diff options
7 files changed, 92 insertions, 112 deletions
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 43dd2b6e226..ccfcc0e4b0d 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 @@ -278,7 +278,7 @@ public class ApplicationController { Version version; if (options.deployCurrentVersion) version = application.currentVersion(controller, zone); - else if (canDeployDirectlyTo(zone, options)) + else if (application.deploymentJobs().isSelfTriggering()) // legacy mode: let the client decide version = options.vespaVersion.map(Version::new).orElse(controller.systemVersion()); else if ( ! application.deploying().isPresent() && ! zone.environment().isManuallyDeployed()) return unexpectedDeployment(applicationId, zone, applicationPackage); @@ -286,8 +286,10 @@ public class ApplicationController { version = application.currentDeployVersion(controller, zone); // Ensure that the deploying change is tested - if (! canDeployDirectlyTo(zone, options) && - ! application.deploymentJobs().isDeployableTo(zone.environment(), application.deploying())) + // FIXME: For now only for non-self-triggering applications - VESPA-8418 + if ( ! application.deploymentJobs().isSelfTriggering() && + ! zone.environment().isManuallyDeployed() && + ! application.deploymentJobs().isDeployableTo(zone.environment(), application.deploying())) throw new IllegalArgumentException("Rejecting deployment of " + application + " to " + zone + " as pending " + application.deploying().get() + " is untested"); @@ -303,7 +305,7 @@ public class ApplicationController { application = application.withProjectId(options.screwdriverBuildJob.get().screwdriverId.value()); if (application.deploying().isPresent() && application.deploying().get() instanceof Change.ApplicationChange) application = application.withDeploying(Optional.of(Change.ApplicationChange.of(revision))); - if ( ! triggeredWith(revision, application, jobType) && !canDeployDirectlyTo(zone, options) && jobType != null) { + if ( ! triggeredWith(revision, application, jobType) && !zone.environment().isManuallyDeployed() && jobType != null) { // Triggering information is used to store which changes were made or attempted // - For self-triggered applications we don't have any trigger information, so we add it here. // - For all applications, we don't have complete control over which revision is actually built, @@ -588,11 +590,6 @@ public class ApplicationController { return curator.lock(application, Duration.ofMinutes(10)); } - /** Returns whether a direct deployment to given zone is allowed */ - private static boolean canDeployDirectlyTo(Zone zone, DeployOptions options) { - return !options.screwdriverBuildJob.isPresent() || zone.environment().isManuallyDeployed(); - } - private static final class ApplicationRotation { private final ImmutableSet<String> cnames; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 8826ee5bf67..d2af5fc50cb 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -285,11 +285,6 @@ public class DeploymentTrigger { return application; } - // Ignore applications that are not associated with a project - if (!application.deploymentJobs().projectId().isPresent()) { - return application; - } - log.info(String.format("Triggering %s for %s, %s: %s", jobType, application, application.deploying().map(d -> "deploying " + d).orElse("restarted deployment"), cause)); 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(); |