Avoid ZK read when checking maintainer system

4 files changed, 57 insertions, 14 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java
index 0e7b84a8d13..bf5743e2d3c 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java
@@ -31,7 +31,7 @@ public class ContactInformationMaintainer extends Maintainer {
     private final Organization organization;
 
     public ContactInformationMaintainer(Controller controller, Duration interval, JobControl jobControl, Organization organization) {
-        super(controller, interval, jobControl, EnumSet.of(SystemName.cd, SystemName.main));
+        super(controller, interval, jobControl, null, EnumSet.of(SystemName.cd, SystemName.main));
         this.organization = Objects.requireNonNull(organization, "organization must be non-null");
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java
index a8e70aedacb..206e1dc7e98 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.component.Version;
+import com.yahoo.config.provision.SystemName;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.UpgradePolicy;
@@ -11,6 +12,7 @@ import com.yahoo.yolean.Exceptions;
 
 import java.time.Duration;
 import java.util.Comparator;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Function;
@@ -29,7 +31,7 @@ public abstract class InfrastructureUpgrader extends Maintainer {
 
     public InfrastructureUpgrader(Controller controller, Duration interval, JobControl jobControl,
                                   UpgradePolicy upgradePolicy, String name) {
-        super(controller, interval, jobControl, name);
+        super(controller, interval, jobControl, name, EnumSet.allOf(SystemName.class));
         this.upgradePolicy = upgradePolicy;
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java
index 0869f28f977..bec3e969887 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.maintenance;
 
+import com.google.common.collect.ImmutableSet;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.vespa.curator.Lock;
@@ -30,26 +31,18 @@ public abstract class Maintainer extends AbstractComponent implements Runnable {
     private final JobControl jobControl;
     private final ScheduledExecutorService service;
     private final String name;
-    private final Set<SystemName> systemNames;
+    private final Set<SystemName> permittedSystems;
 
     public Maintainer(Controller controller, Duration interval, JobControl jobControl) {
         this(controller, interval, jobControl, null, EnumSet.allOf(SystemName.class));
     }
 
-    public Maintainer(Controller controller, Duration interval, JobControl jobControl, String name) {
-        this(controller, interval, jobControl, name, EnumSet.allOf(SystemName.class));
-    }
-
-    public Maintainer(Controller controller, Duration interval, JobControl jobControl, Set<SystemName> systemNames) {
-        this(controller, interval, jobControl, null, systemNames);
-    }
-
-    public Maintainer(Controller controller, Duration interval, JobControl jobControl, String name, Set<SystemName> systemNames) {
+    public Maintainer(Controller controller, Duration interval, JobControl jobControl, String name, Set<SystemName> permittedSystems) {
         this.controller = controller;
         this.maintenanceInterval = interval;
         this.jobControl = jobControl;
         this.name = name;
-        this.systemNames = systemNames;
+        this.permittedSystems = ImmutableSet.copyOf(permittedSystems);
 
         service = new ScheduledThreadPoolExecutor(1);
         service.scheduleAtFixedRate(this, interval.toMillis(), interval.toMillis(), TimeUnit.MILLISECONDS);
@@ -61,7 +54,10 @@ public abstract class Maintainer extends AbstractComponent implements Runnable {
     @Override
     public void run() {
         try {
-            if (jobControl.isActive(name()) && systemNames.contains(controller.system())) {
+            if (!permittedSystems.contains(controller.system())) {
+                return;
+            }
+            if (jobControl.isActive(name())) {
                 try (Lock lock = jobControl.curator().lockMaintenanceJob(name())) {
                     maintain();
                 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java
new file mode 100644
index 00000000000..c10ceef6adb
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java
@@ -0,0 +1,45 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.controller.maintenance;
+
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.vespa.hosted.controller.ControllerTester;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.time.Duration;
+import java.util.EnumSet;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author mpolden
+ */
+public class MaintainerTest {
+
+    private ControllerTester tester;
+
+    @Before
+    public void before() {
+        tester = new ControllerTester();
+    }
+
+    @Test
+    public void only_runs_in_permitted_systems() {
+        AtomicInteger executions = new AtomicInteger();
+        maintainerIn(SystemName.cd, executions).run();
+        maintainerIn(SystemName.main, executions).run();
+        assertEquals(1, executions.get());
+    }
+
+    private Maintainer maintainerIn(SystemName system, AtomicInteger executions) {
+        return new Maintainer(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator()),
+                              "MockMaintainer", EnumSet.of(system)) {
+            @Override
+            protected void maintain() {
+                executions.incrementAndGet();
+            }
+        };
+    }
+
+}