Merge pull request #24613 from vespa-engine/jonmv/no-real-changes

No real changes

4 files changed, 46 insertions, 53 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
index c09fff19d58..e16b1378377 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
@@ -625,20 +625,18 @@ public class DeploymentStatus {
 
         // If neither change is ready, we guess based on the specified rollout.
         if (platformReadyAt.isEmpty() && revisionReadyAt.isEmpty()) {
-            switch (rollout) {
-                case separate: return List.of(change.withoutApplication(), change);  // Platform should stay ahead.
-                case leading: return List.of(change);                                // They should eventually join.
-                case simultaneous: return List.of(change.withoutPlatform(), change); // Revision should get ahead.
-            }
+            return switch (rollout) {
+                case separate -> List.of(change.withoutApplication(), change);  // Platform should stay ahead.
+                case leading -> List.of(change);                                // They should eventually join.
+                case simultaneous -> List.of(change.withoutPlatform(), change); // Revision should get ahead.
+            };
         }
 
         // If only the revision is ready, we run that first.
         if (platformReadyAt.isEmpty()) return List.of(change.withoutPlatform(), change);
 
         // If only the platform is ready, we run that first.
-        if (revisionReadyAt.isEmpty()) {
-            return List.of(change.withoutApplication(), change);
-        }
+        if (revisionReadyAt.isEmpty()) return List.of(change.withoutApplication(), change);
 
         // Both changes are ready for this step, and we look to the specified rollout to decide.
         boolean platformReadyFirst = platformReadyAt.get().isBefore(revisionReadyAt.get());
@@ -646,21 +644,20 @@ public class DeploymentStatus {
         boolean failingUpgradeOnlyTests = ! jobs().type(systemTest(job.type()), stagingTest(job.type()))
                                                   .failingHardOn(Versions.from(change.withoutApplication(), application, deploymentFor(job), () -> systemVersion))
                                                   .isEmpty();
-        switch (rollout) {
-            case separate:      // Let whichever change rolled out first, keep rolling first, unless upgrade alone is failing.
-                return (platformReadyFirst || platformReadyAt.get().equals(Instant.EPOCH)) // Assume platform was first if no jobs have run yet.
-                       ? step.job().flatMap(jobs()::get).flatMap(JobStatus::firstFailing).isPresent() || failingUpgradeOnlyTests
-                         ? List.of(change)                                 // Platform was first, but is failing.
-                         : List.of(change.withoutApplication(), change)    // Platform was first, and is OK.
-                       : revisionReadyFirst
-                         ? List.of(change.withoutPlatform(), change)       // Revision was first.
-                         : List.of(change);                                // Both ready at the same time, probably due to earlier failure.
-            case leading:      // When one change catches up, they fuse and continue together.
-                return List.of(change);
-            case simultaneous: // Revisions are allowed to run ahead, but the job where it caught up should have both changes.
-                return platformReadyFirst ? List.of(change) : List.of(change.withoutPlatform(), change);
-            default: throw new IllegalStateException("Unknown upgrade rollout policy");
-        }
+        return switch (rollout) {
+            case separate ->      // Let whichever change rolled out first, keep rolling first, unless upgrade alone is failing.
+                    (platformReadyFirst || platformReadyAt.get().equals(Instant.EPOCH)) // Assume platform was first if no jobs have run yet.
+                    ? step.job().flatMap(jobs()::get).flatMap(JobStatus::firstFailing).isPresent() || failingUpgradeOnlyTests
+                      ? List.of(change)                                 // Platform was first, but is failing.
+                      : List.of(change.withoutApplication(), change)    // Platform was first, and is OK.
+                    : revisionReadyFirst
+                      ? List.of(change.withoutPlatform(), change)       // Revision was first.
+                      : List.of(change);                                // Both ready at the same time, probably due to earlier failure.
+            case leading ->      // When one change catches up, they fuse and continue together.
+                    List.of(change);
+            case simultaneous -> // Revisions are allowed to run ahead, but the job where it caught up should have both changes.
+                    platformReadyFirst ? List.of(change) : List.of(change.withoutPlatform(), change);
+        };
     }
 
     /** The test jobs that need to run prior to the given production deployment jobs. */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
index d9726edc496..ea9cf0385d9 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
@@ -59,7 +59,6 @@ import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.sta
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.systemTest;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.testApNortheast1;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.testApNortheast2;
-import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.testAwsUsEast1a;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.testEuWest1;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.testUsCentral1;
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.testUsEast3;
@@ -70,7 +69,6 @@ import static java.util.Collections.emptyList;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index 5b141716eaa..07d9efdf8fc 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -402,30 +402,28 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
                                                                    Optional.of("dns-zone-1"))));
         }
 
-        // TODO jonmv: compute on deploy, not when getting the result.
-        return () -> {
-            Application application = applications.get(id);
-            application.activate();
-            List<Node> nodes = nodeRepository.list(id.zoneId(), NodeFilter.all().applications(id.applicationId()));
-            for (Node node : nodes) {
-                nodeRepository.putNodes(id.zoneId(), Node.builder(node)
-                                                         .state(Node.State.active)
-                                                         .wantedVersion(application.version().get())
-                                                         .build());
-            }
-            serviceStatus.put(id, new ServiceConvergence(id.applicationId(),
-                                                         id.zoneId(),
-                                                         false,
-                                                         2,
-                                                         nodes.stream()
-                                                                      .map(node -> new ServiceConvergence.Status(node.hostname(),
-                                                                                                                 43,
-                                                                                                                 "container",
-                                                                                                                 1))
-                                                                      .collect(Collectors.toList())));
-
-            return new DeploymentResult("foo", warnings.getOrDefault(id, List.of()));
-        };
+        Application application = applications.get(id);
+        application.activate();
+        List<Node> nodes = nodeRepository.list(id.zoneId(), NodeFilter.all().applications(id.applicationId()));
+        for (Node node : nodes) {
+            nodeRepository.putNodes(id.zoneId(), Node.builder(node)
+                                                     .state(Node.State.active)
+                                                     .wantedVersion(application.version().get())
+                                                     .build());
+        }
+        serviceStatus.put(id, new ServiceConvergence(id.applicationId(),
+                                                     id.zoneId(),
+                                                     false,
+                                                     2,
+                                                     nodes.stream()
+                                                          .map(node -> new ServiceConvergence.Status(node.hostname(),
+                                                                                                     43,
+                                                                                                     "container",
+                                                                                                     1))
+                                                          .collect(Collectors.toList())));
+
+        DeploymentResult result = new DeploymentResult("foo", warnings.getOrDefault(id, List.of()));
+        return () -> result;
     }
 
     @Override
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
index 45038fc4a63..11110d6edaa 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
@@ -747,13 +747,13 @@ public class UpgraderTest {
 
         // Application change recorded together with ongoing upgrade
         assertTrue(app.instance().change().platform().get().equals(version) &&
-                app.instance().change().revision().get().equals(revision),
-                "Change contains both upgrade and application change");
+                   app.instance().change().revision().get().equals(revision),
+                   "Change contains both upgrade and application change");
 
         // Deployment completes
         app.runJob(systemTest).runJob(stagingTest)
-                .runJob(productionUsWest1)
-                .runJob(productionUsEast3);
+           .runJob(productionUsWest1)
+           .runJob(productionUsEast3);
         assertEquals(List.of(), tester.jobs().active(), "All jobs consumed");
 
         for (Deployment deployment : app.instance().deployments().values()) {