Only consider production deployments

When reasoning about versions, we should not take deployments to "dev"
and "perf" into account.

4 files changed, 26 insertions, 23 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
index 8a49ab1083e..ab33a99298e 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
@@ -23,6 +23,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Collector;
 import java.util.stream.Collectors;
 
 /**
@@ -93,7 +95,17 @@ public class Application {
     
     /** Returns an immutable map of the current deployments of this */
     public Map<Zone, Deployment> deployments() { return deployments; }
-    
+
+    /** 
+     * Returns an immutable map of the current *production* deployments of this
+     * (deployments also includes manually deployed environments)
+     */
+    public Map<Zone, Deployment> productionDeployments() {
+        return deployments.values().stream()
+                                  .filter(deployment -> deployment.zone().environment() == Environment.prod)
+                                   .collect(Collectors.toMap(Deployment::zone, Function.identity()));
+    }
+
     public DeploymentJobs deploymentJobs() { return deploymentJobs; }
 
     /**
@@ -113,11 +125,10 @@ public class Application {
      * or empty version if it is not deployed anywhere
      */
     public Optional<Version> deployedVersion() {
-        return deployments().values().stream()
-                                     .filter(deployment -> isPermanent(deployment.zone().environment()))
-                                     .sorted(Comparator.comparing(Deployment::version))
-                                     .findFirst()
-                                     .map(Deployment::version);
+        return productionDeployments().values().stream()
+                                               .sorted(Comparator.comparing(Deployment::version))
+                                               .findFirst()
+                                               .map(Deployment::version);
     }
 
     /** The version that should be used to compile this application */
@@ -268,14 +279,6 @@ public class Application {
         return "application '" + id + "'";
     }
 
-    private boolean isPermanent(Environment environment) {
-        if (environment == Environment.dev) return false;
-        if (environment == Environment.perf) return false;
-        if (environment == Environment.test) return false;
-        if (environment == Environment.staging) return false;
-        return true;
-    }
-    
     /** Returns true if there is no current change to deploy - i.e deploying is empty or completely deployed */
     public boolean deployingCompleted() { 
         if ( ! deploying.isPresent()) return true;
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 530516b8e26..8d0cdc1d041 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -355,8 +355,7 @@ public class ApplicationController {
     }
 
     private Application deleteRemovedDeployments(Application application, Lock lock) {
-        List<Deployment> deploymentsToRemove = application.deployments().values().stream()
-                .filter(deployment -> deployment.zone().environment() == Environment.prod)
+        List<Deployment> deploymentsToRemove = application.productionDeployments().values().stream()
                 .filter(deployment -> ! application.deploymentSpec().includes(deployment.zone().environment(), 
                                                                               Optional.of(deployment.zone().region())))
                 .collect(Collectors.toList());
@@ -482,7 +481,7 @@ public class ApplicationController {
         try (Lock lock = lock(id)) {
             Optional<Application> application = get(id);
             if ( ! application.isPresent()) return null;
-            if ( ! application.get().deployments().isEmpty())
+            if ( ! application.get().productionDeployments().isEmpty())
                 throw new IllegalArgumentException("Could not delete '" + application + "': It has active deployments");
             
             Tenant tenant = controller.tenants().tenant(new TenantId(id.tenant().value())).get();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
index 7ff5a23e178..785aeefc57f 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
@@ -80,7 +80,7 @@ public class ApplicationList {
 
     /** Returns the subset of applications which have at least one deployment */
     public ApplicationList hasDeployment() {
-        return listOf(list.stream().filter(a -> !a.deployments().isEmpty()));
+        return listOf(list.stream().filter(a -> !a.productionDeployments().isEmpty()));
     }
 
     /** Returns the subset of applications which started failing after the given instant */
@@ -101,7 +101,8 @@ public class ApplicationList {
     /** Returns the subset of applications which have at least one deployment on a lower version than the given one */
     public ApplicationList onLowerVersionThan(Version version) {
         return listOf(list.stream()
-                          .filter(a -> a.deployments().values().stream().anyMatch(d -> d.version().isBefore(version))));
+                          .filter(a -> a.productionDeployments().values().stream()
+                                                                         .anyMatch(d -> d.version().isBefore(version))));
     }
 
     /**
@@ -114,8 +115,7 @@ public class ApplicationList {
 
     /** Returns the subset of applications which have at least one production deployment */
     public ApplicationList hasProductionDeployment() {
-        return listOf(list.stream().filter(a -> a.deployments().keySet().stream()
-                .anyMatch(zone -> zone.environment() == Environment.prod)));
+        return listOf(list.stream().filter(a -> ! a.productionDeployments().isEmpty()));
     }
 
     /** Returns the subset of applications that are allowed to upgrade at the given time */
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 c3b2da42861..73e4eb4d527 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
@@ -136,8 +136,9 @@ public class VersionStatus {
         for (Application application : ApplicationList.from(applications).notPullRequest().asList()) {
             DeploymentJobs jobs = application.deploymentJobs();
 
-            // Note that each version deployed on this application exists
-            for (Deployment deployment : application.deployments().values()) {
+            // Note that each version deployed on this application in production exists
+            // (ignore non-production versions)
+            for (Deployment deployment : application.productionDeployments().values()) {
                 versionMap.computeIfAbsent(deployment.version(), DeploymentStatistics::empty);
             }