Revert "Allow deploy without project"

5 files changed, 86 insertions, 98 deletions

diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerClientMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerClientMock.java
index 13525969358..6018c99206e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerClientMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerClientMock.java
@@ -51,17 +51,8 @@ public class ConfigServerClientMock extends AbstractComponent implements ConfigS
     /** The exception to throw on the next prepare run, or null to continue normally */
     private RuntimeException prepareException = null;
     
-    private Optional<Version> lastPrepareVersion = Optional.empty();
-
-    /** The version given in the previous prepare call, or empty if no call has been made */
-    public Optional<Version> lastPrepareVersion() {
-        return lastPrepareVersion;
-    }
-
-    /** Return map of applications that may have been activated */
-    public Map<ApplicationId, Boolean> activated() {
-        return Collections.unmodifiableMap(applicationActivated);
-    }
+    /** The version given in the previous prepare call, or null if no call has been made */
+    public Optional<Version> lastPrepareVersion = null;
     
     @Override
     public PreparedApplication prepare(DeploymentId deployment, DeployOptions deployOptions, Set<String> rotationCnames, Set<Rotation> rotations, byte[] content) {
@@ -210,5 +201,4 @@ public class ConfigServerClientMock extends AbstractComponent implements ConfigS
                 ? endpoints.get(endpoint)
                 : result;
     }
-
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index 61cc84a07ba..91fca0d37d1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -17,10 +17,16 @@ import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.api.Tenant;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus;
+import com.yahoo.vespa.hosted.controller.api.application.v4.model.GitRevision;
+import com.yahoo.vespa.hosted.controller.api.application.v4.model.ScrewdriverBuildJob;
 import com.yahoo.vespa.hosted.controller.api.identifiers.AthensDomain;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
+import com.yahoo.vespa.hosted.controller.api.identifiers.GitBranch;
+import com.yahoo.vespa.hosted.controller.api.identifiers.GitCommit;
+import com.yahoo.vespa.hosted.controller.api.identifiers.GitRepository;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Property;
 import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId;
+import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserGroup;
 import com.yahoo.vespa.hosted.controller.api.integration.BuildService.BuildJob;
