Merge pull request #8279 from vespa-engine/mpolden/log-os-version-change

Log when system changes OS version

4 files changed, 32 insertions, 9 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
index dd89dbc14ae..745436e8924 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
@@ -26,6 +26,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.github.GitHub;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.Mailer;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.CloudName;
+import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 import com.yahoo.vespa.hosted.controller.athenz.impl.ZmsClientFacade;
 import com.yahoo.vespa.hosted.controller.deployment.JobController;
@@ -48,6 +49,7 @@ import java.util.TreeSet;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
 /**
  * API to the controller. This contains the object model of everything the controller cares about, mainly tenants and
@@ -231,7 +233,7 @@ public class Controller extends AbstractComponent {
         if (version.isEmpty()) {
             throw new IllegalArgumentException("Invalid version '" + version.toFullString() + "'");
         }
-        if (zoneRegistry.zones().all().ids().stream().noneMatch(zone -> cloud.equals(zone.cloud()))) {
+        if (!clouds().contains(cloud)) {
             throw new IllegalArgumentException("Cloud '" + cloud.value() + "' does not exist in this system");
         }
         try (Lock lock = curator.lockOsVersions()) {
@@ -255,6 +257,14 @@ public class Controller extends AbstractComponent {
     /** Replace the current OS version status with a new one */
     public void updateOsVersionStatus(OsVersionStatus newStatus) {
         try (Lock lock = curator.lockOsVersionStatus()) {
+            OsVersionStatus currentStatus = curator.readOsVersionStatus();
+            for (CloudName cloud : clouds()) {
+                Set<Version> newVersions = newStatus.versionsIn(cloud);
+                if (currentStatus.versionsIn(cloud).size() > 1 && newVersions.size() == 1) {
+                    log.info("All nodes in " + cloud + " cloud upgraded to OS version " +
+                             newVersions.iterator().next());
+                }
+            }
             curator.writeOsVersionStatus(newStatus);
         }
     }
@@ -288,6 +298,12 @@ public class Controller extends AbstractComponent {
         return curator;
     }
 
+    private Set<CloudName> clouds() {
+        return zoneRegistry.zones().all().ids().stream()
+                           .map(ZoneId::cloud)
+                           .collect(Collectors.toUnmodifiableSet());
+    }
+
     private static String printableVersion(Optional<VespaVersion> vespaVersion) {
         return vespaVersion.map(v -> v.versionNumber().toFullString()).orElse("unknown");
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
index 7f3b2400736..dcdd28544c1 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
@@ -61,7 +61,7 @@ public class OsUpgrader extends InfrastructureUpgrader {
     protected Optional<Version> targetVersion() {
         // Return target if we have nodes in this cloud on a lower version
         return controller().osVersion(cloud)
-                           .filter(target -> controller().osVersionStatus().nodeVersionsIn(cloud).stream()
+                           .filter(target -> controller().osVersionStatus().nodesIn(cloud).stream()
                                                          .anyMatch(node -> node.version().isBefore(target.version())))
                            .map(OsVersion::version);
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java
index 871a7872a9c..aa174b0df2b 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java
@@ -19,10 +19,9 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.stream.Collectors;
 
-import static java.util.stream.Collectors.collectingAndThen;
-
 /**
  * Information about OS versions in this system.
  *
@@ -44,12 +43,20 @@ public class OsVersionStatus {
         return versions;
     }
 
-    /** Returns node versions that exist in given cloud */
-    public List<Node> nodeVersionsIn(CloudName cloud) {
+    /** Returns nodes eligible for OS upgrades that exist in given cloud */
+    public List<Node> nodesIn(CloudName cloud) {
         return versions.entrySet().stream()
                        .filter(entry -> entry.getKey().cloud().equals(cloud))
                        .flatMap(entry -> entry.getValue().stream())
-                       .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList));
+                       .collect(Collectors.toUnmodifiableList());
+    }
+
+    /** Returns versions that exist in given cloud */
+    public Set<Version> versionsIn(CloudName cloud) {
+        return versions.keySet().stream()
+                       .filter(osVersion -> osVersion.cloud().equals(cloud))
+                       .map(OsVersion::version)
+                       .collect(Collectors.toUnmodifiableSet());
     }
 
     /** Compute the current OS versions in this system. This is expensive and should be called infrequently */
@@ -84,7 +91,7 @@ public class OsVersionStatus {
         return controller.zoneRegistry().osUpgradePolicies().stream()
                          .flatMap(upgradePolicy -> upgradePolicy.asList().stream())
                          .flatMap(Collection::stream)
-                         .collect(Collectors.toList());
+                         .collect(Collectors.toUnmodifiableList());
     }
 
     /** A node in this system and its current OS version */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
index c3915a4b772..c406cd346be 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
@@ -114,7 +114,7 @@ public class OsUpgraderTest {
         osUpgrader.maintain();
         assertWanted(version1, SystemApplication.zone, zone1, zone2, zone3, zone4);
         statusUpdater.maintain();
-        assertTrue("All nodes on target version", tester.controller().osVersionStatus().nodeVersionsIn(cloud).stream()
+        assertTrue("All nodes on target version", tester.controller().osVersionStatus().nodesIn(cloud).stream()
                                                         .allMatch(node -> node.version().equals(version1)));
     }