diff options
author | Martin Polden <mpolden@mpolden.no> | 2018-02-06 11:47:03 +0100 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2018-02-20 13:27:00 +0100 |
commit | a0d596375786e2ae3ec439f11c051313e6ab2dc4 (patch) | |
tree | a3339560ce22e6308a446168607055aeebb29ca0 /controller-server/src | |
parent | 47c80b0ff1976d0cf39f78bda1f5af6a7454a24b (diff) |
Require application version
Diffstat (limited to 'controller-server/src')
26 files changed, 520 insertions, 389 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java index 216025215d3..603f16a9ef6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java @@ -32,8 +32,7 @@ public final class Change { Objects.requireNonNull(platform, "platform cannot be null"); Objects.requireNonNull(application, "application cannot be null"); if (application.isPresent() && application.get().isUnknown()) { - // TODO: Require version to be known for application change - //throw new IllegalArgumentException("Application version to deploy must be a known version"); + throw new IllegalArgumentException("Application version to deploy must be a known version"); } this.platform = platform; this.application = application; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java index 7e1dcfe2bc6..d317fed0b73 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java @@ -264,8 +264,7 @@ public class DeploymentJobs { Objects.requireNonNull(jobError, "jobError cannot be null"); if (jobType == JobType.component && !sourceRevision.isPresent()) { - // TODO: Throw after 2018-03-01 - //throw new IllegalArgumentException("sourceRevision is required for job " + jobType); + throw new IllegalArgumentException("sourceRevision is required for job " + jobType); } this.applicationId = applicationId; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java index fb9ab8735d8..63c9fc5ebb9 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java @@ -172,7 +172,6 @@ public class JobList { private static boolean failingApplicationChange(JobStatus job) { if ( job.isSuccess()) return false; if ( ! job.lastSuccess().isPresent()) return true; // An application which never succeeded is surely bad. - if ( job.lastSuccess().get().applicationVersion().isUnknown()) return true; // Indicates the component job, which is always an application change. if ( ! job.firstFailing().get().version().equals(job.lastSuccess().get().version())) return false; // Version change may be to blame. return ! job.firstFailing().get().applicationVersion().equals(job.lastSuccess().get().applicationVersion()); // Return whether there is an application change. } 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 067d8d41f53..8c93d53ea9e 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 @@ -136,8 +136,6 @@ public class DeploymentTrigger { if (change.platform().get().isAfter(deployment.version())) return false; // later is ok } if (change.application().isPresent()) { - // If we don't yet know the application version we are deploying, then we are not complete - if (change.application().get().isUnknown()) return false; if ( ! change.application().get().equals(deployment.applicationVersion())) return false; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 670338d10e5..107685be994 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -10,7 +10,6 @@ import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; -import com.yahoo.slime.Type; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService.ApplicationMetrics; @@ -64,7 +63,6 @@ public class ApplicationSerializer { private final String deployTimeField = "deployTime"; private final String applicationBuildNumberField = "applicationBuildNumber"; private final String applicationPackageRevisionField = "applicationPackageRevision"; - private final String applicationPackageHashField = "applicationPackageHash"; private final String sourceRevisionField = "sourceRevision"; private final String repositoryField = "repositoryField"; private final String branchField = "branchField"; @@ -203,11 +201,6 @@ public class ApplicationSerializer { if (applicationVersion.buildNumber().isPresent() && applicationVersion.source().isPresent()) { object.setLong(applicationBuildNumberField, applicationVersion.buildNumber().get()); toSlime(applicationVersion.source().get(), object.setObject(sourceRevisionField)); - } else if (applicationVersion.applicationPackageHash().isPresent()) { // TODO: Remove after 2018-03-01 - object.setString(applicationPackageHashField, applicationVersion.applicationPackageHash().get()); - if (applicationVersion.source().isPresent()){ - toSlime(applicationVersion.source().get(), object.setObject(sourceRevisionField)); - } } } @@ -271,7 +264,7 @@ public class ApplicationSerializer { List<Deployment> deployments = deploymentsFromSlime(root.field(deploymentsField)); DeploymentJobs deploymentJobs = deploymentJobsFromSlime(root.field(deploymentJobsField)); Change deploying = changeFromSlime(root.field(deployingField)); - Change outstandingChange = outstandingChangeFromSlime(root.field(outstandingChangeField)); + Change outstandingChange = changeFromSlime(root.field(outstandingChangeField)); Optional<IssueId> ownershipIssueId = optionalString(root.field(ownershipIssueIdField)).map(IssueId::from); ApplicationMetrics metrics = new ApplicationMetrics(root.field(queryQualityField).asDouble(), root.field(writeQualityField).asDouble()); @@ -349,13 +342,8 @@ public class ApplicationSerializer { private ApplicationVersion applicationVersionFromSlime(Inspector object) { if ( ! object.valid()) return ApplicationVersion.unknown; - Optional<String> applicationPackageHash = optionalString(object.field(applicationPackageHashField)); Optional<Long> applicationBuildNumber = optionalLong(object.field(applicationBuildNumberField)); Optional<SourceRevision> sourceRevision = sourceRevisionFromSlime(object.field(sourceRevisionField)); - if (applicationPackageHash.isPresent()) { // TODO: Remove after 2018-03-01 - return sourceRevision.map(sr -> ApplicationVersion.from(applicationPackageHash.get(), sr)) - .orElseGet(() -> ApplicationVersion.from(applicationPackageHash.get())); - } if (!sourceRevision.isPresent() || !applicationBuildNumber.isPresent()) { return ApplicationVersion.unknown; } @@ -383,23 +371,13 @@ public class ApplicationSerializer { Change change = Change.empty(); if (versionFieldValue.valid()) change = Change.of(Version.fromString(versionFieldValue.asString())); - if (object.field(applicationBuildNumberField).valid() || - object.field(applicationPackageHashField).valid()) // TODO: Remove after 2018-03-01 + if (object.field(applicationBuildNumberField).valid()) change = change.with(applicationVersionFromSlime(object)); if ( ! change.isPresent()) // A deploy object with no fields -> unknown application change - change = Change.of(ApplicationVersion.unknown); + change = Change.empty(); return change; } - // TODO: Remove and inline after 2018-03-01 - private Change outstandingChangeFromSlime(Inspector object) { - if (object.type() == Type.BOOL) { - boolean outstandingChange = object.asBool(); - return outstandingChange ? Change.of(ApplicationVersion.unknown) : Change.empty(); - } - return changeFromSlime(object); - } - private List<JobStatus> jobStatusListFromSlime(Inspector array) { List<JobStatus> jobStatusList = new ArrayList<>(); array.traverse((ArrayTraverser) (int i, Inspector item) -> jobStatusList.add(jobStatusFromSlime(item))); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ArtifactRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ArtifactRepositoryMock.java index cfe60cd3e96..6c7137273a7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ArtifactRepositoryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ArtifactRepositoryMock.java @@ -1,6 +1,7 @@ // 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; +import com.yahoo.component.AbstractComponent; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ArtifactRepository; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; @@ -12,7 +13,7 @@ import java.util.Objects; /** * @author mpolden */ -public class ArtifactRepositoryMock implements ArtifactRepository { +public class ArtifactRepositoryMock extends AbstractComponent implements ArtifactRepository { private final Map<Integer, Artifact> repository = new HashMap<>(); @@ -28,19 +29,17 @@ public class ArtifactRepositoryMock implements ArtifactRepository { return artifact == null ? 0 : artifact.hits; } - public ArtifactRepository resetHits() { - repository.values().forEach(Artifact::resetHits); - return this; + public boolean contains(ApplicationId applicationId, String applicationVersion) { + return repository.containsKey(artifactHash(applicationId, applicationVersion)); } @Override public byte[] getApplicationPackage(ApplicationId applicationId, String applicationVersion) { - int artifactHash = artifactHash(applicationId, applicationVersion); - if (!repository.containsKey(artifactHash)) { + Artifact artifact = repository.get(artifactHash(applicationId, applicationVersion)); + if (artifact == null) { throw new IllegalArgumentException("No application package found for " + applicationId + " with version " + applicationVersion); } - Artifact artifact = repository.get(artifactHash); artifact.recordHit(); return artifact.data; } @@ -62,10 +61,6 @@ public class ArtifactRepositoryMock implements ArtifactRepository { hits++; } - private void resetHits() { - hits = 0; - } - } } 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 1f2a9c4e910..5b3f2c74548 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 @@ -23,7 +23,6 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.api.identifiers.UserGroup; -import com.yahoo.vespa.hosted.controller.api.integration.BuildService.BuildJob; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; @@ -40,6 +39,7 @@ import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.BuildSystem; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; +import com.yahoo.vespa.hosted.controller.deployment.BuildJob; import com.yahoo.vespa.hosted.controller.persistence.ApplicationSerializer; import com.yahoo.vespa.hosted.controller.rotation.RotationId; import com.yahoo.vespa.hosted.controller.rotation.RotationLock; @@ -104,9 +104,9 @@ public class ControllerTest { // staging job - succeeding Version version1 = tester.defaultVespaVersion(); Application app1 = tester.createApplication("app1", "tenant1", 1, 11L); - tester.notifyJobCompletion(component, app1, true); - assertEquals("Application version is currently not known", - ApplicationVersion.unknown, + tester.jobCompletion(component).application(app1).uploadArtifact(applicationPackage).submit(); + assertEquals("Application version is known from completion of initial job", + ApplicationVersion.from(BuildJob.defaultSourceRevision, BuildJob.defaultBuildNumber), tester.controller().applications().require(app1.id()).change().application().get()); tester.deployAndNotify(app1, applicationPackage, true, systemTest); tester.deployAndNotify(app1, applicationPackage, true, stagingTest); @@ -147,10 +147,11 @@ public class ControllerTest { tester.clock().advance(Duration.ofHours(1)); - tester.notifyJobCompletion(productionCorpUsEast1, app1, false); // Need to complete the job, or new jobs won't start. + // Need to complete the job, or new jobs won't start. + tester.jobCompletion(productionCorpUsEast1).application(app1).unsuccessful().submit(); // system and staging test job - succeeding - tester.notifyJobCompletion(component, app1, true); + tester.jobCompletion(component).application(app1).submit(); tester.deployAndNotify(app1, applicationPackage, true, false, systemTest); assertStatus(JobStatus.initial(systemTest) .withTriggering(version1, applicationVersion, false, "", tester.clock().instant().minus(Duration.ofMillis(1))) @@ -177,7 +178,7 @@ public class ControllerTest { .environment(Environment.prod) .region("us-east-3") .build(); - tester.notifyJobCompletion(component, app1, true); + tester.jobCompletion(component).application(app1).buildNumber(43).uploadArtifact(applicationPackage).submit(); try { tester.deploy(systemTest, app1, applicationPackage); fail("Expected exception due to unallowed production deployment removal"); @@ -199,6 +200,7 @@ public class ControllerTest { .environment(Environment.prod) .region("us-east-3") .build(); + tester.jobCompletion(component).application(app1).buildNumber(44).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app1, applicationPackage, true, systemTest); assertNull("Zone was removed", applications.require(app1.id()).deployments().get(productionCorpUsEast1.zone(SystemName.main).get())); @@ -217,8 +219,7 @@ public class ControllerTest { SourceRevision source = new SourceRevision("repo", "master", "commit1"); ApplicationVersion applicationVersion = ApplicationVersion.from(source, 101); - tester.artifactRepository().put(app.id(), applicationPackage, applicationVersion.id()); - runDeployment(tester, app.id(), applicationVersion, Optional.empty(), Optional.of(source), 101); + runDeployment(tester, app.id(), applicationVersion, applicationPackage, source,101); assertEquals("Artifact is downloaded twice in staging and once for other zones", 5, tester.artifactRepository().hits(app.id(), applicationVersion.id())); @@ -228,44 +229,6 @@ public class ControllerTest { } @Test - public void testDeploymentApplicationVersionMigration() { - DeploymentTester tester = new DeploymentTester(); - Application app = tester.createApplication("app1", "tenant1", 1, 11L); - ApplicationPackage applicationPackage = new ApplicationPackageBuilder() - .environment(Environment.prod) - .region("corp-us-east-1") - .region("us-east-3") - .build(); - SourceRevision source = new SourceRevision("repo", "master", "commit1"); - - // Scenario 1: Old fashioned deployment. Simulates existing production deployments - ApplicationVersion v0 = ApplicationVersion.from(applicationPackage.hash(), source); - runDeployment(tester, app.id(), v0, Optional.of(applicationPackage), Optional.empty(), 100); - assertEquals("Nothing downloaded from repository", 0, - tester.artifactRepository().hits(app.id(), v0.id())); - - // Scenario 2: New application version number is reported and package is downloaded by controller. In staging, - // the application package from the deployer is used as v0 cannot be downloaded from repository. - ApplicationVersion v1 = ApplicationVersion.from(source, 101); - tester.artifactRepository().put(app.id(), applicationPackage, v1.id()); - runDeployment(tester, app.id(), v1, Optional.of(applicationPackage), Optional.of(source), 101); - assertEquals("Artifact is downloaded once per zone", 4, - tester.artifactRepository().hits(app.id(), v1.id())); - assertEquals("v0 is never downloaded", 0, - tester.artifactRepository().hits(app.id(), v0.id())); - tester.artifactRepository().resetHits(); - - // Scenario 3: Both application versions are available in repository - ApplicationVersion v2 = ApplicationVersion.from(source, 102); - tester.artifactRepository().put(app.id(), applicationPackage, v2.id()); - runDeployment(tester, app.id(), v2, Optional.empty(), Optional.of(source), 102); - assertEquals("Previous artifact is downloaded once", 1, - tester.artifactRepository().hits(app.id(), v1.id())); - assertEquals("Artifact is downloaded once per zone", 4, - tester.artifactRepository().hits(app.id(), v2.id())); - } - - @Test public void testDeployVersion() { // Setup system DeploymentTester tester = new DeploymentTester(); @@ -279,7 +242,7 @@ public class ControllerTest { Application app1 = tester.createApplication("application1", "tenant1", 1, 1L); // First deployment: An application change - tester.notifyJobCompletion(component, app1, true); + tester.jobCompletion(component).application(app1).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app1, applicationPackage, true, systemTest); tester.deployAndNotify(app1, applicationPackage, true, stagingTest); tester.deployAndNotify(app1, applicationPackage, true, productionUsWest1); @@ -302,7 +265,7 @@ public class ControllerTest { .region("us-west-1") .region("us-east-3") .build(); - tester.notifyJobCompletion(component, app1, true); + tester.jobCompletion(component).application(app1).buildNumber(43).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app1, applicationPackage, true, systemTest); tester.deployAndNotify(app1, applicationPackage, true, stagingTest); tester.deployAndNotify(app1, applicationPackage, true, productionUsWest1); @@ -410,7 +373,7 @@ public class ControllerTest { // Initial failure Instant initialFailure = tester.clock().instant(); - tester.notifyJobCompletion(component, app, true); + tester.jobCompletion(component).application(app).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app, applicationPackage, false, systemTest); assertEquals("Failure age is right at initial failure", initialFailure.plus(Duration.ofMillis(2)), firstFailing(app, tester).get().at()); @@ -434,7 +397,7 @@ public class ControllerTest { // Initial failure tester.clock().advance(Duration.ofMillis(1000)); initialFailure = tester.clock().instant(); - tester.notifyJobCompletion(component, app, true); + tester.jobCompletion(component).application(app).submit(); tester.deployAndNotify(app, applicationPackage, false, systemTest); assertEquals("Failure age is right at initial failure", initialFailure.plus(Duration.ofMillis(2)), firstFailing(app, tester).get().at()); @@ -501,13 +464,13 @@ public class ControllerTest { BuildSystem buildSystem = tester.controller().applications().deploymentTrigger().buildSystem(); // all applications: system-test completes successfully - tester.notifyJobCompletion(component, app1, true); + tester.jobCompletion(component).application(app1).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app1, applicationPackage, true, systemTest); - tester.notifyJobCompletion(component, app2, true); + tester.jobCompletion(component).application(app2).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app2, applicationPackage, true, systemTest); - tester.notifyJobCompletion(component, app3, true); + tester.jobCompletion(component).application(app3).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app3, applicationPackage, true, systemTest); // all applications: staging test jobs queued @@ -515,7 +478,10 @@ public class ControllerTest { // app1: staging-test job fails with out of capacity and is added to the front of the queue tester.deploy(stagingTest, app1, applicationPackage); - tester.notifyJobCompletion(stagingTest, app1, Optional.of(JobError.outOfCapacity)); + tester.jobCompletion(stagingTest) + .application(app1) + .error(JobError.outOfCapacity) + .submit(); assertEquals(stagingTest.jobName(), buildSystem.jobs().get(0).jobName()); assertEquals(project1, buildSystem.jobs().get(0).projectId()); @@ -528,20 +494,20 @@ public class ControllerTest { // app1: 15 minutes pass, staging-test job is still failing due out of capacity, but is no longer re-queued by // out of capacity retry mechanism tester.clock().advance(Duration.ofMinutes(15)); - tester.notifyJobCompletion(stagingTest, app1, Optional.of(JobError.outOfCapacity)); // Clear the previous staging test - tester.notifyJobCompletion(component, app1, true); + tester.jobCompletion(stagingTest).application(app1).error(JobError.outOfCapacity).submit(); // Clear the previous staging test + tester.jobCompletion(component).application(app1).buildNumber(43).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app1, applicationPackage, true, false, systemTest); tester.deploy(stagingTest, app1, applicationPackage); assertEquals(1, buildSystem.takeJobsToRun().size()); - tester.notifyJobCompletion(stagingTest, app1, Optional.of(JobError.outOfCapacity)); + tester.jobCompletion(stagingTest).application(app1).error(JobError.outOfCapacity).submit(); assertTrue("No jobs queued", buildSystem.jobs().isEmpty()); // app2 and app3: New change triggers system-test jobs // Provide a changed application package, too, or the deployment is a no-op. - tester.notifyJobCompletion(component, app2, true); + tester.jobCompletion(component).application(app2).buildNumber(43).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app2, applicationPackage2, true, systemTest); - tester.notifyJobCompletion(component, app3, true); + tester.jobCompletion(component).application(app3).buildNumber(43).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app3, applicationPackage2, true, systemTest); assertEquals(2, buildSystem.jobs().size()); @@ -552,7 +518,7 @@ public class ControllerTest { tester.clock().advance(Duration.ofMinutes(50)); tester.readyJobTrigger().maintain(); - List<BuildJob> nextJobs = buildSystem.takeJobsToRun(); + List<com.yahoo.vespa.hosted.controller.api.integration.BuildService.BuildJob> nextJobs = buildSystem.takeJobsToRun(); assertEquals(2, nextJobs.size()); assertEquals(stagingTest.jobName(), nextJobs.get(0).jobName()); assertEquals(project2, nextJobs.get(0).projectId()); @@ -626,7 +592,8 @@ public class ControllerTest { tester.controllerTester().zoneRegistry().setZones(ZoneId.from("prod", "cd-us-central-1")); Supplier<Map<JobType, JobStatus>> statuses = () -> - tester.application(ApplicationId.from("vespa", "canary", "default")).deploymentJobs().jobStatus(); + tester.application(ApplicationId.from("vespa", "canary", "default")) + .deploymentJobs().jobStatus(); // Current system version, matches version in test data Version version = Version.fromString("6.141.117"); @@ -664,7 +631,7 @@ public class ControllerTest { tester.deploy(DeploymentJobs.JobType.systemTest, application, applicationPackage); tester.buildSystem().takeJobsToRun(); tester.clock().advance(Duration.ofMinutes(10)); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, application, true); + tester.jobCompletion(systemTest).application(application).submit(); long newCdJobsCount = statuses.get().keySet().stream() .filter(type -> type.zone(SystemName.cd).isPresent()) @@ -744,7 +711,7 @@ public class ControllerTest { .environment(Environment.prod) .allow(ValidationId.deploymentRemoval) .build(); - tester.notifyJobCompletion(component, app1, true); + tester.jobCompletion(component).application(app1).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app1, applicationPackage, true, systemTest); tester.applications().deactivate(app1, ZoneId.from(Environment.test, RegionName.from("us-east-1"))); tester.applications().deactivate(app1, ZoneId.from(Environment.staging, RegionName.from("us-east-3"))); @@ -862,16 +829,20 @@ public class ControllerTest { } private void runDeployment(DeploymentTester tester, ApplicationId application, ApplicationVersion version, - Optional<ApplicationPackage> applicationPackage, Optional<SourceRevision> sourceRevision, - long initialBuildNumber) { + ApplicationPackage applicationPackage, SourceRevision sourceRevision, long buildNumber) { Application app = tester.applications().require(application); - tester.notifyJobCompletion(component, app, Optional.empty(), sourceRevision, initialBuildNumber); - ApplicationVersion change = sourceRevision.map(sr -> ApplicationVersion.from(sr, initialBuildNumber)) - .orElse(ApplicationVersion.unknown); + tester.jobCompletion(component) + .application(app) + .buildNumber(buildNumber) + .sourceRevision(sourceRevision) + .uploadArtifact(applicationPackage) + .submit(); + + ApplicationVersion change = ApplicationVersion.from(sourceRevision, buildNumber); assertEquals(change.id(), tester.controller().applications() .require(application) .change().application().get().id()); - runDeployment(tester, app, version, Optional.empty(), applicationPackage); + runDeployment(tester, app, version, Optional.empty(), Optional.of(applicationPackage)); } private void runDeployment(DeploymentTester tester, Application app, ApplicationVersion version, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java new file mode 100644 index 00000000000..0ea50221cfc --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java @@ -0,0 +1,119 @@ +// 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.config.provision.ApplicationId; +import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.ArtifactRepositoryMock; +import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; +import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; +import com.yahoo.vespa.hosted.controller.application.SourceRevision; + +import java.util.Objects; +import java.util.Optional; +import java.util.function.Consumer; + +/** + * Create a build job for testing purposes. In most cases this should be constructed by calling + * DeploymentTester.jobCompletion. + * + * @author mpolden + */ +public class BuildJob { + + public static final SourceRevision defaultSourceRevision = new SourceRevision("repository1", + "master", "commit1"); + public static final long defaultBuildNumber = 42; + + private DeploymentJobs.JobType job; + private ApplicationId applicationId; + private Optional<DeploymentJobs.JobError> jobError = Optional.empty(); + private Optional<SourceRevision> sourceRevision = Optional.of(defaultSourceRevision); + private long projectId; + private long buildNumber = defaultBuildNumber; + + private final Consumer<DeploymentJobs.JobReport> reportConsumer; + private final ArtifactRepositoryMock artifactRepository; + + public BuildJob(Consumer<DeploymentJobs.JobReport> reportConsumer, ArtifactRepositoryMock artifactRepository) { + Objects.requireNonNull(reportConsumer, "reportConsumer cannot be null"); + Objects.requireNonNull(artifactRepository, "artifactRepository cannot be null"); + this.reportConsumer = reportConsumer; + this.artifactRepository = artifactRepository; + } + + public BuildJob type(DeploymentJobs.JobType job) { + this.job = job; + return this; + } + + public BuildJob application(Application application) { + this.applicationId = application.id(); + if (application.deploymentJobs().projectId().isPresent()) { + this.projectId = application.deploymentJobs().projectId().get(); + } + return this; + } + + public BuildJob application(ApplicationId applicationId) { + this.applicationId = applicationId; + return this; + } + + public BuildJob error(DeploymentJobs.JobError jobError) { + this.jobError = Optional.of(jobError); + return this; + } + + public BuildJob sourceRevision(SourceRevision sourceRevision) { + this.sourceRevision = Optional.of(sourceRevision); + return this; + } + + public BuildJob buildNumber(long buildNumber) { + this.buildNumber = buildNumber; + return this; + } + + public BuildJob projectId(long projectId) { + this.projectId = projectId; + return this; + } + + public BuildJob success(boolean success) { + this.jobError = success ? Optional.empty() : Optional.of(DeploymentJobs.JobError.unknown); + return this; + } + + public BuildJob unsuccessful() { + return success(false); + } + + /** Create a job report for this build job */ + public DeploymentJobs.JobReport report() { + return new DeploymentJobs.JobReport(applicationId, job, projectId, buildNumber, sourceRevision, jobError); + } + + /** Upload given application package to artifact repository as part of this job */ + public BuildJob uploadArtifact(ApplicationPackage applicationPackage) { + Objects.requireNonNull(job, "job cannot be null"); + if (job != DeploymentJobs.JobType.component) { + throw new IllegalStateException(job + " cannot upload artifact"); + } + artifactRepository.put(applicationId, applicationPackage, applicationVersion()); + return this; + } + + /** Send report for this build job to the controller */ + public void submit() { + if (job == DeploymentJobs.JobType.component && + !artifactRepository.contains(applicationId, applicationVersion())) { + throw new IllegalStateException(job + " must upload artifact before reporting completion"); + } + reportConsumer.accept(report()); + } + + private String applicationVersion() { + return String.format("1.0.%d-%s", buildNumber, sourceRevision.get().commit()); + } + +} 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 c923542b8c9..2d508d09b50 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 @@ -17,7 +17,6 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; import com.yahoo.vespa.hosted.controller.maintenance.JobControl; import com.yahoo.vespa.hosted.controller.maintenance.ReadyJobsTrigger; import com.yahoo.vespa.hosted.controller.maintenance.Upgrader; @@ -25,12 +24,10 @@ import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import java.time.Duration; import java.util.List; -import java.util.NoSuchElementException; import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; -import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError.unknown; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -45,7 +42,6 @@ public class DeploymentTester { // Set a long interval so that maintainers never do scheduled runs during tests private static final Duration maintenanceInterval = Duration.ofDays(1); - private static final int defaultBuildNumber = 42; private final ControllerTester tester; private final Upgrader upgrader; @@ -72,8 +68,6 @@ public class DeploymentTester { public ApplicationController applications() { return tester.controller().applications(); } - // TODO: This thing simulates the wrong thing: the build system won't hold the jobs that are running, - // and so these should be consumed immediately upon triggering, and be "somewhere else" while running. public BuildSystem buildSystem() { return tester.controller().applications().deploymentTrigger().buildSystem(); } public DeploymentTrigger deploymentTrigger() { return tester.controller().applications().deploymentTrigger(); } @@ -120,6 +114,11 @@ public class DeploymentTester { public void restartController() { tester.createNewController(); } + /** Notify the controller about a job completing */ + public BuildJob jobCompletion(JobType job) { + return new BuildJob(this::notifyJobCompletion, tester.artifactRepository()).type(job); + } + /** Simulate the full lifecycle of an application deployment as declared in given application package */ public Application createAndDeploy(String applicationName, int projectId, ApplicationPackage applicationPackage) { TenantId tenantId = tester.createTenant("tenant1", "domain1", 1L); @@ -151,33 +150,16 @@ public class DeploymentTester { /** Deploy application completely using the given application package */ public void deployCompletely(Application application, ApplicationPackage applicationPackage) { - notifyJobCompletion(JobType.component, application, true); - assertTrue(applications().require(application.id()).change().isPresent()); - completeDeployment(application, applicationPackage, Optional.empty(), true); - } - - public static DeploymentJobs.JobReport jobReport(Application application, JobType jobType, boolean success) { - return jobReport(application, jobType, Optional.ofNullable(success ? null : unknown), Optional.empty(), defaultBuildNumber); + deployCompletely(application, applicationPackage, BuildJob.defaultBuildNumber); } - public static DeploymentJobs.JobReport jobReport(Application application, JobType jobType, - Optional<DeploymentJobs.JobError> jobError, - Optional<SourceRevision> sourceRevision, long buildNumber) { - return new DeploymentJobs.JobReport( - application.id(), - jobType, - application.deploymentJobs().projectId().get(), - buildNumber, - sourceRevision, - jobError - ); - } - - /** Deploy application using the given application package, but expecting to stop after test phases */ - public void deployTestOnly(Application application, ApplicationPackage applicationPackage) { - notifyJobCompletion(JobType.component, application, true); + public void deployCompletely(Application application, ApplicationPackage applicationPackage, long buildNumber) { + jobCompletion(JobType.component).application(application) + .buildNumber(buildNumber) + .uploadArtifact(applicationPackage) + .submit(); assertTrue(applications().require(application.id()).change().isPresent()); - completeDeployment(application, applicationPackage, Optional.empty(), false); + completeDeployment(application, applicationPackage, Optional.empty(), true); } private void completeDeployment(Application application, ApplicationPackage applicationPackage, @@ -204,20 +186,6 @@ public class DeploymentTester { } } - public void notifyJobCompletion(JobType jobType, Application application, boolean success) { - notifyJobCompletion(jobType, application, Optional.ofNullable(success ? null : unknown)); - } - - public void notifyJobCompletion(JobType jobType, Application application, Optional<DeploymentJobs.JobError> jobError) { - notifyJobCompletion(jobType, application, jobError, Optional.empty(), defaultBuildNumber); - } - - public void notifyJobCompletion(JobType jobType, Application application, Optional<DeploymentJobs.JobError> jobError, - Optional<SourceRevision> source, long buildNumber) { - clock().advance(Duration.ofMillis(1)); - applications().notifyJobCompletion(jobReport(application, jobType, jobError, source, buildNumber)); - } - public void completeUpgrade(Application application, Version version, String upgradePolicy) { completeUpgrade(application, version, applicationPackage(upgradePolicy)); } @@ -282,11 +250,11 @@ public class DeploymentTester { } deploy(job, application, applicationPackage, false); } - notifyJobCompletion(job, application, success); // Deactivate test deployments after deploy. This replicates the behaviour of the tenant pipeline if (job.isTest()) { controller().applications().deactivate(application, job.zone(controller().system()).get()); } + jobCompletion(job).application(application).success(success).submit(); } } @@ -304,16 +272,24 @@ public class DeploymentTester { private BuildService.BuildJob findJob(Application application, JobType jobType) { for (BuildService.BuildJob job : buildSystem().jobs()) { - if (job.projectId() == application.deploymentJobs().projectId().get() && job.jobName().equals(jobType.jobName())) + if (job.projectId() == application.deploymentJobs().projectId().get() && + job.jobName().equals(jobType.jobName())) { return job; + } } - throw new NoSuchElementException(jobType + " is not scheduled for " + application); + throw new IllegalArgumentException(jobType + " is not scheduled for " + application); } private int countJobsOf(Application application) { - return (int)buildSystem().jobs().stream() - .filter(job -> job.projectId() == application.deploymentJobs().projectId().get()) - .count(); + return (int) buildSystem().jobs().stream() + .filter(job -> job.projectId() == application.deploymentJobs() + .projectId().get()) + .count(); + } + + private void notifyJobCompletion(DeploymentJobs.JobReport report) { + clock().advance(Duration.ofMillis(1)); + applications().notifyJobCompletion(report); } public static ApplicationPackage applicationPackage(String upgradePolicy) { @@ -321,7 +297,6 @@ public class DeploymentTester { .upgradePolicy(upgradePolicy) .environment(Environment.prod) .region("us-west-1") - .environment(Environment.prod) .region("us-east-3") .build(); } 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 029eb335d82..4d7d373a157 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 @@ -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.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; +import com.yahoo.vespa.hosted.controller.application.SourceRevision; import com.yahoo.vespa.hosted.controller.maintenance.JobControl; import com.yahoo.vespa.hosted.controller.maintenance.ReadyJobsTrigger; import org.junit.Test; @@ -20,6 +21,9 @@ import org.junit.Test; import java.time.Duration; import java.time.Instant; +import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.component; +import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.stagingTest; +import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.systemTest; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -45,7 +49,7 @@ public class DeploymentTriggerTest { tester.upgrader().maintain(); // Deploy completely once - tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); + tester.jobCompletion(component).application(app).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); tester.deployAndNotify(app, applicationPackage, true, JobType.productionUsWest1); @@ -89,7 +93,7 @@ public class DeploymentTriggerTest { .build(); // Component job finishes - tester.notifyJobCompletion(JobType.component, application, true); + tester.jobCompletion(component).application(application).uploadArtifact(applicationPackage).submit(); // Application is deployed to all test environments and declared zones tester.deployAndNotify(application, applicationPackage, true, JobType.systemTest); @@ -117,7 +121,7 @@ public class DeploymentTriggerTest { .build(); // Component job finishes - tester.notifyJobCompletion(JobType.component, application, true); + tester.jobCompletion(component).application(application).uploadArtifact(applicationPackage).submit(); // Test jobs pass tester.deployAndNotify(application, applicationPackage, true, JobType.systemTest); @@ -141,7 +145,7 @@ public class DeploymentTriggerTest { // us-west-1 completes tester.deploy(JobType.productionUsWest1, application, applicationPackage); - tester.notifyJobCompletion(JobType.productionUsWest1, application, true); + tester.jobCompletion(JobType.productionUsWest1).application(application).submit(); // Delayed trigger does nothing as not enough time has passed after us-west-1 completion tester.deploymentTrigger().triggerReadyJobs(); @@ -172,7 +176,7 @@ public class DeploymentTriggerTest { .build(); // Component job finishes - tester.notifyJobCompletion(JobType.component, application, true); + tester.jobCompletion(component).application(application).uploadArtifact(applicationPackage).submit(); // Test jobs pass tester.deployAndNotify(application, applicationPackage, true, JobType.systemTest); @@ -189,11 +193,11 @@ public class DeploymentTriggerTest { tester.buildSystem().takeJobsToRun(); tester.deploy(JobType.productionUsWest1, application, applicationPackage, false); - tester.notifyJobCompletion(JobType.productionUsWest1, application, true); + tester.jobCompletion(JobType.productionUsWest1).application(application).submit(); assertTrue("No more jobs triggered at this time", tester.buildSystem().jobs().isEmpty()); tester.deploy(JobType.productionUsEast3, application, applicationPackage, false); - tester.notifyJobCompletion(JobType.productionUsEast3, application, true); + tester.jobCompletion(JobType.productionUsEast3).application(application).submit(); // Last region completes assertEquals(1, tester.buildSystem().jobs().size()); @@ -210,7 +214,7 @@ public class DeploymentTriggerTest { .build(); Application app = tester.createApplication("app1", "tenant1", 1, 11L); - tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); + tester.jobCompletion(component).application(app).uploadArtifact(applicationPackage).submit(); // Test environments pass tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); @@ -218,13 +222,13 @@ public class DeploymentTriggerTest { // Last declared job completes first tester.deploy(DeploymentJobs.JobType.productionUsWest1, app, applicationPackage); - tester.notifyJobCompletion(DeploymentJobs.JobType.productionUsWest1, app, true); + tester.jobCompletion(DeploymentJobs.JobType.productionUsWest1).application(app).submit(); assertTrue("Change is present as not all jobs are complete", tester.applications().require(app.id()).change().isPresent()); // All jobs complete tester.deploy(DeploymentJobs.JobType.productionUsEast3, app, applicationPackage); - tester.notifyJobCompletion(DeploymentJobs.JobType.productionUsEast3, app, true); + tester.jobCompletion(JobType.productionUsEast3).application(app).submit(); assertFalse("Change has been deployed", tester.applications().require(app.id()).change().isPresent()); } @@ -250,7 +254,7 @@ public class DeploymentTriggerTest { .build(); // Component job finishes - tester.notifyJobCompletion(JobType.component, application, true); + tester.jobCompletion(component).application(application).uploadArtifact(newApplicationPackage).submit(); // Application is deployed to all test environments and declared zones tester.deployAndNotify(application, newApplicationPackage, true, JobType.systemTest); @@ -299,8 +303,15 @@ public class DeploymentTriggerTest { " }\n" + "}\n"; ApplicationPackage changedApplication = applicationPackageBuilder.searchDefinition(searchDefinition).build(); - - tester.deployTestOnly(app, changedApplication); + tester.jobCompletion(component) + .application(app) + .buildNumber(43) + .sourceRevision(new SourceRevision("repository1", "master", "cafed00d")) + .uploadArtifact(changedApplication) + .submit(); + assertTrue(tester.applications().require(app.id()).change().isPresent()); + tester.deployAndNotify(app, changedApplication, true, systemTest); + tester.deployAndNotify(app, changedApplication, true, stagingTest); readyJobsTrigger.run(); assertEquals(0, tester.buildSystem().jobs().size()); @@ -339,7 +350,7 @@ public class DeploymentTriggerTest { .build(); // Component job finishes - tester.notifyJobCompletion(JobType.component, application, true); + tester.jobCompletion(component).application(application).uploadArtifact(applicationPackage).submit(); // Application is deployed to all test environments and declared zones tester.deployAndNotify(application, applicationPackage, true, JobType.systemTest); @@ -347,7 +358,7 @@ public class DeploymentTriggerTest { tester.deployAndNotify(application, applicationPackage, true, JobType.productionCorpUsEast1); // Extra notification for last job - tester.notifyJobCompletion(JobType.productionCorpUsEast1, application, true); + tester.jobCompletion(JobType.productionCorpUsEast1).application(application).submit(); assertFalse("Change has been deployed", tester.applications().require(application.id()).change().isPresent()); assertTrue("All jobs consumed", buildSystem.jobs().isEmpty()); 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 e57edcf6da0..155c9f14f77 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 @@ -78,14 +78,14 @@ public class DeploymentIssueReporterTest { // NOTE: All maintenance should be idempotent within a small enough time interval, so maintain is called twice in succession throughout. // apps 1 and 3 have one failure each. - tester.notifyJobCompletion(component, app1, true); + tester.jobCompletion(component).application(app1).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app1, applicationPackage, true, systemTest); tester.deployAndNotify(app1, applicationPackage, false, stagingTest); // app2 is successful, but will fail later. tester.deployCompletely(app2, canaryPackage); - tester.notifyJobCompletion(component, app3, true); + tester.jobCompletion(component).application(app3).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app3, applicationPackage, true, systemTest); tester.deployAndNotify(app3, applicationPackage, true, stagingTest); tester.deployAndNotify(app3, applicationPackage, false, productionCorpUsEast1); @@ -134,7 +134,7 @@ public class DeploymentIssueReporterTest { // app3 now has a new failure past max failure age; see that a new issue is filed. - tester.notifyJobCompletion(component, app3, true); + tester.jobCompletion(component).application(app3).submit(); tester.deployAndNotify(app3, applicationPackage, false, systemTest); tester.clock().advance(maxInactivity.plus(maxFailureAge)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java index 8aa92ec5fce..5ed4c3a186e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java @@ -4,11 +4,11 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; -import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; -import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.athenz.api.NToken; +import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; @@ -74,7 +74,8 @@ public class DnsMaintainerTest { .environment(Environment.prod) .allow(ValidationId.deploymentRemoval) .build(); - tester.notifyJobCompletion(component, application, true); + tester.jobCompletion(component).application(application).uploadArtifact(applicationPackage).submit(); + tester.deployAndNotify(application, applicationPackage, true, systemTest); tester.applications().deactivate(application, ZoneId.from(Environment.test, RegionName.from("us-east-1"))); tester.applications().deactivate(application, ZoneId.from(Environment.staging, RegionName.from("us-east-3"))); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java index 11e85c6be9f..6b8901ace88 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java @@ -18,6 +18,7 @@ import java.nio.file.Files; import java.nio.file.Paths; import java.time.Duration; +import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.component; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -39,7 +40,7 @@ public class FailureRedeployerTest { tester.updateVersionStatus(version); Application app = tester.createApplication("app1", "tenant1", 1, 11L); - tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); + tester.jobCompletion(component).application(app).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsEast3); @@ -82,7 +83,7 @@ public class FailureRedeployerTest { tester.deployAndNotify(app, applicationPackage, false, DeploymentJobs.JobType.productionUsEast3); tester.buildSystem().takeJobsToRun(); tester.clock().advance(Duration.ofMinutes(10)); - tester.notifyJobCompletion(DeploymentJobs.JobType.productionUsEast3, app, false); + tester.jobCompletion(DeploymentJobs.JobType.productionUsEast3).application(app).unsuccessful().submit(); assertTrue("Retries exhausted", tester.buildSystem().jobs().isEmpty()); assertTrue("Failure is recorded", tester.application(app.id()).deploymentJobs().hasFailures()); @@ -108,7 +109,7 @@ public class FailureRedeployerTest { .build(); Application app = tester.createApplication("app1", "tenant1", 1, 11L); - tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); + tester.jobCompletion(component).application(app).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); // staging-test starts, but does not complete @@ -123,7 +124,7 @@ public class FailureRedeployerTest { // Deployment completes tester.deploy(DeploymentJobs.JobType.stagingTest, app, applicationPackage, true); - tester.notifyJobCompletion(DeploymentJobs.JobType.stagingTest, app, true); + tester.jobCompletion(DeploymentJobs.JobType.stagingTest).application(app).submit(); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsEast3); assertTrue("All jobs consumed", tester.buildSystem().jobs().isEmpty()); } @@ -140,7 +141,7 @@ public class FailureRedeployerTest { tester.updateVersionStatus(version); Application app = tester.createApplication("app1", "tenant1", 1, 11L); - tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); + tester.jobCompletion(component).application(app).uploadArtifact(applicationPackage).submit(); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsEast3); @@ -156,7 +157,7 @@ public class FailureRedeployerTest { tester.deployAndNotify(app, applicationPackage, false, DeploymentJobs.JobType.systemTest); tester.buildSystem().takeJobsToRun(); tester.clock().advance(Duration.ofMinutes(10)); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, app, false); + tester.jobCompletion(DeploymentJobs.JobType.systemTest).application(app).unsuccessful().submit(); assertTrue("Retries exhausted", tester.buildSystem().jobs().isEmpty()); // Another version is released @@ -207,12 +208,12 @@ public class FailureRedeployerTest { tester.deploy(DeploymentJobs.JobType.systemTest, application, applicationPackage); tester.buildSystem().takeJobsToRun(); tester.clock().advance(Duration.ofMinutes(10)); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, application, true); + tester.jobCompletion(DeploymentJobs.JobType.systemTest).application(application).submit(); tester.deploy(DeploymentJobs.JobType.stagingTest, application, applicationPackage); tester.buildSystem().takeJobsToRun(); tester.clock().advance(Duration.ofMinutes(10)); - tester.notifyJobCompletion(DeploymentJobs.JobType.stagingTest, application, true); + tester.jobCompletion(DeploymentJobs.JobType.stagingTest).application(application).submit(); // Production job starts, but does not complete assertEquals(1, tester.buildSystem().jobs().size()); @@ -224,12 +225,12 @@ public class FailureRedeployerTest { assertTrue("No jobs retried", tester.buildSystem().jobs().isEmpty()); // Deployment notifies completeness but has not actually made a deployment - tester.notifyJobCompletion(DeploymentJobs.JobType.productionCdUsCentral1, application, true); + tester.jobCompletion(DeploymentJobs.JobType.productionCdUsCentral1).application(application).submit(); assertTrue("Change not really deployed", tester.application(application.id()).change().isPresent()); // Deployment actually deploys and notifies completeness tester.deploy(DeploymentJobs.JobType.productionCdUsCentral1, application, applicationPackage); - tester.notifyJobCompletion(DeploymentJobs.JobType.productionCdUsCentral1, application, true); + tester.jobCompletion(DeploymentJobs.JobType.productionCdUsCentral1).application(application).submit(); assertFalse("Change not really deployed", tester.application(application.id()).change().isPresent()); } 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 a7458f9f8ed..7b0ca7cb618 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 @@ -85,7 +85,7 @@ public class MetricsReporterTest { assertEquals(0.0, metricsMock.getMetric(MetricsReporter.deploymentFailMetric)); // 1 app fails system-test - tester.notifyJobCompletion(component, app4, true); + tester.jobCompletion(component).application(app4).submit(); tester.deployAndNotify(app4, applicationPackage, false, systemTest); metricsReporter.maintain(); 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 4bed276fcac..f6a2177bab8 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 @@ -2,18 +2,20 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; +import com.yahoo.config.provision.Environment; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.integration.BuildService; +import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import org.junit.Test; import java.time.Duration; import java.util.List; -import java.util.Optional; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -30,6 +32,11 @@ public class OutstandingChangeDeployerTest { tester.configServer().setDefaultVersion(new Version(6, 1)); OutstandingChangeDeployer deployer = new OutstandingChangeDeployer(tester.controller(), Duration.ofMinutes(10), new JobControl(new MockCuratorDb())); + ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + .environment(Environment.prod) + .region("us-west-1") + .build(); + tester.createAndDeploy("app1", 11, "default"); tester.createAndDeploy("app2", 22, "default"); @@ -38,14 +45,17 @@ public class OutstandingChangeDeployerTest { assertEquals(Change.of(version), tester.application("app1").change()); assertFalse(tester.application("app1").outstandingChange().isPresent()); - tester.notifyJobCompletion(DeploymentJobs.JobType.component, tester.application("app1"), - Optional.empty(), Optional.of(new SourceRevision("repo", "master", - "cafed00d")), - 42); + + tester.jobCompletion(DeploymentJobs.JobType.component) + .application(tester.application("app1")) + .sourceRevision(new SourceRevision("repository1","master", "cafed00d")) + .buildNumber(43) + .uploadArtifact(applicationPackage) + .submit(); Application app = tester.application("app1"); assertTrue(app.outstandingChange().isPresent()); - assertEquals("1.0.42-cafed00d", app.outstandingChange().application().get().id()); + assertEquals("1.0.43-cafed00d", app.outstandingChange().application().get().id()); assertEquals(1, tester.buildSystem().jobs().size()); deployer.maintain(); 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 ccc029a9654..7c4dd58fa33 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 @@ -19,6 +19,10 @@ import org.junit.Test; import java.time.Duration; import java.time.Instant; +import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.component; +import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.productionUsWest1; +import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.stagingTest; +import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.systemTest; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -105,11 +109,10 @@ public class UpgraderTest { assertEquals("Version broken, but Canaries should keep trying", 2, tester.buildSystem().jobs().size()); // Exhaust canary retries. - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, canary1, false); + tester.jobCompletion(systemTest).application(canary1).unsuccessful().submit(); tester.clock().advance(Duration.ofHours(1)); tester.deployAndNotify(canary0, DeploymentTester.applicationPackage("canary"), false, DeploymentJobs.JobType.stagingTest); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, canary1, false); - //tester.deployAndNotify(canary1, DeploymentTester.applicationPackage("canary"), false, DeploymentJobs.JobType.stagingTest); + tester.jobCompletion(systemTest).application(canary1).unsuccessful().submit(); // --- A new version is released - which repairs the Canary app and fails a default version = Version.fromString("5.3"); @@ -145,7 +148,7 @@ public class UpgraderTest { // Finish previous run, with exhausted retry. tester.clock().advance(Duration.ofHours(1)); - tester.notifyJobCompletion(DeploymentJobs.JobType.stagingTest, default0, false); + tester.jobCompletion(stagingTest).application(default0).unsuccessful().submit(); // --- Failing application is repaired by changing the application, causing confidence to move above 'high' threshold // Deploy application change @@ -196,7 +199,12 @@ public class UpgraderTest { assertEquals(version54, tester.application(default4.id()).change().platform().get()); tester.completeUpgrade(default1, version54, "default"); tester.completeUpgrade(default2, version54, "default"); + tester.completeUpgradeWithError(default3, version54, "default", DeploymentJobs.JobType.stagingTest); + // Exhaust immediate retries for upgrade + tester.clock().advance(Duration.ofHours(1)); + tester.jobCompletion(stagingTest).application(default3).unsuccessful().submit(); + tester.completeUpgradeWithError(default4, version54, "default", DeploymentJobs.JobType.productionUsWest1); // State: Default applications started upgrading to 5.5 tester.upgrader().maintain(); @@ -209,7 +217,7 @@ public class UpgraderTest { // Finish running job, without retry. tester.clock().advance(Duration.ofHours(1)); - tester.notifyJobCompletion(DeploymentJobs.JobType.productionUsWest1, default3, false); + tester.jobCompletion(DeploymentJobs.JobType.productionUsWest1).application(default3).unsuccessful().submit(); tester.upgrader().maintain(); assertEquals("Upgrade of defaults are scheduled on 5.4 instead, since 5.5 broken: " + @@ -270,10 +278,10 @@ public class UpgraderTest { assertEquals("Canaries done: Should upgrade defaults", 10, tester.buildSystem().jobs().size()); tester.completeUpgrade(default0, version, "default"); - tester.completeUpgradeWithError(default1, version, "default", DeploymentJobs.JobType.systemTest); - tester.completeUpgradeWithError(default2, version, "default", DeploymentJobs.JobType.systemTest); - tester.completeUpgradeWithError(default3, version, "default", DeploymentJobs.JobType.systemTest); - tester.completeUpgradeWithError(default4, version, "default", DeploymentJobs.JobType.systemTest); + tester.completeUpgradeWithError(default1, version, "default", systemTest); + tester.completeUpgradeWithError(default2, version, "default", systemTest); + tester.completeUpgradeWithError(default3, version, "default", systemTest); + tester.completeUpgradeWithError(default4, version, "default", systemTest); // > 40% and at least 4 failed - version is broken tester.updateVersionStatus(version); @@ -294,8 +302,8 @@ public class UpgraderTest { tester.updateVersionStatus(version); Application app = tester.createApplication("app1", "tenant1", 1, 11L); - tester.notifyJobCompletion(DeploymentJobs.JobType.component, app, true); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.jobCompletion(component).application(app).uploadArtifact(applicationPackage).submit(); + tester.deployAndNotify(app, applicationPackage, true, systemTest); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsEast3); @@ -310,13 +318,13 @@ public class UpgraderTest { tester.upgrader().maintain(); // system-test completes successfully - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, systemTest); // staging-test fails multiple times, exhausts retries and failure is recorded tester.deployAndNotify(app, applicationPackage, false, DeploymentJobs.JobType.stagingTest); tester.buildSystem().takeJobsToRun(); tester.clock().advance(Duration.ofMinutes(10)); - tester.notifyJobCompletion(DeploymentJobs.JobType.stagingTest, app, false); + tester.jobCompletion(stagingTest).application(app).unsuccessful().submit(); assertTrue("Retries exhausted", tester.buildSystem().jobs().isEmpty()); assertTrue("Failure is recorded", tester.application(app.id()).deploymentJobs().hasFailures()); assertTrue("Application has pending change", tester.application(app.id()).change().isPresent()); @@ -369,17 +377,17 @@ public class UpgraderTest { assertEquals("Upgrade scheduled for remaining apps", 5, tester.buildSystem().jobs().size()); // 4/5 applications fail and lowers confidence - tester.completeUpgradeWithError(default0, version, "default", DeploymentJobs.JobType.systemTest); - tester.completeUpgradeWithError(default1, version, "default", DeploymentJobs.JobType.systemTest); - tester.completeUpgradeWithError(default2, version, "default", DeploymentJobs.JobType.systemTest); - tester.completeUpgradeWithError(default3, version, "default", DeploymentJobs.JobType.systemTest); + tester.completeUpgradeWithError(default0, version, "default", systemTest); + tester.completeUpgradeWithError(default1, version, "default", systemTest); + tester.completeUpgradeWithError(default2, version, "default", systemTest); + tester.completeUpgradeWithError(default3, version, "default", systemTest); tester.updateVersionStatus(version); assertEquals(VespaVersion.Confidence.broken, tester.controller().versionStatus().systemVersion().get().confidence()); tester.upgrader().maintain(); // 5th app passes system-test, but does not trigger next job as upgrade is cancelled assertFalse("No change present", tester.applications().require(default4.id()).change().isPresent()); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default4, true); + tester.jobCompletion(systemTest).application(default4).submit(); assertTrue("All jobs consumed", tester.buildSystem().jobs().isEmpty()); } @@ -457,14 +465,14 @@ public class UpgraderTest { tester.deploymentTrigger().triggerReadyJobs(); assertEquals("Testing of 5.1 for 5 applications is triggered", 5, tester.buildSystem().jobs().size()); - assertEquals(DeploymentJobs.JobType.systemTest.jobName(), tester.buildSystem().jobs().get(0).jobName()); - assertEquals(DeploymentJobs.JobType.systemTest.jobName(), tester.buildSystem().jobs().get(1).jobName()); - assertEquals(DeploymentJobs.JobType.systemTest.jobName(), tester.buildSystem().jobs().get(2).jobName()); - assertEquals(DeploymentJobs.JobType.systemTest.jobName(), tester.buildSystem().jobs().get(3).jobName()); - assertEquals(DeploymentJobs.JobType.systemTest.jobName(), tester.buildSystem().jobs().get(4).jobName()); + assertEquals(systemTest.jobName(), tester.buildSystem().jobs().get(0).jobName()); + assertEquals(systemTest.jobName(), tester.buildSystem().jobs().get(1).jobName()); + assertEquals(systemTest.jobName(), tester.buildSystem().jobs().get(2).jobName()); + assertEquals(systemTest.jobName(), tester.buildSystem().jobs().get(3).jobName()); + assertEquals(systemTest.jobName(), tester.buildSystem().jobs().get(4).jobName()); // The tester code for completing upgrades does not handle this scenario, so we trigger each step manually (for one app) - tester.deployAndNotify(tester.application("default0"), "default", true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(tester.application("default0"), "default", true, systemTest); tester.deployAndNotify(tester.application("default0"), "default", true, DeploymentJobs.JobType.stagingTest); // prod zone on 5.2 (usWest1) is skipped, but we still trigger the next zone from triggerReadyJobs: tester.clock().advance(Duration.ofHours(13)); // Currently we don't cancel running jobs, so this is necessary to allow a new triggering below @@ -484,14 +492,17 @@ public class UpgraderTest { Version version = Version.fromString("5.0"); tester.updateVersionStatus(version); + ApplicationPackage canaryPolicy = DeploymentTester.applicationPackage("canary"); + ApplicationPackage defaultPolicy = DeploymentTester.applicationPackage("default"); + // Setup applications - Application canary0 = tester.createAndDeploy("canary0", 1, "canary"); - Application canary1 = tester.createAndDeploy("canary1", 2, "canary"); - Application default0 = tester.createAndDeploy("default0", 3, "default"); - Application default1 = tester.createAndDeploy("default1", 4, "default"); - Application default2 = tester.createAndDeploy("default2", 5, "default"); - Application default3 = tester.createAndDeploy("default3", 6, "default"); - Application default4 = tester.createAndDeploy("default4", 7, "default"); + Application canary0 = tester.createAndDeploy("canary0", 1, canaryPolicy); + Application canary1 = tester.createAndDeploy("canary1", 2, canaryPolicy); + Application default0 = tester.createAndDeploy("default0", 3, defaultPolicy); + Application default1 = tester.createAndDeploy("default1", 4, defaultPolicy); + Application default2 = tester.createAndDeploy("default2", 5, defaultPolicy); + Application default3 = tester.createAndDeploy("default3", 6, defaultPolicy); + Application default4 = tester.createAndDeploy("default4", 7, defaultPolicy); // New version is released version = Version.fromString("5.1"); @@ -517,12 +528,12 @@ public class UpgraderTest { // Multiple application changes are triggered and fail, but does not affect version confidence as upgrade has // completed successfully - tester.notifyJobCompletion(DeploymentJobs.JobType.component, default0, false); - tester.notifyJobCompletion(DeploymentJobs.JobType.component, default1, false); - tester.notifyJobCompletion(DeploymentJobs.JobType.component, default2, true); - tester.notifyJobCompletion(DeploymentJobs.JobType.component, default3, true); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default2, false); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default3, false); + tester.jobCompletion(component).application(default0).buildNumber(43).uploadArtifact(canaryPolicy).unsuccessful().submit(); + tester.jobCompletion(component).application(default1).buildNumber(43).uploadArtifact(canaryPolicy).unsuccessful().submit(); + tester.jobCompletion(component).application(default2).buildNumber(43).uploadArtifact(defaultPolicy).submit(); + tester.jobCompletion(component).application(default3).buildNumber(43).uploadArtifact(defaultPolicy).submit(); + tester.jobCompletion(component).application(default2).buildNumber(44).uploadArtifact(canaryPolicy).unsuccessful().submit(); + tester.jobCompletion(component).application(default3).buildNumber(44).uploadArtifact(canaryPolicy).unsuccessful().submit(); tester.updateVersionStatus(version); assertEquals(VespaVersion.Confidence.normal, tester.controller().versionStatus().systemVersion().get().confidence()); } @@ -560,7 +571,7 @@ public class UpgraderTest { tester.clock().advance(Duration.ofHours(1)); tester.upgrader().maintain(); assertFalse("Job is scheduled", tester.buildSystem().jobs().isEmpty()); - tester.completeUpgrade(app, version, "canary"); + tester.completeUpgrade(app, version, applicationPackage); assertTrue("All jobs consumed", tester.buildSystem().jobs().isEmpty()); } @@ -592,10 +603,10 @@ public class UpgraderTest { // Application upgrade starts tester.upgrader().maintain(); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, systemTest); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); clock.advance(Duration.ofHours(1)); // Entering block window after prod job is triggered - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsWest1); + tester.deployAndNotify(app, applicationPackage, true, productionUsWest1); assertTrue(tester.buildSystem().jobs().isEmpty()); // Next job not triggered due to being in the block window // One hour passes, time is 19:00, still no upgrade @@ -647,9 +658,9 @@ public class UpgraderTest { // Application upgrade starts tester.upgrader().maintain(); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, systemTest); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsWest1); + tester.deployAndNotify(app, applicationPackage, true, productionUsWest1); clock.advance(Duration.ofHours(1)); // Entering block window after prod job is triggered tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsCentral1); assertTrue(tester.buildSystem().jobs().isEmpty()); // Next job not triggered due to being in the block window @@ -669,9 +680,9 @@ public class UpgraderTest { readyJobsTrigger.maintain(); // We proceed with the new version in the expected order, not starting with the previously blocked version: // Test jobs are run with the new version, but not production as we are in the block window - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.systemTest); + tester.deployAndNotify(app, applicationPackage, true, systemTest); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.stagingTest); - tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsWest1); + tester.deployAndNotify(app, applicationPackage, true, productionUsWest1); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsCentral1); tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsEast3); assertTrue("All jobs consumed", tester.buildSystem().jobs().isEmpty()); @@ -727,10 +738,10 @@ public class UpgraderTest { assertEquals("Upgrade scheduled for remaining apps", 5, tester.buildSystem().jobs().size()); // 4/5 applications fail, confidence is lowered and upgrade is cancelled - tester.completeUpgradeWithError(default0, version, defaultApplicationPackage, DeploymentJobs.JobType.systemTest); - tester.completeUpgradeWithError(default1, version, defaultApplicationPackage, DeploymentJobs.JobType.systemTest); - tester.completeUpgradeWithError(default2, version, defaultApplicationPackage, DeploymentJobs.JobType.systemTest); - tester.completeUpgradeWithError(default3, version, defaultApplicationPackage, DeploymentJobs.JobType.systemTest); + tester.completeUpgradeWithError(default0, version, defaultApplicationPackage, systemTest); + tester.completeUpgradeWithError(default1, version, defaultApplicationPackage, systemTest); + tester.completeUpgradeWithError(default2, version, defaultApplicationPackage, systemTest); + tester.completeUpgradeWithError(default3, version, defaultApplicationPackage, systemTest); tester.updateVersionStatus(version); assertEquals(VespaVersion.Confidence.broken, tester.controller().versionStatus().systemVersion().get().confidence()); @@ -738,10 +749,10 @@ public class UpgraderTest { // Exhaust retries and finish runs tester.clock().advance(Duration.ofHours(1)); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default0, false); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default1, false); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default2, false); - tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default3, false); + tester.jobCompletion(systemTest).application(default0).unsuccessful().submit(); + tester.jobCompletion(systemTest).application(default1).unsuccessful().submit(); + tester.jobCompletion(systemTest).application(default2).unsuccessful().submit(); + tester.jobCompletion(systemTest).application(default3).unsuccessful().submit(); // 5th app never reports back and has a dead job, but no ongoing change Application deadLocked = tester.applications().require(default4.id()); @@ -755,10 +766,10 @@ public class UpgraderTest { .environment(Environment.prod) .region("us-west-1") .build(); - tester.deployCompletely(default0, defaultApplicationPackageV2); - tester.deployCompletely(default1, defaultApplicationPackageV2); - tester.deployCompletely(default2, defaultApplicationPackageV2); - tester.deployCompletely(default3, defaultApplicationPackageV2); + tester.deployCompletely(default0, defaultApplicationPackageV2, 43); + tester.deployCompletely(default1, defaultApplicationPackageV2, 43); + tester.deployCompletely(default2, defaultApplicationPackageV2, 43); + tester.deployCompletely(default3, defaultApplicationPackageV2, 43); tester.updateVersionStatus(version); assertEquals(VespaVersion.Confidence.normal, tester.controller().versionStatus().systemVersion().get().confidence()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index 9c19cdb66ac..0ad01f57579 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -148,14 +148,13 @@ public class ApplicationSerializerTest { assertEquals(6, serialized.deployments().get(zone2).metrics().writeLatencyMillis(), Double.MIN_VALUE); { // test more deployment serialization cases - Application original2 = writable(original).withChange(Change.of(ApplicationVersion.from("hash1"))); + Application original2 = writable(original).withChange(Change.of(ApplicationVersion.from(new SourceRevision("repo1", "branch1", "commit1"), 42))); Application serialized2 = applicationSerializer.fromSlime(applicationSerializer.toSlime(original2)); assertEquals(original2.change(), serialized2.change()); assertEquals(serialized2.change().application().get().source(), original2.change().application().get().source()); - Application original3 = writable(original).withChange(Change.of(ApplicationVersion.from("hash1", - new SourceRevision("a", "b", "c")))); + Application original3 = writable(original).withChange(Change.of(ApplicationVersion.from(new SourceRevision("a", "b", "c"), 42))); Application serialized3 = applicationSerializer.fromSlime(applicationSerializer.toSlime(original3)); assertEquals(original3.change(), serialized3.change()); assertEquals(serialized3.change().application().get().source(), @@ -164,11 +163,11 @@ public class ApplicationSerializerTest { Application serialized4 = applicationSerializer.fromSlime(applicationSerializer.toSlime(original4)); assertEquals(original4.change(), serialized4.change()); - Application original5 = writable(original).withChange(Change.of(ApplicationVersion.unknown)); + Application original5 = writable(original).withChange(Change.of(ApplicationVersion.from(new SourceRevision("a", "b", "c"), 42))); Application serialized5 = applicationSerializer.fromSlime(applicationSerializer.toSlime(original5)); assertEquals(original5.change(), serialized5.change()); - Application original6 = writable(original).withOutstandingChange(Change.of(ApplicationVersion.unknown)); + Application original6 = writable(original).withOutstandingChange(Change.of(ApplicationVersion.from(new SourceRevision("a", "b", "c"), 42))); Application serialized6 = applicationSerializer.fromSlime(applicationSerializer.toSlime(original6)); assertEquals(original6.outstandingChange(), serialized6.outstandingChange()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json index 59cb709eb3f..eb3269fafc8 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json @@ -501,7 +501,7 @@ ] }, "deployingField": { - "applicationPackageHash": "ec548fa61cbfab7a270a51d46b1263ec1be5d9a8", + "buildNumber": 42, "sourceRevision": { "repositoryField": "git@git.host:user/repo.git", "branchField": "origin/master", 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 5b806d580e2..256dcca7a05 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 @@ -7,6 +7,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.athenz.utils.AthenzIdentities; import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.ArtifactRepositoryMock; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.TestIdentities; import com.yahoo.vespa.hosted.controller.api.Tenant; @@ -27,6 +28,7 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock; +import com.yahoo.vespa.hosted.controller.deployment.BuildJob; import com.yahoo.vespa.hosted.controller.maintenance.JobControl; import com.yahoo.vespa.hosted.controller.maintenance.Upgrader; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; @@ -56,6 +58,11 @@ public class ContainerControllerTester { public Controller controller() { return containerTester.controller(); } + public ArtifactRepositoryMock artifactRepository() { + return (ArtifactRepositoryMock) containerTester.container().components() + .getComponent(ArtifactRepositoryMock.class.getName()); + } + public Upgrader upgrader() { return upgrader; } /** Returns the wrapped generic container tester */ @@ -79,36 +86,14 @@ public class ContainerControllerTester { public Application deploy(Application application, ApplicationPackage applicationPackage, ZoneId zone, long projectId) { ScrewdriverId app1ScrewdriverId = new ScrewdriverId(String.valueOf(projectId)); GitRevision app1RevisionId = new GitRevision(new GitRepository("repo"), new GitBranch("master"), new GitCommit("commit1")); - controller().applications().deployApplication(application.id(), - zone, - Optional.of(applicationPackage), + controller().applications().deployApplication(application.id(), zone, Optional.of(applicationPackage), new DeployOptions(Optional.of(new ScrewdriverBuildJob(app1ScrewdriverId, app1RevisionId)), Optional.empty(), false, false)); return application; } - public void notifyJobCompletion(ApplicationId applicationId, long projectId, boolean success, DeploymentJobs.JobType job) { - try { - Thread.sleep(1); - } - catch (InterruptedException e) { - throw new RuntimeException(e); - } - controller().applications().notifyJobCompletion(new DeploymentJobs.JobReport(applicationId, job, projectId, - 42, - Optional.empty(), - success ? Optional.empty() : Optional.of(DeploymentJobs.JobError.unknown) - )); - } - - public AthenzDomain addTenantAthenzDomain(String domainName, String userName) { - AthenzClientFactoryMock mock = (AthenzClientFactoryMock) containerTester.container().components() - .getComponent(AthenzClientFactoryMock.class.getName()); - AthenzDomain athensDomain = new AthenzDomain(domainName); - AthenzDbMock.Domain domain = new AthenzDbMock.Domain(athensDomain); - domain.markAsVespaTenant(); - domain.admin(AthenzIdentities.from(new AthenzDomain("domain"), userName)); - mock.getSetup().addDomain(domain); - return athensDomain; + /** Notify the controller about a job completing */ + public BuildJob jobCompletion(DeploymentJobs.JobType job) { + return new BuildJob(this::notifyJobCompletion, artifactRepository()).type(job); } // ---- Delegators: @@ -134,4 +119,24 @@ public class ContainerControllerTester { .addRoleMember(action, HostedAthenzIdentities.from(screwdriverId)); } + private void notifyJobCompletion(DeploymentJobs.JobReport jobReport) { + try { + Thread.sleep(1); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + controller().applications().notifyJobCompletion(jobReport); + } + + private AthenzDomain addTenantAthenzDomain(String domainName, String userName) { + AthenzClientFactoryMock mock = (AthenzClientFactoryMock) containerTester.container().components() + .getComponent(AthenzClientFactoryMock.class.getName()); + AthenzDomain athensDomain = new AthenzDomain(domainName); + AthenzDbMock.Domain domain = new AthenzDbMock.Domain(athensDomain); + domain.markAsVespaTenant(); + domain.admin(AthenzIdentities.from(new AthenzDomain("domain"), userName)); + mock.getSetup().addDomain(domain); + return athensDomain; + } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java index abc5f9f8aa1..566014e9830 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java @@ -8,7 +8,8 @@ import com.yahoo.application.container.handler.Response; import org.junit.After; import org.junit.Before; -import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.CharacterCodingException; import static org.junit.Assert.assertEquals; @@ -90,11 +91,15 @@ public class ControllerContainerTest { " </handler>\n" + "</jdisc>"; - protected void assertResponse(Request request, int responseStatus, String responseMessage) throws IOException { + protected void assertResponse(Request request, int responseStatus, String responseMessage) { Response response = container.handleRequest(request); // Compare both status and message at once for easier diagnosis - assertEquals("status: " + responseStatus + "\nmessage: " + responseMessage, - "status: " + response.getStatus() + "\nmessage: " + response.getBodyAsString()); + try { + assertEquals("status: " + responseStatus + "\nmessage: " + responseMessage, + "status: " + response.getStatus() + "\nmessage: " + response.getBodyAsString()); + } catch (CharacterCodingException e) { + throw new UncheckedIOException(e); + } } } 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 e6fe7531fdc..2b198c9897f 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 @@ -200,7 +200,11 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default", DELETE) .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default"); - controllerTester.notifyJobCompletion(id, screwdriverProjectId, true, DeploymentJobs.JobType.systemTest); // Called through the separate screwdriver/v1 API + // Called through the separate screwdriver/v1 API + controllerTester.jobCompletion(DeploymentJobs.JobType.systemTest) + .application(id) + .projectId(screwdriverProjectId) + .submit(); // ... staging tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default/", POST) @@ -210,14 +214,21 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default", DELETE) .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated tenant/tenant1/application/application1/environment/staging/region/us-east-3/instance/default"); - controllerTester.notifyJobCompletion(id, screwdriverProjectId, true, DeploymentJobs.JobType.stagingTest); + controllerTester.jobCompletion(DeploymentJobs.JobType.stagingTest) + .application(id) + .projectId(screwdriverProjectId) + .submit(); // ... prod zone tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/", POST) .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId))) .screwdriverIdentity(SCREWDRIVER_ID), new File("deploy-result.json")); - controllerTester.notifyJobCompletion(id, screwdriverProjectId, false, DeploymentJobs.JobType.productionCorpUsEast1); + controllerTester.jobCompletion(DeploymentJobs.JobType.productionCorpUsEast1) + .application(id) + .projectId(screwdriverProjectId) + .unsuccessful() + .submit(); // GET tenant screwdriver projects tester.assertResponse(request("/application/v4/tenant-pipeline/", GET), @@ -405,16 +416,18 @@ public class ApplicationApiTest extends ControllerContainerTest { .build(); ApplicationId id = ApplicationId.from("tenant1", "application1", "default"); long projectId = 1; - HttpEntity deployData = createApplicationDeployData(applicationPackage, Optional.of(projectId)); - - startAndTestChange(controllerTester, id, projectId, deployData); + HttpEntity deployData = createApplicationDeployData(Optional.empty(), Optional.of(projectId)); + startAndTestChange(controllerTester, id, projectId, applicationPackage, deployData, 100); // us-east-3 tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-east-3/instance/default/deploy", POST) .data(deployData) .screwdriverIdentity(SCREWDRIVER_ID), new File("deploy-result.json")); - controllerTester.notifyJobCompletion(id, projectId, true, DeploymentJobs.JobType.productionUsEast3); + controllerTester.jobCompletion(DeploymentJobs.JobType.productionUsEast3) + .application(id) + .projectId(projectId) + .submit(); // New zone is added before us-east-3 applicationPackage = new ApplicationPackageBuilder() @@ -423,21 +436,26 @@ public class ApplicationApiTest extends ControllerContainerTest { .region("us-west-1") .region("us-east-3") .build(); - deployData = createApplicationDeployData(applicationPackage, Optional.of(projectId)); - startAndTestChange(controllerTester, id, projectId, deployData); + startAndTestChange(controllerTester, id, projectId, applicationPackage, deployData, 101); // us-west-1 tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-west-1/instance/default/deploy", POST) .data(deployData) .screwdriverIdentity(SCREWDRIVER_ID), new File("deploy-result.json")); - controllerTester.notifyJobCompletion(id, projectId, true, DeploymentJobs.JobType.productionUsWest1); + controllerTester.jobCompletion(DeploymentJobs.JobType.productionUsWest1) + .application(id) + .projectId(projectId) + .submit(); // us-east-3 tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-east-3/instance/default/deploy", POST) .data(deployData).screwdriverIdentity(SCREWDRIVER_ID), new File("deploy-result.json")); - controllerTester.notifyJobCompletion(id, projectId, true, DeploymentJobs.JobType.productionUsEast3); + controllerTester.jobCompletion(DeploymentJobs.JobType.productionUsEast3) + .application(id) + .projectId(projectId) + .submit(); setDeploymentMaintainedInfo(controllerTester); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", GET), @@ -695,7 +713,11 @@ public class ApplicationApiTest extends ControllerContainerTest { controllerTester.authorize(ATHENZ_TENANT_DOMAIN, screwdriverId, ApplicationAction.deploy, application); // Allow systemtest to succeed by notifying completion of system test - controllerTester.notifyJobCompletion(application.id(), screwdriverProjectId, true, DeploymentJobs.JobType.component); + controllerTester.jobCompletion(DeploymentJobs.JobType.component) + .application(application.id()) + .projectId(screwdriverProjectId) + .uploadArtifact(applicationPackage) + .submit(); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-east-1/instance/default/", POST) .data(createApplicationDeployData(applicationPackage, Optional.of(screwdriverProjectId))) .screwdriverIdentity(screwdriverId), @@ -704,9 +726,13 @@ public class ApplicationApiTest extends ControllerContainerTest { } private HttpEntity createApplicationDeployData(ApplicationPackage applicationPackage, Optional<Long> screwdriverJobId) { + return createApplicationDeployData(Optional.of(applicationPackage), screwdriverJobId); + } + + private HttpEntity createApplicationDeployData(Optional<ApplicationPackage> applicationPackage, Optional<Long> screwdriverJobId) { MultipartEntityBuilder builder = MultipartEntityBuilder.create(); builder.addTextBody("deployOptions", deployOptions(screwdriverJobId), ContentType.APPLICATION_JSON); - builder.addBinaryBody("applicationZip", applicationPackage.zippedContent()); + applicationPackage.ifPresent(ap -> builder.addBinaryBody("applicationZip", ap.zippedContent())); return builder.build(); } @@ -811,12 +837,19 @@ public class ApplicationApiTest extends ControllerContainerTest { athenzApplication.addRoleMember(ApplicationAction.deploy, screwdriverIdentity); } - private void startAndTestChange(ContainerControllerTester controllerTester, ApplicationId application, long projectId, - HttpEntity deployData) throws IOException { + private void startAndTestChange(ContainerControllerTester controllerTester, ApplicationId application, + long projectId, ApplicationPackage applicationPackage, + HttpEntity deployData, long buildNumber) throws IOException { ContainerTester tester = controllerTester.containerTester(); // Trigger application change - controllerTester.notifyJobCompletion(application, projectId, true, DeploymentJobs.JobType.component); + controllerTester.artifactRepository().put(application, applicationPackage,"1.0." + buildNumber + + "-commit1"); + controllerTester.jobCompletion(DeploymentJobs.JobType.component) + .application(application) + .projectId(projectId) + .buildNumber(buildNumber) + .submit(); // system-test String testPath = String.format("/application/v4/tenant/%s/application/%s/environment/test/region/us-east-1/instance/default", @@ -828,7 +861,10 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request(testPath, DELETE) .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated " + testPath.replaceFirst("/application/v4/", "")); - controllerTester.notifyJobCompletion(application, projectId, true, DeploymentJobs.JobType.systemTest); + controllerTester.jobCompletion(DeploymentJobs.JobType.systemTest) + .application(application) + .projectId(projectId) + .submit(); // staging String stagingPath = String.format("/application/v4/tenant/%s/application/%s/environment/staging/region/us-east-3/instance/default", @@ -840,7 +876,10 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request(stagingPath, DELETE) .screwdriverIdentity(SCREWDRIVER_ID), "Deactivated " + stagingPath.replaceFirst("/application/v4/", "")); - controllerTester.notifyJobCompletion(application, projectId, true, DeploymentJobs.JobType.stagingTest); + controllerTester.jobCompletion(DeploymentJobs.JobType.stagingTest) + .application(application) + .projectId(projectId) + .submit(); } /** diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json index 8bb5cc6da46..d3b765551f0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json @@ -6,14 +6,30 @@ "type": "component", "success": true, "lastCompleted": { - "id": 42, + "id": 101, "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + } + }, "reason": "Application commit", "at": "(ignore)" }, "lastSuccess": { - "id": 42, + "id": 101, "version": "(ignore)", + "revision": { + "hash": "(ignore)", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + } + }, "reason": "Application commit", "at": "(ignore)" } @@ -78,7 +94,7 @@ "gitCommit": "commit1" } }, - "reason":"system-test completed", + "reason": "system-test completed", "at": "(ignore)" }, "lastCompleted": { @@ -92,7 +108,7 @@ "gitCommit": "commit1" } }, - "reason":"system-test completed", + "reason": "system-test completed", "at": "(ignore)" }, "lastSuccess": { @@ -106,7 +122,7 @@ "gitCommit": "commit1" } }, - "reason":"system-test completed", + "reason": "system-test completed", "at": "(ignore)" } }, @@ -124,7 +140,7 @@ "gitCommit": "commit1" } }, - "reason":"staging-test completed", + "reason": "staging-test completed", "at": "(ignore)" }, "lastCompleted": { @@ -138,7 +154,7 @@ "gitCommit": "commit1" } }, - "reason":"staging-test completed", + "reason": "staging-test completed", "at": "(ignore)" }, "lastSuccess": { @@ -152,7 +168,7 @@ "gitCommit": "commit1" } }, - "reason":"staging-test completed", + "reason": "staging-test completed", "at": "(ignore)" } }, @@ -170,7 +186,7 @@ "gitCommit": "commit1" } }, - "reason":"production-us-west-1 completed", + "reason": "production-us-west-1 completed", "at": "(ignore)" }, "lastCompleted": { @@ -184,7 +200,7 @@ "gitCommit": "commit1" } }, - "reason":"production-us-west-1 completed", + "reason": "production-us-west-1 completed", "at": "(ignore)" }, "lastSuccess": { @@ -198,7 +214,7 @@ "gitCommit": "commit1" } }, - "reason":"production-us-west-1 completed", + "reason": "production-us-west-1 completed", "at": "(ignore)" } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java index f3c8e57b1b5..538f2ac51e5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java @@ -6,10 +6,11 @@ import com.yahoo.application.container.handler.Request; import com.yahoo.component.Version; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; -import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; +import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; @@ -23,11 +24,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; -import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.component; -import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.productionCorpUsEast1; -import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.stagingTest; -import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.systemTest; - /** * @author bratseth */ @@ -101,17 +97,31 @@ public class DeploymentApiTest extends ControllerContainerTest { private void deployCompletely(Application application, ApplicationPackage applicationPackage, long projectId, boolean success) { - tester.notifyJobCompletion(application.id(), projectId, true, component); + tester.jobCompletion(DeploymentJobs.JobType.component) + .application(application) + .projectId(projectId) + .uploadArtifact(applicationPackage) + .submit(); tester.deploy(application, applicationPackage, ZoneId.from(Environment.test, RegionName.from("us-east-1")), projectId); - tester.notifyJobCompletion(application.id(), projectId, true, systemTest); + tester.jobCompletion(DeploymentJobs.JobType.systemTest) + .application(application) + .projectId(projectId) + .submit(); tester.deploy(application, applicationPackage, ZoneId.from(Environment.staging, RegionName.from("us-east-3")), projectId); - tester.notifyJobCompletion(application.id(), projectId, success, stagingTest); + tester.jobCompletion(DeploymentJobs.JobType.stagingTest) + .application(application) + .projectId(projectId) + .success(success) + .submit(); if (success) { tester.deploy(application, applicationPackage, ZoneId.from(Environment.prod, RegionName.from("corp-us-east-1")), projectId); - tester.notifyJobCompletion(application.id(), projectId, true, productionCorpUsEast1); + tester.jobCompletion(DeploymentJobs.JobType.productionCorpUsEast1) + .application(application) + .projectId(projectId) + .submit(); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java index 679b0114721..81470bac618 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java @@ -4,17 +4,20 @@ package com.yahoo.vespa.hosted.controller.restapi.screwdriver; import com.yahoo.application.container.handler.Request; import com.yahoo.application.container.handler.Response; import com.yahoo.component.Version; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; -import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Slime; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; +import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; 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.BuildSystem; import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; @@ -23,8 +26,8 @@ import org.junit.Test; import java.io.File; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; -import java.util.Optional; import static junit.framework.TestCase.assertNotNull; import static junit.framework.TestCase.assertNull; @@ -68,16 +71,16 @@ public class ScrewdriverApiTest extends ControllerContainerTest { Version vespaVersion = new Version("6.1"); // system version from mock config server client - // Make web service calls. - notifyCompletion(app.id(), projectId, JobType.component, Optional.empty()); + BuildJob job = new BuildJob(this::notifyCompletion, tester.artifactRepository()) + .application(app) + .projectId(projectId); + job.type(JobType.component).uploadArtifact(applicationPackage).submit(); tester.deploy(app, applicationPackage, testZone, projectId); - notifyCompletion(app.id(), projectId, JobType.systemTest, Optional.empty()); + job.type(JobType.systemTest).submit(); // Notifying about unknown job fails tester.containerTester().assertResponse(new Request("http://localhost:8080/application/v4/tenant/tenant1/application/application1/jobreport", - jsonReport(app.id(), JobType.productionUsEast3, projectId, 1L, - Optional.empty()) - .getBytes(StandardCharsets.UTF_8), + asJson(job.type(JobType.productionUsEast3).report()), Request.Method.POST), new File("unexpected-completion.json"), 400); @@ -113,7 +116,7 @@ public class ScrewdriverApiTest extends ControllerContainerTest { } @Test - public void testJobStatusReportingOutOfCapacity() throws Exception { + public void testJobStatusReportingOutOfCapacity() { ContainerControllerTester tester = new ContainerControllerTester(container, responseFiles); tester.containerTester().updateSystemVersion(); @@ -125,11 +128,15 @@ public class ScrewdriverApiTest extends ControllerContainerTest { .build(); // Report job failing with out of capacity - notifyCompletion(app.id(), projectId, JobType.component, Optional.empty()); + BuildJob job = new BuildJob(this::notifyCompletion, tester.artifactRepository()) + .application(app) + .projectId(projectId); + job.type(JobType.component).uploadArtifact(applicationPackage).submit(); + tester.deploy(app, applicationPackage, testZone, projectId); - notifyCompletion(app.id(), projectId, JobType.systemTest, Optional.empty()); + job.type(JobType.systemTest).submit(); tester.deploy(app, applicationPackage, stagingZone, projectId); - notifyCompletion(app.id(), projectId, JobType.stagingTest, Optional.of(JobError.outOfCapacity)); + job.type(JobType.stagingTest).error(JobError.outOfCapacity).submit(); // Appropriate error is recorded JobStatus jobStatus = tester.controller().applications().get(app.id()) @@ -142,7 +149,7 @@ public class ScrewdriverApiTest extends ControllerContainerTest { } @Test - public void testTriggerJobForApplication() throws Exception { + public void testTriggerJobForApplication() { ContainerControllerTester tester = new ContainerControllerTester(container, responseFiles); BuildSystem buildSystem = tester.controller().applications().deploymentTrigger().buildSystem(); tester.containerTester().updateSystemVersion(); @@ -182,26 +189,35 @@ public class ScrewdriverApiTest extends ControllerContainerTest { assertEquals(JobType.stagingTest.jobName(), buildSystem.jobs().get(0).jobName()); assertEquals(1L, buildSystem.jobs().get(0).projectId()); } - - private void notifyCompletion(ApplicationId app, long projectId, JobType jobType, Optional<JobError> error) throws IOException { + + private void notifyCompletion(DeploymentJobs.JobReport report) { assertResponse(new Request("http://localhost:8080/application/v4/tenant/tenant1/application/application1/jobreport", - jsonReport(app, jobType, projectId, 1L, error).getBytes(StandardCharsets.UTF_8), + asJson(report), Request.Method.POST), 200, "{\"message\":\"ok\"}"); } - private static String jsonReport(ApplicationId applicationId, JobType jobType, long projectId, long buildNumber, - Optional<JobError> jobError) { - return - "{\n" + - " \"projectId\" : " + projectId + ",\n" + - " \"jobName\" :\"" + jobType.jobName() + "\",\n" + - " \"buildNumber\" : " + buildNumber + ",\n" + - jobError.map(message -> " \"jobError\" : \"" + message + "\",\n").orElse("") + - " \"tenant\" :\"" + applicationId.tenant().value() + "\",\n" + - " \"application\" :\"" + applicationId.application().value() + "\",\n" + - " \"instance\" :\"" + applicationId.instance().value() + "\"\n" + - "}"; + private static byte[] asJson(DeploymentJobs.JobReport report) { + Slime slime = new Slime(); + Cursor cursor = slime.setObject(); + cursor.setLong("projectId", report.projectId()); + cursor.setString("jobName", report.jobType().jobName()); + cursor.setLong("buildNumber", report.buildNumber()); + report.jobError().ifPresent(jobError -> cursor.setString("jobError", jobError.name())); + report.sourceRevision().ifPresent(sr -> { + Cursor sourceRevision = cursor.setObject("sourceRevision"); + sourceRevision.setString("repository", sr.repository()); + sourceRevision.setString("branch", sr.branch()); + sourceRevision.setString("commit", sr.commit()); + }); + cursor.setString("tenant", report.applicationId().tenant().value()); + cursor.setString("application", report.applicationId().application().value()); + cursor.setString("instance", report.applicationId().instance().value()); + try { + return SlimeUtils.toJsonBytes(slime); + } catch (IOException e) { + throw new UncheckedIOException(e); + } } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationTest.java index c259ae0ca60..c73bf9ac34e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/rotation/RotationTest.java @@ -79,7 +79,7 @@ public class RotationTest { .region("us-west-1") .searchDefinition("search foo { }") // Update application package so there is something to deploy .build(); - tester.deployCompletely(application, applicationPackage); + tester.deployCompletely(application, applicationPackage, 43); assertEquals(expected.id(), tester.applications().require(application.id()).rotation().get().id()); } 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 ee6b61f5582..868ea50128d 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 @@ -6,7 +6,6 @@ import com.yahoo.component.Version; import com.yahoo.component.Vtag; import com.yahoo.config.provision.Environment; import com.yahoo.vespa.hosted.controller.Application; -import com.yahoo.vespa.hosted.controller.ApplicationController; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; @@ -21,7 +20,6 @@ import java.net.URISyntaxException; import java.time.Duration; import java.util.List; -import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.component; import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.productionUsEast3; import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.productionUsWest1; import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.stagingTest; @@ -285,30 +283,6 @@ public class VersionStatusTest { tester.controllerTester().curator().writeIgnoreConfidence(false); } - @Test - public void testComputeIgnoresVersionWithUnknownGitMetadata() { - ControllerTester tester = new ControllerTester(); - ApplicationController applications = tester.controller().applications(); - - tester.gitHub() - .mockAny(false) - .knownTag(Vtag.currentVersion.toFullString(), "foo") // controller - .knownTag("6.1.0", "bar"); // config server - - Version versionWithUnknownTag = new Version("6.1.2"); - - Application app = tester.createAndDeploy("tenant1", "domain1","application1", Environment.test, 11); - tester.clock().advance(Duration.ofMillis(1)); - applications.notifyJobCompletion(DeploymentTester.jobReport(app, component, true)); - applications.notifyJobCompletion(DeploymentTester.jobReport(app, systemTest, true)); - - List<VespaVersion> vespaVersions = VersionStatus.compute(tester.controller()).versions(); - - assertEquals(2, vespaVersions.size()); // controller and config server - assertTrue("Version referencing unknown tag is skipped", - vespaVersions.stream().noneMatch(v -> v.versionNumber().equals(versionWithUnknownTag))); - } - private Confidence confidence(Controller controller, Version version) { return controller.versionStatus().versions().stream() .filter(v -> v.statistics().version().equals(version)) |