@@ -211,7 +217,7 @@ public class ControllerTest {
 
         app1 = applications.require(app1.id());
         assertEquals("First deployment gets system version", systemVersion, app1.deployedVersion().get());
-        assertEquals(systemVersion, tester.configServer().lastPrepareVersion().get());
+        assertEquals(systemVersion, tester.configServer().lastPrepareVersion.get());
 
         // Unexpected deployment
         tester.deploy(productionUsWest1, app1, applicationPackage);
@@ -234,7 +240,7 @@ public class ControllerTest {
 
         app1 = applications.require(app1.id());
         assertEquals("Application change preserves version", systemVersion, app1.deployedVersion().get());
-        assertEquals(systemVersion, tester.configServer().lastPrepareVersion().get());
+        assertEquals(systemVersion, tester.configServer().lastPrepareVersion.get());
 
         // A deployment to the new region gets the same version
         applicationPackage = new ApplicationPackageBuilder()
@@ -245,7 +251,7 @@ public class ControllerTest {
         tester.deployAndNotify(app1, applicationPackage, true, productionUsEast3);
         app1 = applications.require(app1.id());
         assertEquals("Application change preserves version", systemVersion, app1.deployedVersion().get());
-        assertEquals(systemVersion, tester.configServer().lastPrepareVersion().get());
+        assertEquals(systemVersion, tester.configServer().lastPrepareVersion.get());
 
         // Version upgrade changes system version
         Change.VersionChange change = new Change.VersionChange(newSystemVersion);
@@ -257,7 +263,7 @@ public class ControllerTest {
 
         app1 = applications.require(app1.id());
         assertEquals("Version upgrade changes version", newSystemVersion, app1.deployedVersion().get());
-        assertEquals(newSystemVersion, tester.configServer().lastPrepareVersion().get());
+        assertEquals(newSystemVersion, tester.configServer().lastPrepareVersion.get());
     }
 
     /** Adds a new version, higher than the current system version, makes it the system version and returns it */
@@ -527,6 +533,44 @@ public class ControllerTest {
     }
 
     @Test
+    public void testLegacyDeployments() {
+        // Setup system
+        DeploymentTester tester = new DeploymentTester();
+        ApplicationController applications = tester.controller().applications();
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .environment(Environment.prod)
+                .region("us-east-3")
+                .build();
+        Version systemVersion = tester.controller().versionStatus().systemVersion().get().versionNumber();
+
+        Application app1 = tester.createApplication("application1", "tenant1", 1, 1L);
+        applications.store(app1.with(app1.deploymentJobs().asSelfTriggering(true)), applications.lock(app1.id()));
+
+        // Scenario: App already on 6.0, Upgrade to 6.1 (systemversion)
+        Zone prodZone = new Zone(Environment.prod, RegionName.from("us-east-3"));
+        Zone stagingZone = new Zone(Environment.staging, RegionName.from("us-east-3"));
+        Version existingVersion = Version.fromString("6.0");
+
+        // Add deployment on existing version
+        legacyDeploy(tester.controller(), app1, applicationPackage, prodZone, Optional.of(existingVersion), false);
+
+        // Add dev/perf deployment on old version to verify that this does not affect Initialize staging step. VESPA-8469
+        Version devVersion = Version.fromString("5.0");
+        legacyDeploy(tester.controller(), app1, applicationPackage, new Zone(Environment.dev, RegionName.from("us-east-1")), Optional.of(devVersion), false);
+        legacyDeploy(tester.controller(), app1, applicationPackage, new Zone(Environment.perf, RegionName.from("us-east-3")), Optional.of(devVersion), false);
+
+        // Initialize staging on existing version
+        legacyDeploy(tester.controller(), app1, applicationPackage, stagingZone, Optional.of(systemVersion), true);
+        app1 = applications.require(app1.id());
+        assertEquals(existingVersion, app1.currentDeployVersion(tester.controller(), stagingZone));
+
+        // Upgrade to the new version in staging
+        legacyDeploy(tester.controller(), app1, applicationPackage, stagingZone, Optional.of(systemVersion), false);
+        app1 = applications.require(app1.id());
+        assertEquals(systemVersion, app1.currentDeployVersion(tester.controller(), stagingZone));
+    }
+
+    @Test
     public void testDeployUntestedChangeFails() {
         ControllerTester tester = new ControllerTester();
         ApplicationController applications = tester.controller().applications();TenantId tenant = tester.createTenant("tenant1", "domain1", 11L);
@@ -543,6 +587,16 @@ public class ControllerTest {
         }
     }
 
+    private void legacyDeploy(Controller controller, Application application, ApplicationPackage applicationPackage, Zone zone, Optional<Version> version, boolean deployCurrentVersion) {
+        ScrewdriverId app1ScrewdriverId = new ScrewdriverId(String.valueOf(application.deploymentJobs().projectId().get()));
+        GitRevision app1RevisionId = new GitRevision(new GitRepository("repo"), new GitBranch("master"), new GitCommit("commit1"));
+        controller.applications().deployApplication(application.id(),
+                zone,
+                applicationPackage,
+                new DeployOptions(Optional.of(new ScrewdriverBuildJob(app1ScrewdriverId, app1RevisionId)), version, false, deployCurrentVersion));
+
+    }
+
     @Test
     public void testCleanupOfStaleDeploymentData() throws IOException {
         DeploymentTester tester = new DeploymentTester();
@@ -625,31 +679,4 @@ public class ControllerTest {
         assertEquals("fake-global-rotation-tenant1.app1", record.get().value());
     }
 
-    @Test
-    public void testDeployWithoutProjectId() {
-        DeploymentTester tester = new DeploymentTester();
-        tester.controllerTester().zoneRegistry().setSystem(SystemName.cd);
-        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
-                .environment(Environment.prod)
-                .region("cd-us-central-1")
-                .build();
-
-        // Create application
-        Application app = tester.createApplication("app1", "tenant1", 1, 2L);
-
-        // Direct deploy is allowed when project ID is missing
-        Zone zone = new Zone(Environment.prod, RegionName.from("cd-us-central-1"));
-        // Same options as used in our integration tests
-        DeployOptions options = new DeployOptions(Optional.empty(), Optional.empty(), false,
-                                                  false);
-        tester.controller().applications().deployApplication(app.id(), zone, applicationPackage, options);
-
-        assertTrue("Application deployed and activated",
-                   tester.controllerTester().configServer().activated().getOrDefault(app.id(), false));
-
-        assertTrue("No job status added",
-                   tester.applications().require(app.id()).deploymentJobs().jobStatus().isEmpty());
-
-    }
-
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystemTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystemTest.java
index c869bd90924..779af370ff4 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystemTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/PolledBuildSystemTest.java
@@ -2,9 +2,12 @@
 package com.yahoo.vespa.hosted.controller.deployment;
 
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.Environment;
+import com.yahoo.vespa.curator.mock.MockCurator;
+import com.yahoo.vespa.hosted.controller.ControllerTester;
 import com.yahoo.vespa.hosted.controller.api.integration.BuildService.BuildJob;
-import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType;
+import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -34,16 +37,15 @@ public class PolledBuildSystemTest {
 
     @Test
     public void throttle_capacity_constrained_jobs() {
-        DeploymentTester tester = new DeploymentTester();
+        ControllerTester tester = new ControllerTester();
         BuildSystem buildSystem = new PolledBuildSystem(tester.controller(), new MockCuratorDb());
 
-        int fooProjectId = 1;
-        int barProjectId = 2;
-        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
-                .region("us-west-1")
-                .build();
-        ApplicationId foo = tester.createAndDeploy("app1", fooProjectId, applicationPackage).id();
-        ApplicationId bar = tester.createAndDeploy("app2", barProjectId, applicationPackage).id();
+        long fooProjectId = 1;
+        long barProjectId = 2;
+        ApplicationId foo = tester.createAndDeploy("tenant1", "domain1", "app1",
+                                                   Environment.prod, fooProjectId).id();
+        ApplicationId bar = tester.createAndDeploy("tenant2", "domain2", "app2",
+                                                   Environment.prod, barProjectId).id();
 
         // Trigger jobs in capacity constrained environment
         buildSystem.addJob(foo, jobType, false);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java
index ef0b05f9bb2..ee4f3631b54 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirerTest.java
@@ -1,22 +1,16 @@
 // Copyright 2017 Yahoo Holdings. 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.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.Zone;
-import com.yahoo.vespa.hosted.controller.Application;
-import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
-import com.yahoo.vespa.hosted.controller.application.Deployment;
-import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
-import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
+import com.yahoo.vespa.hosted.controller.ControllerTester;
 import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
-import org.junit.Before;
 import org.junit.Test;
 
 import java.io.IOException;
 import java.time.Duration;
-import java.util.List;
-import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
 
@@ -25,53 +19,28 @@ import static org.junit.Assert.assertEquals;
  */
 public class DeploymentExpirerTest {
 
-    private DeploymentTester tester;
-
-    @Before
-    public void before() {
-        tester = new DeploymentTester();
-    }
-
     @Test
     public void testDeploymentExpiry() throws IOException, InterruptedException {
-        tester.controllerTester().zoneRegistry().setDeploymentTimeToLive(
-                new Zone(Environment.dev, RegionName.from("us-east-1")),
-                Duration.ofDays(14)
-        );
+        ControllerTester tester = new ControllerTester();
+        tester.zoneRegistry().setDeploymentTimeToLive(new Zone(Environment.dev, RegionName.from("us-east-1")), Duration.ofDays(14));
         DeploymentExpirer expirer = new DeploymentExpirer(tester.controller(), Duration.ofDays(10),
                                                           tester.clock(), new JobControl(new MockCuratorDb()));
-        Application devApp = tester.createApplication("app1", "tenant1", 123L, 1L);
-        Application prodApp = tester.createApplication("app2", "tenant2", 456L, 2L);
-
-        // Deploy dev
-        tester.controllerTester().deploy(devApp, tester.controllerTester().toZone(Environment.dev));
+        ApplicationId devApp = tester.createAndDeploy("tenant1", "domain1", "app1", Environment.dev, 123).id();
+        ApplicationId prodApp = tester.createAndDeploy("tenant2", "domain2", "app2", Environment.prod, 456).id();
 
-        // Deploy prod
-        ApplicationPackage prodAppPackage = new ApplicationPackageBuilder()
-                .region("us-west-1")
-                .build();
-        tester.deployCompletely(prodApp, prodAppPackage);
-
-        assertEquals(1, permanentDeployments(devApp).size());
-        assertEquals(1, permanentDeployments(prodApp).size());
+        assertEquals(1, tester.controller().applications().get(devApp).get().deployments().size());
+        assertEquals(1, tester.controller().applications().get(prodApp).get().deployments().size());
 
         // Not expired at first
         expirer.maintain();
-        assertEquals(1, permanentDeployments(devApp).size());
-        assertEquals(1, permanentDeployments(prodApp).size());
+        assertEquals(1, tester.controller().applications().get(devApp).get().deployments().size());
+        assertEquals(1, tester.controller().applications().get(prodApp).get().deployments().size());
 
         // The dev application is removed
         tester.clock().advance(Duration.ofDays(15));
         expirer.maintain();
-        assertEquals(0, permanentDeployments(devApp).size());
-        assertEquals(1, permanentDeployments(prodApp).size());
-    }
-
-    private List<Deployment> permanentDeployments(Application application) {
-        return tester.controller().applications().get(application.id()).get().deployments().values().stream()
-                .filter(deployment -> deployment.zone().environment() != Environment.test &&
-                                      deployment.zone().environment() != Environment.staging)
-                .collect(Collectors.toList());
+        assertEquals(0, tester.controller().applications().get(devApp).get().deployments().size());
+        assertEquals(1, tester.controller().applications().get(prodApp).get().deployments().size());
     }
 
 }
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 6709098ba06..ccdfc8d042e 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
@@ -57,7 +57,7 @@ public class UpgraderTest {
 
         assertEquals("New system version: Should upgrade Canaries", 2, tester.buildSystem().jobs().size());
         tester.completeUpgrade(canary0, version, "canary");
-        assertEquals(version, tester.configServer().lastPrepareVersion().get());
+        assertEquals(version, tester.configServer().lastPrepareVersion.get());
 
         tester.updateVersionStatus(version);
         tester.upgrader().maintain();
@@ -107,7 +107,7 @@ public class UpgraderTest {
 
         assertEquals("New system version: Should upgrade Canaries", 2, tester.buildSystem().jobs().size());
         tester.completeUpgrade(canary0, version, "canary");
-        assertEquals(version, tester.configServer().lastPrepareVersion().get());
+        assertEquals(version, tester.configServer().lastPrepareVersion.get());
 
         tester.updateVersionStatus(version);
         tester.upgrader().maintain();
@@ -187,7 +187,7 @@ public class UpgraderTest {
 
         assertEquals("New system version: Should upgrade Canaries", 2, tester.buildSystem().jobs().size());
         tester.completeUpgrade(canary0, version, "canary");
-        assertEquals(version, tester.configServer().lastPrepareVersion().get());
+        assertEquals(version, tester.configServer().lastPrepareVersion.get());
 
         tester.updateVersionStatus(version);
         tester.upgrader().maintain();