Keep unofficial version if root version is active

3 files changed, 27 insertions, 15 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
index e937e8af60d..add95c2f7b2 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
@@ -85,7 +85,11 @@ public record VersionStatus(List<VespaVersion> versions) {
 
     /** Returns whether given version is active in this system */
     public boolean isActive(Version version) {
-        return version(version) != null;
+        if (version(version) != null) return true;
+        // Occasionally we may deploy unofficial versions of a given Vespa version, i.e. given the version 8.42.1,
+        // an unofficial version 8.42.1.a may exist. Count such versions as active if their root version is active
+        Version rootVersion = new Version(version.getMajor(), version.getMinor(), version.getMicro());
+        return version(rootVersion) != null;
     }
 
     /** Create the empty version status */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRegistryMock.java
index ef0445e06c6..a36bddc618e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRegistryMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRegistryMock.java
@@ -8,7 +8,6 @@ import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 /**
  * @author mpolden
@@ -22,6 +21,7 @@ public class ArtifactRegistryMock implements ArtifactRegistry {
     private final Map<String, Artifact> images = new HashMap<>();
 
     public ArtifactRegistryMock add(Artifact image) {
+        if (images.containsKey(image.id())) throw new IllegalArgumentException("Image with ID '" + image.id() + "' already exists");
         images.put(image.id(), image);
         return this;
     }
@@ -33,7 +33,7 @@ public class ArtifactRegistryMock implements ArtifactRegistry {
 
     @Override
     public List<Artifact> list() {
-        return images.values().stream().sorted(comparator).collect(Collectors.toUnmodifiableList());
+        return images.values().stream().sorted(comparator).toList();
     }
 
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirerTest.java
index c0f0322d192..6280725794c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ArtifactExpirerTest.java
@@ -27,35 +27,43 @@ public class ArtifactExpirerTest {
 
         Instant instant = tester.clock().instant();
         Artifact image0 = new Artifact("image0", "registry.example.com", "vespa/vespa", "7.1", instant, Version.fromString("7.1"));
-        Artifact image1 = new Artifact("image1", "registry.example.com", "vespa/vespa", "7.2-amd64", instant, Version.fromString("7.2"));
-        Artifact image2 = new Artifact("image2", "registry.example.com", "vespa/vespa", "7.4-amd64", instant, Version.fromString("7.4"));
+        Artifact image1 = new Artifact("image1", "registry.example.com", "vespa/vespa", "7.2.42-amd64", instant, Version.fromString("7.2.42"));
+        Artifact image2 = new Artifact("image2", "registry.example.com", "vespa/vespa", "7.2.42.a-amd64", instant, Version.fromString("7.2.42.a"));
+        Artifact image3 = new Artifact("image3", "registry.example.com", "vespa/vespa", "7.4-amd64", instant, Version.fromString("7.4"));
         registry.add(image0)
                 .add(image1)
-                .add(image2);
+                .add(image2)
+                .add(image3);
 
         // Make one image active
         tester.controllerTester().upgradeSystem(image1.version());
 
         // Nothing is expired initially
         expirer.maintain();
-        assertEquals(List.of(image0, image1, image2), registry.list());
+        assertEquals(List.of(image0, image1, image2, image3), registry.list());
 
-        // Nothing happens as not enough time has passed since image creation
+        // Nothing is expired as not enough time has passed since image creation
         tester.clock().advance(Duration.ofDays(1));
         expirer.maintain();
-        assertEquals(List.of(image0, image1, image2), registry.list());
+        assertEquals(List.of(image0, image1, image2, image3), registry.list());
 
         // Enough time passes to expire unused image
         tester.clock().advance(Duration.ofDays(13).plus(Duration.ofSeconds(1)));
         expirer.maintain();
-        assertEquals(List.of(image1, image2), registry.list());
+        assertEquals(List.of(image1, image2, image3), registry.list());
 
-        // A new version becomes active. The active and future version are kept
-        Artifact image3 = new Artifact("image3", "registry.example.com", "vespa/vespa", "7.3-arm64", tester.clock().instant(), Version.fromString("7.3"));
-        registry.add(image3);
-        tester.controllerTester().upgradeSystem(image3.version());
+        // A new version becomes is published and controllers upgrade. This version, the system version + its unofficial
+        // version and future versions are all kept
+        Artifact image4 = new Artifact("image4", "registry.example.com", "vespa/vespa", "7.3.0-arm64", tester.clock().instant(), Version.fromString("7.3.0"));
+        registry.add(image4);
+        tester.controllerTester().upgradeController(image4.version());
         expirer.maintain();
-        assertEquals(List.of(image3, image2), registry.list());
+        assertEquals(List.of(image1, image2, image4, image3), registry.list());
+
+        // The system upgrades, only the active and future version are kept
+        tester.controllerTester().upgradeSystem(image4.version());
+        expirer.maintain();
+        assertEquals(List.of(image4, image3), registry.list());
     }
 
 }