Default vcmr status is no-op. Increase maintainer frequency and retirement time. Add test

4 files changed, 27 insertions, 7 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VespaChangeRequest.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VespaChangeRequest.java
index eec5684d522..ea56521dc3b 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VespaChangeRequest.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VespaChangeRequest.java
@@ -88,6 +88,7 @@ public class VespaChangeRequest extends ChangeRequest {
         IN_PROGRESS,
         PENDING_ACTION,
         PENDING_ASSESSMENT,
-        REQUIRES_OPERATOR_ACTION
+        REQUIRES_OPERATOR_ACTION,
+        NOOP
     }
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
index 9513348303f..861e0c9a151 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
@@ -156,7 +156,7 @@ public class ControllerMaintenance extends AbstractComponent {
             this.archiveAccessMaintainer = duration(10, MINUTES);
             this.tenantRoleMaintainer = duration(5, MINUTES);
             this.changeRequestMaintainer = duration(12, HOURS);
-            this.vcmrMaintainer = duration(12, HOURS);
+            this.vcmrMaintainer = duration(4, HOURS);
         }
 
         private Duration duration(long amount, TemporalUnit unit) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainer.java
index 9c81327d442..595c2ef26d6 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainer.java
@@ -20,7 +20,6 @@ import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import java.time.Duration;
 import java.time.Instant;
 import java.time.ZonedDateTime;
-import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -37,6 +36,7 @@ import java.util.stream.Collectors;
 public class VCMRMaintainer extends ControllerMaintainer {
 
     private final Logger logger = Logger.getLogger(VCMRMaintainer.class.getName());
+    private final Duration ALLOWED_RETIREMENT_TIME = Duration.ofHours(60);
     private final CuratorDb curator;
     private final NodeRepository nodeRepository;
 
@@ -92,7 +92,11 @@ public class VCMRMaintainer extends ControllerMaintainer {
             return Status.IN_PROGRESS;
         }
 
-        return Status.PENDING_ACTION;
+        if (byActionState.getOrDefault(State.PENDING_RETIREMENT, 0L) > 0) {
+            return Status.PENDING_ACTION;
+        }
+
+        return Status.NOOP;
     }
 
     private List<HostAction> getNextActions(List<Node> nodes, VespaChangeRequest changeRequest) {
@@ -163,7 +167,7 @@ public class VCMRMaintainer extends ControllerMaintainer {
     private boolean shouldRetire(VespaChangeRequest changeRequest, HostAction action) {
         return action.getState() == State.PENDING_RETIREMENT &&
                 changeRequest.getChangeRequestSource().getPlannedStartTime()
-                        .minus(Duration.ofDays(2))
+                        .minus(ALLOWED_RETIREMENT_TIME)
                         .isBefore(ZonedDateTime.now());
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainerTest.java
index f5cea0eb391..0983b7869bb 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VCMRMaintainerTest.java
@@ -58,7 +58,7 @@ public class VCMRMaintainerTest {
         nodeRepo.putNodes(zoneId, List.of(configNode, activeNode));
         nodeRepo.hasSpareCapacity(true);
 
-        tester.curator().writeChangeRequest(openChangeRequest());
+        tester.curator().writeChangeRequest(futureChangeRequest());
         maintainer.maintain();
 
         var writtenChangeRequest = tester.curator().readChangeRequest(changeRequestId).get();
@@ -124,11 +124,26 @@ public class VCMRMaintainerTest {
         assertEquals(Status.IN_PROGRESS, writtenChangeRequest.getStatus());
     }
 
+    @Test
+    public void pending_retirement_when_vcmr_is_far_ahead() {
+        var activeNode = createNode(host2, NodeType.host, Node.State.active, false);
+        nodeRepo.putNodes(zoneId, List.of(activeNode));
+        nodeRepo.hasSpareCapacity(true);
+
+        tester.curator().writeChangeRequest(futureChangeRequest());
+        maintainer.maintain();
+
+        var writtenChangeRequest = tester.curator().readChangeRequest(changeRequestId).get();
+        var tenantHostAction = writtenChangeRequest.getHostActionPlan().get(0);
+        assertEquals(State.PENDING_RETIREMENT, tenantHostAction.getState());
+        assertEquals(Status.PENDING_ACTION, writtenChangeRequest.getStatus());
+    }
+
     private VespaChangeRequest canceledChangeRequest() {
         return newChangeRequest(ChangeRequestSource.Status.CANCELED, State.RETIRED, State.RETIRING, ZonedDateTime.now());
     }
 
-    private VespaChangeRequest openChangeRequest() {
+    private VespaChangeRequest futureChangeRequest() {
         return newChangeRequest(ChangeRequestSource.Status.WAITING_FOR_APPROVAL, State.NONE, State.NONE, ZonedDateTime.now().plus(Duration.ofDays(5L)));
     }