Merge pull request #16082 from vespa-engine/hakonhall/always-use-permanently-down-status

Always use permanently down status

5 files changed, 15 insertions, 40 deletions

diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index 671815d0ab1..c4805b3651b 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -43,7 +43,7 @@ public class Flags {
     private static volatile TreeMap<FlagId, FlagDefinition> flags = new TreeMap<>();
 
     public static final UnboundBooleanFlag RETIRE_WITH_PERMANENTLY_DOWN = defineFeatureFlag(
-            "retire-with-permanently-down", false,
+            "retire-with-permanently-down", true,
             List.of("hakonhall"), "2020-12-02", "2021-02-01",
             "If enabled, retirement will end with setting the host status to PERMANENTLY_DOWN, " +
             "instead of ALLOWED_TO_BE_DOWN (old behavior).",
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
index fbb25baa806..d69cbcf7e9f 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
@@ -1,7 +1,6 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.orchestrator;
 
-import java.util.logging.Level;
 import com.yahoo.time.TimeBudget;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientTimeouts;
@@ -11,6 +10,7 @@ import java.time.Duration;
 import java.time.Instant;
 import java.util.HashMap;
 import java.util.Optional;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -31,7 +31,6 @@ public class OrchestratorContext implements AutoCloseable {
     private final TimeBudget timeBudget;
     private final boolean probe;
     private final boolean largeLocks;
-    private final boolean usePermanentlyDownStatus;
 
     // The key set is the set of applications locked by this context tree: Only the
     // root context has a non-empty set. The value is an unlock callback to be called
@@ -42,37 +41,30 @@ public class OrchestratorContext implements AutoCloseable {
     public static OrchestratorContext createContextForMultiAppOp(Clock clock) {
         return new OrchestratorContext(null, clock, TimeBudget.fromNow(clock, DEFAULT_TIMEOUT_FOR_BATCH_OP),
                 false, // probe
-                true, // large locks
-                false); // use permanently down status
+                true); // large locks
     }
 
     /** Create an OrchestratorContext for an operation on a single application. */
     public static OrchestratorContext createContextForSingleAppOp(Clock clock) {
-        return createContextForSingleAppOp(clock, false);
-    }
-
-    public static OrchestratorContext createContextForSingleAppOp(Clock clock, boolean usePermanentlyDownStatus) {
         return new OrchestratorContext(null, clock, TimeBudget.fromNow(clock, DEFAULT_TIMEOUT_FOR_SINGLE_OP),
-                false, false, usePermanentlyDownStatus);
+                false, false);
     }
 
     public static OrchestratorContext createContextForAdminOp(Clock clock) {
         return new OrchestratorContext(null, clock, TimeBudget.fromNow(clock, DEFAULT_TIMEOUT_FOR_ADMIN_OP),
-                false, false, false);
+                false, false);
     }
 
     private OrchestratorContext(OrchestratorContext parentOrNull,
                                 Clock clock,
                                 TimeBudget timeBudget,
                                 boolean probe,
-                                boolean largeLocks,
-                                boolean usePermanentlyDownStatus) {
+                                boolean largeLocks) {
         this.parent = Optional.ofNullable(parentOrNull);
         this.clock = clock;
         this.timeBudget = timeBudget;
         this.probe = probe;
         this.largeLocks = largeLocks;
-        this.usePermanentlyDownStatus = usePermanentlyDownStatus;
     }
 
     public Duration getTimeLeft() {
@@ -91,9 +83,6 @@ public class OrchestratorContext implements AutoCloseable {
     /** Whether application locks acquired during probing of a batch suspend should be closed after the non-probe is done. */
     public boolean largeLocks() { return largeLocks; }
 
-    /** Whether the PERMANENTLY_DOWN host status should be used (where appropriate). */
-    public boolean usePermanentlyDownStatus() { return usePermanentlyDownStatus; }
-
     /**
      * Returns true if 1. large locks is enabled, and 2.
      * {@link #registerLockAcquisition(ApplicationInstanceReference, Runnable) registerLockAcquisition}
@@ -131,7 +120,7 @@ public class OrchestratorContext implements AutoCloseable {
         // Move deadline towards past by a fixed amount to ensure there's time to process exceptions and
         // access ZooKeeper before the lock times out.
         TimeBudget subTimeBudget = timeBudget.withDeadline(timeBudget.deadline().get().minus(TIMEOUT_OVERHEAD));
-        return new OrchestratorContext(this, clock, subTimeBudget, probe, largeLocks, usePermanentlyDownStatus);
+        return new OrchestratorContext(this, clock, subTimeBudget, probe, largeLocks);
     }
 
     /** Create an OrchestratorContext for an operation on a single application, but limited to current timeout. */
@@ -144,7 +133,7 @@ public class OrchestratorContext implements AutoCloseable {
         }
 
         TimeBudget timeBudget = TimeBudget.from(clock, now, Optional.of(Duration.between(now, deadline)));
-        return new OrchestratorContext(this, clock, timeBudget, probe, largeLocks, usePermanentlyDownStatus);
+        return new OrchestratorContext(this, clock, timeBudget, probe, largeLocks);
     }
 
     @Override
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
index 333edd4b3f9..630427f49a8 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
@@ -11,10 +11,7 @@ import com.yahoo.vespa.applicationmodel.ClusterId;
 import com.yahoo.vespa.applicationmodel.HostName;
 import com.yahoo.vespa.applicationmodel.ServiceCluster;
 import com.yahoo.vespa.applicationmodel.ServiceInstance;
-import com.yahoo.vespa.flags.BooleanFlag;
-import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.FlagSource;
-import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.orchestrator.config.OrchestratorConfig;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient;
 import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory;
@@ -65,7 +62,6 @@ public class OrchestratorImpl implements Orchestrator {
     private final ClusterControllerClientFactory clusterControllerClientFactory;
     private final Clock clock;
     private final ApplicationApiFactory applicationApiFactory;
-    private final BooleanFlag retireWithPermanentlyDownFlag;
 
     @Inject
     public OrchestratorImpl(ClusterControllerClientFactory clusterControllerClientFactory,
@@ -103,7 +99,6 @@ public class OrchestratorImpl implements Orchestrator {
         this.serviceMonitor = serviceMonitor;
         this.clock = clock;
         this.applicationApiFactory = applicationApiFactory;
-        this.retireWithPermanentlyDownFlag = Flags.RETIRE_WITH_PERMANENTLY_DOWN.bindTo(flagSource);
 
         serviceMonitor.registerListener(statusService);
     }
@@ -208,10 +203,7 @@ public class OrchestratorImpl implements Orchestrator {
         ApplicationInstance appInstance = getApplicationInstance(hostName);
         NodeGroup nodeGroup = new NodeGroup(appInstance, hostName);
 
-        boolean usePermanentlyDownStatus = retireWithPermanentlyDownFlag
-                .with(FetchVector.Dimension.HOSTNAME, hostName.s())
-                .value();
-        OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock, usePermanentlyDownStatus);
+        OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock);
         try (ApplicationLock lock = statusService.lockApplication(context, appInstance.reference())) {
             ApplicationApi applicationApi = applicationApiFactory.create(nodeGroup, lock, clusterControllerClientFactory);
 
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
index f9b914e391e..8090a4e95c4 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
@@ -99,15 +99,9 @@ public class HostedVespaPolicy implements Policy {
             storageNode.setNodeState(context, ClusterControllerNodeState.DOWN);
         }
 
-        if (context.usePermanentlyDownStatus()) {
-            // Ensure all nodes in the group are marked as permanently down
-            for (HostName hostName : applicationApi.getNodesInGroupWith(status -> status != HostStatus.PERMANENTLY_DOWN)) {
-                applicationApi.setHostState(context, hostName, HostStatus.PERMANENTLY_DOWN);
-            }
-        } else {
-            for (HostName hostName : applicationApi.getNodesInGroupWith(status -> status != HostStatus.ALLOWED_TO_BE_DOWN)) {
-                applicationApi.setHostState(context, hostName, HostStatus.ALLOWED_TO_BE_DOWN);
-            }
+        // Ensure all nodes in the group are marked as permanently down
+        for (HostName hostName : applicationApi.getNodesInGroupWith(status -> status != HostStatus.PERMANENTLY_DOWN)) {
+            applicationApi.setHostState(context, hostName, HostStatus.PERMANENTLY_DOWN);
         }
     }
 
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java
index 599b50548b7..1799b95c65a 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java
@@ -140,9 +140,9 @@ public class HostedVespaPolicyTest {
         order.verify(storageNode3).setNodeState(context, ClusterControllerNodeState.DOWN);
 
         order.verify(applicationApi).getNodesInGroupWith(any());
-        order.verify(applicationApi).setHostState(context, hostName1, HostStatus.ALLOWED_TO_BE_DOWN);
-        order.verify(applicationApi).setHostState(context, hostName2, HostStatus.ALLOWED_TO_BE_DOWN);
-        order.verify(applicationApi).setHostState(context, hostName3, HostStatus.ALLOWED_TO_BE_DOWN);
+        order.verify(applicationApi).setHostState(context, hostName1, HostStatus.PERMANENTLY_DOWN);
+        order.verify(applicationApi).setHostState(context, hostName2, HostStatus.PERMANENTLY_DOWN);
+        order.verify(applicationApi).setHostState(context, hostName3, HostStatus.PERMANENTLY_DOWN);
 
         order.verifyNoMoreInteractions();
     }