diff options
Diffstat (limited to 'controller-server/src/main/java')
10 files changed, 88 insertions, 191 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 f26502e4630..d5ce613b98d 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 @@ -7,9 +7,9 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService.ApplicationMetrics; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; -import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationRotation; import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; @@ -149,16 +149,6 @@ public class Application { .min(Comparator.naturalOrder()); } - /** - * Returns the oldest application version this has deployed in a permanent zone (not test or staging), - * or empty version if it is not deployed anywhere - */ - public Optional<ApplicationVersion> oldestDeployedApplicationVersion() { - return productionDeployments().values().stream() - .map(Deployment::applicationVersion) - .min(Comparator.naturalOrder()); - } - /** Returns the version a new deployment to this zone should use for this application */ public Version deployVersionIn(ZoneId zone, Controller controller) { return change.platform().orElse(versionIn(zone, controller)); @@ -167,18 +157,19 @@ public class Application { /** Returns the current version this application has, or if none; should use, in the given zone */ public Version versionIn(ZoneId zone, Controller controller) { return Optional.ofNullable(deployments().get(zone)).map(Deployment::version) // Already deployed in this zone: Use that version - .orElse(oldestDeployedVersion().orElse(controller.systemVersion())); + .orElse(oldestDeployedVersion().orElse(controller.systemVersion())); } /** Returns the application version a deployment to this zone should use, or empty if we don't know */ public Optional<ApplicationVersion> deployApplicationVersionIn(ZoneId zone) { if (change().application().isPresent()) { ApplicationVersion version = change().application().get(); - if (version.isUnknown()) + if (version == ApplicationVersion.unknown) return Optional.empty(); else return Optional.of(version); } + return applicationVersionIn(zone); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 407f5b65e81..83c3d6a5d11 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -297,51 +297,41 @@ public class ApplicationController { version = application.deployVersionIn(zone, controller); } - // Determine application package to use - ApplicationVersion applicationVersion; - ApplicationPackage applicationPackage; - Optional<DeploymentJobs.JobType> job = DeploymentJobs.JobType.from(controller.system(), zone); + Optional<DeploymentJobs.JobType> jobType = DeploymentJobs.JobType.from(controller.system(), zone); + if (!jobType.isPresent() && !applicationPackageFromDeployer.isPresent()) { + throw new IllegalArgumentException("Unable to determine job type from zone '" + zone + + "' and no application package was given"); + } - // TODO: Simplify after new application version is always available - if (canDownloadReportedApplicationVersion(application) && !canDeployDirectlyTo(zone, options)) { - if (!job.isPresent()) { - throw new IllegalArgumentException("Cannot determine job for zone " + zone); - } - applicationVersion = application.deployApplicationVersion(job.get(), controller, - options.deployCurrentVersion) - .orElseThrow(() -> new IllegalArgumentException("Cannot determine application version for " + applicationId)); - if (canDownloadArtifact(applicationVersion)) { - applicationPackage = new ApplicationPackage( - artifactRepository.getApplicationPackage(applicationId, applicationVersion.id()) - ); - } else { - applicationPackage = applicationPackageFromDeployer.orElseThrow( - () -> new IllegalArgumentException("Application package with version " + - applicationVersion.id() + " cannot be downloaded, and " + - "no package was given by deployer")); - } - } else { // ..otherwise we use the package sent by the deployer and deduce version from the package - // TODO: Only allow this for environments that are allowed to deploy directly - applicationPackage = applicationPackageFromDeployer.orElseThrow( - () -> new IllegalArgumentException("Application package must be given as new application " + - "version is not known for " + applicationId) + // Determine which application package to use + ApplicationPackage applicationPackage; + ApplicationVersion applicationVersion; + if (applicationPackageFromDeployer.isPresent()) { + applicationVersion = toApplicationPackageRevision(applicationPackageFromDeployer.get(), + options.screwdriverBuildJob); + applicationPackage = applicationPackageFromDeployer.get(); + } else { + applicationVersion = application.deployApplicationVersion(jobType.get(), controller) + .orElseThrow(() -> new IllegalArgumentException("Cannot determine application version to use in " + zone)); + applicationPackage = new ApplicationPackage(artifactRepository.getApplicationPackage( + applicationId, applicationVersion.id()) ); - applicationVersion = toApplicationPackageRevision(applicationPackage, options.screwdriverBuildJob); } + validate(applicationPackage.deploymentSpec()); - // TODO: Remove after introducing new application version - if (!options.deployCurrentVersion && !canDownloadReportedApplicationVersion(application)) { + // TODO: Remove after introducing new application version number + if ( ! options.deployCurrentVersion && applicationPackageFromDeployer.isPresent()) { if (application.change().application().isPresent()) { - application = application.withChange(application.change().with(applicationVersion)); + application = application.withDeploying(application.change().with(applicationVersion)); } - if (!canDeployDirectlyTo(zone, options) && job.isPresent()) { + if (!canDeployDirectlyTo(zone, options) && jobType.isPresent()) { // Update with (potentially) missing information about what we triggered: // * When someone else triggered the job, we need to store a stand-in triggering event. // * When this is the system test job, we need to record the new application version, // for future use. - JobStatus.JobRun triggering = getOrCreateTriggering(application, version, job.get()); - application = application.withJobTriggering(job.get(), + JobStatus.JobRun triggering = getOrCreateTriggering(application, version, jobType.get()); + application = application.withJobTriggering(jobType.get(), application.change(), triggering.at(), version, @@ -426,8 +416,7 @@ public class ApplicationController { Log logEntry = new Log(); logEntry.level = "WARNING"; logEntry.time = clock.instant().toEpochMilli(); - logEntry.message = "Ignoring deployment of " + require(applicationId) + " to " + zone + - " as a deployment is not currently expected"; + logEntry.message = "Ignoring deployment of " + get(applicationId) + " to " + zone + " as a deployment is not currently expected"; PrepareResponse prepareResponse = new PrepareResponse(); prepareResponse.log = Collections.singletonList(logEntry); prepareResponse.configChangeActions = new ConfigChangeActions(Collections.emptyList(), Collections.emptyList()); @@ -709,20 +698,6 @@ public class ApplicationController { zone.environment().isManuallyDeployed(); } - /** Returns whether artifact for given version number is available in artifact repository */ - private static boolean canDownloadArtifact(ApplicationVersion applicationVersion) { - return applicationVersion.buildNumber().isPresent() && applicationVersion.source().isPresent(); - } - - /** Returns whether component has reported a version number that is availabe in artifact repository */ - private static boolean canDownloadReportedApplicationVersion(Application application) { - return Optional.ofNullable(application.deploymentJobs().jobStatus().get(DeploymentJobs.JobType.component)) - .flatMap(JobStatus::lastSuccess) - .map(JobStatus.JobRun::applicationVersion) - .filter(ApplicationController::canDownloadArtifact) - .isPresent(); - } - /** Verify that each of the production zones listed in the deployment spec exist in this system. */ private void validate(DeploymentSpec deploymentSpec) { deploymentSpec.zones().stream() 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 4b6213733a0..e744df0da68 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 @@ -60,11 +60,8 @@ public class LockedApplication extends Application { return new LockedApplication(new Builder(this).with(deploymentJobs().with(issueId))); } - public LockedApplication withJobCompletion(DeploymentJobs.JobReport report, ApplicationVersion applicationVersion, - Instant notificationTime, Controller controller) { - return new LockedApplication(new Builder(this).with(deploymentJobs().withCompletion( - report, applicationVersion, notificationTime, controller)) - ); + public LockedApplication withJobCompletion(DeploymentJobs.JobReport report, Instant notificationTime, Controller controller) { + return new LockedApplication(new Builder(this).with(deploymentJobs().withCompletion(report, notificationTime, controller))); } public LockedApplication withJobTriggering(JobType type, Change change, Instant triggerTime, @@ -122,8 +119,8 @@ public class LockedApplication extends Application { return new LockedApplication(new Builder(this).with(validationOverrides)); } - public LockedApplication withChange(Change change) { - return new LockedApplication(new Builder(this).withChange(change)); + public LockedApplication withDeploying(Change deploying) { + return new LockedApplication(new Builder(this).withDeploying(deploying)); } public LockedApplication withOutstandingChange(boolean outstandingChange) { @@ -149,17 +146,6 @@ public class LockedApplication extends Application { } public Optional<ApplicationVersion> deployApplicationVersion(DeploymentJobs.JobType jobType, Controller controller) { - return deployApplicationVersion(jobType, controller, false); - } - - public Optional<ApplicationVersion> deployApplicationVersion(DeploymentJobs.JobType jobType, Controller controller, - boolean currentVersion) { - if (currentVersion) { - Optional<ApplicationVersion> version = oldestDeployedApplicationVersion(); - if (version.isPresent()) { - return version; - } - } return jobType == JobType.component ? Optional.empty() : deployApplicationVersionIn(jobType.zone(controller.system()).get()); @@ -219,7 +205,7 @@ public class LockedApplication extends Application { return this; } - private Builder withChange(Change deploying) { + private Builder withDeploying(Change deploying) { this.deploying = deploying; return this; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationVersion.java index 31ee5fa6d44..304d82b2bec 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationVersion.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationVersion.java @@ -11,19 +11,18 @@ import java.util.Optional; * @author bratseth * @author mpolden */ -public class ApplicationVersion implements Comparable<ApplicationVersion> { +public class ApplicationVersion { - /** - * Used in cases where application version cannot be determined, such as manual deployments (e.g. in dev - * environment) - */ + // TODO: Remove the need for this public static final ApplicationVersion unknown = new ApplicationVersion(); - // This never changes and is only used to create a valid semantic version number, as required by application bundles + // Never changes. Only used to create a valid version number for the bundle private static final String majorVersion = "1.0"; - // TODO: Remove after 2018-03-01 + // TODO: Remove after introducing new application version private final Optional<String> applicationPackageHash; + + // TODO: Make mandatory private final Optional<SourceRevision> source; private final Optional<Long> buildNumber; @@ -38,11 +37,11 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { Objects.requireNonNull(applicationPackageHash, "applicationPackageHash cannot be null"); Objects.requireNonNull(source, "source cannot be null"); Objects.requireNonNull(buildNumber, "buildNumber cannot be null"); - if (!applicationPackageHash.isPresent() && source.isPresent() != buildNumber.isPresent()) { - throw new IllegalArgumentException("both buildNumber and source must be set together"); + if (buildNumber.isPresent() && !source.isPresent()) { + throw new IllegalArgumentException("both buildNumber and source must be set if buildNumber is set"); } - if (buildNumber.isPresent() && buildNumber.get() <= 0) { - throw new IllegalArgumentException("buildNumber must be > 0"); + if ( ! buildNumber.isPresent() && ! applicationPackageHash.isPresent()) { + throw new IllegalArgumentException("applicationPackageHash must be given if buildNumber is unset"); } this.applicationPackageHash = applicationPackageHash; this.source = source; @@ -69,14 +68,9 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { if (applicationPackageHash.isPresent()) { return applicationPackageHash.get(); } - return source.map(sourceRevision -> String.format("%s.%d-%s", majorVersion, buildNumber.get(), - abbreviateCommit(source.get().commit()))) - .orElse("unknown"); + return String.format("%s.%d-%s", majorVersion, buildNumber.get(), abbreviateCommit(source.get().commit())); } - /** Returns the application package hash, if known */ - public Optional<String> applicationPackageHash() { return applicationPackageHash; } - /** * Returns information about the source of this revision, or empty if the source is not know/defined * (which is the case for command-line deployment from developers, but never for deployment jobs) @@ -86,29 +80,23 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { /** Returns the build number that built this version */ public Optional<Long> buildNumber() { return buildNumber; } - /** Returns whether this is unknown */ - public boolean isUnknown() { - return this.equals(unknown); - } - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ApplicationVersion that = (ApplicationVersion) o; - return Objects.equals(applicationPackageHash, that.applicationPackageHash) && - Objects.equals(source, that.source) && - Objects.equals(buildNumber, that.buildNumber); - } + public int hashCode() { return applicationPackageHash.hashCode(); } @Override - public int hashCode() { - return Objects.hash(applicationPackageHash, source, buildNumber); + public boolean equals(Object other) { + if (this == other) return true; + if ( ! (other instanceof ApplicationVersion)) return false; + return this.applicationPackageHash.equals(((ApplicationVersion)other).applicationPackageHash); } @Override public String toString() { - return "Application package version: " + id() + source.map(s -> ", " + s.toString()).orElse(""); + if (buildNumber.isPresent()) { + return "Application package version: " + abbreviateCommit(source.get().commit()) + "-" + buildNumber.get(); + } + return "Application package revision '" + applicationPackageHash + "'" + + (source.isPresent() ? " with " + source.get() : ""); } /** Abbreviate given commit hash to 9 characters */ @@ -116,11 +104,4 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { return hash.length() <= 9 ? hash : hash.substring(0, 9); } - @Override - public int compareTo(ApplicationVersion o) { - if (!buildNumber().isPresent() || !o.buildNumber().isPresent()) { - return 0; // No sorting for application package hash - } - return buildNumber().get().compareTo(o.buildNumber().get()); - } } 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 932777e690d..bb7b39eed0f 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 @@ -55,13 +55,11 @@ public class DeploymentJobs { } /** Return a new instance with the given completion */ - public DeploymentJobs withCompletion(JobReport report, ApplicationVersion applicationVersion, - Instant notificationTime, Controller controller) { + public DeploymentJobs withCompletion(JobReport report, Instant notificationTime, Controller controller) { Map<JobType, JobStatus> status = new LinkedHashMap<>(this.status); status.compute(report.jobType(), (type, job) -> { if (job == null) job = JobStatus.initial(report.jobType()); - return job.withCompletion(report.buildNumber(), applicationVersion, report.jobError(), notificationTime, - controller); + return job.withCompletion(report.buildNumber(), report.jobError(), notificationTime, controller); }); return new DeploymentJobs(Optional.of(report.projectId()), status, issueId); } @@ -256,16 +254,11 @@ public class DeploymentJobs { Objects.requireNonNull(sourceRevision, "sourceRevision cannot be null"); 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); - } - this.applicationId = applicationId; this.projectId = projectId; this.buildNumber = buildNumber; this.jobType = jobType; - this.sourceRevision = sourceRevision; + this.sourceRevision = sourceRevision; // TODO: Require non-empty source revision if jobType == component this.jobError = jobError; } 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..41060a7af4c 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,7 @@ 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.lastSuccess().get().applicationVersion() == ApplicationVersion.unknown) 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/application/JobStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java index e963fde8f94..e165d3c9fe5 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 @@ -57,31 +57,27 @@ public class JobStatus { 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)), + return new JobStatus(type, jobError, Optional.of(new JobRun(-1, version, applicationVersion, upgrade, reason, triggerTime)), lastCompleted, firstFailing, lastSuccess); } - public JobStatus withCompletion(long runId, Optional<DeploymentJobs.JobError> jobError, Instant completionTime, - Controller controller) { - return withCompletion(runId, ApplicationVersion.unknown, jobError, completionTime, controller); - } - - public JobStatus withCompletion(long runId, ApplicationVersion applicationVersion, - Optional<DeploymentJobs.JobError> jobError, Instant completionTime, - Controller controller) { + public JobStatus withCompletion(long runId, Optional<DeploymentJobs.JobError> jobError, Instant completionTime, Controller controller) { Version version; + ApplicationVersion applicationVersion; boolean upgrade; String reason; if (type == DeploymentJobs.JobType.component) { // not triggered by us version = controller.systemVersion(); + applicationVersion = ApplicationVersion.unknown; upgrade = false; reason = "Application commit"; - } else if (! lastTriggered.isPresent()) { + } + else if (! lastTriggered.isPresent()) { throw new IllegalStateException("Got notified about completion of " + this + ", but that has neither been triggered nor deployed"); - } else { + } + else { version = lastTriggered.get().version(); applicationVersion = lastTriggered.get().applicationVersion(); upgrade = lastTriggered.get().upgrade(); @@ -201,7 +197,7 @@ public class JobStatus { /** Returns the Vespa version used on this run */ public Version version() { return version; } - /** Returns the application version used in this run */ + /** Returns the application version used for this run, or empty when not known */ public ApplicationVersion applicationVersion() { return applicationVersion; } /** Returns a human-readable reason for this particular job run */ 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 6560423f3b9..1beab1307c1 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 @@ -78,8 +78,7 @@ public class DeploymentTrigger { */ public void triggerFromCompletion(JobReport report) { applications().lockOrThrow(report.applicationId(), application -> { - application = application.withJobCompletion(report, applicationVersionFrom(report), clock.instant(), - controller); + application = application.withJobCompletion(report, clock.instant(), controller); application = application.withProjectId(report.projectId()); // Handle successful starting and ending @@ -90,9 +89,8 @@ public class DeploymentTrigger { if ( ! ( application.change().platform().isPresent())) { ApplicationVersion applicationVersion = ApplicationVersion.unknown; if (report.sourceRevision().isPresent()) - applicationVersion = ApplicationVersion.from(report.sourceRevision().get(), - report.buildNumber()); - application = application.withChange(Change.of(applicationVersion)); + applicationVersion = ApplicationVersion.from(report.sourceRevision().get(), report.buildNumber()); + application = application.withDeploying(Change.of(applicationVersion)); } } else { // postpone @@ -102,7 +100,7 @@ public class DeploymentTrigger { } else if (deploymentComplete(application)) { // change completed - application = application.withChange(Change.empty()); + application = application.withDeploying(Change.empty()); } } @@ -141,7 +139,7 @@ public class DeploymentTrigger { } 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() == ApplicationVersion.unknown) return false; if ( ! change.application().get().equals(deployment.applicationVersion())) return false; } } @@ -262,7 +260,7 @@ public class DeploymentTrigger { if (application.change().isPresent() && ! application.deploymentJobs().hasFailures()) throw new IllegalArgumentException("Could not start " + change + " on " + application + ": " + application.change() + " is already in progress"); - application = application.withChange(change); + application = application.withDeploying(change); if (change.application().isPresent()) application = application.withOutstandingChange(false); application = trigger(JobType.systemTest, application, false, change.toString()); @@ -278,7 +276,7 @@ public class DeploymentTrigger { public void cancelChange(ApplicationId applicationId) { applications().lockOrThrow(applicationId, application -> { buildSystem.removeJobs(application.id()); - applications().store(application.withChange(Change.empty())); + applications().store(application.withDeploying(Change.empty())); }); } @@ -357,17 +355,10 @@ public class DeploymentTrigger { application.change(), clock.instant(), application.deployVersionFor(jobType, controller), - application.deployApplicationVersion(jobType, controller) - .orElse(ApplicationVersion.unknown), + application.deployApplicationVersion(jobType, controller).orElse(ApplicationVersion.unknown), reason); } - /** Create application version from job report */ - private ApplicationVersion applicationVersionFrom(JobReport report) { - return report.sourceRevision().map(sr -> ApplicationVersion.from(sr, report.buildNumber())) - .orElse(ApplicationVersion.unknown); - } - /** Returns true if the given proposed job triggering should be effected */ private boolean allowedTriggering(JobType jobType, LockedApplication application) { // Note: We could make a more fine-grained and more correct determination about whether to block 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 4b7993b3b7d..652f95a2d13 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 @@ -6,6 +6,7 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; @@ -14,7 +15,6 @@ import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService.ApplicationMetrics; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; -import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.ClusterInfo; @@ -61,7 +61,6 @@ public class ApplicationSerializer { private final String environmentField = "environment"; private final String regionField = "region"; 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"; @@ -199,15 +198,9 @@ public class ApplicationSerializer { } private void toSlime(ApplicationVersion applicationVersion, Cursor object) { - if (applicationVersion.buildNumber().isPresent() && applicationVersion.source().isPresent()) { - object.setLong(applicationBuildNumberField, applicationVersion.buildNumber().get()); + object.setString(applicationPackageHashField, applicationVersion.id()); + if (applicationVersion.source().isPresent()) 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)); - } - } } private void toSlime(SourceRevision sourceRevision, Cursor object) { @@ -243,7 +236,7 @@ public class ApplicationSerializer { Cursor object = parent.setObject(jobRunObjectName); object.setLong(jobRunIdField, jobRun.get().id()); object.setString(versionField, jobRun.get().version().toString()); - if ( ! jobRun.get().applicationVersion().isUnknown()) + if ( jobRun.get().applicationVersion() != ApplicationVersion.unknown) toSlime(jobRun.get().applicationVersion(), object.setObject(revisionField)); object.setBool(upgradeField, jobRun.get().upgrade()); object.setString(reasonField, jobRun.get().reason()); @@ -256,7 +249,7 @@ public class ApplicationSerializer { Cursor object = parentObject.setObject(deployingField); if (deploying.platform().isPresent()) object.setString(versionField, deploying.platform().get().toString()); - if (deploying.application().isPresent() && !deploying.application().get().isUnknown()) + if (deploying.application().isPresent() && deploying.application().get() != ApplicationVersion.unknown) toSlime(deploying.application().get(), object); } @@ -349,17 +342,10 @@ 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)); + String applicationPackageHash = object.field(applicationPackageHashField).asString(); 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; - } - return ApplicationVersion.from(sourceRevision.get(), applicationBuildNumber.get()); + return sourceRevision.isPresent() ? ApplicationVersion.from(applicationPackageHash, sourceRevision.get()) + : ApplicationVersion.from(applicationPackageHash); } private Optional<SourceRevision> sourceRevisionFromSlime(Inspector object) { @@ -383,8 +369,7 @@ 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(applicationPackageHashField).valid()) change = change.with(applicationVersionFromSlime(object)); if ( ! change.isPresent()) // A deploy object with no fields -> unknown application change change = Change.of(ApplicationVersion.unknown); 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 77e0fb1ef05..9b2174b881d 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 @@ -478,10 +478,9 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private void toSlime(ApplicationVersion applicationVersion, Cursor object) { - if (!applicationVersion.isUnknown()) { - object.setString("hash", applicationVersion.id()); + object.setString("hash", applicationVersion.id()); + if (applicationVersion.source().isPresent()) sourceRevisionToSlime(applicationVersion.source(), object.setObject("source")); - } } private void sourceRevisionToSlime(Optional<SourceRevision> revision, Cursor object) { @@ -971,7 +970,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private void toSlime(JobStatus.JobRun jobRun, Cursor object) { object.setLong("id", jobRun.id()); object.setString("version", jobRun.version().toFullString()); - if (!jobRun.applicationVersion().isUnknown()) + if (jobRun.applicationVersion() != ApplicationVersion.unknown) toSlime(jobRun.applicationVersion(), object.setObject("revision")); object.setString("reason", jobRun.reason()); object.setLong("at", jobRun.at().toEpochMilli()); |