Always activate or remove infrastructure application

2 files changed, 33 insertions, 24 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisioner.java
index 443cbf4c358..76091c1ceb4 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisioner.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.provision.maintenance;
 
 import com.yahoo.component.Version;
+import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.HostSpec;
 import com.yahoo.config.provision.NodeType;
@@ -11,19 +12,22 @@ import com.yahoo.transaction.Mutex;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
+import com.yahoo.vespa.service.duper.DuperModel;
 import com.yahoo.vespa.service.monitor.DuperModelInfraApi;
 import com.yahoo.vespa.service.monitor.InfraApplicationApi;
 
 import java.time.Duration;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 /**
- * This maintainer makes sure that infrastructure nodes are allocated with correct wanted
- * version. Source for the wanted version comes from the target version set using
- * /nodes/v2/upgrade/ endpoint.
+ * This maintainer makes sure that 1. infrastructure nodes are allocated with correct wanted
+ * version and 2. keeping {@link DuperModel} up to date. Source for the wanted version comes
+ * from the target version set using /nodes/v2/upgrade/ endpoint.
  *
  * @author freva
  */
@@ -34,6 +38,7 @@ public class InfrastructureProvisioner extends Maintainer {
     private final Provisioner provisioner;
     private final InfrastructureVersions infrastructureVersions;
     private final DuperModelInfraApi duperModel;
+    private final Map<ApplicationId, Version> lastActivations = new HashMap<>();
 
     public InfrastructureProvisioner(Provisioner provisioner, NodeRepository nodeRepository,
                                      InfrastructureVersions infrastructureVersions, Duration interval, JobControl jobControl,
@@ -52,8 +57,8 @@ public class InfrastructureProvisioner extends Maintainer {
 
                 Optional<Version> targetVersion = infrastructureVersions.getTargetVersionFor(nodeType);
                 if (!targetVersion.isPresent()) {
-                    logger.log(LogLevel.DEBUG, "Skipping provision of " + nodeType + ": No target version set");
-                    duperModel.infraApplicationRemoved(application.getApplicationId());
+                    logger.log(LogLevel.DEBUG, "No target version set for " + nodeType + ", removing application");
+                    removeApplication(application.getApplicationId());
                     continue;
                 }
 
@@ -65,16 +70,8 @@ public class InfrastructureProvisioner extends Maintainer {
                                 .orElse(null))
                         .collect(Collectors.toList());
                 if (wantedVersions.isEmpty()) {
-                    // TODO: Unprovision active nodes from application?
-                    logger.log(LogLevel.DEBUG, "Skipping provision of " + nodeType + ": No nodes to provision");
-                    duperModel.infraApplicationRemoved(application.getApplicationId());
-                    continue;
-                }
-
-                if (wantedVersions.stream().allMatch(targetVersion.get()::equals) &&
-                        duperModel.infraApplicationIsActive(application.getApplicationId())) {
-                    logger.log(LogLevel.DEBUG, "Skipping provision of " + nodeType +
-                            ": Already provisioned to target version " + targetVersion);
+                    logger.log(LogLevel.DEBUG, "No nodes to provision for " + nodeType + ", removing application");
+                    removeApplication(application.getApplicationId());
                     continue;
                 }
 
@@ -99,11 +96,18 @@ public class InfrastructureProvisioner extends Maintainer {
                 } else {
                     detail = " with " + hostSpecs.size() + " hosts";
                 }
-                logger.log(LogLevel.INFO, "Infrastructure application " + application.getApplicationId() + " activated" + detail);
+                logger.log(LogLevel.DEBUG, "Infrastructure application " + application.getApplicationId() + " activated" + detail);
             } catch (RuntimeException e) {
                 logger.log(LogLevel.INFO, "Failed to activate " + application.getApplicationId(), e);
                 // loop around to activate the next application
             }
         }
     }
+
+    private void removeApplication(ApplicationId applicationId) {
+        NestedTransaction nestedTransaction = new NestedTransaction();
+        provisioner.remove(nestedTransaction, applicationId);
+        nestedTransaction.commit();
+        duperModel.infraApplicationRemoved(applicationId);
+    }
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisionerTest.java
index 6f27888fbfb..85092053bdd 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisionerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureProvisionerTest.java
@@ -33,8 +33,8 @@ import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 /**
@@ -80,7 +80,7 @@ public class InfrastructureProvisionerTest {
         addNode(1, Node.State.active, Optional.of(target));
         infrastructureProvisioner.maintain();
         verify(duperModelInfraApi).infraApplicationRemoved(application.getApplicationId());
-        verifyNoMoreInteractions(provisioner);
+        verifyRemoved(1);
     }
 
     @Test
@@ -90,11 +90,11 @@ public class InfrastructureProvisionerTest {
         addNode(2, Node.State.parked, Optional.empty());
         infrastructureProvisioner.maintain();
         verify(duperModelInfraApi).infraApplicationRemoved(application.getApplicationId());
-        verifyNoMoreInteractions(provisioner);
+        verifyRemoved(1);
     }
 
     @Test
-    public void no_op_if_nodes_active_and_on_target_version() {
+    public void activate_when_no_op() {
         when(infrastructureVersions.getTargetVersionFor(eq(nodeType))).thenReturn(Optional.of(target));
 
         addNode(1, Node.State.failed, Optional.of(oldVersion));
@@ -107,8 +107,8 @@ public class InfrastructureProvisionerTest {
 
         infrastructureProvisioner.maintain();
         verify(duperModelInfraApi, never()).infraApplicationRemoved(any());
-        verify(duperModelInfraApi, never()).infraApplicationActivated(any(), any());
-        verifyNoMoreInteractions(provisioner);
+        verify(duperModelInfraApi).infraApplicationActivated(any(), any());
+        verify(provisioner).activate(any(), any(), any());
     }
 
     @Test
@@ -196,14 +196,19 @@ public class InfrastructureProvisionerTest {
         when(infrastructureVersions.getTargetVersionFor(eq(nodeType))).thenReturn(Optional.of(target));
 
         infrastructureProvisioner.maintain();
-        verifyNoMoreInteractions(provisioner);
+        verifyRemoved(1);
 
         // Add nodes in non-provisionable states
         addNode(1, Node.State.dirty, Optional.empty());
         addNode(2, Node.State.failed, Optional.empty());
 
         infrastructureProvisioner.maintain();
-        verifyNoMoreInteractions(provisioner);
+        verifyRemoved(2);
+    }
+
+    private void verifyRemoved(int removedCount) {
+        verify(provisioner, times(removedCount)).remove(any(), any());
+        verify(duperModelInfraApi, times(removedCount)).infraApplicationRemoved(any());
     }
 
     private Node addNode(int id, Node.State state, Optional<Version> wantedVespaVersion) {