diff options
author | Martin Polden <mpolden@mpolden.no> | 2018-02-22 14:02:37 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-02-22 14:02:37 +0100 |
commit | 89ee93f4a1e98b199e68d7d0c6533116073f5ea6 (patch) | |
tree | 214c730c837928c25380000ede8983337ab2a9e3 /controller-server/src | |
parent | 3dcd634419265771a6989e53d13119761651e89d (diff) | |
parent | 9075606ec19d1d8f5f5d159a4a59b9dd2f02da31 (diff) |
Merge pull request #5113 from vespa-engine/mpolden/remove-unused-upgrade-field
Remove unused upgrade field
Diffstat (limited to 'controller-server/src')
6 files changed, 21 insertions, 47 deletions
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 d317fed0b73..bca3fbdc960 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 @@ -77,7 +77,6 @@ public class DeploymentJobs { if (job == null) job = JobStatus.initial(jobType); return job.withTriggering(version, applicationVersion, - change.platform().isPresent(), reason, triggerTime); }); 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 63c9fc5ebb9..c19c8a31ec1 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 @@ -65,7 +65,7 @@ public class JobList { return new JobList(list, ! negate); } - /** Returns the subset of jobs which are current upgrading */ + /** Returns the subset of jobs which are currently upgrading */ public JobList upgrading() { // TODO: Centralise and standardise reasoning about upgrades and application versions. return filter(job -> job.lastSuccess().isPresent() && job.lastTriggered().isPresent() @@ -155,10 +155,6 @@ public class JobList { return filter(run -> run.version().equals(version)); } - public JobList upgrade() { - return filter(JobRun::upgrade); - } - /** Transforms the JobRun condition to a JobStatus condition, by considering only the JobRun mapped by which, and executes */ private JobList filter(Predicate<JobRun> condition) { return JobList.this.filter(job -> which.apply(job).filter(condition).isPresent()); @@ -166,7 +162,6 @@ public class JobList { } - // ----------------------------------- Internal helpers private static boolean failingApplicationChange(JobStatus job) { 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 e963fde8f94..10fc1b41733 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 @@ -55,10 +55,10 @@ public class JobStatus { return new JobStatus(type, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); } - public JobStatus withTriggering(Version version, ApplicationVersion applicationVersion, - boolean upgrade, String reason, Instant triggerTime) { - return new JobStatus(type, jobError, Optional.of(new JobRun(-1, version, applicationVersion, upgrade, - reason, triggerTime)), + public JobStatus withTriggering(Version version, ApplicationVersion applicationVersion, String reason, + Instant triggerTime) { + return new JobStatus(type, jobError, Optional.of(new JobRun(-1, version, applicationVersion, reason, + triggerTime)), lastCompleted, firstFailing, lastSuccess); } @@ -71,11 +71,9 @@ public class JobStatus { Optional<DeploymentJobs.JobError> jobError, Instant completionTime, Controller controller) { Version version; - boolean upgrade; String reason; if (type == DeploymentJobs.JobType.component) { // not triggered by us version = controller.systemVersion(); - upgrade = false; reason = "Application commit"; } else if (! lastTriggered.isPresent()) { throw new IllegalStateException("Got notified about completion of " + this + @@ -84,11 +82,10 @@ public class JobStatus { } else { version = lastTriggered.get().version(); applicationVersion = lastTriggered.get().applicationVersion(); - upgrade = lastTriggered.get().upgrade(); reason = lastTriggered.get().reason(); } - JobRun thisCompletion = new JobRun(runId, version, applicationVersion, upgrade, reason, completionTime); + JobRun thisCompletion = new JobRun(runId, version, applicationVersion, reason, completionTime); Optional<JobRun> firstFailing = this.firstFailing; if (jobError.isPresent() && ! this.firstFailing.isPresent()) @@ -172,12 +169,10 @@ public class JobStatus { private final long id; private final Version version; private final ApplicationVersion applicationVersion; - private final boolean upgrade; private final String reason; private final Instant at; - public JobRun(long id, Version version, ApplicationVersion applicationVersion, - boolean upgrade, String reason, Instant at) { + public JobRun(long id, Version version, ApplicationVersion applicationVersion,String reason, Instant at) { Objects.requireNonNull(version, "version cannot be null"); Objects.requireNonNull(applicationVersion, "applicationVersion cannot be null"); Objects.requireNonNull(reason, "Reason cannot be null"); @@ -185,7 +180,6 @@ public class JobStatus { this.id = id; this.version = version; this.applicationVersion = applicationVersion; - this.upgrade = upgrade; this.reason = reason; this.at = at; } @@ -194,10 +188,6 @@ public class JobStatus { /** Returns the id of this run of this job, or -1 if not known */ public long id() { return id; } - // TODO: Fix how this is set, and add an applicationChange() method as well, in the same vein. - /** Returns whether this job run was a Vespa upgrade */ - public boolean upgrade() { return upgrade; } - /** Returns the Vespa version used on this run */ public Version version() { return version; } @@ -220,7 +210,7 @@ public class JobStatus { @Override public int hashCode() { - return Objects.hash(version, applicationVersion, upgrade, at); + return Objects.hash(version, applicationVersion, at); } @Override @@ -231,12 +221,11 @@ public class JobStatus { return id == jobRun.id && Objects.equals(version, jobRun.version) && Objects.equals(applicationVersion, jobRun.applicationVersion) && - upgrade == jobRun.upgrade && Objects.equals(at, jobRun.at); } @Override - public String toString() { return "job run " + id + " of version " + (upgrade() ? "upgrade " : "") + version + " " + public String toString() { return "job run " + id + " of version " + version + " " + applicationVersion + " at " + at; } } 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 107685be994..269cfee3b9f 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 @@ -85,7 +85,6 @@ public class ApplicationSerializer { private final String jobRunIdField = "id"; private final String versionField = "version"; private final String revisionField = "revision"; - private final String upgradeField = "upgrade"; private final String reasonField = "reason"; private final String atField = "at"; @@ -238,7 +237,6 @@ public class ApplicationSerializer { object.setLong(jobRunIdField, jobRun.get().id()); object.setString(versionField, jobRun.get().version().toString()); toSlime(jobRun.get().applicationVersion(), object.setObject(revisionField)); - object.setBool(upgradeField, jobRun.get().upgrade()); object.setString(reasonField, jobRun.get().reason()); object.setLong(atField, jobRun.get().at().toEpochMilli()); } @@ -403,7 +401,6 @@ public class ApplicationSerializer { return Optional.of(new JobStatus.JobRun(optionalLong(object.field(jobRunIdField)).orElse(-1L), // TODO: Make non-optional after November 2017 -- what about lastTriggered? new Version(object.field(versionField).asString()), applicationVersionFromSlime(object.field(revisionField)), - object.field(upgradeField).asBool(), optionalString(object.field(reasonField)).orElse(""), // TODO: Make non-optional after November 2017 Instant.ofEpochMilli(object.field(atField).asLong()))); } 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 243dfdff4df..25ea29f92cc 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 @@ -115,12 +115,12 @@ public class ControllerTest { ApplicationVersion applicationVersion = tester.controller().applications().require(app1.id()).change().application().get(); assertTrue("Application version has been set during deployment", applicationVersion != ApplicationVersion.unknown); assertStatus(JobStatus.initial(stagingTest) - .withTriggering(version1, applicationVersion, false, "", tester.clock().instant().minus(Duration.ofMillis(1))) + .withTriggering(version1, applicationVersion, "", tester.clock().instant().minus(Duration.ofMillis(1))) .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller()); // Causes first deployment job to be triggered assertStatus(JobStatus.initial(productionCorpUsEast1) - .withTriggering(version1, applicationVersion, false, "", tester.clock().instant()), app1.id(), tester.controller()); + .withTriggering(version1, applicationVersion, "", tester.clock().instant()), app1.id(), tester.controller()); tester.clock().advance(Duration.ofSeconds(1)); // production job (failing) @@ -128,9 +128,9 @@ public class ControllerTest { assertEquals(4, applications.require(app1.id()).deploymentJobs().jobStatus().size()); JobStatus expectedJobStatus = JobStatus.initial(productionCorpUsEast1) - .withTriggering(version1, applicationVersion, false, "", tester.clock().instant()) // Triggered first without application version info + .withTriggering(version1, applicationVersion, "", tester.clock().instant()) // Triggered first without application version info .withCompletion(42, Optional.of(JobError.unknown), tester.clock().instant(), tester.controller()) - .withTriggering(version1, applicationVersion, false, "", tester.clock().instant()); // Re-triggering (due to failure) has application version info + .withTriggering(version1, applicationVersion, "", tester.clock().instant()); // Re-triggering (due to failure) has application version info assertStatus(expectedJobStatus, app1.id(), tester.controller()); @@ -154,20 +154,20 @@ public class ControllerTest { 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))) + .withTriggering(version1, applicationVersion, "", tester.clock().instant().minus(Duration.ofMillis(1))) .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller()); tester.deployAndNotify(app1, applicationPackage, true, stagingTest); // production job succeeding now tester.deployAndNotify(app1, applicationPackage, true, productionCorpUsEast1); expectedJobStatus = expectedJobStatus - .withTriggering(version1, applicationVersion, false, "", tester.clock().instant().minus(Duration.ofMillis(1))) + .withTriggering(version1, applicationVersion, "", tester.clock().instant().minus(Duration.ofMillis(1))) .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()); assertStatus(expectedJobStatus, app1.id(), tester.controller()); // causes triggering of next production job assertStatus(JobStatus.initial(productionUsEast3) - .withTriggering(version1, applicationVersion, false, "", tester.clock().instant()), + .withTriggering(version1, applicationVersion, "", tester.clock().instant()), app1.id(), tester.controller()); tester.deployAndNotify(app1, applicationPackage, true, productionUsEast3); @@ -854,7 +854,7 @@ public class ControllerTest { tester.deployAndNotify(app, applicationPackage, true, true, systemTest); tester.deployAndNotify(app, applicationPackage, true, true, stagingTest); assertStatus(JobStatus.initial(stagingTest) - .withTriggering(vespaVersion, version, upgrade.isPresent(), "", + .withTriggering(vespaVersion, version, "", tester.clock().instant().minus(Duration.ofMillis(1))) .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()), app.id(), tester.controller()); @@ -862,13 +862,13 @@ public class ControllerTest { // Deploy in production tester.deployAndNotify(app, applicationPackage, true, true, productionCorpUsEast1); assertStatus(JobStatus.initial(productionCorpUsEast1) - .withTriggering(vespaVersion, version, upgrade.isPresent(), "", + .withTriggering(vespaVersion, version, "", tester.clock().instant().minus(Duration.ofMillis(1))) .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()), app.id(), tester.controller()); tester.deployAndNotify(app, applicationPackage, true, true, productionUsEast3); assertStatus(JobStatus.initial(productionUsEast3) - .withTriggering(vespaVersion, version, upgrade.isPresent(), "", + .withTriggering(vespaVersion, version, "", tester.clock().instant().minus(Duration.ofMillis(1))) .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()), app.id(), tester.controller()); 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 0ad01f57579..31f8901914c 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 @@ -73,10 +73,10 @@ public class ApplicationSerializerTest { List<JobStatus> statusList = new ArrayList<>(); statusList.add(JobStatus.initial(DeploymentJobs.JobType.systemTest) - .withTriggering(Version.fromString("5.6.7"), ApplicationVersion.unknown, true, "Test", Instant.ofEpochMilli(7)) + .withTriggering(Version.fromString("5.6.7"), ApplicationVersion.unknown, "Test", Instant.ofEpochMilli(7)) .withCompletion(30, Optional.empty(), Instant.ofEpochMilli(8), tester.controller())); statusList.add(JobStatus.initial(DeploymentJobs.JobType.stagingTest) - .withTriggering(Version.fromString("5.6.6"), ApplicationVersion.unknown, true, "Test 2", Instant.ofEpochMilli(5)) + .withTriggering(Version.fromString("5.6.6"), ApplicationVersion.unknown, "Test 2", Instant.ofEpochMilli(5)) .withCompletion(11, Optional.of(JobError.unknown), Instant.ofEpochMilli(6), tester.controller())); DeploymentJobs deploymentJobs = new DeploymentJobs(projectId, statusList, Optional.empty()); @@ -213,12 +213,6 @@ public class ApplicationSerializerTest { } @Test - public void testLegacySerializationWithoutUpgradeField() { - Application application = applicationSerializer.fromSlime(applicationSlime(false)); - assertFalse(application.deploymentJobs().jobStatus().get(DeploymentJobs.JobType.systemTest).lastCompleted().get().upgrade()); - } - - @Test public void testCompleteApplicationDeserialization() throws Exception { byte[] applicationJson = Files.readAllBytes(testData.resolve("complete-application.json")); applicationSerializer.fromSlime(SlimeUtils.jsonToSlime(applicationJson)); |