diff options
4 files changed, 67 insertions, 41 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 01b12a5136d..f3368d0918b 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 @@ -7,7 +7,6 @@ import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; -import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; @@ -44,14 +43,14 @@ 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.ActivateResult; -import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageValidator; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.DeploymentQuotaCalculator; import com.yahoo.vespa.hosted.controller.application.QuotaUsage; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; +import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; +import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageValidator; 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; @@ -378,7 +377,7 @@ public class ApplicationController { endpointCertificateMetadata = endpointCertificates.getMetadata(instance, zone, applicationPackage.deploymentSpec()); - containerEndpoints = controller.routing().containerEndpointsOf(application.get(), job.application().instance(), zone); + containerEndpoints = controller.routing().containerEndpointsOf(application, job.application().instance(), zone); } // Release application lock while doing the deployment, which is a lengthy task. @@ -434,10 +433,6 @@ public class ApplicationController { application = withoutDeletedDeployments(application, name); } - for (InstanceName instance : declaredInstances) - if (applicationPackage.deploymentSpec().requireInstance(instance).concerns(Environment.prod)) - application = controller.routing().assignRotations(application, instance); - // Validate new deployment spec thoroughly before storing it. controller.jobController().deploymentStatus(application.get()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index 37266dd6dac..b6b02a7cab4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -241,7 +241,7 @@ public class RoutingController { * Assigns one or more global rotations to given application, if eligible. The given application is implicitly * stored, ensuring that the assigned rotation(s) are persisted when this returns. */ - public LockedApplication assignRotations(LockedApplication application, InstanceName instanceName) { + private LockedApplication assignRotations(LockedApplication application, InstanceName instanceName) { try (RotationLock rotationLock = rotationRepository.lock()) { var rotations = rotationRepository.getOrAssignRotations(application.get().deploymentSpec(), application.get().require(instanceName), @@ -253,14 +253,21 @@ public class RoutingController { } /** Returns the global and application-level endpoints for given deployment, as container endpoints */ - public Set<ContainerEndpoint> containerEndpointsOf(Application application, InstanceName instanceName, ZoneId zone) { - Instance instance = application.require(instanceName); - boolean registerLegacyNames = requiresLegacyNames(application.deploymentSpec(), instanceName); + public Set<ContainerEndpoint> containerEndpointsOf(LockedApplication application, InstanceName instanceName, ZoneId zone) { + // Assign rotations to application + for (var deploymentInstanceSpec : application.get().deploymentSpec().instances()) { + if (deploymentInstanceSpec.concerns(Environment.prod)) { + application = controller.routing().assignRotations(application, deploymentInstanceSpec.name()); + } + } + + // Add endpoints backed by a rotation, and register them in DNS if necessary + boolean registerLegacyNames = requiresLegacyNames(application.get().deploymentSpec(), instanceName); + Instance instance = application.get().require(instanceName); Set<ContainerEndpoint> containerEndpoints = new HashSet<>(); DeploymentId deployment = new DeploymentId(instance.id(), zone); - EndpointList endpoints = declaredEndpointsOf(application).targets(deployment); + EndpointList endpoints = declaredEndpointsOf(application.get()).targets(deployment); EndpointList globalEndpoints = endpoints.scope(Endpoint.Scope.global); - // Add endpoints backed by a rotation, and register them in DNS if necessary for (var assignedRotation : instance.rotations()) { var names = new ArrayList<String>(); EndpointList rotationEndpoints = globalEndpoints.named(assignedRotation.endpointId()) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java index b9c1e9f3e72..d98789591ab 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java @@ -53,6 +53,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.failed; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.unfinished; import static org.junit.Assert.assertEquals; @@ -292,13 +293,32 @@ public class DeploymentContext { /** Fail current deployment in given job */ private DeploymentContext failDeployment(JobType type, RuntimeException exception) { + configServer().throwOnNextPrepare(exception); + runJobExpectingFailure(type, Optional.empty()); + return this; + } + + /** Run given job and expect it to fail with given message, if any */ + public DeploymentContext runJobExpectingFailure(JobType type, Optional<String> messagePart) { triggerJobs(); var job = jobId(type); RunId id = currentRun(job).id(); - configServer().throwOnNextPrepare(exception); runner.advance(currentRun(job)); - assertTrue(jobs.run(id).get().hasFailed()); - assertTrue(jobs.run(id).get().hasEnded()); + Run run = jobs.run(id).get(); + assertTrue(run.hasFailed()); + assertTrue(run.hasEnded()); + if (messagePart.isPresent()) { + Optional<Step> firstFailing = run.stepStatuses().entrySet().stream() + .filter(kv -> kv.getValue() == failed) + .map(Map.Entry::getKey) + .findFirst(); + assertTrue("Found failing step", firstFailing.isPresent()); + Optional<RunLog> details = jobs.details(id); + assertTrue("Found log entries for run " + id, details.isPresent()); + assertTrue("Found log message containing '" + messagePart.get() + "'", + details.get().get(firstFailing.get()).stream() + .anyMatch(entry -> entry.message().contains(messagePart.get()))); + } return this; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepositoryTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepositoryTest.java index b767e8a791f..e7c2eacbd02 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepositoryTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationRepositoryTest.java @@ -5,19 +5,20 @@ import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.vespa.hosted.controller.ControllerTester; -import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.AssignedRotation; +import com.yahoo.vespa.hosted.controller.application.SystemApplication; +import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import java.net.URI; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -30,22 +31,19 @@ import static org.junit.Assert.assertTrue; */ public class RotationRepositoryTest { - @Rule - public ExpectedException thrown = ExpectedException.none(); - - private final RotationsConfig rotationsConfig = new RotationsConfig( + private static final RotationsConfig rotationsConfig = new RotationsConfig( new RotationsConfig.Builder() .rotations("foo-1", "foo-1.com") .rotations("foo-2", "foo-2.com") ); - private final RotationsConfig rotationsConfigWhitespaces = new RotationsConfig( + private static final RotationsConfig rotationsConfigWhitespaces = new RotationsConfig( new RotationsConfig.Builder() .rotations("foo-1", "\n \t foo-1.com \n") .rotations("foo-2", "foo-2.com") ); - private final ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + private static final ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .globalServiceId("foo") .region("us-east-3") .region("us-west-1") @@ -57,8 +55,8 @@ public class RotationRepositoryTest { @Test public void assigns_and_reuses_rotation() { - // Submitting assigns a rotation - application.submit(applicationPackage); + // Deploying assigns a rotation + application.submit(applicationPackage).deploy(); Rotation expected = new Rotation(new RotationId("foo-1"), "foo-1.com"); assertEquals(List.of(expected.id()), rotationIds(application.instance().rotations())); @@ -108,17 +106,16 @@ public class RotationRepositoryTest { @Test public void out_of_rotations() { // Assigns 1 rotation - application.submit(applicationPackage); + application.submit(applicationPackage).deploy(); // Assigns 1 more var application2 = tester.newDeploymentContext("tenant2", "app2", "default"); - application2.submit(applicationPackage); + application2.submit(applicationPackage).deploy(); - // We're now out of rotations - thrown.expect(IllegalStateException.class); - thrown.expectMessage("out of rotations"); + // We're now out of rotations and next deployment fails var application3 = tester.newDeploymentContext("tenant3", "app3", "default"); - application3.submit(applicationPackage); + application3.submit(applicationPackage) + .runJobExpectingFailure(JobType.systemTest, Optional.of("out of rotations")); } @Test @@ -127,9 +124,7 @@ public class RotationRepositoryTest { .globalServiceId("foo") .region("us-east-3") .build(); - thrown.expect(RuntimeException.class); - thrown.expectMessage("less than 2 prod zones are defined"); - application.submit(applicationPackage); + application.submit(applicationPackage).runJobExpectingFailure(JobType.systemTest, Optional.of("less than 2 prod zones are defined")); } @Test @@ -149,13 +144,18 @@ public class RotationRepositoryTest { .region("cd-us-east-1") .region("cd-us-west-1") .build(); - var zones = List.of(ZoneApiMock.fromId("prod.cd-us-east-1"), ZoneApiMock.fromId("prod.cd-us-west-1")); + var zones = List.of( + ZoneApiMock.fromId("test.cd-us-west-1"), + ZoneApiMock.fromId("staging.cd-us-west-1"), + ZoneApiMock.fromId("prod.cd-us-east-1"), + ZoneApiMock.fromId("prod.cd-us-west-1")); tester.controllerTester().zoneRegistry() .setZones(zones) .setRoutingMethod(zones, RoutingMethod.shared) .setSystemName(SystemName.cd); + tester.configServer().bootstrap(tester.controllerTester().zoneRegistry().zones().all().ids(), SystemApplication.notController()); var application2 = tester.newDeploymentContext("tenant2", "app2", "default"); - application2.submit(applicationPackage); + application2.submit(applicationPackage).deploy(); assertEquals(List.of(new RotationId("foo-1")), rotationIds(application2.instance().rotations())); assertEquals("https://cd--app2--tenant2.global.vespa.oath.cloud:4443/", tester.controller().routing().readDeclaredEndpointsOf(application2.instanceId()).primary().get().url().toString()); @@ -169,7 +169,9 @@ public class RotationRepositoryTest { .parallel("us-west-1", "us-east-3") .globalServiceId("global") .build(); - var instance1 = tester.newDeploymentContext("tenant1", "application1", "instance1").submit(applicationPackage); + var instance1 = tester.newDeploymentContext("tenant1", "application1", "instance1") + .submit(applicationPackage) + .deploy(); var instance2 = tester.newDeploymentContext("tenant1", "application1", "instance2"); assertEquals(List.of(new RotationId("foo-1")), rotationIds(instance1.instance().rotations())); assertEquals(List.of(new RotationId("foo-2")), rotationIds(instance2.instance().rotations())); @@ -187,7 +189,9 @@ public class RotationRepositoryTest { .parallel("us-west-1", "us-east-3") .endpoint("default", "foo", "us-central-1", "us-west-1") .build(); - var instance1 = tester.newDeploymentContext("tenant1", "application1", "instance1").submit(applicationPackage); + var instance1 = tester.newDeploymentContext("tenant1", "application1", "instance1") + .submit(applicationPackage) + .deploy(); var instance2 = tester.newDeploymentContext("tenant1", "application1", "instance2"); assertEquals(List.of(new RotationId("foo-1")), rotationIds(instance1.instance().rotations())); |