Avoid periodic removal

The controller log contains errors about refusing to handle the config server
application since the controller application is active.

The message is misleading: Only the controller application will be activated,
while the config server application will be tried removed, and the message
comes as part of this removal.

This PR adds a check that actual removal from duper model and provisioner will
only be done if it is active in the duper model.

There are edge cases that the application may be active in the provisioner
while not in duper model, but it's fixed on a restart.

2 files changed, 25 insertions, 6 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 9d87a835960..68d597fb839 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
@@ -108,9 +108,12 @@ public class InfrastructureProvisioner extends Maintainer {
     }
 
     private void removeApplication(ApplicationId applicationId) {
-        NestedTransaction nestedTransaction = new NestedTransaction();
-        provisioner.remove(nestedTransaction, applicationId);
-        nestedTransaction.commit();
-        duperModel.infraApplicationRemoved(applicationId);
+        // Use the DuperModel as source-of-truth on whether it has also been activated (to avoid periodic removals)
+        if (duperModel.infraApplicationIsActive(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 bc83e3525ad..4fd20d6991b 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
@@ -78,6 +78,7 @@ public class InfrastructureProvisionerTest {
     public void remove_application_if_without_target_version() {
         when(infrastructureVersions.getTargetVersionFor(eq(nodeType))).thenReturn(Optional.empty());
         addNode(1, Node.State.active, Optional.of(target));
+        when(duperModelInfraApi.infraApplicationIsActive(eq(application.getApplicationId()))).thenReturn(true);
         infrastructureProvisioner.maintain();
         verify(duperModelInfraApi).infraApplicationRemoved(application.getApplicationId());
         verifyRemoved(1);
@@ -85,12 +86,26 @@ public class InfrastructureProvisionerTest {
 
     @Test
     public void remove_application_if_without_nodes() {
+        remove_application_without_nodes(true);
+    }
+
+    @Test
+    public void skip_remove_unless_active() {
+        remove_application_without_nodes(false);
+    }
+
+    private void remove_application_without_nodes(boolean applicationIsActive) {
         when(infrastructureVersions.getTargetVersionFor(eq(nodeType))).thenReturn(Optional.of(target));
         addNode(1, Node.State.failed, Optional.of(target));
         addNode(2, Node.State.parked, Optional.empty());
+        when(duperModelInfraApi.infraApplicationIsActive(eq(application.getApplicationId()))).thenReturn(applicationIsActive);
         infrastructureProvisioner.maintain();
-        verify(duperModelInfraApi).infraApplicationRemoved(application.getApplicationId());
-        verifyRemoved(1);
+        if (applicationIsActive) {
+            verify(duperModelInfraApi).infraApplicationRemoved(application.getApplicationId());
+            verifyRemoved(1);
+        } else {
+            verifyRemoved(0);
+        }
     }
 
     @Test
@@ -199,6 +214,7 @@ public class InfrastructureProvisionerTest {
     @Test
     public void avoid_provisioning_if_no_usable_nodes() {
         when(infrastructureVersions.getTargetVersionFor(eq(nodeType))).thenReturn(Optional.of(target));
+        when(duperModelInfraApi.infraApplicationIsActive(eq(application.getApplicationId()))).thenReturn(true);
 
         infrastructureProvisioner.maintain();
         verifyRemoved(1);