diff options
author | Martin Polden <mpolden@mpolden.no> | 2018-04-17 21:00:49 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-04-17 21:00:49 +0200 |
commit | 31eb7a0ec654ef296549970f4589379ba0f5f7a1 (patch) | |
tree | 0993d87924d453e58dce19615785502c70669197 | |
parent | 67ab5565091d5688fb85abed37824547f4bea205 (diff) | |
parent | 634864e1b9442c5c1953c9bbff1f21caabd11479 (diff) |
Merge pull request #5611 from vespa-engine/jvenstad/dep-orch-minors
Fixes to earlier comments
17 files changed, 70 insertions, 83 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index 5e6d045afc4..33137202ea4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.OptionalLong; import java.util.function.Function; import java.util.stream.Collectors; @@ -50,7 +51,7 @@ public class Application { /** Creates an empty application */ public Application(ApplicationId id) { this(id, DeploymentSpec.empty, ValidationOverrides.empty, Collections.emptyMap(), - new DeploymentJobs(Optional.empty(), Collections.emptyList(), Optional.empty()), + new DeploymentJobs(OptionalLong.empty(), Collections.emptyList(), Optional.empty()), Change.empty(), Change.empty(), Optional.empty(), new ApplicationMetrics(0, 0), Optional.empty()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java index 505775aef12..979578c1a37 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java @@ -21,13 +21,13 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobStatus; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.rotation.RotationId; import java.time.Instant; import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; +import java.util.OptionalLong; /** * A combination of an application instance and a lock for that application. Provides methods for updating application @@ -54,7 +54,7 @@ public class LockedApplication extends Application { builder.outstandingChange, builder.ownershipIssueId, builder.metrics, builder.rotation); } - public LockedApplication withProjectId(Optional<Long> projectId) { + public LockedApplication withProjectId(OptionalLong projectId) { return new LockedApplication(new Builder(this).with(deploymentJobs().withProjectId(projectId))); } 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 dca1736d2c5..711853d2f9c 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 @@ -17,6 +17,7 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.OptionalLong; import java.util.stream.Stream; /** @@ -28,16 +29,16 @@ import java.util.stream.Stream; */ public class DeploymentJobs { - private final Optional<Long> projectId; + private final OptionalLong projectId; private final ImmutableMap<JobType, JobStatus> status; private final Optional<IssueId> issueId; - public DeploymentJobs(Optional<Long> projectId, Collection<JobStatus> jobStatusEntries, + public DeploymentJobs(OptionalLong projectId, Collection<JobStatus> jobStatusEntries, Optional<IssueId> issueId) { this(projectId, asMap(jobStatusEntries), issueId); } - private DeploymentJobs(Optional<Long> projectId, Map<JobType, JobStatus> status, Optional<IssueId> issueId) { + private DeploymentJobs(OptionalLong projectId, Map<JobType, JobStatus> status, Optional<IssueId> issueId) { requireId(projectId, "projectId must be a positive integer"); Objects.requireNonNull(status, "status cannot be null"); Objects.requireNonNull(issueId, "issueId cannot be null"); @@ -62,7 +63,7 @@ public class DeploymentJobs { return job.withCompletion(report.buildNumber(), applicationVersion, report.jobError(), notificationTime, controller); }); - return new DeploymentJobs(Optional.of(report.projectId()), status, issueId); + return new DeploymentJobs(OptionalLong.of(report.projectId()), status, issueId); } public DeploymentJobs withTriggering(JobType jobType, JobStatus.JobRun jobRun) { @@ -74,7 +75,7 @@ public class DeploymentJobs { return new DeploymentJobs(projectId, status, issueId); } - public DeploymentJobs withProjectId(Optional<Long> projectId) { + public DeploymentJobs withProjectId(OptionalLong projectId) { return new DeploymentJobs(projectId, status, issueId); } @@ -120,7 +121,7 @@ public class DeploymentJobs { * - or empty when this is not known or does not exist. * It is not known until the jobs have run once and reported back to the controller. */ - public Optional<Long> projectId() { return projectId; } + public OptionalLong projectId() { return projectId; } public Optional<IssueId> issueId() { return issueId; } @@ -132,12 +133,12 @@ public class DeploymentJobs { .map(JobStatus.JobRun::at); } - private static Optional<Long> requireId(Optional<Long> id, String message) { + private static OptionalLong requireId(OptionalLong id, String message) { Objects.requireNonNull(id, message); if ( ! id.isPresent()) { return id; } - if (id.get() <= 0) { + if (id.getAsLong() <= 0) { throw new IllegalArgumentException(message); } return id; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java index febaac8b18f..697ef7dbd4e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.component.Version; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import java.time.Instant; import java.util.Objects; 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 77cbaef3a0d..b8f49ab9ca7 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 @@ -31,11 +31,11 @@ import java.util.List; import java.util.NoSuchElementException; import java.util.Objects; import java.util.Optional; +import java.util.OptionalLong; import java.util.Set; import java.util.function.Supplier; import java.util.logging.Logger; import java.util.stream.Collectors; -import java.util.stream.Stream; import static java.util.Comparator.comparing; import static java.util.Comparator.naturalOrder; @@ -96,7 +96,7 @@ public class DeploymentTrigger { ApplicationVersion applicationVersion = report.sourceRevision().map(sr -> ApplicationVersion.from(sr, report.buildNumber())) .orElse(ApplicationVersion.unknown); application = application.withJobCompletion(report, applicationVersion, clock.instant(), controller); - application = application.withProjectId(Optional.of(report.projectId())); + application = application.withProjectId(OptionalLong.of(report.projectId())); if (report.jobType() == JobType.component && report.success()) { if (acceptNewApplicationVersion(application)) @@ -116,7 +116,8 @@ public class DeploymentTrigger { * Only one job is triggered each run for test jobs, since their environments have limited capacity. */ public long triggerReadyJobs() { - return computeReadyJobs().collect(partitioningBy(job -> job.jobType().isTest())) + return computeReadyJobs().stream() + .collect(partitioningBy(job -> job.jobType().isTest())) .entrySet().stream() .flatMap(entry -> (entry.getKey() // True for capacity constrained zones -- sort by priority and make a task for each job type. @@ -152,9 +153,9 @@ public class DeploymentTrigger { return true; } catch (RuntimeException e) { - if (e instanceof NoSuchElementException || e instanceof IllegalArgumentException) - applications().lockOrThrow(job.id, application -> applications().store(application.withProjectId(Optional.empty()))); log.log(LogLevel.WARNING, String.format("Exception triggering %s for %s (%s): %s", job.jobType, job.id, job.projectId, e)); + if (e instanceof NoSuchElementException || e instanceof IllegalArgumentException) + applications().lockOrThrow(job.id, application -> applications().store(application.withProjectId(OptionalLong.empty()))); return false; } } @@ -189,21 +190,22 @@ public class DeploymentTrigger { } /** Returns the set of all jobs which have changes to propagate from the upstream steps. */ - public Stream<Job> computeReadyJobs() { + public List<Job> computeReadyJobs() { return ApplicationList.from(applications().asList()) .notPullRequest() .withProjectId() .deploying() .idList().stream() .map(this::computeReadyJobs) - .flatMap(List::stream); + .flatMap(List::stream) + .collect(Collectors.toList()); } /** Returns whether the given job is currently running; false if completed since last triggered, asking the build service othewise. */ public boolean isRunning(Application application, JobType jobType) { return ! application.deploymentJobs().statusOf(jobType) .flatMap(job -> job.lastCompleted().map(run -> run.at().isAfter(job.lastTriggered().get().at()))).orElse(false) - && buildService.isRunning(new BuildService.BuildJob(application.deploymentJobs().projectId().get(), jobType.jobName())); + && buildService.isRunning(new BuildService.BuildJob(application.deploymentJobs().projectId().getAsLong(), jobType.jobName())); } public Job forcedDeploymentJob(Application application, JobType jobType, String reason) { @@ -357,7 +359,7 @@ public class DeploymentTrigger { private Job(Application application, JobType jobType, String reason, Instant availableSince, Collection<JobType> concurrentlyWith, boolean isRetry, Change change, Version platformVersion, ApplicationVersion applicationVersion) { this.id = application.id(); this.jobType = jobType; - this.projectId = application.deploymentJobs().projectId().get(); + this.projectId = application.deploymentJobs().projectId().getAsLong(); this.availableSince = availableSince; this.concurrentlyWith = concurrentlyWith; this.reason = reason; 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 269cfee3b9f..efe882b2c74 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 @@ -34,6 +34,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.OptionalLong; /** * Serializes applications to/from slime. @@ -340,12 +341,12 @@ public class ApplicationSerializer { private ApplicationVersion applicationVersionFromSlime(Inspector object) { if ( ! object.valid()) return ApplicationVersion.unknown; - Optional<Long> applicationBuildNumber = optionalLong(object.field(applicationBuildNumberField)); + OptionalLong applicationBuildNumber = optionalLong(object.field(applicationBuildNumberField)); Optional<SourceRevision> sourceRevision = sourceRevisionFromSlime(object.field(sourceRevisionField)); if (!sourceRevision.isPresent() || !applicationBuildNumber.isPresent()) { return ApplicationVersion.unknown; } - return ApplicationVersion.from(sourceRevision.get(), applicationBuildNumber.get()); + return ApplicationVersion.from(sourceRevision.get(), applicationBuildNumber.getAsLong()); } private Optional<SourceRevision> sourceRevisionFromSlime(Inspector object) { @@ -356,7 +357,7 @@ public class ApplicationSerializer { } private DeploymentJobs deploymentJobsFromSlime(Inspector object) { - Optional<Long> projectId = optionalLong(object.field(projectIdField)); + OptionalLong projectId = optionalLong(object.field(projectIdField)); List<JobStatus> jobStatusList = jobStatusListFromSlime(object.field(jobStatusField)); Optional<IssueId> issueId = optionalString(object.field(issueIdField)).map(IssueId::from); @@ -409,8 +410,8 @@ public class ApplicationSerializer { return field.valid() ? optionalString(field).map(RotationId::new) : Optional.empty(); } - private Optional<Long> optionalLong(Inspector field) { - return field.valid() ? Optional.of(field.asLong()) : Optional.empty(); + private OptionalLong optionalLong(Inspector field) { + return field.valid() ? OptionalLong.of(field.asLong()) : OptionalLong.empty(); } private Optional<String> optionalString(Inspector field) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 2e4861744ed..6d7ae0ccada 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -275,7 +275,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if ( ! application.deploymentJobs().projectId().isPresent()) continue; Cursor pipelineObject = pipelinesArray.addObject(); - pipelineObject.setString("screwdriverId", String.valueOf(application.deploymentJobs().projectId().get())); + pipelineObject.setString("screwdriverId", String.valueOf(application.deploymentJobs().projectId().getAsLong())); pipelineObject.setString("tenant", application.id().tenant().value()); pipelineObject.setString("application", application.id().application().value()); pipelineObject.setString("instance", application.id().instance().value()); @@ -469,8 +469,8 @@ public class ApplicationApiHandler extends LoggingRequestHandler { controller.zoneRegistry().getDeploymentTimeToLive(deploymentId.zoneId()) .ifPresent(deploymentTimeToLive -> response.setLong("expiryTimeEpochMs", deployment.at().plus(deploymentTimeToLive).toEpochMilli())); - controller.applications().get(deploymentId.applicationId()).flatMap(application -> application.deploymentJobs().projectId()) - .ifPresent(i -> response.setString("screwdriverId", String.valueOf(i))); + controller.applications().require(deploymentId.applicationId()).deploymentJobs().projectId() + .ifPresent(i -> response.setString("screwdriverId", String.valueOf(i))); sourceRevisionToSlime(deployment.applicationVersion().source(), response); // Cost diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java index edb7d40f304..0f7d5b0eab2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java @@ -70,7 +70,7 @@ public class ScrewdriverApiHandler extends LoggingRequestHandler { return vespaVersion(); } if (path.matches("/screwdriver/v1/jobsToRun")) - return buildJobs(controller.applications().deploymentTrigger().computeReadyJobs().collect(groupingBy(job -> job.jobType()))); + return buildJobs(controller.applications().deploymentTrigger().computeReadyJobs().stream().collect(groupingBy(job -> job.jobType()))); return notFound(request); } 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 79dd6156220..8e5f09346b1 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 @@ -436,42 +436,40 @@ public class ControllerTest { assertEquals(3, mockBuildService.jobs().size()); // Abort all running jobs, so we have three candidate jobs, of which only one should be triggered at a time. - synchronized (tester.buildService()) { - tester.buildService().clear(); - } + tester.buildService().clear(); tester.clock().advance(Duration.ofHours(13)); List<BuildService.BuildJob> jobs = new ArrayList<>(); assertEquals(jobs, tester.buildService().jobs()); tester.readyJobTrigger().maintain(); - jobs.add(new BuildService.BuildJob(app2.deploymentJobs().projectId().get(), stagingTest.jobName())); + jobs.add(new BuildService.BuildJob(app2.deploymentJobs().projectId().getAsLong(), stagingTest.jobName())); assertEquals(jobs, tester.buildService().jobs()); tester.readyJobTrigger().maintain(); - jobs.add(new BuildService.BuildJob(app1.deploymentJobs().projectId().get(), stagingTest.jobName())); + jobs.add(new BuildService.BuildJob(app1.deploymentJobs().projectId().getAsLong(), stagingTest.jobName())); assertEquals(jobs, tester.buildService().jobs()); tester.readyJobTrigger().maintain(); - jobs.add(new BuildService.BuildJob(app3.deploymentJobs().projectId().get(), stagingTest.jobName())); + jobs.add(new BuildService.BuildJob(app3.deploymentJobs().projectId().getAsLong(), stagingTest.jobName())); assertEquals(jobs, tester.buildService().jobs()); // Remove the jobs for app1 and app2, and then let app3 fail with outOfCapacity. // All three jobs are now eligible, but the one for app3 should trigger first as an outOfCapacity-retry. - tester.buildService().removeJob(app1.deploymentJobs().projectId().get(), stagingTest.jobName()); - tester.buildService().removeJob(app2.deploymentJobs().projectId().get(), stagingTest.jobName()); + tester.buildService().removeJob(app1.deploymentJobs().projectId().getAsLong(), stagingTest.jobName()); + tester.buildService().removeJob(app2.deploymentJobs().projectId().getAsLong(), stagingTest.jobName()); tester.clock().advance(Duration.ofHours(13)); - jobs.remove(new BuildService.BuildJob(app1.deploymentJobs().projectId().get(), stagingTest.jobName())); - jobs.remove(new BuildService.BuildJob(app2.deploymentJobs().projectId().get(), stagingTest.jobName())); + jobs.remove(new BuildService.BuildJob(app1.deploymentJobs().projectId().getAsLong(), stagingTest.jobName())); + jobs.remove(new BuildService.BuildJob(app2.deploymentJobs().projectId().getAsLong(), stagingTest.jobName())); tester.jobCompletion(stagingTest).application(app3).error(JobError.outOfCapacity).submit(); assertEquals(jobs, tester.buildService().jobs()); tester.readyJobTrigger().maintain(); - jobs.add(new BuildService.BuildJob(app2.deploymentJobs().projectId().get(), stagingTest.jobName())); + jobs.add(new BuildService.BuildJob(app2.deploymentJobs().projectId().getAsLong(), stagingTest.jobName())); assertEquals(jobs, tester.buildService().jobs()); tester.readyJobTrigger().maintain(); - jobs.add(new BuildService.BuildJob(app1.deploymentJobs().projectId().get(), stagingTest.jobName())); + jobs.add(new BuildService.BuildJob(app1.deploymentJobs().projectId().getAsLong(), stagingTest.jobName())); assertEquals(jobs, tester.buildService().jobs()); // Finish deployment for apps 2 and 3, then release a new version, leaving only app1 with an application upgrade. @@ -491,21 +489,21 @@ public class ControllerTest { // Let the last system test job start, then remove the ones for apps 1 and 2, and let app3 fail with outOfCapacity again. tester.readyJobTrigger().maintain(); - tester.buildService().removeJob(app1.deploymentJobs().projectId().get(), systemTest.jobName()); - tester.buildService().removeJob(app2.deploymentJobs().projectId().get(), systemTest.jobName()); + tester.buildService().removeJob(app1.deploymentJobs().projectId().getAsLong(), systemTest.jobName()); + tester.buildService().removeJob(app2.deploymentJobs().projectId().getAsLong(), systemTest.jobName()); tester.clock().advance(Duration.ofHours(13)); jobs.clear(); - jobs.add(new BuildService.BuildJob(app1.deploymentJobs().projectId().get(), stagingTest.jobName())); - jobs.add(new BuildService.BuildJob(app3.deploymentJobs().projectId().get(), systemTest.jobName())); + jobs.add(new BuildService.BuildJob(app1.deploymentJobs().projectId().getAsLong(), stagingTest.jobName())); + jobs.add(new BuildService.BuildJob(app3.deploymentJobs().projectId().getAsLong(), systemTest.jobName())); tester.jobCompletion(systemTest).application(app3).error(JobError.outOfCapacity).submit(); assertEquals(jobs, tester.buildService().jobs()); tester.readyJobTrigger().maintain(); - jobs.add(new BuildService.BuildJob(app1.deploymentJobs().projectId().get(), systemTest.jobName())); + jobs.add(new BuildService.BuildJob(app1.deploymentJobs().projectId().getAsLong(), systemTest.jobName())); assertEquals(jobs, tester.buildService().jobs()); tester.readyJobTrigger().maintain(); - jobs.add(new BuildService.BuildJob(app2.deploymentJobs().projectId().get(), systemTest.jobName())); + jobs.add(new BuildService.BuildJob(app2.deploymentJobs().projectId().getAsLong(), systemTest.jobName())); assertEquals(jobs, tester.buildService().jobs()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index 5abc6c4e14e..110455dac35 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -44,6 +44,7 @@ import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; import java.util.Optional; +import java.util.OptionalLong; import java.util.logging.Logger; import static org.junit.Assert.assertNotNull; @@ -217,7 +218,7 @@ public final class ControllerTester { ApplicationId applicationId = ApplicationId.from(tenant.value(), applicationName, instanceName); controller().applications().createApplication(applicationId, Optional.of(TestIdentities.userNToken)); controller().applications().lockOrThrow(applicationId, lockedApplication -> - controller().applications().store(lockedApplication.withProjectId(Optional.of(projectId)))); + controller().applications().store(lockedApplication.withProjectId(OptionalLong.of(projectId)))); return controller().applications().require(applicationId); } @@ -234,7 +235,7 @@ public final class ControllerTester { } public void deploy(Application application, ZoneId zone, Optional<ApplicationPackage> applicationPackage, boolean deployCurrentVersion) { - ScrewdriverId app1ScrewdriverId = new ScrewdriverId(String.valueOf(application.deploymentJobs().projectId().get())); + ScrewdriverId app1ScrewdriverId = new ScrewdriverId(String.valueOf(application.deploymentJobs().projectId().getAsLong())); GitRevision app1RevisionId = new GitRevision(new GitRepository("repo"), new GitBranch("master"), new GitCommit("commit1")); controller().applications().deployApplication(application.id(), zone, 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 index f1e6108710d..821eb237530 100644 --- 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 @@ -49,7 +49,7 @@ public class BuildJob { public BuildJob application(Application application) { this.applicationId = application.id(); if (application.deploymentJobs().projectId().isPresent()) { - this.projectId = application.deploymentJobs().projectId().get(); + this.projectId = application.deploymentJobs().projectId().getAsLong(); } return this; } 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 4caf4645233..f752cfd4f7a 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 @@ -279,7 +279,7 @@ public class DeploymentTester { } public void assertRunning(ApplicationId id, JobType jobType) { - assertRunning(application(id).deploymentJobs().projectId().get(), jobType); + assertRunning(application(id).deploymentJobs().projectId().getAsLong(), jobType); } public void assertRunning(long projectId, JobType jobType) { 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 097066dc847..28d6a516dbf 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 @@ -78,9 +78,7 @@ public class DeploymentTriggerTest { tester.deployAndNotify(app, applicationPackage, true, JobType.systemTest); // staging-test times out and is retried - synchronized (tester.buildService()) { - tester.buildService().clear(); - } + tester.buildService().clear(); tester.clock().advance(Duration.ofHours(12).plus(Duration.ofSeconds(1))); tester.readyJobTrigger().maintain(); assertEquals("Retried dead job", 1, tester.buildService().jobs().size()); @@ -326,7 +324,7 @@ public class DeploymentTriggerTest { tester.clock().advance(Duration.ofHours(2)); // ---------------- Exit block window: 20:30 tester.deploymentTrigger().triggerReadyJobs(); // Schedules the blocked production job(s) - assertEquals(singletonList(new BuildService.BuildJob(app.deploymentJobs().projectId().get(), "production-us-west-1")), + assertEquals(singletonList(new BuildService.BuildJob(app.deploymentJobs().projectId().getAsLong(), "production-us-west-1")), tester.buildService().jobs()); } 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 2d76d395804..0a330bfa4c5 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 @@ -85,7 +85,7 @@ public class FailureRedeployerTest { // Production job fails again, and is retried tester.deployAndNotify(app, applicationPackage, false, DeploymentJobs.JobType.productionUsEast3); tester.readyJobTrigger().maintain(); - assertEquals("Job is retried", Collections.singletonList(new BuildService.BuildJob(app.deploymentJobs().projectId().get(), productionUsEast3.jobName())), tester.buildService().jobs()); + assertEquals("Job is retried", Collections.singletonList(new BuildService.BuildJob(app.deploymentJobs().projectId().getAsLong(), productionUsEast3.jobName())), tester.buildService().jobs()); // Production job finally succeeds tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsEast3); @@ -125,10 +125,7 @@ public class FailureRedeployerTest { tester.updateVersionStatus(version); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); - boolean result; - synchronized (tester.buildService()) { - result = tester.buildService().removeJob((long) 1, systemTest.jobName()); - } + tester.buildService().removeJob((long) 1, systemTest.jobName()); tester.upgrader().maintain(); tester.readyJobTrigger().maintain(); assertEquals("Application has pending upgrade to " + version, version, tester.application(app.id()).change().platform().get()); 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 7de53d47fe4..424141137b0 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 @@ -123,14 +123,8 @@ public class UpgraderTest { tester.updateVersionStatus(version); assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber()); tester.upgrader().maintain(); - boolean result1; - synchronized (tester.buildService()) { - result1 = tester.buildService().removeJob(canary0.deploymentJobs().projectId().get(), stagingTest.jobName()); - } - boolean result; - synchronized (tester.buildService()) { - result = tester.buildService().removeJob(canary1.deploymentJobs().projectId().get(), systemTest.jobName()); - } + tester.buildService().removeJob(canary0.deploymentJobs().projectId().getAsLong(), stagingTest.jobName()); + tester.buildService().removeJob(canary1.deploymentJobs().projectId().getAsLong(), systemTest.jobName()); tester.readyJobTrigger().maintain(); tester.readyJobTrigger().maintain(); @@ -260,9 +254,7 @@ public class UpgraderTest { tester.jobCompletion(DeploymentJobs.JobType.productionUsWest1).application(default3).unsuccessful().submit(); tester.upgrader().maintain(); - synchronized (tester.buildService()) { - tester.buildService().clear(); - } + tester.buildService().clear(); tester.readyJobTrigger().maintain(); assertEquals("Upgrade of defaults are scheduled on 5.4 instead, since 5.5 broken: " + "This is default3 since it failed upgrade on both 5.4 and 5.5", @@ -347,9 +339,7 @@ public class UpgraderTest { // > 40% and at least 4 failed - version is broken tester.updateVersionStatus(version); tester.upgrader().maintain(); - synchronized (tester.buildService()) { - tester.buildService().clear(); - } + tester.buildService().clear(); tester.readyJobTrigger().maintain(); assertEquals(VespaVersion.Confidence.broken, tester.controller().versionStatus().systemVersion().get().confidence()); assertEquals("Upgrades are cancelled", 0, tester.buildService().jobs().size()); @@ -527,9 +517,7 @@ public class UpgraderTest { tester.deploymentTrigger().cancelChange(default1.id(), false); tester.deploymentTrigger().cancelChange(default2.id(), false); tester.deploymentTrigger().cancelChange(default3.id(), false); - synchronized (tester.buildService()) { - tester.buildService().clear(); - } + tester.buildService().clear(); tester.clock().advance(Duration.ofHours(13)); // Currently we don't cancel running jobs, so this is necessary to allow a new triggering below // Applications with default policy start upgrading to V2 @@ -553,9 +541,7 @@ public class UpgraderTest { assertEquals(v2, tester.application("default0").deployments().get(ZoneId.from("prod.us-west-1")).version()); assertEquals(v0, tester.application("default0").deployments().get(ZoneId.from("prod.us-east-3")).version()); tester.upgrader().maintain(); - synchronized (tester.buildService()) { - tester.buildService().clear(); - } + tester.buildService().clear(); tester.clock().advance(Duration.ofHours(13)); // TODO jvenstad: Reduce all these when build service is polled for status. tester.readyJobTrigger().maintain(); tester.readyJobTrigger().maintain(); 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 31f8901914c..9cb7e9f9636 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 @@ -36,6 +36,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.OptionalLong; import static com.yahoo.vespa.hosted.controller.ControllerTester.writable; import static org.junit.Assert.assertEquals; @@ -69,7 +70,7 @@ public class ApplicationSerializerTest { deployments.add(new Deployment(zone2, applicationVersion2, Version.fromString("1.2.3"), Instant.ofEpochMilli(5), createClusterUtils(3, 0.2), createClusterInfo(3, 4),new DeploymentMetrics(2,3,4,5,6))); - Optional<Long> projectId = Optional.of(123L); + OptionalLong projectId = OptionalLong.of(123L); List<JobStatus> statusList = new ArrayList<>(); statusList.add(JobStatus.initial(DeploymentJobs.JobType.systemTest) 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 42f291d5f96..a8e4c970cf7 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 @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.restapi.screwdriver; import com.yahoo.application.container.handler.Request; import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; @@ -11,6 +12,7 @@ import org.junit.Test; import java.io.File; import java.nio.charset.StandardCharsets; import java.util.Optional; +import java.util.OptionalLong; /** * @author bratseth @@ -39,7 +41,7 @@ public class ScrewdriverApiTest extends ControllerContainerTest { Application app = tester.createApplication(); tester.controller().applications().lockOrThrow(app.id(), application -> - tester.controller().applications().store(application.withProjectId(Optional.of(1L)))); + tester.controller().applications().store(application.withProjectId(OptionalLong.of(1L)))); // Unknown application assertResponse(new Request("http://localhost:8080/screwdriver/v1/trigger/tenant/foo/application/bar", |