diff options
author | jonmv <venstad@gmail.com> | 2023-03-31 13:24:26 +0200 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2023-03-31 13:24:26 +0200 |
commit | 71163e6de96cfeb8a24eb167d1d17a7741b0ba1e (patch) | |
tree | ca60b820c73a38da1dcfbb8bb61fd7560f2ca183 | |
parent | d65d548169183b47b931b3c5e39ad5a8fae06ce5 (diff) |
Validate all future jobs (submit) and current (deploy) have zone in account
4 files changed, 23 insertions, 6 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 a1534ebc533..44ea693d8cd 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 @@ -47,6 +47,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.RestartFilter; import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore; +import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics.Warning; @@ -60,6 +61,7 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageValid import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; import com.yahoo.vespa.hosted.controller.certificate.EndpointCertificates; import com.yahoo.vespa.hosted.controller.concurrent.Once; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.deployment.JobStatus; import com.yahoo.vespa.hosted.controller.deployment.Run; @@ -586,7 +588,16 @@ public class ApplicationController { } // Validate new deployment spec thoroughly before storing it. - controller.jobController().deploymentStatus(application.get()); + DeploymentStatus status = controller.jobController().deploymentStatus(application.get()); + Change dummyChange = Change.of(RevisionId.forProduction(Long.MAX_VALUE)); // Should always run everywhere. + for (var jobs : status.jobsToRun(applicationPackage.deploymentSpec().instanceNames().stream() + .collect(toMap(name -> name, __ -> dummyChange))) + .entrySet()) { + for (var job : jobs.getValue()) { + decideCloudAccountOf(new DeploymentId(jobs.getKey().application(), job.type().zone()), + applicationPackage.deploymentSpec()); + } + } for (Notification notification : controller.notificationsDb().listNotifications(NotificationSource.from(application.get().id()), true)) { if ( notification.source().instance().isPresent() @@ -696,7 +707,7 @@ public class ApplicationController { throw new IllegalArgumentException("Requested cloud account '" + requestedAccount.get().value() + "' is not valid for tenant '" + tenant + "'"); } - if (!controller.zoneRegistry().hasZone(zoneId, requestedAccount.get())) { + if ( ! controller.zoneRegistry().hasZone(zoneId, requestedAccount.get())) { throw new IllegalArgumentException("Zone " + zoneId + " is not configured in requested cloud account '" + requestedAccount.get().value() + "'"); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index a76e76611c2..050b77a391e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -390,7 +390,7 @@ public class DeploymentStatus { } /** The set of jobs that need to run for the given changes to be considered complete. */ - private Map<JobId, List<Job>> jobsToRun(Map<InstanceName, Change> changes) { + public Map<JobId, List<Job>> jobsToRun(Map<InstanceName, Change> changes) { return jobsToRun(changes, false); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 73c64be3e47..10e4052f067 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -713,6 +713,7 @@ public class JobController { // TODO(mpolden): Enable for public CD once all tests have been updated if (controller.system() != SystemName.PublicCd) { controller.applications().validatePackage(applicationPackage, application.get()); + controller.applications().decideCloudAccountOf(new DeploymentId(id, type.zone()), applicationPackage.deploymentSpec()); } controller.applications().store(application); }); 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 8f0536480f5..d9ee82f5e90 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 @@ -1429,9 +1429,14 @@ public class ControllerTest { // Deployment fails because zone is not configured in requested cloud account tester.controllerTester().flagSource().withListFlag(PermanentFlags.CLOUD_ACCOUNTS.id(), List.of(cloudAccount), String.class); - context.submit(applicationPackage) - .runJobExpectingFailure(systemTest, "Zone test.us-east-1 is not configured in requested cloud account '012345678912'") - .abortJob(stagingTest); + assertEquals("Zone test.us-east-1 is not configured in requested cloud account '012345678912'", + assertThrows(IllegalArgumentException.class, + () -> context.submit(applicationPackage)) + .getMessage()); + assertEquals("Zone dev.us-east-1 is not configured in requested cloud account '012345678912'", + assertThrows(IllegalArgumentException.class, + () -> context.runJob(devUsEast1, applicationPackage)) + .getMessage()); // Deployment to prod succeeds once all zones are configured in requested account tester.controllerTester().zoneRegistry().configureCloudAccount(CloudAccount.from(cloudAccount), |