Add isActive() to RetirementPolicy

9 files changed, 169 insertions, 52 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
index 707e909ff15..c7ef7f35ce0 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
@@ -6,13 +6,13 @@ import com.yahoo.component.AbstractComponent;
 import com.yahoo.config.provision.Deployer;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostLivenessTracker;
-import com.yahoo.config.provision.RegionName;
-import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.maintenance.retire.RetireIPv4OnlyNodes;
+import com.yahoo.vespa.hosted.provision.maintenance.retire.RetirementPolicy;
+import com.yahoo.vespa.hosted.provision.maintenance.retire.RetirementPolicyList;
 import com.yahoo.vespa.hosted.provision.provisioning.FlavorSpareChecker;
 import com.yahoo.vespa.hosted.provision.provisioning.FlavorSpareCount;
 import com.yahoo.vespa.orchestrator.Orchestrator;
@@ -76,16 +76,10 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
         nodeRebooter = new NodeRebooter(nodeRepository, clock, durationFromEnv("reboot_interval").orElse(defaults.rebootInterval), jobControl);
         metricsReporter = new MetricsReporter(nodeRepository, metric, durationFromEnv("metrics_interval").orElse(defaults.metricsInterval), jobControl);
 
+        RetirementPolicy policy = new RetirementPolicyList(new RetireIPv4OnlyNodes(zone));
         FlavorSpareChecker flavorSpareChecker = new FlavorSpareChecker(
                 NodeRetirer.SPARE_NODES_POLICY, FlavorSpareCount.constructFlavorSpareCountGraph(zone.nodeFlavors().get().getFlavors()));
-        nodeRetirer = new NodeRetirer(nodeRepository, zone, flavorSpareChecker, durationFromEnv("retire_interval").orElse(defaults.nodeRetirerInterval), deployer, jobControl,
-                new RetireIPv4OnlyNodes(),
-                new Zone(SystemName.cd, Environment.dev, RegionName.from("cd-us-central-1")),
-                new Zone(SystemName.cd, Environment.prod, RegionName.from("cd-us-central-1")),
-                new Zone(SystemName.cd, Environment.prod, RegionName.from("cd-us-central-2")),
-                new Zone(SystemName.main, Environment.perf, RegionName.from("us-east-3")),
-                new Zone(SystemName.main, Environment.prod, RegionName.from("us-east-3")),
-                new Zone(SystemName.main, Environment.prod, RegionName.from("us-west-1")));
+        nodeRetirer = new NodeRetirer(nodeRepository, flavorSpareChecker, durationFromEnv("retire_interval").orElse(defaults.nodeRetirerInterval), deployer, jobControl, policy);
     }
 
     @Override
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java
index cd70ea65675..12b63f66d3f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java
@@ -7,7 +7,6 @@ import com.yahoo.config.provision.Deployer;
 import com.yahoo.config.provision.Deployment;
 import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.NodeType;
-import com.yahoo.config.provision.Zone;
 import com.yahoo.log.LogLevel;
 import com.yahoo.transaction.Mutex;
 import com.yahoo.vespa.hosted.provision.Node;
