Check if number of affected instances is equal to number of deprovisioned hosts.

Check if host already wants to deprovision/retire
Change from list to set in test

2 files changed, 29 insertions, 31 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java
index 73b13bb79a6..bd8faaed2e2 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporter.java
@@ -49,28 +49,31 @@ public class CloudEventReporter extends Maintainer {
         for (var awsRegion : zonesByCloudNativeRegion.keySet()) {
             List<CloudEvent> events = eventFetcher.getEvents(awsRegion);
             for (var event : events) {
-                deprovisionHosts(awsRegion, event);
-                submitIssue(event);
+                List<String> deprovisionedHosts = deprovisionHosts(awsRegion, event);
+                submitIssue(event, deprovisionedHosts);
             }
         }
     }
 
-    private void deprovisionHosts(String awsRegion, CloudEvent event) {
-        for (var zone : zonesByCloudNativeRegion.get(awsRegion)) {
-            nodeRepository.list(zone.getId())
-                    .stream()
-                    .filter(shouldDeprovisionHost(event))
-                    .map(node -> node.hostname().value())
-                    .forEach(hostname -> {
-                        log.info(String.format("Setting host %s to wantToRetire and wantToDeprovision", hostname));
-                        nodeRepository.retireAndDeprovision(zone.getId(), hostname);
-                        event.affectedInstances.removeIf(hostname::contains);
-                    });
-        }
+    private List<String> deprovisionHosts(String awsRegion, CloudEvent event) {
+        return zonesByCloudNativeRegion.get(awsRegion)
+                .stream()
+                .flatMap(zone ->
+                    nodeRepository.list(zone.getId())
+                            .stream()
+                            .filter(shouldDeprovisionHost(event))
+                            .map(node -> {
+                                if (!node.wantToDeprovision() || !node.wantToRetire())
+                                    log.info(String.format("Setting host %s to wantToRetire and wantToDeprovision", node.hostname().value()));
+                                    nodeRepository.retireAndDeprovision(zone.getId(), node.hostname().value());
+                                return node.hostname().value();
+                            })
+                )
+                .collect(Collectors.toList());
     }
 
-    private void submitIssue(CloudEvent event) {
-        if (event.affectedInstances.isEmpty())
+    private void submitIssue(CloudEvent event, List<String> deprovisionedHosts) {
+        if (event.affectedInstances.size() == deprovisionedHosts.size())
             return;
         Issue issue = eventFetcher.createIssue(event);
         if (!issueHandler.issueExists(issue)) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java
index 231c5876c4b..cd2a4fd8453 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/CloudEventReporterTest.java
@@ -10,7 +10,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.MockIssueHandler;
 import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock;
-import com.yahoo.vespa.jdk8compat.Set;
 import org.junit.Test;
 
 import java.time.Duration;
@@ -18,6 +17,7 @@ import java.util.Arrays;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.*;
@@ -46,29 +46,24 @@ public class CloudEventReporterTest {
         setUpZones();
         CloudEventReporter cloudEventReporter = new CloudEventReporter(tester.controller(), Duration.ofMinutes(15), new JobControl(tester.curator()));
 
-        equalListsIgnoreOrder(List.of("host1.com", "host2.com", "host3.com"), getHostnames(nonAwsZone.getId()));
-        equalListsIgnoreOrder(List.of("host1.com", "host2.com", "host3.com"), getHostnames(awsZone1.getId()));
-        equalListsIgnoreOrder(List.of("host4.com", "host5.com", "confighost.com"), getHostnames(awsZone2.getId()));
+        assertEquals(Set.of("host1.com", "host2.com", "host3.com"), getHostnames(nonAwsZone.getId()));
+        assertEquals(Set.of("host1.com", "host2.com", "host3.com"), getHostnames(awsZone1.getId()));
+        assertEquals(Set.of("host4.com", "host5.com", "confighost.com"), getHostnames(awsZone2.getId()));
 
         mockEvents();
         cloudEventReporter.maintain();
 
-        equalListsIgnoreOrder(List.of("host1.com", "host2.com", "host3.com"), getHostnames(nonAwsZone.getId()));
-        equalListsIgnoreOrder(List.of("host3.com"), getHostnames(awsZone1.getId()));
-        equalListsIgnoreOrder(List.of("host4.com", "confighost.com"), getHostnames(awsZone2.getId()));
+        assertEquals(Set.of("host1.com", "host2.com", "host3.com"), getHostnames(nonAwsZone.getId()));
+        assertEquals(Set.of("host3.com"), getHostnames(awsZone1.getId()));
+        assertEquals(Set.of("host4.com", "confighost.com"), getHostnames(awsZone2.getId()));
 
         Map<IssueId, MockIssueHandler.MockIssue> createdIssues = tester.serviceRegistry().issueHandler().issues();
         assertEquals(1, createdIssues.size());
         String description = createdIssues.get(IssueId.from("1")).issue().description();
-        assertEquals("[confighost]", description);
+        assertTrue(description.contains("confighost"));
 
     }
 
-    private void equalListsIgnoreOrder(List<?> l1, List<?> l2) {
-        assertTrue(l1 + " and " + l2 + " should be equal",
-                l1.size() == l2.size() && l1.containsAll(l2));
-    }
-
     private void mockEvents() {
         MockAwsEventFetcher mockAwsEventFetcher = (MockAwsEventFetcher)tester.controller().serviceRegistry().eventFetcherService();
 
@@ -144,11 +139,11 @@ public class CloudEventReporterTest {
                 .build();
     }
 
-    private List<String> getHostnames(ZoneId zoneId) {
+    private Set<String> getHostnames(ZoneId zoneId) {
         return tester.configServer().nodeRepository().list(zoneId)
                 .stream()
                 .map(node -> node.hostname().value())
-                .collect(Collectors.toList());
+                .collect(Collectors.toSet());
     }
 
     private ZoneApiMock createZone(String zoneId, String cloudNativeRegionName, String cloud) {