diff options
7 files changed, 112 insertions, 92 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 ccfcc0e4b0d..43dd2b6e226 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 (application.deploymentJobs().isSelfTriggering()) // legacy mode: let the client decide + else if (canDeployDirectlyTo(zone, options)) version = options.vespaVersion.map(Version::new).orElse(controller.systemVersion()); else if ( ! application.deploying().isPresent() && ! zone.environment().isManuallyDeployed()) return unexpectedDeployment(applicationId, zone, applicationPackage); @@ -286,10 +286,8 @@ public class ApplicationController { version = application.currentDeployVersion(controller, zone); // Ensure that the deploying change is tested - // 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())) + if (! canDeployDirectlyTo(zone, options) && + ! application.deploymentJobs().isDeployableTo(zone.environment(), application.deploying())) throw new IllegalArgumentException("Rejecting deployment of " + application + " to " + zone + " as pending " + application.deploying().get() + " is untested"); @@ -305,7 +303,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) && !zone.environment().isManuallyDeployed() && jobType != null) { + if ( ! triggeredWith(revision, application, jobType) && !canDeployDirectlyTo(zone, options) && 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, @@ -590,6 +588,11 @@ 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 d2af5fc50cb..8826ee5bf67 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,6 +285,11 @@ 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 6018c99206e..13525969358 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,8 +51,17 @@ 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; - /** The version given in the previous prepare call, or null if no call has been made */ - public Optional<Version> lastPrepareVersion = 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); + } @Override public PreparedApplication prepare(DeploymentId deployment, DeployOptions deployOptions, Set<String> rotationCnames, Set<Rotation> rotations, byte[] content) { @@ -201,4 +210,5 @@ 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 91fca0d37d1..61cc84a07ba 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,16 +17,10 @@ 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; @@ -217,7 +211,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); @@ -240,7 +234,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() @@ -251,7 +245,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); @@ -263,7 +257,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 */ @@ -533,44 +527,6 @@ 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); @@ -587,16 +543,6 @@ 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(); @@ -679,4 +625,31 @@ 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 779af370ff4..c869bd90924 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,12 +2,9 @@ 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; @@ -37,15 +34,16 @@ public class PolledBuildSystemTest { @Test public void throttle_capacity_constrained_jobs() { - ControllerTester tester = new ControllerTester(); + DeploymentTester tester = new DeploymentTester(); BuildSystem buildSystem = new PolledBuildSystem(tester.controller(), new MockCuratorDb()); - 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(); + 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(); // 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 ee4f3631b54..ef0b05f9bb2 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,16 +1,22 @@ // 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.ControllerTester; +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.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; @@ -19,28 +25,53 @@ 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 { - ControllerTester tester = new ControllerTester(); - tester.zoneRegistry().setDeploymentTimeToLive(new Zone(Environment.dev, RegionName.from("us-east-1")), Duration.ofDays(14)); + tester.controllerTester().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())); - ApplicationId devApp = tester.createAndDeploy("tenant1", "domain1", "app1", Environment.dev, 123).id(); - ApplicationId prodApp = tester.createAndDeploy("tenant2", "domain2", "app2", Environment.prod, 456).id(); + 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)); - assertEquals(1, tester.controller().applications().get(devApp).get().deployments().size()); - assertEquals(1, tester.controller().applications().get(prodApp).get().deployments().size()); + // 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()); // Not expired at first expirer.maintain(); - assertEquals(1, tester.controller().applications().get(devApp).get().deployments().size()); - assertEquals(1, tester.controller().applications().get(prodApp).get().deployments().size()); + assertEquals(1, permanentDeployments(devApp).size()); + assertEquals(1, permanentDeployments(prodApp).size()); // The dev application is removed tester.clock().advance(Duration.ofDays(15)); expirer.maintain(); - assertEquals(0, tester.controller().applications().get(devApp).get().deployments().size()); - assertEquals(1, tester.controller().applications().get(prodApp).get().deployments().size()); + 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()); } } 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 ccdfc8d042e..6709098ba06 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(); |