@@ -17,7 +16,6 @@ import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.provisioning.FlavorSpareChecker;
 
 import java.time.Duration;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -46,15 +44,9 @@ public class NodeRetirer extends Maintainer {
     private final FlavorSpareChecker flavorSpareChecker;
     private final RetirementPolicy retirementPolicy;
 
-    public NodeRetirer(NodeRepository nodeRepository, Zone zone, FlavorSpareChecker flavorSpareChecker, Duration interval,
-                       Deployer deployer, JobControl jobControl, RetirementPolicy retirementPolicy, Zone... applies) {
+    public NodeRetirer(NodeRepository nodeRepository, FlavorSpareChecker flavorSpareChecker, Duration interval,
+                       Deployer deployer, JobControl jobControl, RetirementPolicy retirementPolicy) {
         super(nodeRepository, interval, jobControl);
-        if (! Arrays.asList(applies).contains(zone)) {
-            String targetZones = Arrays.stream(applies).map(Zone::toString).collect(Collectors.joining(", "));
-            log.info("NodeRetirer should only run in " + targetZones + " and not in " + zone + ", stopping.");
-            deconstruct();
-        }
-
         this.deployer = deployer;
         this.retirementPolicy = retirementPolicy;
         this.flavorSpareChecker = flavorSpareChecker;
@@ -62,6 +54,8 @@ public class NodeRetirer extends Maintainer {
 
     @Override
     protected void maintain() {
+        if (! retirementPolicy.isActive()) return;
+
         if (retireUnallocated()) {
             retireAllocated();
         }
@@ -79,7 +73,7 @@ public class NodeRetirer extends Maintainer {
 
             long numFlavorsWithUnsuccessfullyRetiredNodes = allNodes.stream()
                     .filter(node -> node.state() == Node.State.ready)
-                    .filter(retirementPolicy::shouldRetire)
+                    .filter(node -> retirementPolicy.shouldRetire(node).isPresent())
                     .collect(Collectors.groupingBy(
                             Node::flavor,
                             Collectors.toSet()))
@@ -90,10 +84,11 @@ public class NodeRetirer extends Maintainer {
                             Node nodeToRetire = iter.next();
                             if (! flavorSpareChecker.canRetireUnallocatedNodeWithFlavor(nodeToRetire.flavor())) break;
 
-                            nodeRepository().write(nodeToRetire.with(nodeToRetire.status().withWantToDeprovision(true)));
-                            nodeRepository().park(nodeToRetire.hostname(), Agent.NodeRetirer,
-                                    "Policy: " + retirementPolicy.getClass().getSimpleName());
-                            iter.remove();
+                            retirementPolicy.shouldRetire(nodeToRetire).ifPresent(reason -> {
+                                nodeRepository().write(nodeToRetire.with(nodeToRetire.status().withWantToDeprovision(true)));
+                                nodeRepository().park(nodeToRetire.hostname(), Agent.NodeRetirer, reason);
+                                iter.remove();
+                            });
                         }
 
                         if (! nodesThatShouldBeRetiredForFlavor.isEmpty()) {
@@ -155,16 +150,17 @@ public class NodeRetirer extends Maintainer {
                         .flatMap(node -> node.map(Stream::of).orElseGet(Stream::empty))
                         .collect(Collectors.toSet());
 
-                nodesToRetire.forEach(node -> {
-                    log.info("Setting wantToRetire and wantToDeprovision for host " + node.hostname() +
-                            " with flavor " + node.flavor().name() +
-                            " allocated to " + node.allocation().get().owner() + ". Policy: " +
-                            retirementPolicy.getClass().getSimpleName());
-                    Node updatedNode = node.with(node.status()
-                            .withWantToRetire(true)
-                            .withWantToDeprovision(true));
-                    nodeRepository().write(updatedNode);
-                });
+                nodesToRetire.forEach(node ->
+                        retirementPolicy.shouldRetire(node).ifPresent(reason -> {
+                            log.info("Setting wantToRetire and wantToDeprovision for host " + node.hostname() +
+                                    " with flavor " + node.flavor().name() +
+                                    " allocated to " + node.allocation().get().owner() + ". Reason: " + reason);
+
+                            Node updatedNode = node.with(node.status()
+                                    .withWantToRetire(true)
+                                    .withWantToDeprovision(true));
+                            nodeRepository().write(updatedNode);
+                        }));
             }
 
             // This takes a while, so do it outside of the application lock
@@ -208,7 +204,7 @@ public class NodeRetirer extends Maintainer {
         return nodes.stream()
                 .filter(node -> node.state() == Node.State.active)
                 .filter(node -> !node.status().wantToRetire())
-                .filter(retirementPolicy::shouldRetire)
+                .filter(node -> retirementPolicy.shouldRetire(node).isPresent())
                 .collect(Collectors.toSet());
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetireIPv4OnlyNodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetireIPv4OnlyNodes.java
index 3aa1ad2c910..4fb58b3714b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetireIPv4OnlyNodes.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetireIPv4OnlyNodes.java
@@ -2,21 +2,50 @@
 package com.yahoo.vespa.hosted.provision.maintenance.retire;
 
 import com.google.common.net.InetAddresses;
+import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.Flavor;
+import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.hosted.provision.Node;
 
 import java.net.Inet4Address;
+import java.util.Optional;
 
 /**
  * @author freva
  */
 public class RetireIPv4OnlyNodes implements RetirementPolicy {
+    private final Zone zone;
+
+    public RetireIPv4OnlyNodes(Zone zone) {
+        this.zone = zone;
+    }
+
+    @Override
+    public boolean isActive() {
+        if(zone.system() == SystemName.cd) {
+            return zone.environment() == Environment.dev || zone.environment() == Environment.prod;
+        }
+
+        if (zone.system() == SystemName.main) {
+            if (zone.region().equals(RegionName.from("us-east-3"))) {
+                return zone.environment() == Environment.perf || zone.environment() == Environment.prod;
+            } else if (zone.region().equals(RegionName.from("us-west-1"))) {
+                return zone.environment() == Environment.prod;
+            }
+        }
+
+        return false;
+    }
 
     @Override
-    public boolean shouldRetire(Node node) {
-        if (node.flavor().getType() == Flavor.Type.VIRTUAL_MACHINE) return false;
-        return node.ipAddresses().stream()
+    public Optional<String> shouldRetire(Node node) {
+        if (node.flavor().getType() == Flavor.Type.VIRTUAL_MACHINE) return Optional.empty();
+        boolean shouldRetire = node.ipAddresses().stream()
                 .map(InetAddresses::forString)
                 .allMatch(address -> address instanceof Inet4Address);
+
+        return shouldRetire ? Optional.of("Node is IPv4-only") : Optional.empty();
     }
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetirementPolicy.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetirementPolicy.java
index d1147b64dba..4971d8ea1c9 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetirementPolicy.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetirementPolicy.java
@@ -3,9 +3,17 @@ package com.yahoo.vespa.hosted.provision.maintenance.retire;
 
 import com.yahoo.vespa.hosted.provision.Node;
 
+import java.util.Optional;
+
 /**
  * @author freva
  */
 public interface RetirementPolicy {
-    boolean shouldRetire(Node node);
+
+    boolean isActive();
+
+    /**
+     * Returns reason for retiring the node, empty if node should not be retired
+     */
+    Optional<String> shouldRetire(Node node);
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetirementPolicyCache.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetirementPolicyCache.java
new file mode 100644
index 00000000000..c112daadcc9
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetirementPolicyCache.java
@@ -0,0 +1,28 @@
+// 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.provision.maintenance.retire;
+
+import com.yahoo.vespa.hosted.provision.Node;
+
+import java.util.Optional;
+
+/**
+ * @author freva
+ */
+public class RetirementPolicyCache implements RetirementPolicy {
+    private final RetirementPolicy policy;
+    private final boolean isActiveCached;
+
+    RetirementPolicyCache(RetirementPolicy policy) {
+        this.policy = policy;
+        this.isActiveCached = policy.isActive();
+    }
+
+    @Override
+    public boolean isActive() {
+        return isActiveCached;
+    }
+
+    public Optional<String> shouldRetire(Node node) {
+        return policy.shouldRetire(node);
+    }
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetirementPolicyList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetirementPolicyList.java
new file mode 100644
index 00000000000..5f4d887b029
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetirementPolicyList.java
@@ -0,0 +1,39 @@
+// 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.provision.maintenance.retire;
+
+import com.yahoo.vespa.hosted.provision.Node;
+
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ * @author freva
+ */
+public class RetirementPolicyList implements RetirementPolicy {
+    private final List<RetirementPolicy> retirementPolicies;
+
+    public RetirementPolicyList(RetirementPolicy... retirementPolicies) {
+        this.retirementPolicies = Stream.of(retirementPolicies)
+                .map(RetirementPolicyCache::new)
+                .collect(Collectors.toList());
+    }
+
+    @Override
+    public boolean isActive() {
+        return retirementPolicies.stream().anyMatch(RetirementPolicy::isActive);
+    }
+
+    @Override
+    public Optional<String> shouldRetire(Node node) {
+        List<String> retirementReasons = retirementPolicies.stream()
+                .filter(RetirementPolicy::isActive)
+                .map(retirementPolicy -> retirementPolicy.shouldRetire(node))
+                .flatMap(reason -> reason.map(Stream::of).orElse(Stream.empty()))
+                .collect(Collectors.toList());
+
+        return retirementReasons.isEmpty() ? Optional.empty() :
+                Optional.of("[" + String.join(", ", retirementReasons) + "]");
+    }
+}
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirerTest.java
index 9d47b8ecea8..a5369e4f89c 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirerTest.java
@@ -12,6 +12,7 @@ import org.junit.Test;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -19,20 +20,33 @@ import java.util.stream.Stream;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 /**
  * @author freva
  */
 public class NodeRetirerTest {
-    private final RetirementPolicy policy = node -> node.ipAddresses().equals(Collections.singleton("::1"));
     private NodeRetirerTester tester;
     private NodeRetirer retirer;
+    private final RetirementPolicy policy = mock(RetirementPolicy.class);
 
     @Before
     public void setup() {
+        doAnswer(invok -> {
+            boolean shouldRetire = ((Node) invok.getArguments()[0]).ipAddresses().equals(Collections.singleton("::1"));
+            return shouldRetire ? Optional.of("Some reason") : Optional.empty();
+        }).when(policy).shouldRetire(any(Node.class));
+        when(policy.isActive()).thenReturn(true);
+
         NodeFlavors nodeFlavors = NodeRetirerTester.makeFlavors(5);
         tester = new NodeRetirerTester(nodeFlavors);
-        retirer = tester.makeNodeRetirer(policy);
+        retirer = spy(tester.makeNodeRetirer(policy));
 
         tester.createReadyNodesByFlavor(21, 42, 27, 15, 8);
         tester.deployApp("vespa",  "calendar",  new int[]{3},       new int[]{7});
@@ -148,4 +162,13 @@ public class NodeRetirerTest {
         long actualOneRetired = retirer.getNumberNodesAllowToRetireForCluster(tester.nodeRepository.getNodes(app), 2);
         assertEquals(1, actualOneRetired);
     }
+
+    @Test
+    public void inactivePolicyDoesNothingTest() {
+        when(policy.isActive()).thenReturn(false);
+        retirer.maintain();
+
+        verify(retirer, never()).retireUnallocated();
+        verify(retirer, never()).retireAllocated();
+    }
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirerTester.java
index 81535fa8d1b..04e782c5562 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirerTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirerTester.java
@@ -74,7 +74,7 @@ public class NodeRetirerTester {
     }
 
     NodeRetirer makeNodeRetirer(RetirementPolicy policy) {
-         return new NodeRetirer(nodeRepository, zone, flavorSpareChecker, Duration.ofDays(1), deployer, jobControl, policy);
+         return new NodeRetirer(nodeRepository, flavorSpareChecker, Duration.ofDays(1), deployer, jobControl, policy);
     }
 
     void createReadyNodesByFlavor(int... nums) {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetireIPv4OnlyNodesTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetireIPv4OnlyNodesTest.java
index bb24e121265..ab2c9d81399 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetireIPv4OnlyNodesTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/retire/RetireIPv4OnlyNodesTest.java
@@ -21,49 +21,49 @@ import static org.junit.Assert.assertTrue;
  * @author freva
  */
 public class RetireIPv4OnlyNodesTest {
-    private final RetireIPv4OnlyNodes policy = new RetireIPv4OnlyNodes();
+    private final RetireIPv4OnlyNodes policy = new RetireIPv4OnlyNodes(null);
     private final List<Flavor> nodeFlavors = initFlavors();
 
     @Test
     public void testSingleIPv4Address() {
         Node node = createNodeWithAddresses("127.0.0.1");
-        assertTrue(policy.shouldRetire(node));
+        assertTrue(policy.shouldRetire(node).isPresent());
     }
 
     @Test
     public void testSingleIPv6Address() {
         Node node = createNodeWithAddresses("::1");
-        assertFalse(policy.shouldRetire(node));
+        assertFalse(policy.shouldRetire(node).isPresent());
     }
 
     @Test
     public void testMultipleIPv4Address() {
         Node node = createNodeWithAddresses("127.0.0.1", "10.0.0.1", "192.168.0.1");
-        assertTrue(policy.shouldRetire(node));
+        assertTrue(policy.shouldRetire(node).isPresent());
     }
 
     @Test
     public void testMultipleIPv6Address() {
         Node node = createNodeWithAddresses("::1", "::2", "1234:5678:90ab::cdef");
-        assertFalse(policy.shouldRetire(node));
+        assertFalse(policy.shouldRetire(node).isPresent());
     }
 
     @Test
     public void testCombinationAddress() {
         Node node = createNodeWithAddresses("127.0.0.1", "::1", "10.0.0.1", "::2");
-        assertFalse(policy.shouldRetire(node));
+        assertFalse(policy.shouldRetire(node).isPresent());
     }
 
     @Test
     public void testNeverRetireVMs() {
         Node node = createVMWithAddresses("127.0.0.1", "10.0.0.1", "192.168.0.1");
-        assertFalse(policy.shouldRetire(node));
+        assertFalse(policy.shouldRetire(node).isPresent());
 
         node = createNodeWithAddresses("::1", "::2", "1234:5678:90ab::cdef");
-        assertFalse(policy.shouldRetire(node));
+        assertFalse(policy.shouldRetire(node).isPresent());
 
         node = createNodeWithAddresses("127.0.0.1", "::1", "10.0.0.1", "::2");
-        assertFalse(policy.shouldRetire(node));
+        assertFalse(policy.shouldRetire(node).isPresent());
     }
 
     private Node createNodeWithAddresses(String... addresses) {