Revert "Expire stale container images"

9 files changed, 2 insertions, 192 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationMetaDataGarbageCollector.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationMetaDataGarbageCollector.java
index 42b442bf7b0..9fa3b91f633 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationMetaDataGarbageCollector.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationMetaDataGarbageCollector.java
@@ -6,9 +6,6 @@ import java.time.Duration;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-/**
- * @author jvenstad
- */
 public class ApplicationMetaDataGarbageCollector extends ControllerMaintainer {
 
     private static final Logger log = Logger.getLogger(ApplicationMetaDataGarbageCollector.class.getName());
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContainerImageExpirer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContainerImageExpirer.java
deleted file mode 100644
index 80a79d004c6..00000000000
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContainerImageExpirer.java
+++ /dev/null
@@ -1,71 +0,0 @@
-// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.controller.maintenance;
-
-import com.yahoo.component.Version;
-import com.yahoo.config.provision.SystemName;
-import com.yahoo.vespa.hosted.controller.Controller;
-import com.yahoo.vespa.hosted.controller.api.integration.container.ContainerImage;
-import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
-import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
-
-import java.time.Duration;
-import java.time.Instant;
-import java.util.Comparator;
-import java.util.EnumSet;
-import java.util.List;
-import java.util.Set;
-import java.util.logging.Level;
-import java.util.logging.Logger;
-import java.util.stream.Collectors;
-
-/**
- * Periodically expire unused container images.
- *
- * @author mpolden
- */
-public class ContainerImageExpirer extends ControllerMaintainer {
-
-    private static final Logger log = Logger.getLogger(ContainerImageExpirer.class.getName());
-
-    private static final Duration MIN_AGE = Duration.ofDays(14);
-
-    public ContainerImageExpirer(Controller controller, Duration interval) {
-        super(controller, interval, null, expiringSystems());
-    }
-
-    @Override
-    protected boolean maintain() {
-        Instant now = controller().clock().instant();
-        VersionStatus versionStatus = controller().readVersionStatus();
-        List<ContainerImage> imagesToExpire = controller().serviceRegistry().containerRegistry().list().stream()
-                                                          .filter(image -> canExpire(image, now, versionStatus))
-                                                          .collect(Collectors.toList());
-        if (!imagesToExpire.isEmpty()) {
-            log.log(Level.INFO, "Expiring container images: " + imagesToExpire);
-            controller().serviceRegistry().containerRegistry().deleteAll(imagesToExpire);
-        }
-        return true;
-    }
-
-    /** Returns whether given image can be expired */
-    private boolean canExpire(ContainerImage image, Instant now, VersionStatus versionStatus) {
-        List<VespaVersion> versions = versionStatus.versions();
-        if (versions.isEmpty()) return false;
-
-        if (versionStatus.isActive(image.version())) return false;
-        if (image.createdAt().isAfter(now.minus(MIN_AGE))) return false;
-
-        Version maxVersion = versions.stream().map(VespaVersion::versionNumber).max(Comparator.naturalOrder()).get();
-        if (image.version().isAfter(maxVersion)) return false; // A future version
-
-        return true;
-    }
-
-    /** Returns systems where images can be expired */
-    private static Set<SystemName> expiringSystems() {
-        // Run only in public and main. Public systems have distinct container registries, while main and CD have
-        // shared registries.
-        return EnumSet.of(SystemName.Public, SystemName.PublicCd, SystemName.main);
-    }
-
-}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
index bc63c235027..6731c30ecd7 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
@@ -46,7 +46,6 @@ public class ControllerMaintenance extends AbstractComponent {
     private final SystemRoutingPolicyMaintainer systemRoutingPolicyMaintainer;
     private final ApplicationMetaDataGarbageCollector applicationMetaDataGarbageCollector;
     private final HostRepairMaintainer hostRepairMaintainer;
-    private final ContainerImageExpirer containerImageExpirer;
 
 
     @Inject
@@ -79,7 +78,6 @@ public class ControllerMaintenance extends AbstractComponent {
         systemRoutingPolicyMaintainer = new SystemRoutingPolicyMaintainer(controller, Duration.ofMinutes(10));
         applicationMetaDataGarbageCollector = new ApplicationMetaDataGarbageCollector(controller, Duration.ofHours(12));
         hostRepairMaintainer = new HostRepairMaintainer(controller, Duration.ofHours(12));
-        containerImageExpirer = new ContainerImageExpirer(controller, Duration.ofHours(2));
     }
 
     public Upgrader upgrader() { return upgrader; }
@@ -107,9 +105,7 @@ public class ControllerMaintenance extends AbstractComponent {
         rotationStatusUpdater.close();
         resourceTagMaintainer.close();
         systemRoutingPolicyMaintainer.close();
-        applicationMetaDataGarbageCollector.close();
         hostRepairMaintainer.close();
-        containerImageExpirer.close();
     }
 
     /** Create one OS upgrader per cloud found in the zone registry of controller */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index d0fa0f1a7f0..0541cc91159 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -1426,7 +1426,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
             VersionStatus versionStatus = controller.readVersionStatus();
             if (version.equals(Version.emptyVersion))
                 version = controller.systemVersion(versionStatus);
-            if (!versionStatus.isActive(version))
+            if ( versionStatus.version(version) == null)
                 throw new IllegalArgumentException("Cannot trigger deployment of version '" + version + "': " +
                                                    "Version is not active in this system. " +
                                                    "Active versions: " + versionStatus.versions()
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 a30409dfa80..022ccbe266c 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
@@ -75,11 +75,6 @@ public class VersionStatus {
         return versions.stream().filter(v -> v.versionNumber().equals(version)).findFirst().orElse(null);
     }
 
-    /** Returns whether given version is active in this system */
-    public boolean isActive(Version version) {
-        return version(version) != null;
-    }
-
     /** Create the empty version status */
     public static VersionStatus empty() { return new VersionStatus(List.of()); }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ContainerRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ContainerRegistryMock.java
deleted file mode 100644
index 9fa2867631a..00000000000
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ContainerRegistryMock.java
+++ /dev/null
@@ -1,39 +0,0 @@
-// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.controller.integration;
-
-import com.yahoo.vespa.hosted.controller.api.integration.container.ContainerImage;
-import com.yahoo.vespa.hosted.controller.api.integration.container.ContainerRegistry;
-
-import java.util.Comparator;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.stream.Collectors;
-
-/**
- * @author mpolden
- */
-public class ContainerRegistryMock implements ContainerRegistry {
-
-    private static final Comparator<ContainerImage> comparator = Comparator.comparing(ContainerImage::registry)
-                                                                           .thenComparing(ContainerImage::repository)
-                                                                           .thenComparing(ContainerImage::version);
-
-    private final Map<String, ContainerImage> images = new HashMap<>();
-
-    public ContainerRegistryMock add(ContainerImage image) {
-        images.put(image.id(), image);
-        return this;
-    }
-
-    @Override
-    public void deleteAll(List<ContainerImage> images) {
-        images.forEach(image -> this.images.remove(image.id()));
-    }
-
-    @Override
-    public List<ContainerImage> list() {
-        return images.values().stream().sorted(comparator).collect(Collectors.toUnmodifiableList());
-    }
-
-}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
index ea31667d249..3ec02c6ceb7 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java
@@ -21,6 +21,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.entity.MemoryEntityServ
 import com.yahoo.vespa.hosted.controller.api.integration.organization.MockContactRetriever;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.MockIssueHandler;
 import com.yahoo.vespa.hosted.controller.api.integration.repair.MockRepairClient;
+import com.yahoo.vespa.hosted.controller.api.integration.repair.HostRepairClient;
 import com.yahoo.vespa.hosted.controller.api.integration.resource.CostReportConsumerMock;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.GlobalRoutingService;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.MemoryGlobalRoutingService;
@@ -63,7 +64,6 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
     private final ApplicationRoleService applicationRoleService = new NoopApplicationRoleService();
     private final BillingController billingController = new MockBillingController();
     private final MockRepairClient repairClient = new MockRepairClient();
-    private final ContainerRegistryMock containerRegistry = new ContainerRegistryMock();
 
     public ServiceRegistryMock(SystemName system) {
         this.zoneRegistryMock = new ZoneRegistryMock(system);
@@ -200,11 +200,6 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg
         return repairClient;
     }
 
-    @Override
-    public ContainerRegistryMock containerRegistry() {
-        return containerRegistry;
-    }
-
     public ConfigServerMock configServerMock() {
         return configServerMock;
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContainerImageExpirerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContainerImageExpirerTest.java
deleted file mode 100644
index 36a0c57f716..00000000000
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContainerImageExpirerTest.java
+++ /dev/null
@@ -1,60 +0,0 @@
-// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.controller.maintenance;
-
-import com.yahoo.component.Version;
-import com.yahoo.vespa.hosted.controller.api.integration.container.ContainerImage;
-import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
-import com.yahoo.vespa.hosted.controller.integration.ContainerRegistryMock;
-import org.junit.Test;
-
-import java.time.Duration;
-import java.time.Instant;
-import java.util.List;
-
-import static org.junit.Assert.assertEquals;
-
-/**
- * @author mpolden
- */
-public class ContainerImageExpirerTest {
-
-    @Test
-    public void maintain() {
-        DeploymentTester tester = new DeploymentTester();
-        ContainerImageExpirer expirer = new ContainerImageExpirer(tester.controller(), Duration.ofDays(1));
-        ContainerRegistryMock registry = tester.controllerTester().serviceRegistry().containerRegistry();
-
-        Instant instant = tester.clock().instant();
-        ContainerImage image0 = new ContainerImage("image0", "registry.example.com", "vespa/vespa", instant, Version.fromString("7.1"));
-        ContainerImage image1 = new ContainerImage("image1", "registry.example.com", "vespa/vespa", instant, Version.fromString("7.2"));
-        ContainerImage image2 = new ContainerImage("image2", "registry.example.com", "vespa/vespa", instant, Version.fromString("7.4"));
-        registry.add(image0)
-                .add(image1)
-                .add(image2);
-
-        // Make one image active
-        tester.controllerTester().upgradeSystem(image1.version());
-
-        // Nothing is expired initially
-        expirer.maintain();
-        assertEquals(List.of(image0, image1, image2), registry.list());
-
-        // Nothing happens 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());
-
-        // 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());
-
-        // A new version becomes active. The active and future version are kept
-        ContainerImage image3 = new ContainerImage("image3", "registry.example.com", "vespa/vespa", tester.clock().instant(), Version.fromString("7.3"));
-        registry.add(image3);
-        tester.controllerTester().upgradeSystem(image3.version());
-        expirer.maintain();
-        assertEquals(List.of(image3, image2), registry.list());
-    }
-
-}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json
index c1ee1489cd4..bb3578b2482 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/responses/maintenance.json
@@ -13,9 +13,6 @@
       "name": "ContactInformationMaintainer"
     },
     {
-      "name": "ContainerImageExpirer"
-    },
-    {
       "name": "CostReportMaintainer"
     },
     {