From f3808f84558c8306c3e5541f8c1c4c860fd01c2a Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:27:45 +0100 Subject: Allow creation of a ControllerTester on top of a ContainerTester --- .../vespa/hosted/controller/ControllerTest.java | 2 -- .../vespa/hosted/controller/ControllerTester.java | 22 ++++++++++++++++++++-- .../maintenance/RoutingPoliciesTest.java | 2 -- .../hosted/controller/restapi/ContainerTester.java | 6 ++++++ 4 files changed, 26 insertions(+), 6 deletions(-) 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 c374787aaa4..4015ac0452b 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 @@ -30,7 +30,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; -import com.yahoo.vespa.hosted.controller.deployment.BuildJob; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; @@ -384,7 +383,6 @@ public class ControllerTest { var west = ZoneId.from("prod", "us-west-1"); var central = ZoneId.from("prod", "us-central-1"); var east = ZoneId.from("prod", "us-east-3"); - var buildNumber = BuildJob.defaultBuildNumber; // Application is deployed with endpoint pointing to 2/3 zones ApplicationPackage applicationPackage = new ApplicationPackageBuilder() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index 351b139f747..35b79cdbf8f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -37,6 +37,8 @@ import com.yahoo.vespa.hosted.controller.integration.ServiceRegistryMock; import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; +import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; +import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; import com.yahoo.vespa.hosted.controller.security.AthenzCredentials; import com.yahoo.vespa.hosted.controller.security.AthenzTenantSpec; import com.yahoo.vespa.hosted.controller.security.Credentials; @@ -73,6 +75,7 @@ public final class ControllerTester { public static final int availableRotations = 10; + private final boolean inContainer; private final AthenzDbMock athenzDb; private final ManualClock clock; private final ZoneRegistryMock zoneRegistry; @@ -106,11 +109,12 @@ public final class ControllerTester { this(defaultRotationsConfig(), new MockCuratorDb()); } - private ControllerTester(AthenzDbMock athenzDb, + private ControllerTester(AthenzDbMock athenzDb, boolean inContainer, ZoneRegistryMock zoneRegistry, CuratorDb curator, RotationsConfig rotationsConfig, ServiceRegistryMock serviceRegistry, Controller controller) { this.athenzDb = athenzDb; + this.inContainer = inContainer; this.clock = serviceRegistry.clock(); this.zoneRegistry = zoneRegistry; this.serviceRegistry = serviceRegistry; @@ -130,10 +134,22 @@ public final class ControllerTester { ZoneRegistryMock zoneRegistry, CuratorDb curator, RotationsConfig rotationsConfig, ServiceRegistryMock serviceRegistry) { - this(athenzDb, zoneRegistry, curator, rotationsConfig, serviceRegistry, + this(athenzDb, false, zoneRegistry, curator, rotationsConfig, serviceRegistry, createController(curator, rotationsConfig, zoneRegistry, athenzDb, serviceRegistry)); } + /** Creates a ControllerTester built on the ContainerTester's controller. This controller can not be recreated. */ + public ControllerTester(ContainerTester tester) { + this(tester.athenzClientFactory().getSetup(), + true, + tester.serviceRegistry().zoneRegistryMock(), + tester.controller().curator(), + null, + tester.serviceRegistry(), + tester.controller()); + } + + public void configureDefaultLogHandler(Consumer configureFunc) { Arrays.stream(Logger.getLogger("").getHandlers()) // Do not mess with log configuration if a custom one has been set @@ -179,6 +195,8 @@ public final class ControllerTester { /** Create a new controller instance. Useful to verify that controller state is rebuilt from persistence */ public final void createNewController() { + if (inContainer) + throw new UnsupportedOperationException("Cannot recreate this controller"); controller = createController(curator, rotationsConfig, zoneRegistry, athenzDb, serviceRegistry); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java index f5f1605a699..bc184715589 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPoliciesTest.java @@ -14,7 +14,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; -import com.yahoo.vespa.hosted.controller.deployment.BuildJob; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import org.junit.Test; @@ -53,7 +52,6 @@ public class RoutingPoliciesTest { @Test public void maintains_global_routing_policies() { - long buildNumber = BuildJob.defaultBuildNumber; int clustersPerZone = 2; int numberOfDeployments = 2; var applicationPackage = new ApplicationPackageBuilder() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java index 300eddd6291..55249670c5c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java @@ -11,9 +11,11 @@ import com.yahoo.container.http.filter.FilterChainRepository; import com.yahoo.jdisc.http.filter.SecurityRequestFilter; import com.yahoo.jdisc.http.filter.SecurityRequestFilterChain; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactoryMock; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.integration.ServiceRegistryMock; +import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.vespa.hosted.controller.versions.ControllerVersion; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import org.junit.ComparisonFailure; @@ -58,6 +60,10 @@ public class ContainerTester { return serviceRegistry().configServerMock(); } + public AthenzClientFactoryMock athenzClientFactory() { + return (AthenzClientFactoryMock) container.components().getComponent(AthenzClientFactoryMock.class.getName()); + } + public ServiceRegistryMock serviceRegistry() { return (ServiceRegistryMock) container.components().getComponent(ServiceRegistryMock.class.getName()); } -- cgit v1.2.3 From 1c7999b7258e5c9a46f55c046715c20b624ea814 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:28:17 +0100 Subject: ApplicationVersion equality on id parts only --- .../controller/api/integration/deployment/ApplicationVersion.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java index b1dad7d814e..fb9745d90cb 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java @@ -115,15 +115,12 @@ public class ApplicationVersion implements Comparable { if (!(o instanceof ApplicationVersion)) return false; ApplicationVersion that = (ApplicationVersion) o; return Objects.equals(source, that.source) && - Objects.equals(authorEmail, that.authorEmail) && - Objects.equals(buildNumber, that.buildNumber) && - Objects.equals(compileVersion, that.compileVersion) && - Objects.equals(buildTime, that.buildTime); + Objects.equals(buildNumber, that.buildNumber); } @Override public int hashCode() { - return Objects.hash(source, authorEmail, buildNumber, compileVersion, buildTime); + return Objects.hash(source, buildNumber); } @Override -- cgit v1.2.3 From 1fb8ad0c9eea7b096f85251e17a7d6b2ad3e1177 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:28:46 +0100 Subject: Change complete when _all_ instances have it, not _any_ --- .../vespa/hosted/controller/deployment/DeploymentTrigger.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 9573f5d07f5..359374e8575 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 @@ -565,13 +565,13 @@ public class DeploymentTrigger { : steps.productionJobs(); Change change = application.change(); - for (Instance instance : application.instances().values()) { - if (jobs.stream().allMatch(job -> isComplete(application.change().withoutApplication(), application.change(), instance, job))) + if (application.instances().values().stream().allMatch(instance -> + jobs.stream().allMatch(job -> isComplete(application.change().withoutApplication(), application.change(), instance, job)))) change = change.withoutPlatform(); - if (jobs.stream().allMatch(job -> isComplete(application.change().withoutPlatform(), application.change(), instance, job))) + if (application.instances().values().stream().allMatch(instance -> + jobs.stream().allMatch(job -> isComplete(application.change().withoutPlatform(), application.change(), instance, job)))) change = change.withoutApplication(); - } return change; } -- cgit v1.2.3 From 7d85d248c66e9789fecf60525b5968a609cc542f Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:29:37 +0100 Subject: Do not cancel change when submitting, as all apps are on internal pipeline --- .../com/yahoo/vespa/hosted/controller/deployment/JobController.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 c4f394d237b..cc483dff765 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 @@ -343,9 +343,7 @@ public class JobController { controller.applications().applicationStore().put(instance.id().tenant(), instance.id().application(), appVersion, content); }); } - // Make sure any ongoing upgrade is cancelled, since future jobs will require the tester artifact. - return application.withChange(application.get().change().withoutPlatform().withoutApplication()) - .withBuiltInternally(true); + return application.withBuiltInternally(true); } /** Orders a run of the given type, or throws an IllegalStateException if that job type is already running. */ -- cgit v1.2.3 From 23f47d7495df1b038887a4413b8be6150cbc734b Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:32:54 +0100 Subject: Rename deploymentContext() to newDeploymentContext() --- .../vespa/hosted/controller/ControllerTest.java | 18 ++++++------- .../controller/deployment/DeploymentContext.java | 3 ++- .../controller/deployment/DeploymentTester.java | 4 +-- .../deployment/DeploymentTriggerTest.java | 30 +++++++++++----------- .../deployment/InternalStepRunnerTest.java | 1 + .../DeploymentMetricsMaintainerTest.java | 2 +- .../maintenance/MetricsReporterTest.java | 10 ++++---- .../controller/maintenance/UpgraderTest.java | 18 ++++++------- .../controller/versions/VersionStatusTest.java | 2 +- 9 files changed, 45 insertions(+), 43 deletions(-) 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 4015ac0452b..dbe451fd433 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 @@ -80,7 +80,7 @@ public class ControllerTest { // staging job - succeeding Version version1 = tester.configServer().initialVersion(); - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); context.submit(applicationPackage); assertEquals("Application version is known from completion of initial job", ApplicationVersion.from(DeploymentContext.defaultSourceRevision, 1, "a@b", new Version("6.1"), Instant.ofEpochSecond(1)), @@ -501,7 +501,7 @@ public class ControllerTest { @Test public void testUnassignRotations() { - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .endpoint("default", "qrs", "us-west-1", "us-central-1") @@ -633,7 +633,7 @@ public class ControllerTest { .build(); // Create application - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); // Direct deploy is allowed when deployDirectly is true ZoneId zone = ZoneId.from("prod", "cd-us-central-1"); @@ -664,7 +664,7 @@ public class ControllerTest { .build(); // Create application - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); ZoneId zone = ZoneId.from("dev", "us-east-1"); // Deploy @@ -678,7 +678,7 @@ public class ControllerTest { @Test public void testSuspension() { - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .region("us-west-1") @@ -702,7 +702,7 @@ public class ControllerTest { // second time will not fail @Test public void testDeletingApplicationThatHasAlreadyBeenDeleted() { - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .region("us-east-3") @@ -721,12 +721,12 @@ public class ControllerTest { .environment(Environment.prod) .region("us-west-1") .build(true); - tester.deploymentContext().submit(applicationPackage); + tester.newDeploymentContext().submit(applicationPackage); } @Test public void testDeployApplicationWithWarnings() { - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) .region("us-west-1") @@ -788,7 +788,7 @@ public class ControllerTest { ZoneApiMock.fromId("prod.us-west-1"), ZoneApiMock.newBuilder().with(CloudName.from("aws")).withId("prod.aws-us-east-1").build() ); - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); var applicationPackage = new ApplicationPackageBuilder() .region("aws-us-east-1") .region("us-west-1") 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 a5840cea3bd..5bbce118b1a 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 @@ -57,12 +57,13 @@ import static org.junit.Assert.assertTrue; * A deployment context for an application. This allows fine-grained control of the deployment of an application's * instances. * - * References to this should be acquired through {@link DeploymentTester#deploymentContext}. + * References to this should be acquired through {@link DeploymentTester#newDeploymentContext}. * * Tester code that is not specific to deployments should be added to either {@link ControllerTester} or * {@link DeploymentTester} instead of this class. * * @author mpolden + * @author jonmv */ public class DeploymentContext { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index 0ca035b85b2..c9e2b2c0954 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -140,8 +140,8 @@ public class DeploymentTester { } /** Returns the default deployment context owned by this */ - public DeploymentContext deploymentContext() { - return defaultContext; + public DeploymentContext newDeploymentContext() { + return newDeploymentContext(instanceId); } /** Create a new deployment context for given application */ diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 07cddbe2c7e..754065225ae 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -59,7 +59,7 @@ public class DeploymentTriggerTest { .build(); // Deploy completely once - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); // New version is released Version version = Version.fromString("6.3"); @@ -141,7 +141,7 @@ public class DeploymentTriggerTest { @Test public void abortsJobsOnNewApplicationChange() { - var app = tester.deploymentContext(); + var app = tester.newDeploymentContext(); app.submit() .runJob(systemTest) .runJob(stagingTest); @@ -188,7 +188,7 @@ public class DeploymentTriggerTest { .region("us-central-1") .delay(Duration.ofMinutes(10)) // Delays after last region are valid, but have no effect .build(); - var app = tester.deploymentContext().submit(applicationPackage); + var app = tester.newDeploymentContext().submit(applicationPackage); // Test jobs pass app.runJob(systemTest).runJob(stagingTest); @@ -242,7 +242,7 @@ public class DeploymentTriggerTest { .region("eu-west-1") .build(); - var app = tester.deploymentContext().submit(applicationPackage); + var app = tester.newDeploymentContext().submit(applicationPackage); // Test jobs pass app.runJob(systemTest).runJob(stagingTest); @@ -279,7 +279,7 @@ public class DeploymentTriggerTest { .region("us-central-1") .parallel("us-west-1", "us-east-3") .build(); - var application = tester.deploymentContext().submit().deploy(); + var application = tester.newDeploymentContext().submit().deploy(); // The first production zone is suspended: tester.configServer().setSuspended(application.deploymentIdIn(ZoneId.from("prod", "us-central-1")), true); @@ -317,7 +317,7 @@ public class DeploymentTriggerTest { .region("us-east-3") .build(); - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); tester.clock().advance(Duration.ofHours(1)); // --------------- Enter block window: 18:30 @@ -352,7 +352,7 @@ public class DeploymentTriggerTest { .region("us-west-1") .region("us-east-3") .build(); - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); // Application on (6.1, 1.0.1) Version v1 = Version.fromString("6.1"); @@ -399,7 +399,7 @@ public class DeploymentTriggerTest { .region("us-west-1") .region("us-east-3") .build(); - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); tester.controllerTester().upgradeSystem(new Version("9.8.7")); tester.upgrader().maintain(); @@ -441,7 +441,7 @@ public class DeploymentTriggerTest { .region("us-central-1") .region("eu-west-1") .build(); - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); // productionUsCentral1 fails after deployment, causing a mismatch between deployed and successful state. app.submit(applicationPackage) @@ -561,7 +561,7 @@ public class DeploymentTriggerTest { .parallel("eu-west-1", "us-east-3") .build(); // Application version 1 and platform version 6.1. - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); // Success in first prod zone, change cancelled between triggering and completion of eu west job. // One of the parallel zones get a deployment, but both fail their jobs. @@ -613,7 +613,7 @@ public class DeploymentTriggerTest { .build(); // Deploy completely on default application and platform versions - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); // New application change is deployed and fails in system-test for a while app.submit(applicationPackage).runJob(stagingTest).failDeployment(systemTest); @@ -666,7 +666,7 @@ public class DeploymentTriggerTest { // Initial failure Instant initialFailure = tester.clock().instant().truncatedTo(MILLIS); - var app = tester.deploymentContext().submit(applicationPackage); + var app = tester.newDeploymentContext().submit(applicationPackage); app.failDeployment(systemTest); assertEquals("Failure age is right at initial failure", initialFailure, app.instance().deploymentJobs().jobStatus().get(systemTest).firstFailing().get().at()); @@ -710,7 +710,7 @@ public class DeploymentTriggerTest { .region("us-west-1") .build(); Version version1 = tester.controller().versionStatus().systemVersion().get().versionNumber(); - var app1 = tester.deploymentContext(); + var app1 = tester.newDeploymentContext(); // First deployment: An application change app1.submit(applicationPackage).deploy(); @@ -855,7 +855,7 @@ public class DeploymentTriggerTest { @Test public void testUserInstancesNotInDeploymentSpec() { - var app = tester.deploymentContext(); + var app = tester.newDeploymentContext(); tester.controller().applications().createInstance(app.application().id().instance("user")); app.submit().deploy(); } @@ -868,7 +868,7 @@ public class DeploymentTriggerTest { .environment(Environment.prod) .region("us-east-3") .build(); - var app = tester.deploymentContext().submit(applicationPackage); // TODO jonmv: support instances in deployment context> + var app = tester.newDeploymentContext().submit(applicationPackage); // TODO jonmv: support instances in deployment context> app.deploy(); assertEquals(2, app.application().instances().size()); assertEquals(2, app.application().productionDeployments().values().stream() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index 7be2b6a9797..e0f11a0b925 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -74,6 +74,7 @@ public class InternalStepRunnerTest { @Before public void setup() { tester = new DeploymentTester(); + app = tester.newDeploymentContext(); } private SystemName system() { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java index 4e4fbe00bb7..06a815819f4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java @@ -35,7 +35,7 @@ public class DeploymentMetricsMaintainerTest { @Test public void updates_metrics() { - var application = tester.deploymentContext(); + var application = tester.newDeploymentContext(); application.runJob(JobType.devUsEast1, new ApplicationPackage(new byte[0]), Version.fromString("7.1")); DeploymentMetricsMaintainer maintainer = maintainer(tester.controller()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java index 31f55be92a9..7c87cbf3610 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java @@ -78,7 +78,7 @@ public class MetricsReporterTest { MetricsReporter reporter = createReporter(tester.controller()); - var context = tester.deploymentContext() + var context = tester.newDeploymentContext() .submit(applicationPackage) .deploy(); reporter.maintain(); @@ -122,7 +122,7 @@ public class MetricsReporterTest { .build(); MetricsReporter reporter = createReporter(tester.controller()); - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); // Initial deployment without failures context.submit(applicationPackage).deploy(); @@ -174,7 +174,7 @@ public class MetricsReporterTest { .region("us-east-3") .build(); MetricsReporter reporter = createReporter(tester.controller()); - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); tester.configServer().generateWarnings(context.deploymentIdIn(ZoneId.from("prod", "us-west-1")), 3); tester.configServer().generateWarnings(context.deploymentIdIn(ZoneId.from("prod", "us-west-1")), 4); context.submit(applicationPackage).deploy(); @@ -186,7 +186,7 @@ public class MetricsReporterTest { public void build_time_reporting() { var tester = new DeploymentTester(); var applicationPackage = new ApplicationPackageBuilder().region("us-west-1").build(); - var context = tester.deploymentContext() + var context = tester.newDeploymentContext() .submit(applicationPackage) .deploy(); assertEquals(1000, context.lastSubmission().get().buildTime().get().toEpochMilli()); @@ -207,7 +207,7 @@ public class MetricsReporterTest { .region("us-east-3") .build(); MetricsReporter reporter = createReporter(tester.controller()); - var context = tester.deploymentContext() + var context = tester.newDeploymentContext() .deferDnsUpdates(); reporter.maintain(); assertEquals("Queue is empty initially", 0, metrics.getMetric(MetricsReporter.NAME_SERVICE_REQUESTS_QUEUED).intValue()); 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 58a49307733..36039c47025 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 @@ -461,7 +461,7 @@ public class UpgraderTest { .region("us-west-1") .build(); - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); // New version is released version = Version.fromString("6.3"); @@ -504,7 +504,7 @@ public class UpgraderTest { .region("us-east-3") .build(); - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); // New version is released version = Version.fromString("6.3"); @@ -541,7 +541,7 @@ public class UpgraderTest { .region("us-east-3") .build(); - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); // New version is released version = Version.fromString("6.3"); @@ -662,7 +662,7 @@ public class UpgraderTest { // Setup applications var canary0 = createAndDeploy("canary0", "canary"); - var default0 = tester.deploymentContext().submit(version6ApplicationPackage).deploy(); + var default0 = tester.newDeploymentContext().submit(version6ApplicationPackage).deploy(); // New major version is released version = Version.fromString("7.0"); @@ -690,7 +690,7 @@ public class UpgraderTest { // Setup applications var canary0 = createAndDeploy("canary", "canary"); - var default0 = tester.deploymentContext().submit().deploy(); + var default0 = tester.newDeploymentContext().submit().deploy(); tester.applications().lockApplicationOrThrow(default0.application().id(), a -> tester.applications().store(a.withMajorVersion(6))); assertEquals(OptionalInt.of(6), default0.application().majorVersion()); @@ -819,7 +819,7 @@ public class UpgraderTest { .region("us-east-3") .build(); - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); // Application upgrade starts. app.submit(applicationPackage); @@ -861,7 +861,7 @@ public class UpgraderTest { .region("us-east-3") .build(); - var app = tester.deploymentContext().submit(applicationPackage).deploy(); + var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); // Application revision starts rolling out. @@ -895,7 +895,7 @@ public class UpgraderTest { tester.controllerTester().upgradeSystem(version0); // Create an application with pinned platform version. - var context = tester.deploymentContext(); + var context = tester.newDeploymentContext(); tester.deploymentTrigger().forceChange(context.application().id(), Change.empty().withPin()); context.submit().deploy(); @@ -993,7 +993,7 @@ public class UpgraderTest { .region("us-west-1") .region("us-east-3") .build(); - var application = tester.deploymentContext().submit(applicationPackage).deploy(); + var application = tester.newDeploymentContext().submit(applicationPackage).deploy(); // Next version is released and 2/3 deployments upgrade Version v2 = Version.fromString("6.2"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index 5c68fd2e370..06260da833f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -401,7 +401,7 @@ public class VersionStatusTest { assertEquals(commitDate0, tester.controller().versionStatus().systemVersion().get().committedAt()); // Deploy app on version0 to keep computing statistics for that version - tester.deploymentContext().submit().deploy(); + tester.newDeploymentContext().submit().deploy(); // Commit details are updated for new version var version1 = tester.controllerTester().nextVersion(); -- cgit v1.2.3 From 57def4af1e20262f39f03b5668f02aecd294db31 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:33:42 +0100 Subject: Applications with a custom, pre-existing setup, is OK --- .../yahoo/vespa/hosted/controller/deployment/DeploymentContext.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 5bbce118b1a..8045fab876e 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 @@ -119,9 +119,7 @@ public class DeploymentContext { try { var tenant = tester.createTenant(instanceId.tenant().value()); tester.createApplication(tenant, instanceId.application().value(), instanceId.instance().value()); - } catch (IllegalArgumentException ignored) { - // TODO(mpolden): Application already exists. Remove this once InternalDeploymentTester stops implicitly creating applications - } + } catch (IllegalArgumentException ignored) { } // Tenant and or application may already exist with custom setup. } public Application application() { -- cgit v1.2.3 From a3a216bff21a22e322e18529a9c90854d355ac93 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:34:10 +0100 Subject: Use 1000 as a default project ID if none has been set --- .../com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8045fab876e..58d1c3e3ced 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 @@ -204,7 +204,7 @@ public class DeploymentContext { var projectId = tester.controller().applications() .requireApplication(applicationId) .projectId() - .orElseThrow(() -> new IllegalArgumentException("No project ID set for " + applicationId)); + .orElse(1000); // These are really set through submission, so just pick one if it hasn't been set. lastSubmission = jobs.submit(applicationId, sourceRevision, "a@b", projectId, applicationPackage, new byte[0]); return this; } -- cgit v1.2.3 From 2180cc01b9600355602af8cd6308601c4215d39d Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:34:42 +0100 Subject: Return more Mock types in MockServiceRegistry --- .../integration/ServiceRegistryMock.java | 34 +++++++++++----------- .../restapi/application/ApplicationApiTest.java | 4 +-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java index a3c06c75a26..9cbfa54798e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java @@ -97,62 +97,62 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg } @Override - public Mailer mailer() { + public MockMailer mailer() { return mockMailer; } @Override - public ApplicationCertificateProvider applicationCertificateProvider() { + public ApplicationCertificateMock applicationCertificateProvider() { return applicationCertificateMock; } @Override - public MeteringClient meteringService() { + public MockMeteringClient meteringService() { return mockMeteringClient; } @Override - public ContactRetriever contactRetriever() { + public MockContactRetriever contactRetriever() { return mockContactRetriever; } @Override - public IssueHandler issueHandler() { + public MockIssueHandler issueHandler() { return mockIssueHandler; } @Override - public OwnershipIssues ownershipIssues() { + public DummyOwnershipIssues ownershipIssues() { return dummyOwnershipIssues; } @Override - public DeploymentIssues deploymentIssues() { + public LoggingDeploymentIssues deploymentIssues() { return loggingDeploymentIssues; } @Override - public EntityService entityService() { + public MemoryEntityService entityService() { return memoryEntityService; } @Override - public CostReportConsumer costReportConsumer() { + public CostReportConsumerMock costReportConsumer() { return costReportConsumerMock; } @Override - public Billing billingService() { + public MockBilling billingService() { return mockBilling; } @Override - public AwsEventFetcher eventFetcherService() { + public MockAwsEventFetcher eventFetcherService() { return mockAwsEventFetcher; } @Override - public ArtifactRepository artifactRepository() { + public ArtifactRepositoryMock artifactRepository() { return artifactRepositoryMock; } @@ -162,27 +162,27 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg } @Override - public ApplicationStore applicationStore() { + public ApplicationStoreMock applicationStore() { return applicationStoreMock; } @Override - public RunDataStore runDataStore() { + public MockRunDataStore runDataStore() { return mockRunDataStore; } @Override - public BuildService buildService() { + public MockBuildService buildService() { return mockBuildService; } @Override - public NameService nameService() { + public MemoryNameService nameService() { return memoryNameService; } @Override - public TenantCost tenantCost() { return mockTenantCost;} + public MockTenantCost tenantCost() { return mockTenantCost;} public ZoneRegistryMock zoneRegistryMock() { return zoneRegistryMock; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index cf28845b4e1..18b59814e5d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -1067,7 +1067,7 @@ public class ApplicationApiTest extends ControllerContainerTest { @Test public void testMeteringResponses() { - MockMeteringClient mockMeteringClient = (MockMeteringClient) controllerTester.containerTester().serviceRegistry().meteringService(); + MockMeteringClient mockMeteringClient = controllerTester.containerTester().serviceRegistry().meteringService(); // Mock response for MeteringClient ResourceAllocation currentSnapshot = new ResourceAllocation(1, 2, 3); @@ -1090,7 +1090,7 @@ public class ApplicationApiTest extends ControllerContainerTest { @Test public void testTenantCostResponse() { ApplicationId applicationId = createTenantAndApplication(); - MockTenantCost mockTenantCost = (MockTenantCost) controllerTester.containerTester().serviceRegistry().tenantCost(); + MockTenantCost mockTenantCost = controllerTester.containerTester().serviceRegistry().tenantCost(); mockTenantCost.setMonthsWithMetering( new TreeSet<>(Set.of( -- cgit v1.2.3 From 88f48f2f433c9b59a783e44c9d25d66ca474459c Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:35:48 +0100 Subject: Use DeploymentContext instead of DeploymentTester directly --- .../controller/deployment/DeploymentContext.java | 2 + .../controller/deployment/DeploymentTester.java | 121 ---------------- .../deployment/InternalStepRunnerTest.java | 161 +++++++++++---------- .../ApplicationOwnershipConfirmerTest.java | 46 +++--- .../maintenance/DeploymentExpirerTest.java | 23 ++- .../maintenance/DeploymentIssueReporterTest.java | 63 ++++---- .../maintenance/OutstandingChangeDeployerTest.java | 41 +++--- .../JobControllerApiHandlerHelperTest.java | 79 +++++----- 8 files changed, 199 insertions(+), 337 deletions(-) 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 58d1c3e3ced..28057c3787a 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 @@ -138,6 +138,8 @@ public class DeploymentContext { return instanceId; } + public TesterId testerId() { return testerId; } + public DeploymentId deploymentIdIn(ZoneId zone) { return new DeploymentId(instanceId, zone); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index c9e2b2c0954..9316ea632cf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.deployment; -import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.log.LogLevel; @@ -14,13 +13,9 @@ import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzDbMock; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGeneratorMock; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockTesterCloud; -import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.maintenance.JobControl; @@ -36,12 +31,8 @@ import java.time.Instant; import java.time.LocalDateTime; import java.time.ZoneOffset; import java.util.Collections; -import java.util.Optional; import java.util.logging.Logger; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; /** @@ -59,7 +50,6 @@ public class DeploymentTester { public static final ApplicationId instanceId = appId.defaultInstance(); public static final TesterId testerId = TesterId.of(instanceId); - private final DeploymentContext defaultContext; private final ControllerTester tester; private final JobController jobs; private final RoutingGeneratorMock routing; @@ -104,7 +94,6 @@ public class DeploymentTester { outstandingChangeDeployer = new OutstandingChangeDeployer(tester.controller(), maintenanceInterval, jobControl); nameServiceDispatcher = new NameServiceDispatcher(tester.controller(), maintenanceInterval, jobControl, Integer.MAX_VALUE); - defaultContext = newDeploymentContext(instanceId); routing.putEndpoints(new DeploymentId(null, null), Collections.emptyList()); // Turn off default behaviour for the mock. // Get deployment job logs to stderr. @@ -159,29 +148,6 @@ public class DeploymentTester { return newDeploymentContext(tenantName, applicationName, instanceName).application(); } - /** Submits a new application, and returns the version of the new submission. */ - public ApplicationVersion newSubmission(TenantAndApplicationId id, ApplicationPackage applicationPackage, SourceRevision sourceRevision) { - return newDeploymentContext(id.defaultInstance()).submit(applicationPackage, sourceRevision).lastSubmission().get(); - } - - public ApplicationVersion newSubmission(TenantAndApplicationId id, ApplicationPackage applicationPackage) { - return newSubmission(id, applicationPackage, DeploymentContext.defaultSourceRevision); - } - - /** - * Submits a new application package, and returns the version of the new submission. - */ - public ApplicationVersion newSubmission(ApplicationPackage applicationPackage) { - return newSubmission(appId, applicationPackage); - } - - /** - * Submits a new application, and returns the version of the new submission. - */ - public ApplicationVersion newSubmission() { - return defaultContext.submit().lastSubmission().get(); - } - /** * Sets a single endpoint in the routing mock; this matches that required for the tester. */ @@ -189,34 +155,6 @@ public class DeploymentTester { newDeploymentContext(id).setEndpoints(zone); } - /** Completely deploys the given application version, assuming it is the last to be submitted. */ - public void deployNewSubmission(ApplicationVersion version) { - deployNewSubmission(appId, version); - } - - /** Completely deploys the given application version, assuming it is the last to be submitted. */ - public void deployNewSubmission(TenantAndApplicationId id, ApplicationVersion version) { - var context = newDeploymentContext(id.defaultInstance()); - var application = context.application(); - assertFalse(application.instances().values().stream() - .anyMatch(instance -> instance.deployments().values().stream() - .anyMatch(deployment -> deployment.applicationVersion().equals(version)))); - assertEquals(version, application.change().application().get()); - assertFalse(application.change().platform().isPresent()); - context.completeRollout(); - assertFalse(context.application().change().hasTargets()); - } - - /** Completely deploys the given, new platform. */ - public void deployNewPlatform(Version version) { - deployNewPlatform(appId, version); - } - - /** Completely deploys the given, new platform. */ - public void deployNewPlatform(TenantAndApplicationId id, Version version) { - newDeploymentContext(id.defaultInstance()).deployPlatform(version); - } - /** Aborts and finishes all running jobs. */ public void abortAll() { triggerJobs(); @@ -234,63 +172,4 @@ public class DeploymentTester { return triggered; } - /** Starts a manual deployment of the given package, and then runs the whole of the given job, successfully. */ - public void runJob(ApplicationId instanceId, JobType type, ApplicationPackage applicationPackage) { - jobs.deploy(instanceId, type, Optional.empty(), applicationPackage); - newDeploymentContext(instanceId).runJob(type); - } - - /** Pulls the ready job trigger, and then runs the whole of the given job, successfully. */ - public void runJob(JobType type) { - defaultContext.runJob(type); - } - - /** Pulls the ready job trigger, and then runs the whole of the given job, successfully. */ - public void runJob(ApplicationId instanceId, JobType type) { - if (type.environment().isManuallyDeployed()) - throw new IllegalArgumentException("Use overload with application package for dev/perf jobs"); - newDeploymentContext(instanceId).runJob(type); - } - - public void failDeployment(JobType type) { - defaultContext.failDeployment(type); - } - - public void failDeployment(ApplicationId instanceId, JobType type) { - newDeploymentContext(instanceId).failDeployment(type); - } - - public void timeOutUpgrade(JobType type) { - defaultContext.timeOutUpgrade(type); - } - - public void timeOutUpgrade(ApplicationId instanceId, JobType type) { - newDeploymentContext(instanceId).timeOutConvergence(type); - } - - public void timeOutConvergence(JobType type) { - defaultContext.timeOutConvergence(type); - } - - public void timeOutConvergence(ApplicationId instanceId, JobType type) { - newDeploymentContext(instanceId).timeOutConvergence(type); - } - - public RunId startSystemTestTests() { - return defaultContext.startSystemTestTests(); - } - - /** Creates and submits a new application, and then starts the job of the given type. Use only once per test. */ - public RunId newRun(JobType type) { - return defaultContext.newRun(type); - } - - public void assertRunning(JobType type) { - assertRunning(instanceId, type); - } - - public void assertRunning(ApplicationId id, JobType type) { - assertTrue(jobs.active().stream().anyMatch(run -> run.id().application().equals(id) && run.id().type() == type)); - } - } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index e0f11a0b925..d50399c6c78 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -53,7 +53,6 @@ import static com.yahoo.vespa.hosted.controller.api.integration.LogEntry.Type.wa import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTester.instanceId; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.applicationPackage; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.publicCdApplicationPackage; -import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTester.testerId; 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; @@ -70,6 +69,7 @@ import static org.junit.Assert.fail; public class InternalStepRunnerTest { private DeploymentTester tester; + private DeploymentContext app; @Before public void setup() { @@ -83,15 +83,16 @@ public class InternalStepRunnerTest { @Test public void canRegisterAndRunDirectly() { - tester.deploymentContext().submit().deploy(); + app.submit().deploy(); } @Test public void testerHasAthenzIdentity() { - tester.newRun(JobType.stagingTest); + app.submit(); + tester.triggerJobs(); tester.runner().run(); DeploymentSpec spec = tester.configServer() - .application(DeploymentTester.testerId.id(), JobType.stagingTest.zone(system())).get() + .application(app.testerId().id(), JobType.stagingTest.zone(system())).get() .applicationPackage().deploymentSpec(); assertEquals("domain", spec.athenzDomain().get().value()); ZoneId zone = JobType.stagingTest.zone(system()); @@ -100,21 +101,21 @@ public class InternalStepRunnerTest { @Test public void refeedRequirementBlocksDeployment() { - RunId id = tester.newRun(JobType.stagingTest); + RunId id = app.newRun(JobType.stagingTest); - tester.setEndpoints(testerId.id(), JobType.stagingTest.zone(system())); + tester.setEndpoints(app.testerId().id(), JobType.stagingTest.zone(system())); tester.runner().run(); assertEquals(unfinished, tester.jobs().run(id).get().steps().get(Step.installInitialReal)); - tester.setEndpoints(instanceId, JobType.stagingTest.zone(system())); - tester.configServer().convergeServices(instanceId, JobType.stagingTest.zone(system())); + tester.setEndpoints(app.instanceId(), JobType.stagingTest.zone(system())); + tester.configServer().convergeServices(app.instanceId(), JobType.stagingTest.zone(system())); tester.configServer().setConfigChangeActions(new ConfigChangeActions(Collections.emptyList(), - singletonList(new RefeedAction("Refeed", - false, - "doctype", - "cluster", - Collections.emptyList(), - singletonList("Refeed it!"))))); + singletonList(new RefeedAction("Refeed", + false, + "doctype", + "cluster", + Collections.emptyList(), + singletonList("Refeed it!"))))); tester.runner().run(); assertEquals(failed, tester.jobs().run(id).get().steps().get(Step.deployReal)); @@ -122,11 +123,11 @@ public class InternalStepRunnerTest { @Test public void restartsServicesAndWaitsForRestartAndReboot() { - RunId id = tester.newRun(JobType.productionUsCentral1); + RunId id = app.newRun(JobType.productionUsCentral1); ZoneId zone = id.type().zone(system()); HostName host = tester.configServer().hostFor(instanceId, zone); - tester.setEndpoints(testerId.id(), JobType.productionUsCentral1.zone(system())); + tester.setEndpoints(app.testerId().id(), JobType.productionUsCentral1.zone(system())); tester.runner().run(); tester.configServer().setConfigChangeActions(new ConfigChangeActions(singletonList(new RestartAction("cluster", @@ -141,11 +142,11 @@ public class InternalStepRunnerTest { tester.runner().run(); assertEquals(succeeded, tester.jobs().run(id).get().steps().get(Step.deployReal)); - tester.configServer().convergeServices(instanceId, zone); + tester.configServer().convergeServices(app.instanceId(), zone); assertEquals(unfinished, tester.jobs().run(id).get().steps().get(Step.installReal)); - tester.configServer().nodeRepository().doRestart(new DeploymentId(instanceId, zone), Optional.of(host)); - tester.configServer().nodeRepository().requestReboot(new DeploymentId(instanceId, zone), Optional.of(host)); + tester.configServer().nodeRepository().doRestart(app.deploymentIdIn(zone), Optional.of(host)); + tester.configServer().nodeRepository().requestReboot(app.deploymentIdIn(zone), Optional.of(host)); tester.runner().run(); assertEquals(unfinished, tester.jobs().run(id).get().steps().get(Step.installReal)); @@ -156,85 +157,85 @@ public class InternalStepRunnerTest { @Test public void waitsForEndpointsAndTimesOut() { - tester.newRun(JobType.systemTest); + app.newRun(JobType.systemTest); // Tester fails to show up for staging tests, and the real deployment for system tests. - tester.setEndpoints(testerId.id(), JobType.systemTest.zone(system())); - tester.setEndpoints(instanceId, JobType.stagingTest.zone(system())); + tester.setEndpoints(app.testerId().id(), JobType.systemTest.zone(system())); + tester.setEndpoints(app.instanceId(), JobType.stagingTest.zone(system())); tester.runner().run(); - tester.configServer().convergeServices(instanceId, JobType.stagingTest.zone(system())); + tester.configServer().convergeServices(app.instanceId(), JobType.stagingTest.zone(system())); tester.runner().run(); - tester.configServer().convergeServices(instanceId, JobType.systemTest.zone(system())); - tester.configServer().convergeServices(testerId.id(), JobType.systemTest.zone(system())); - tester.configServer().convergeServices(instanceId, JobType.stagingTest.zone(system())); - tester.configServer().convergeServices(testerId.id(), JobType.stagingTest.zone(system())); + tester.configServer().convergeServices(app.instanceId(), JobType.systemTest.zone(system())); + tester.configServer().convergeServices(app.testerId().id(), JobType.systemTest.zone(system())); + tester.configServer().convergeServices(app.instanceId(), JobType.stagingTest.zone(system())); + tester.configServer().convergeServices(app.testerId().id(), JobType.stagingTest.zone(system())); tester.runner().run(); tester.clock().advance(InternalStepRunner.endpointTimeout.plus(Duration.ofSeconds(1))); tester.runner().run(); - assertEquals(failed, tester.jobs().last(instanceId, JobType.systemTest).get().steps().get(Step.installReal)); - assertEquals(failed, tester.jobs().last(instanceId, JobType.stagingTest).get().steps().get(Step.installTester)); + assertEquals(failed, tester.jobs().last(app.instanceId(), JobType.systemTest).get().steps().get(Step.installReal)); + assertEquals(failed, tester.jobs().last(app.instanceId(), JobType.stagingTest).get().steps().get(Step.installTester)); } @Test public void installationFailsIfDeploymentExpires() { - tester.newRun(JobType.systemTest); + app.newRun(JobType.systemTest); tester.runner().run(); - tester.configServer().convergeServices(instanceId, JobType.systemTest.zone(system())); - tester.setEndpoints(instanceId, JobType.systemTest.zone(system())); + tester.configServer().convergeServices(app.instanceId(), JobType.systemTest.zone(system())); + tester.setEndpoints(app.instanceId(), JobType.systemTest.zone(system())); tester.runner().run(); - assertEquals(succeeded, tester.jobs().last(instanceId, JobType.systemTest).get().steps().get(Step.installReal)); + assertEquals(succeeded, tester.jobs().last(app.instanceId(), JobType.systemTest).get().steps().get(Step.installReal)); - tester.applications().deactivate(instanceId, JobType.systemTest.zone(system())); + tester.applications().deactivate(app.instanceId(), JobType.systemTest.zone(system())); tester.runner().run(); - assertEquals(failed, tester.jobs().last(instanceId, JobType.systemTest).get().steps().get(Step.installTester)); - assertTrue(tester.jobs().last(instanceId, JobType.systemTest).get().hasEnded()); - assertTrue(tester.jobs().last(instanceId, JobType.systemTest).get().hasFailed()); + assertEquals(failed, tester.jobs().last(app.instanceId(), JobType.systemTest).get().steps().get(Step.installTester)); + assertTrue(tester.jobs().last(app.instanceId(), JobType.systemTest).get().hasEnded()); + assertTrue(tester.jobs().last(app.instanceId(), JobType.systemTest).get().hasFailed()); } @Test public void startTestsFailsIfDeploymentExpires() { - tester.newRun(JobType.systemTest); + app.newRun(JobType.systemTest); tester.runner().run(); - tester.configServer().convergeServices(instanceId, JobType.systemTest.zone(system())); - tester.configServer().convergeServices(testerId.id(), JobType.systemTest.zone(system())); + tester.configServer().convergeServices(app.instanceId(), JobType.systemTest.zone(system())); + tester.configServer().convergeServices(app.testerId().id(), JobType.systemTest.zone(system())); tester.runner().run(); - tester.applications().deactivate(instanceId, JobType.systemTest.zone(system())); + tester.applications().deactivate(app.instanceId(), JobType.systemTest.zone(system())); tester.runner().run(); - assertEquals(unfinished, tester.jobs().last(instanceId, JobType.systemTest).get().steps().get(Step.startTests)); + assertEquals(unfinished, tester.jobs().last(app.instanceId(), JobType.systemTest).get().steps().get(Step.startTests)); } @Test public void alternativeEndpointsAreDetected() { - tester.newRun(JobType.systemTest); + app.newRun(JobType.systemTest); tester.runner().run();; - tester.configServer().convergeServices(instanceId, JobType.systemTest.zone(system())); - tester.configServer().convergeServices(testerId.id(), JobType.systemTest.zone(system())); - assertEquals(unfinished, tester.jobs().last(instanceId, JobType.systemTest).get().steps().get(Step.installReal)); - assertEquals(unfinished, tester.jobs().last(instanceId, JobType.systemTest).get().steps().get(Step.installTester)); - - tester.controller().curator().writeRoutingPolicies(instanceId, Set.of(new RoutingPolicy(instanceId, + tester.configServer().convergeServices(app.instanceId(), JobType.systemTest.zone(system())); + tester.configServer().convergeServices(app.testerId().id(), JobType.systemTest.zone(system())); + assertEquals(unfinished, tester.jobs().last(app.instanceId(), JobType.systemTest).get().steps().get(Step.installReal)); + assertEquals(unfinished, tester.jobs().last(app.instanceId(), JobType.systemTest).get().steps().get(Step.installTester)); + + tester.controller().curator().writeRoutingPolicies(app.instanceId(), Set.of(new RoutingPolicy(app.instanceId(), + ClusterSpec.Id.from("default"), + JobType.systemTest.zone(system()), + HostName.from("host"), + Optional.empty(), + emptySet()))); + tester.controller().curator().writeRoutingPolicies(app.testerId().id(), Set.of(new RoutingPolicy(app.testerId().id(), ClusterSpec.Id.from("default"), JobType.systemTest.zone(system()), HostName.from("host"), Optional.empty(), emptySet()))); - tester.controller().curator().writeRoutingPolicies(testerId.id(), Set.of(new RoutingPolicy(testerId.id(), - ClusterSpec.Id.from("default"), - JobType.systemTest.zone(system()), - HostName.from("host"), - Optional.empty(), - emptySet()))); tester.runner().run();; - assertEquals(succeeded, tester.jobs().last(instanceId, JobType.systemTest).get().steps().get(Step.installReal)); - assertEquals(succeeded, tester.jobs().last(instanceId, JobType.systemTest).get().steps().get(Step.installTester)); + assertEquals(succeeded, tester.jobs().last(app.instanceId(), JobType.systemTest).get().steps().get(Step.installReal)); + assertEquals(succeeded, tester.jobs().last(app.instanceId(), JobType.systemTest).get().steps().get(Step.installTester)); } @Test public void testsFailIfTesterRestarts() { - RunId id = tester.startSystemTestTests(); + RunId id = app.startSystemTestTests(); tester.cloud().set(TesterCloud.Status.NOT_STARTED); tester.runner().run(); assertEquals(failed, tester.jobs().run(id).get().steps().get(Step.endTests)); @@ -242,7 +243,7 @@ public class InternalStepRunnerTest { @Test public void testsFailIfTestsFailRemotely() { - RunId id = tester.startSystemTestTests(); + RunId id = app.startSystemTestTests(); tester.cloud().add(new LogEntry(123, Instant.ofEpochMilli(321), error, "Failure!")); tester.cloud().set(TesterCloud.Status.FAILURE); @@ -256,7 +257,7 @@ public class InternalStepRunnerTest { @Test public void testsFailIfTestsErr() { - RunId id = tester.startSystemTestTests(); + RunId id = app.startSystemTestTests(); tester.cloud().add(new LogEntry(0, Instant.ofEpochMilli(123), error, "Error!")); tester.cloud().set(TesterCloud.Status.ERROR); @@ -270,13 +271,13 @@ public class InternalStepRunnerTest { @Test public void testsSucceedWhenTheyDoRemotely() { - RunId id = tester.startSystemTestTests(); + RunId id = app.startSystemTestTests(); tester.runner().run(); assertEquals(unfinished, tester.jobs().run(id).get().steps().get(Step.endTests)); - assertEquals(URI.create(tester.routing().endpoints(new DeploymentId(testerId.id(), JobType.systemTest.zone(system()))).get(0).endpoint()), + assertEquals(URI.create(tester.routing().endpoints(new DeploymentId(app.testerId().id(), JobType.systemTest.zone(system()))).get(0).endpoint()), tester.cloud().testerUrl()); Inspector configObject = SlimeUtils.jsonToSlime(tester.cloud().config()).get(); - assertEquals(instanceId.serializedForm(), configObject.field("application").asString()); + assertEquals(app.instanceId().serializedForm(), configObject.field("application").asString()); assertEquals(JobType.systemTest.zone(system()).value(), configObject.field("zone").asString()); assertEquals(system().value(), configObject.field("system").asString()); assertEquals(1, configObject.field("endpoints").children()); @@ -309,36 +310,36 @@ public class InternalStepRunnerTest { @Test public void deployToDev() { ZoneId zone = JobType.devUsEast1.zone(system()); - tester.jobs().deploy(instanceId, JobType.devUsEast1, Optional.empty(), applicationPackage); + tester.jobs().deploy(app.instanceId(), JobType.devUsEast1, Optional.empty(), applicationPackage); tester.runner().run(); - RunId id = tester.jobs().last(instanceId, JobType.devUsEast1).get().id(); + RunId id = tester.jobs().last(app.instanceId(), JobType.devUsEast1).get().id(); assertEquals(unfinished, tester.jobs().run(id).get().steps().get(Step.installReal)); Version version = new Version("7.8.9"); Future concurrentDeployment = Executors.newSingleThreadExecutor().submit(() -> { - tester.jobs().deploy(instanceId, JobType.devUsEast1, Optional.of(version), applicationPackage); + tester.jobs().deploy(app.instanceId(), JobType.devUsEast1, Optional.of(version), applicationPackage); }); while ( ! concurrentDeployment.isDone()) tester.runner().run(); - assertEquals(id.number() + 1, tester.jobs().last(instanceId, JobType.devUsEast1).get().id().number()); + assertEquals(id.number() + 1, tester.jobs().last(app.instanceId(), JobType.devUsEast1).get().id().number()); ApplicationPackage otherPackage = new ApplicationPackageBuilder().region("us-central-1").build(); - tester.jobs().deploy(instanceId, JobType.perfUsEast3, Optional.empty(), otherPackage); + tester.jobs().deploy(app.instanceId(), JobType.perfUsEast3, Optional.empty(), otherPackage); tester.runner().run(); // Job run order determined by JobType enum order per application. - tester.configServer().convergeServices(instanceId, zone); - tester.setEndpoints(instanceId, zone); + tester.configServer().convergeServices(app.instanceId(), zone); + tester.setEndpoints(app.instanceId(), zone); assertEquals(unfinished, tester.jobs().run(id).get().steps().get(Step.installReal)); - assertEquals(applicationPackage.hash(), tester.configServer().application(instanceId, zone).get().applicationPackage().hash()); - assertEquals(otherPackage.hash(), tester.configServer().application(instanceId, JobType.perfUsEast3.zone(system())).get().applicationPackage().hash()); + assertEquals(applicationPackage.hash(), tester.configServer().application(app.instanceId(), zone).get().applicationPackage().hash()); + assertEquals(otherPackage.hash(), tester.configServer().application(app.instanceId(), JobType.perfUsEast3.zone(system())).get().applicationPackage().hash()); - tester.configServer().setVersion(instanceId, zone, version); + tester.configServer().setVersion(app.instanceId(), zone, version); tester.runner().run(); assertEquals(1, tester.jobs().active().size()); - assertEquals(version, tester.instance(instanceId).deployments().get(zone).version()); + assertEquals(version, tester.instance(app.instanceId()).deployments().get(zone).version()); try { - tester.jobs().deploy(instanceId, JobType.productionApNortheast1, Optional.empty(), applicationPackage); + tester.jobs().deploy(app.instanceId(), JobType.productionApNortheast1, Optional.empty(), applicationPackage); fail("Deployments outside dev should not be allowed."); } catch (IllegalArgumentException expected) { } @@ -346,10 +347,10 @@ public class InternalStepRunnerTest { @Test public void notificationIsSent() { - tester.startSystemTestTests(); + app.startSystemTestTests(); tester.cloud().set(TesterCloud.Status.NOT_STARTED); tester.runner().run(); - MockMailer mailer = ((MockMailer) tester.controller().serviceRegistry().mailer()); + MockMailer mailer = tester.controllerTester().serviceRegistry().mailer(); assertEquals(1, mailer.inbox("a@b").size()); assertEquals("Vespa application tenant.application: System test failing due to system error", mailer.inbox("a@b").get(0).subject()); @@ -360,7 +361,7 @@ public class InternalStepRunnerTest { @Test public void vespaLogIsCopied() { - RunId id = tester.startSystemTestTests(); + RunId id = app.startSystemTestTests(); tester.cloud().set(TesterCloud.Status.ERROR); tester.configServer().setLogStream(vespaLog); long lastId = tester.jobs().details(id).get().lastId().getAsLong(); @@ -385,11 +386,11 @@ public class InternalStepRunnerTest { public void certificateTimeoutAbortsJob() { tester.controllerTester().zoneRegistry().setSystemName(SystemName.PublicCd); tester.controllerTester().zoneRegistry().setZones(ZoneApiMock.fromId("prod.aws-us-east-1c")); - RunId id = tester.startSystemTestTests(); + RunId id = app.startSystemTestTests(); List trusted = new ArrayList<>(publicCdApplicationPackage.trustedCertificates()); trusted.add(tester.jobs().run(id).get().testerCertificate().get()); - assertEquals(trusted, tester.configServer().application(instanceId, id.type().zone(system())).get().applicationPackage().trustedCertificates()); + assertEquals(trusted, tester.configServer().application(app.instanceId(), id.type().zone(system())).get().applicationPackage().trustedCertificates()); tester.clock().advance(InternalStepRunner.certificateTimeout.plus(Duration.ofSeconds(1))); tester.runner().run(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java index 88d1626b834..8997f34fb98 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java @@ -2,15 +2,12 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.InstanceName; -import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.LockedTenant; import com.yahoo.vespa.hosted.controller.api.integration.organization.ApplicationSummary; import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; -import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import com.yahoo.vespa.hosted.controller.tenant.UserTenant; @@ -20,7 +17,6 @@ import org.junit.Test; import java.time.Duration; import java.util.List; import java.util.Optional; -import java.util.function.Supplier; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTester.appId; import static org.junit.Assert.assertEquals; @@ -46,20 +42,18 @@ public class ApplicationOwnershipConfirmerTest { @Test public void testConfirmation() { Optional contact = Optional.of(tester.controllerTester().serviceRegistry().contactRetrieverMock().contact()); + var propertyApp = tester.newDeploymentContext(); tester.controller().tenants().lockOrThrow(appId.tenant(), LockedTenant.Athenz.class, tenant -> tester.controller().tenants().store(tenant.with(contact.get()))); - Supplier propertyApp = tester::application; - tester.deployNewSubmission(tester.newSubmission()); + propertyApp.submit().deploy(); UserTenant user = UserTenant.create("by-user", contact); tester.controller().tenants().createUser(user); - tester.createApplication(user.name().value(), "application", "default"); - TenantAndApplicationId userAppId = TenantAndApplicationId.from("by-user", "application"); - Supplier userApp = () -> tester.controller().applications().requireApplication(userAppId); - tester.deployNewSubmission(userAppId, tester.newSubmission(userAppId, DeploymentContext.applicationPackage)); + var userApp = tester.newDeploymentContext("by-user", "application", "default"); + userApp.submit().deploy(); - assertFalse("No issue is initially stored for a new application.", propertyApp.get().ownershipIssueId().isPresent()); - assertFalse("No issue is initially stored for a new application.", userApp.get().ownershipIssueId().isPresent()); + assertFalse("No issue is initially stored for a new application.", propertyApp.application().ownershipIssueId().isPresent()); + assertFalse("No issue is initially stored for a new application.", userApp.application().ownershipIssueId().isPresent()); assertFalse("No escalation has been attempted for a new application", issues.escalatedToContact || issues.escalatedToTerminator); // Set response from the issue mock, which will be obtained by the maintainer on issue filing. @@ -67,14 +61,14 @@ public class ApplicationOwnershipConfirmerTest { issues.response = issueId; confirmer.maintain(); - assertFalse("No issue is stored for an application newer than 3 months.", propertyApp.get().ownershipIssueId().isPresent()); - assertFalse("No issue is stored for an application newer than 3 months.", userApp.get().ownershipIssueId().isPresent()); + assertFalse("No issue is stored for an application newer than 3 months.", propertyApp.application().ownershipIssueId().isPresent()); + assertFalse("No issue is stored for an application newer than 3 months.", userApp.application().ownershipIssueId().isPresent()); tester.clock().advance(Duration.ofDays(91)); confirmer.maintain(); - assertEquals("Confirmation issue has been filed for property owned application.", issueId, propertyApp.get().ownershipIssueId()); - assertEquals("Confirmation issue has been filed for user owned application.", issueId, userApp.get().ownershipIssueId()); + assertEquals("Confirmation issue has been filed for property owned application.", issueId, propertyApp.application().ownershipIssueId()); + assertEquals("Confirmation issue has been filed for user owned application.", issueId, userApp.application().ownershipIssueId()); assertTrue(issues.escalatedToTerminator); assertTrue("Both applications have had their responses ensured.", issues.escalatedToContact); @@ -82,14 +76,14 @@ public class ApplicationOwnershipConfirmerTest { issues.response = Optional.empty(); confirmer.maintain(); - assertEquals("Confirmation issue reference is not updated when no issue id is returned.", issueId, propertyApp.get().ownershipIssueId()); - assertEquals("Confirmation issue reference is not updated when no issue id is returned.", issueId, userApp.get().ownershipIssueId()); + assertEquals("Confirmation issue reference is not updated when no issue id is returned.", issueId, propertyApp.application().ownershipIssueId()); + assertEquals("Confirmation issue reference is not updated when no issue id is returned.", issueId, userApp.application().ownershipIssueId()); // The user deletes all production deployments — see that the issue is forgotten. - assertEquals("Confirmation issue for user is still open.", issueId, userApp.get().ownershipIssueId()); - userApp.get().productionDeployments().values().stream().flatMap(List::stream) - .forEach(deployment -> tester.controller().applications().deactivate(userAppId.defaultInstance(), deployment.zone())); - assertTrue("No production deployments are listed for user.", userApp.get().require(InstanceName.defaultName()).productionDeployments().isEmpty()); + assertEquals("Confirmation issue for user is still open.", issueId, userApp.application().ownershipIssueId()); + userApp.application().productionDeployments().values().stream().flatMap(List::stream) + .forEach(deployment -> tester.controller().applications().deactivate(userApp.instanceId(), deployment.zone())); + assertTrue("No production deployments are listed for user.", userApp.application().require(InstanceName.defaultName()).productionDeployments().isEmpty()); confirmer.maintain(); // Time has passed, and a new confirmation issue is in order for the property which is still in production. @@ -97,13 +91,13 @@ public class ApplicationOwnershipConfirmerTest { issues.response = issueId2; confirmer.maintain(); - assertEquals("A new confirmation issue id is stored when something is returned to the maintainer.", issueId2, propertyApp.get().ownershipIssueId()); - assertEquals("Confirmation issue for application without production deployments has not been filed.", issueId, userApp.get().ownershipIssueId()); + assertEquals("A new confirmation issue id is stored when something is returned to the maintainer.", issueId2, propertyApp.application().ownershipIssueId()); + assertEquals("Confirmation issue for application without production deployments has not been filed.", issueId, userApp.application().ownershipIssueId()); - assertFalse("No owner is stored for application", propertyApp.get().owner().isPresent()); + assertFalse("No owner is stored for application", propertyApp.application().owner().isPresent()); issues.owner = Optional.of(User.from("username")); confirmer.maintain(); - assertEquals("Owner has been added to application", propertyApp.get().owner().get().username(), "username"); + assertEquals("Owner has been added to application", propertyApp.application().owner().get().username(), "username"); } 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 4df1336ac1f..258dad3b6d6 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 @@ -35,35 +35,32 @@ public class DeploymentExpirerTest { ); DeploymentExpirer expirer = new DeploymentExpirer(tester.controller(), Duration.ofDays(10), new JobControl(new MockCuratorDb())); - Application devApp = tester.createApplication("tenant1", "app1", "default"); - Application prodApp = tester.createApplication("tenant2", "app2", "default"); + var devApp = tester.newDeploymentContext("tenant1", "app1", "default"); + var prodApp = tester.newDeploymentContext("tenant2", "app2", "default"); ApplicationPackage appPackage = new ApplicationPackageBuilder() .region("us-west-1") .build(); - Instance devInstance = tester.instance(devApp.id().defaultInstance()); - Instance prodInstance = tester.instance(prodApp.id().defaultInstance()); - // Deploy dev - tester.runJob(devInstance.id(), JobType.devUsEast1, appPackage); + devApp.runJob(JobType.devUsEast1, appPackage); // Deploy prod - tester.deployNewSubmission(prodApp.id(), tester.newSubmission(prodApp.id(), appPackage)); + prodApp.submit(appPackage).deploy(); - assertEquals(1, permanentDeployments(devInstance).size()); - assertEquals(1, permanentDeployments(prodInstance).size()); + assertEquals(1, permanentDeployments(devApp.instance()).size()); + assertEquals(1, permanentDeployments(prodApp.instance()).size()); // Not expired at first expirer.maintain(); - assertEquals(1, permanentDeployments(devInstance).size()); - assertEquals(1, permanentDeployments(prodInstance).size()); + assertEquals(1, permanentDeployments(devApp.instance()).size()); + assertEquals(1, permanentDeployments(prodApp.instance()).size()); // The dev application is removed tester.clock().advance(Duration.ofDays(15)); expirer.maintain(); - assertEquals(0, permanentDeployments(devInstance).size()); - assertEquals(1, permanentDeployments(prodInstance).size()); + assertEquals(0, permanentDeployments(devApp.instance()).size()); + assertEquals(1, permanentDeployments(prodApp.instance()).size()); } private List permanentDeployments(Instance instance) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java index 070171ad399..0afe7377d40 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java @@ -13,6 +13,7 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; 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.persistence.MockCuratorDb; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; @@ -65,33 +66,28 @@ public class DeploymentIssueReporterTest { tester.controllerTester().upgradeSystem(Version.fromString("6.2")); // Create and deploy one application for each of three tenants. - Application app1 = tester.createApplication("application1", "tenant1", "default"); - Application app2 = tester.createApplication("application2", "tenant2", "default"); - Application app3 = tester.createApplication("application3", "tenant3", "default"); + var app1 = tester.newDeploymentContext("application1", "tenant1", "default"); + var app2 = tester.newDeploymentContext("application2", "tenant2", "default"); + var app3 = tester.newDeploymentContext("application3", "tenant3", "default"); Contact contact = tester.controllerTester().serviceRegistry().contactRetrieverMock().contact(); - tester.controller().tenants().lockOrThrow(app1.id().tenant(), LockedTenant.Athenz.class, tenant -> + tester.controller().tenants().lockOrThrow(app1.instanceId().tenant(), LockedTenant.Athenz.class, tenant -> tester.controller().tenants().store(tenant.with(contact))); - tester.controller().tenants().lockOrThrow(app2.id().tenant(), LockedTenant.Athenz.class, tenant -> + tester.controller().tenants().lockOrThrow(app2.instanceId().tenant(), LockedTenant.Athenz.class, tenant -> tester.controller().tenants().store(tenant.with(contact))); - tester.controller().tenants().lockOrThrow(app3.id().tenant(), LockedTenant.Athenz.class, tenant -> + tester.controller().tenants().lockOrThrow(app3.instanceId().tenant(), LockedTenant.Athenz.class, tenant -> tester.controller().tenants().store(tenant.with(contact))); // NOTE: All maintenance should be idempotent within a small enough time interval, so maintain is called twice in succession throughout. // app 1 fails staging tests. - tester.newSubmission(app1.id(), applicationPackage); - tester.runJob(app1.id().defaultInstance(), systemTest); - tester.timeOutConvergence(app1.id().defaultInstance(), stagingTest); + app1.submit(applicationPackage).runJob(systemTest).timeOutConvergence(stagingTest); // app2 is successful, but will fail later. - tester.deployNewSubmission(app2.id(), tester.newSubmission(app2.id(), applicationPackage)); + app2.submit(applicationPackage).deploy(); // app 3 fails a production job. - tester.newSubmission(app3.id(), applicationPackage); - tester.runJob(app3.id().defaultInstance(), systemTest); - tester.runJob(app3.id().defaultInstance(), stagingTest); - tester.failDeployment(app3.id().defaultInstance(), productionUsWest1); + app3.submit(applicationPackage).runJob(systemTest).runJob(stagingTest).failDeployment(productionUsWest1); reporter.maintain(); reporter.maintain(); @@ -103,53 +99,52 @@ public class DeploymentIssueReporterTest { reporter.maintain(); reporter.maintain(); - assertTrue("One issue is produced for app1.", issues.isOpenFor(app1.id())); - assertFalse("No issues are produced for app2.", issues.isOpenFor(app2.id())); - assertTrue("One issue is produced for app3.", issues.isOpenFor(app3.id())); + assertTrue("One issue is produced for app1.", issues.isOpenFor(app1.application().id())); + assertFalse("No issues are produced for app2.", issues.isOpenFor(app2.application().id())); + assertTrue("One issue is produced for app3.", issues.isOpenFor(app3.application().id())); // app3 closes their issue prematurely; see that it is refiled. - issues.closeFor(app3.id()); - assertFalse("No issue is open for app3.", issues.isOpenFor(app3.id())); + issues.closeFor(app3.application().id()); + assertFalse("No issue is open for app3.", issues.isOpenFor(app3.application().id())); reporter.maintain(); reporter.maintain(); - assertTrue("Issue is re-filed for app3.", issues.isOpenFor(app3.id())); + assertTrue("Issue is re-filed for app3.", issues.isOpenFor(app3.application().id())); // Some time passes; tenant1 leaves her issue unattended, while tenant3 starts work and updates the issue. tester.clock().advance(maxInactivity.plus(maxFailureAge)); - issues.touchFor(app3.id()); + issues.touchFor(app3.application().id()); reporter.maintain(); reporter.maintain(); - assertEquals("The issue for app1 is escalated once.", 1, issues.escalationLevelFor(app1.id())); + assertEquals("The issue for app1 is escalated once.", 1, issues.escalationLevelFor(app1.application().id())); // app3 fixes their problems, but the ticket for app3 is left open; see the resolved ticket is not escalated when another escalation period has passed. - tester.runJob(app3.id().defaultInstance(), productionUsWest1); + app3.runJob(productionUsWest1); tester.clock().advance(maxInactivity.plus(Duration.ofDays(1))); reporter.maintain(); reporter.maintain(); assertFalse("We no longer have a platform issue.", issues.platformIssue()); - assertEquals("The issue for app1 is escalated once more.", 2, issues.escalationLevelFor(app1.id())); - assertEquals("The issue for app3 is not escalated.", 0, issues.escalationLevelFor(app3.id())); + assertEquals("The issue for app1 is escalated once more.", 2, issues.escalationLevelFor(app1.application().id())); + assertEquals("The issue for app3 is not escalated.", 0, issues.escalationLevelFor(app3.application().id())); // app3 now has a new failure past max failure age; see that a new issue is filed. - tester.newSubmission(app3.id(), applicationPackage); - tester.failDeployment(app3.id().defaultInstance(), systemTest); + app3.submit(applicationPackage).failDeployment(systemTest); tester.clock().advance(maxInactivity.plus(maxFailureAge)); reporter.maintain(); reporter.maintain(); - assertTrue("A new issue is filed for app3.", issues.isOpenFor(app3.id())); + assertTrue("A new issue is filed for app3.", issues.isOpenFor(app3.application().id())); - // App2 is changed to be a canary - tester.deployNewSubmission(app2.id(), tester.newSubmission(app2.id(), canaryPackage)); - assertEquals(canary, tester.applications().requireApplication(app2.id()).deploymentSpec().requireInstance("default").upgradePolicy()); - assertEquals(Change.empty(), tester.applications().requireApplication(app2.id()).change()); + // app2 is changed to be a canary + app2.submit(canaryPackage).deploy(); + assertEquals(canary, app2.application().deploymentSpec().requireInstance("default").upgradePolicy()); + assertEquals(Change.empty(), app2.application().change()); // Bump system version to upgrade canary app2. Version version = Version.fromString("6.3"); @@ -157,7 +152,7 @@ public class DeploymentIssueReporterTest { tester.upgrader().maintain(); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); - tester.timeOutUpgrade(app2.id().defaultInstance(), systemTest); + app2.timeOutUpgrade(systemTest); tester.controllerTester().upgradeSystem(version); assertEquals(VespaVersion.Confidence.broken, tester.controller().versionStatus().systemVersion().get().confidence()); @@ -169,7 +164,7 @@ public class DeploymentIssueReporterTest { reporter.maintain(); reporter.maintain(); assertTrue("We get a platform issue when confidence is broken", issues.platformIssue()); - assertFalse("No deployment issue is filed for app2, which has a version upgrade failure.", issues.isOpenFor(app2.id())); + assertFalse("No deployment issue is filed for app2, which has a version upgrade failure.", issues.isOpenFor(app2.application().id())); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java index 0be873f80ed..11c77304dd0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java @@ -37,45 +37,40 @@ public class OutstandingChangeDeployerTest { .region("us-west-1") .build(); - Application app1 = tester.createApplication("tenant", "app1", "default"); - tester.deployNewSubmission(app1.id(), tester.newSubmission(app1.id(), applicationPackage)); + var app1 = tester.newDeploymentContext("tenant", "app1", "default").submit(applicationPackage).deploy(); Version version = new Version(6, 2); - tester.deploymentTrigger().triggerChange(app1.id(), Change.of(version)); + tester.deploymentTrigger().triggerChange(app1.application().id(), Change.of(version)); tester.deploymentTrigger().triggerReadyJobs(); - assertEquals(Change.of(version), tester.application(app1.id()).change()); - assertFalse(tester.application(app1.id()).outstandingChange().hasTargets()); + assertEquals(Change.of(version), app1.application().change()); + assertFalse(app1.application().outstandingChange().hasTargets()); - assertEquals(1, tester.application(app1.id()).latestVersion().get().buildNumber().getAsLong()); - tester.newSubmission(app1.id(), applicationPackage, new SourceRevision("repository1", "master", "cafed00d")); + assertEquals(1, app1.application().latestVersion().get().buildNumber().getAsLong()); + app1.submit(applicationPackage, new SourceRevision("repository1", "master", "cafed00d")); - ApplicationId instanceId = app1.id().defaultInstance(); - assertTrue(tester.application(app1.id()).outstandingChange().hasTargets()); - assertEquals("1.0.2-cafed00d", tester.application(app1.id()).outstandingChange().application().get().id()); - tester.assertRunning(instanceId, JobType.systemTest); - tester.assertRunning(instanceId, JobType.stagingTest); + assertTrue(app1.application().outstandingChange().hasTargets()); + assertEquals("1.0.2-cafed00d", app1.application().outstandingChange().application().get().id()); + app1.assertRunning(JobType.systemTest); + app1.assertRunning(JobType.stagingTest); assertEquals(2, tester.jobs().active().size()); deployer.maintain(); - tester.deploymentTrigger().triggerReadyJobs(); + tester.triggerJobs(); assertEquals("No effect as job is in progress", 2, tester.jobs().active().size()); - assertEquals("1.0.2-cafed00d", tester.application(app1.id()).outstandingChange().application().get().id()); + assertEquals("1.0.2-cafed00d", app1.application().outstandingChange().application().get().id()); - tester.runJob(instanceId, JobType.systemTest); - tester.runJob(instanceId, JobType.stagingTest); - tester.runJob(instanceId, JobType.productionUsWest1); - tester.runJob(instanceId, JobType.systemTest); - tester.runJob(instanceId, JobType.stagingTest); + app1.runJob(JobType.systemTest).runJob(JobType.stagingTest).runJob(JobType.productionUsWest1) + .runJob(JobType.stagingTest).runJob(JobType.systemTest); assertEquals("Upgrade done", 0, tester.jobs().active().size()); deployer.maintain(); - tester.deploymentTrigger().triggerReadyJobs(); - assertEquals("1.0.2-cafed00d", tester.application(app1.id()).change().application().get().id()); + tester.triggerJobs(); + assertEquals("1.0.2-cafed00d", app1.application().change().application().get().id()); List runs = tester.jobs().active(); assertEquals(1, runs.size()); - tester.assertRunning(instanceId, JobType.productionUsWest1); - assertFalse(tester.application(app1.id()).outstandingChange().hasTargets()); + app1.assertRunning(JobType.productionUsWest1); + assertFalse(app1.application().outstandingChange().hasTargets()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java index daf1a4c2ea7..8e3a3b8d727 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java @@ -33,9 +33,7 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobTy import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Status.FAILURE; -import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTester.instanceId; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.applicationPackage; -import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTester.testerId; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.installationFailed; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.running; @@ -51,60 +49,59 @@ public class JobControllerApiHandlerHelperTest { @Test public void testResponses() { DeploymentTester tester = new DeploymentTester(); + var app = tester.newDeploymentContext(); tester.clock().setInstant(Instant.EPOCH); // Revision 1 gets deployed everywhere. - ApplicationVersion revision1 = tester.newSubmission(); - tester.deployNewSubmission(revision1); + app.submit().deploy(); + ApplicationVersion revision1 = app.lastSubmission().get(); assertEquals(1000, tester.application().projectId().getAsLong()); tester.clock().advance(Duration.ofMillis(1000)); // Revision 2 gets deployed everywhere except in us-east-3. - ApplicationVersion revision2 = tester.newSubmission(); - tester.runJob(systemTest); - tester.runJob(stagingTest); - tester.runJob(productionUsCentral1); + ApplicationVersion revision2 = app.submit().lastSubmission().get(); + app.runJob(systemTest); + app.runJob(stagingTest); + app.runJob(productionUsCentral1); tester.triggerJobs(); // us-east-3 eats the deployment failure and fails before deployment, while us-west-1 fails after. tester.configServer().throwOnNextPrepare(new ConfigServerException(URI.create("url"), "ERROR!", INVALID_APPLICATION_PACKAGE, null)); tester.runner().run(); - assertEquals(deploymentFailed, tester.jobs().last(instanceId, productionUsEast3).get().status()); + assertEquals(deploymentFailed, tester.jobs().last(app.instanceId(), productionUsEast3).get().status()); ZoneId usWest1 = productionUsWest1.zone(tester.controller().system()); - tester.configServer().convergeServices(instanceId, usWest1); - tester.configServer().convergeServices(testerId.id(), usWest1); - tester.setEndpoints(instanceId, usWest1); - tester.setEndpoints(testerId.id(), usWest1); + tester.configServer().convergeServices(app.instanceId(), usWest1); + tester.configServer().convergeServices(app.testerId().id(), usWest1); + tester.setEndpoints(app.instanceId(), usWest1); + tester.setEndpoints(app.testerId().id(), usWest1); tester.runner().run(); tester.cloud().set(FAILURE); tester.runner().run(); - assertEquals(testFailure, tester.jobs().last(instanceId, productionUsWest1).get().status()); - assertEquals(revision2, tester.instance().deployments().get(productionUsCentral1.zone(tester.controller().system())).applicationVersion()); - assertEquals(revision1, tester.instance().deployments().get(productionUsEast3.zone(tester.controller().system())).applicationVersion()); - assertEquals(revision2, tester.instance().deployments().get(productionUsWest1.zone(tester.controller().system())).applicationVersion()); + assertEquals(testFailure, tester.jobs().last(app.instanceId(), productionUsWest1).get().status()); + assertEquals(revision2, app.deployment(productionUsCentral1.zone(tester.controller().system())).applicationVersion()); + assertEquals(revision1, app.deployment(productionUsEast3.zone(tester.controller().system())).applicationVersion()); + assertEquals(revision2, app.deployment(productionUsWest1.zone(tester.controller().system())).applicationVersion()); tester.clock().advance(Duration.ofMillis(1000)); // Revision 3 starts. - tester.newSubmission(); - tester.runJob(systemTest); - tester.runJob(stagingTest); - tester.triggerJobs(); // Starts a run for us-central-1. - tester.triggerJobs(); // Starts a new staging test run. + app.submit() + .runJob(systemTest).runJob(stagingTest); + tester.triggerJobs(); // Starts runs for us-central-1 and a new staging test run. tester.runner().run(); - assertEquals(running, tester.jobs().last(instanceId, productionUsCentral1).get().status()); - assertEquals(running, tester.jobs().last(instanceId, stagingTest).get().status()); + assertEquals(running, tester.jobs().last(app.instanceId(), productionUsCentral1).get().status()); + assertEquals(running, tester.jobs().last(app.instanceId(), stagingTest).get().status()); - // Staging is expired, and the job fails and won't be retried immediately. - tester.controller().applications().deactivate(instanceId, stagingTest.zone(tester.controller().system())); + // Staging deployment expires, the job fails, and won't be retried immediately. + tester.controller().applications().deactivate(app.instanceId(), stagingTest.zone(tester.controller().system())); tester.runner().run(); - assertEquals(installationFailed, tester.jobs().last(instanceId, stagingTest).get().status()); + assertEquals(installationFailed, tester.jobs().last(app.instanceId(), stagingTest).get().status()); tester.clock().advance(Duration.ofMillis(100_000)); // More than the minute within which there are immediate retries. tester.triggerJobs(); - assertEquals(installationFailed, tester.jobs().last(instanceId, stagingTest).get().status()); + assertEquals(installationFailed, tester.jobs().last(app.instanceId(), stagingTest).get().status()); // System upgrades to a new version, which won't yet start. Version platform = new Version("7.1"); @@ -117,36 +114,38 @@ public class JobControllerApiHandlerHelperTest { // Only us-east-3 is verified, on revision1. // staging-test has 4 runs: one success without sources on revision1, one success from revision1 to revision2, // one success from revision2 to revision3 and one failure from revision1 to revision3. - assertResponse(JobControllerApiHandlerHelper.runResponse(tester.jobs().runs(instanceId, stagingTest), URI.create("https://some.url:43/root")), "staging-runs.json"); - assertResponse(JobControllerApiHandlerHelper.runDetailsResponse(tester.jobs(), tester.jobs().last(instanceId, productionUsEast3).get().id(), "0"), "us-east-3-log-without-first.json"); - assertResponse(JobControllerApiHandlerHelper.jobTypeResponse(tester.controller(), instanceId, URI.create("https://some.url:43/root/")), "overview.json"); + assertResponse(JobControllerApiHandlerHelper.runResponse(tester.jobs().runs(app.instanceId(), stagingTest), URI.create("https://some.url:43/root")), "staging-runs.json"); + assertResponse(JobControllerApiHandlerHelper.runDetailsResponse(tester.jobs(), tester.jobs().last(app.instanceId(), productionUsEast3).get().id(), "0"), "us-east-3-log-without-first.json"); + assertResponse(JobControllerApiHandlerHelper.jobTypeResponse(tester.controller(), app.instanceId(), URI.create("https://some.url:43/root/")), "overview.json"); - tester.runJob(instanceId, JobType.devAwsUsEast2a, applicationPackage); - assertResponse(JobControllerApiHandlerHelper.runResponse(tester.jobs().runs(instanceId, devAwsUsEast2a), URI.create("https://some.url:43/root")), "dev-aws-us-east-2a-runs.json"); + app.runJob(devAwsUsEast2a, applicationPackage); + assertResponse(JobControllerApiHandlerHelper.runResponse(tester.jobs().runs(app.instanceId(), devAwsUsEast2a), URI.create("https://some.url:43/root")), "dev-aws-us-east-2a-runs.json"); } @Test public void testDevResponses() { DeploymentTester tester = new DeploymentTester(); + var app = tester.newDeploymentContext(); tester.clock().setInstant(Instant.EPOCH); ZoneId zone = JobType.devUsEast1.zone(tester.controller().system()); - tester.jobs().deploy(instanceId, JobType.devUsEast1, Optional.empty(), applicationPackage); + tester.jobs().deploy(app.instanceId(), JobType.devUsEast1, Optional.empty(), applicationPackage); tester.configServer().setLogStream("1554970337.935104\t17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\t5480\tcontainer\tstdout\tinfo\tERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)\n"); - assertResponse(JobControllerApiHandlerHelper.runDetailsResponse(tester.jobs(), tester.jobs().last(instanceId, devUsEast1).get().id(), null), "dev-us-east-1-log-first-part.json"); + assertResponse(JobControllerApiHandlerHelper.runDetailsResponse(tester.jobs(), tester.jobs().last(app.instanceId(), devUsEast1).get().id(), null), "dev-us-east-1-log-first-part.json"); tester.configServer().setLogStream("Nope, this won't be logged"); - tester.configServer().convergeServices(instanceId, zone); - tester.setEndpoints(instanceId, zone); + tester.configServer().convergeServices(app.instanceId(), zone); + tester.setEndpoints(app.instanceId(), zone); tester.runner().run(); - assertResponse(JobControllerApiHandlerHelper.jobTypeResponse(tester.controller(), instanceId, URI.create("https://some.url:43/root")), "dev-overview.json"); - assertResponse(JobControllerApiHandlerHelper.runDetailsResponse(tester.jobs(), tester.jobs().last(instanceId, devUsEast1).get().id(), "9"), "dev-us-east-1-log-second-part.json"); + assertResponse(JobControllerApiHandlerHelper.jobTypeResponse(tester.controller(), app.instanceId(), URI.create("https://some.url:43/root")), "dev-overview.json"); + assertResponse(JobControllerApiHandlerHelper.runDetailsResponse(tester.jobs(), tester.jobs().last(app.instanceId(), devUsEast1).get().id(), "9"), "dev-us-east-1-log-second-part.json"); } @Test public void testResponsesWithDirectDeployment() { var tester = new DeploymentTester(); + var app = tester.newDeploymentContext(); tester.clock().setInstant(Instant.EPOCH); var region = "us-west-1"; var applicationPackage = new ApplicationPackageBuilder().region(region).build(); @@ -155,7 +154,7 @@ public class JobControllerApiHandlerHelperTest { Optional.of(applicationPackage), new DeployOptions(true, Optional.empty(), false, false)); - assertResponse(JobControllerApiHandlerHelper.jobTypeResponse(tester.controller(), instanceId, URI.create("https://some.url:43/root/")), + assertResponse(JobControllerApiHandlerHelper.jobTypeResponse(tester.controller(), app.instanceId(), URI.create("https://some.url:43/root/")), "jobs-direct-deployment.json"); } -- cgit v1.2.3 From c9d404683f2fd4ee2a2eaede18aeca40de57ddae Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:36:04 +0100 Subject: Use compliant exception in ApplicationStoreMock --- .../vespa/hosted/controller/integration/ApplicationStoreMock.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java index d915fe06720..3c33051e98b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java @@ -36,7 +36,11 @@ public class ApplicationStoreMock implements ApplicationStore { @Override public byte[] get(TenantName tenant, ApplicationName application, ApplicationVersion applicationVersion) { - return requireNonNull(store.get(appId(tenant, application)).get(applicationVersion)); + byte[] bytes = store.get(appId(tenant, application)).get(applicationVersion); + if (bytes == null) + throw new IllegalArgumentException("No application package found for " + tenant + "." + application + + " with version " + applicationVersion.id()); + return bytes; } @Override -- cgit v1.2.3 From 94e4ff6524f95d27af39dd475a1f58bb0d28205e Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:40:48 +0100 Subject: Refactor Athenz mocking in ApplicationApiTest --- .../restapi/ContainerControllerTester.java | 13 ---------- .../hosted/controller/restapi/ContainerTester.java | 14 ++++++++++- .../restapi/application/ApplicationApiTest.java | 29 +++++++++++----------- 3 files changed, 27 insertions(+), 29 deletions(-) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java index 110aaf2b1a6..d7d3a2c97b1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java @@ -130,19 +130,6 @@ public class ContainerControllerTester { containerTester.assertResponse(() -> request, expectedResponse, expectedStatusCode); } - /* - * Authorize action on tenantDomain/application for a given screwdriverId - */ - public void authorize(AthenzDomain tenantDomain, ScrewdriverId screwdriverId, ApplicationAction action, TenantAndApplicationId id) { - AthenzClientFactoryMock mock = (AthenzClientFactoryMock) containerTester.container().components() - .getComponent(AthenzClientFactoryMock.class.getName()); - - mock.getSetup() - .domains.get(tenantDomain) - .applications.get(new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(id.application().value())) - .addRoleMember(action, HostedAthenzIdentities.from(screwdriverId)); - } - private void notifyJobCompletion(DeploymentJobs.JobReport report) { MockBuildService buildService = containerTester.serviceRegistry().buildServiceMock(); if (report.jobType() != component && ! buildService.remove(report.buildJob())) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java index 55249670c5c..a80ba7af6a5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java @@ -6,16 +6,21 @@ import com.yahoo.application.container.handler.Request; import com.yahoo.application.container.handler.Response; import com.yahoo.component.ComponentSpecification; import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.container.http.filter.FilterChainRepository; import com.yahoo.jdisc.http.filter.SecurityRequestFilter; import com.yahoo.jdisc.http.filter.SecurityRequestFilterChain; +import com.yahoo.vespa.athenz.api.AthenzDomain; +import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.ApplicationAction; import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactoryMock; import com.yahoo.vespa.hosted.controller.application.SystemApplication; +import com.yahoo.vespa.hosted.controller.athenz.HostedAthenzIdentities; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.integration.ServiceRegistryMock; -import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.vespa.hosted.controller.versions.ControllerVersion; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import org.junit.ComparisonFailure; @@ -84,6 +89,13 @@ public class ContainerTester { computeVersionStatus(); } + public void authorize(AthenzDomain tenantDomain, AthenzIdentity identity, ApplicationAction action, ApplicationName application) { + athenzClientFactory().getSetup() + .domains.get(tenantDomain) + .applications.get(new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(application.value())) + .addRoleMember(action, identity); + } + public void assertResponse(Supplier request, File responseFile) { assertResponse(request.get(), responseFile); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 18b59814e5d..a5d72b77187 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -7,6 +7,7 @@ import com.yahoo.application.container.handler.Request; import com.yahoo.component.Version; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; @@ -268,7 +269,7 @@ public class ApplicationApiTest extends ControllerContainerTest { addScrewdriverUserToDeployRole(SCREWDRIVER_ID, ATHENZ_TENANT_DOMAIN, - new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(id.application().value())); // (Necessary but not provided in this API) + id.application()); // Pipeline notifies about completed component job controllerTester.jobCompletion(JobType.component) @@ -354,7 +355,7 @@ public class ApplicationApiTest extends ControllerContainerTest { long screwdriverProjectId2 = 456; addScrewdriverUserToDeployRole(SCREWDRIVER_ID, ATHENZ_TENANT_DOMAIN_2, - new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(app2.application().value())); + app2.application()); // Trigger upgrade and then application change controllerTester.controller().applications().deploymentTrigger().triggerChange(TenantAndApplicationId.from(app2), Change.of(Version.fromString("7.0"))); @@ -974,7 +975,7 @@ public class ApplicationApiTest extends ControllerContainerTest { // Grant deploy access addScrewdriverUserToDeployRole(SCREWDRIVER_ID, ATHENZ_TENANT_DOMAIN, - new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId("application1")); + ApplicationName.from("application1")); // POST (deploy) an application to a prod zone - allowed when project ID is not specified MultiPartStreamer entity = createApplicationDeployData(applicationPackageInstance1, true); @@ -1444,7 +1445,7 @@ public class ApplicationApiTest extends ControllerContainerTest { Application application = controllerTester.createApplication(ATHENZ_TENANT_DOMAIN.getName(), "tenant1", "application1", "default"); ScrewdriverId screwdriverId = new ScrewdriverId(Long.toString(screwdriverProjectId)); - controllerTester.authorize(ATHENZ_TENANT_DOMAIN, screwdriverId, ApplicationAction.deploy, application.id()); + addScrewdriverUserToDeployRole(screwdriverId, ATHENZ_TENANT_DOMAIN, application.id().application()); controllerTester.jobCompletion(JobType.component) .application(application) @@ -1475,7 +1476,8 @@ public class ApplicationApiTest extends ControllerContainerTest { configureAthenzIdentity(new com.yahoo.vespa.athenz.api.AthenzService(ATHENZ_TENANT_DOMAIN, "service"), true); Application application = controllerTester.createApplication(ATHENZ_TENANT_DOMAIN.getName(), "tenant1", "application1", "default"); - controllerTester.authorize(ATHENZ_TENANT_DOMAIN, screwdriverId, ApplicationAction.deploy, application.id()); + tester.authorize(ATHENZ_TENANT_DOMAIN, HostedAthenzIdentities.from(screwdriverId), + ApplicationAction.deploy, application.id().application()); // Allow systemtest to succeed by notifying completion of component controllerTester.jobCompletion(JobType.component) @@ -1572,7 +1574,8 @@ public class ApplicationApiTest extends ControllerContainerTest { configureAthenzIdentity(new com.yahoo.vespa.athenz.api.AthenzService(ATHENZ_TENANT_DOMAIN, "service"), false); Application application = controllerTester.createApplication(ATHENZ_TENANT_DOMAIN.getName(), "tenant1", "application1", "default"); - controllerTester.authorize(ATHENZ_TENANT_DOMAIN, screwdriverId, ApplicationAction.deploy, application.id()); + tester.authorize(ATHENZ_TENANT_DOMAIN, HostedAthenzIdentities.from(screwdriverId), + ApplicationAction.deploy, application.id().application()); // Allow systemtest to succeed by notifying completion of system test controllerTester.jobCompletion(JobType.component) @@ -1609,7 +1612,8 @@ public class ApplicationApiTest extends ControllerContainerTest { configureAthenzIdentity(new com.yahoo.vespa.athenz.api.AthenzService(ATHENZ_TENANT_DOMAIN, "service"), true); Application application = controllerTester.createApplication(ATHENZ_TENANT_DOMAIN.getName(), "tenant1", "application1", "default"); - controllerTester.authorize(ATHENZ_TENANT_DOMAIN, screwdriverId, ApplicationAction.deploy, application.id()); + tester.authorize(ATHENZ_TENANT_DOMAIN, HostedAthenzIdentities.from(screwdriverId), + ApplicationAction.deploy, application.id().application()); // Allow systemtest to succeed by notifying completion of component controllerTester.jobCompletion(JobType.component) @@ -1841,12 +1845,8 @@ public class ApplicationApiTest extends ControllerContainerTest { */ private void addScrewdriverUserToDeployRole(ScrewdriverId screwdriverId, AthenzDomain domain, - com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId applicationId) { - AthenzClientFactoryMock mock = (AthenzClientFactoryMock) container.components() - .getComponent(AthenzClientFactoryMock.class.getName()); - AthenzIdentity screwdriverIdentity = HostedAthenzIdentities.from(screwdriverId); - AthenzDbMock.Application athenzApplication = mock.getSetup().domains.get(domain).applications.get(applicationId); - athenzApplication.addRoleMember(ApplicationAction.deploy, screwdriverIdentity); + ApplicationName application) { + tester.authorize(domain, HostedAthenzIdentities.from(screwdriverId), ApplicationAction.deploy, application); } private ApplicationId createTenantAndApplication() { @@ -1860,8 +1860,7 @@ public class ApplicationApiTest extends ControllerContainerTest { .userIdentity(USER_ID) .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT), new File("instance-reference.json")); - addScrewdriverUserToDeployRole(SCREWDRIVER_ID, ATHENZ_TENANT_DOMAIN, - new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId("application1")); + addScrewdriverUserToDeployRole(SCREWDRIVER_ID, ATHENZ_TENANT_DOMAIN, ApplicationName.from("application1")); return ApplicationId.from("tenant1", "application1", "instance1"); } -- cgit v1.2.3 From 20b0d6bb559d86b51e63a2e99a45dd814a3b58fb Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 10:43:21 +0100 Subject: Print response mismatch before status code mismatch in ContainerTester --- .../java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java index a80ba7af6a5..cd0a3a6f112 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java @@ -157,8 +157,8 @@ public class ContainerTester { FilterResult filterResult = invokeSecurityFilters(request); request = filterResult.request; Response response = filterResult.response != null ? filterResult.response : container.handleRequest(request); - assertEquals("Status code", expectedStatusCode, response.getStatus()); responseAssertion.accept(response); + assertEquals("Status code", expectedStatusCode, response.getStatus()); } // Hack to run request filters as part of the request processing chain. -- cgit v1.2.3 From 59d99b7a98416a0eec86f1557e6f275955a5a5fb Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 4 Nov 2019 11:20:02 +0100 Subject: Revert "Change complete when _all_ instances have it, not _any_" This reverts commit 1fb8ad0c9eea7b096f85251e17a7d6b2ad3e1177. Cannot do this until DeploymentSteps accounts for instances. --- .../vespa/hosted/controller/deployment/DeploymentTrigger.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 359374e8575..9573f5d07f5 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 @@ -565,13 +565,13 @@ public class DeploymentTrigger { : steps.productionJobs(); Change change = application.change(); - if (application.instances().values().stream().allMatch(instance -> - jobs.stream().allMatch(job -> isComplete(application.change().withoutApplication(), application.change(), instance, job)))) + for (Instance instance : application.instances().values()) { + if (jobs.stream().allMatch(job -> isComplete(application.change().withoutApplication(), application.change(), instance, job))) change = change.withoutPlatform(); - if (application.instances().values().stream().allMatch(instance -> - jobs.stream().allMatch(job -> isComplete(application.change().withoutPlatform(), application.change(), instance, job)))) + if (jobs.stream().allMatch(job -> isComplete(application.change().withoutPlatform(), application.change(), instance, job))) change = change.withoutApplication(); + } return change; } -- cgit v1.2.3