diff options
author | Martin Polden <mpolden@mpolden.no> | 2017-08-29 13:07:37 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-08-29 13:07:37 +0200 |
commit | 915b617dd57402b16e70d22b4187d987d64293c0 (patch) | |
tree | b4980839eac81c6fd37ba1d33fa4047c61a46535 /controller-server | |
parent | 5cc377f38e47f7cdbff7a661267293ef3ca14f3e (diff) | |
parent | 655adc2b7a2f3ed451d35da4ad60041c5af1cc8c (diff) |
Merge pull request #3245 from vespa-engine/jvenstad/make-unexpected-deployment-a-noop
Let unexpected deployment return a dummy ActivateResult
Diffstat (limited to 'controller-server')
2 files changed, 31 insertions, 26 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 3ede73963a3..27992220f1c 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 @@ -17,12 +17,15 @@ 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.application.v4.model.configserverbindings.ConfigChangeActions; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerClient; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NoInstanceException; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse; import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordId; @@ -216,22 +219,32 @@ public class ApplicationController { /** Deploys an application. If the application does not exist it is created. */ // TODO: Get rid of the options arg - public ActivateResult deployApplication(ApplicationId applicationId, com.yahoo.config.provision.Zone zone, + public ActivateResult deployApplication(ApplicationId applicationId, Zone zone, ApplicationPackage applicationPackage, DeployOptions options) { try (Lock lock = lock(applicationId)) { // Determine what we are doing Application application = get(applicationId).orElse(new Application(applicationId)); - DeploymentJobs.JobType jobType = DeploymentJobs.JobType.from(controller.zoneRegistry().system(), zone); - Version version = decideVersion(application, zone, options); - ApplicationRevision revision = toApplicationPackageRevision(applicationPackage, options.screwdriverBuildJob); + + // Decide version to deploy, if applicable. + Version version; + if (options.deployCurrentVersion) + version = application.currentVersion(controller, zone); + 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); + else + 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 ( ! 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"); - } + + DeploymentJobs.JobType jobType = DeploymentJobs.JobType.from(controller.zoneRegistry().system(), zone); + ApplicationRevision revision = toApplicationPackageRevision(applicationPackage, options.screwdriverBuildJob); // Don't update/store applicationpackage information when deploying previous application package (initial staging step) if(! options.deployCurrentVersion) { @@ -274,21 +287,17 @@ public class ApplicationController { return new ActivateResult(new RevisionId(applicationPackage.hash()), preparedApplication.messages(), preparedApplication.prepareResponse()); } } - - private Version decideVersion(Application application, Zone zone, DeployOptions options) { - if (options.deployCurrentVersion) - return application.currentVersion(controller, zone); - - if (application.deploymentJobs().isSelfTriggering()) // legacy mode: let the client decide - return options.vespaVersion.map(Version::new).orElse(controller.systemVersion()); - if ( ! application.deploying().isPresent() && ! zone.environment().isManuallyDeployed()) - throw new IllegalArgumentException("Rejecting deployment of " + application + " to " + zone + - " as a deployment is not currently expected"); - - return application.currentDeployVersion(controller, zone); + private ActivateResult unexpectedDeployment(ApplicationId applicationId, Zone zone, ApplicationPackage applicationPackage) { + Log logEntry = new Log(); + logEntry.level = "WARNING"; + logEntry.time = clock.instant().toEpochMilli(); + logEntry.message = "Ignoring deployment of " + get(applicationId) + " to " + zone + " as a deployment is not currently expected"; + PrepareResponse prepareResponse = new PrepareResponse(); + prepareResponse.configChangeActions = new ConfigChangeActions(Collections.emptyList(), Collections.emptyList()); + return new ActivateResult(new RevisionId(applicationPackage.hash()), Collections.singletonList(logEntry), prepareResponse); } - + private Application deleteRemovedDeployments(Application application) { List<Deployment> deploymentsToRemove = application.deployments().values().stream() .filter(deployment -> deployment.zone().environment() == Environment.prod) 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 4204c3d6339..3dddfaf58a1 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 @@ -220,13 +220,9 @@ public class ControllerTest { assertEquals(systemVersion, tester.configServerClientMock().lastPrepareVersion.get()); // Unexpected deployment - try { - tester.deploy(productionUsWest1, app1, applicationPackage); - fail("Expected exception as no change was to be deployed"); - } - catch (IllegalArgumentException expected) { - // success - } + tester.deploy(productionUsWest1, app1, applicationPackage); + // applications are immutable, so any change to one, including deployment changes, would give rise to a new instance. + assertEquals("Unexpected deployment is ignored", app1, applications.require(app1.id())); // Application change after a new system version, and a region added Version newSystemVersion = incrementSystemVersion(tester.controller()); |