diff options
author | Martin Polden <mpolden@mpolden.no> | 2018-01-30 14:57:05 +0100 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2018-02-01 12:17:59 +0100 |
commit | 4be250ecd1fca4293130bfe0286570770d2fb3da (patch) | |
tree | 764884a42d9a89216ec5d51d161a576524de4ba0 | |
parent | 8d66a1532b4dd34b3fb5891dac31234999c75c10 (diff) |
Migrate to new application version
15 files changed, 336 insertions, 222 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 d5ce613b98d..f26502e4630 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,6 +149,16 @@ 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)); @@ -157,19 +167,18 @@ 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 == ApplicationVersion.unknown) + if (version.isUnknown()) 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 83c3d6a5d11..407f5b65e81 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,41 +297,51 @@ public class ApplicationController { version = application.deployVersionIn(zone, controller); } - 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"); - } - - // Determine which application package to use - ApplicationPackage applicationPackage; + // Determine application package to use 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()) + ApplicationPackage applicationPackage; + Optional<DeploymentJobs.JobType> job = DeploymentJobs.JobType.from(controller.system(), zone); + + // 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) ); + applicationVersion = toApplicationPackageRevision(applicationPackage, options.screwdriverBuildJob); } - validate(applicationPackage.deploymentSpec()); - // TODO: Remove after introducing new application version number - if ( ! options.deployCurrentVersion && applicationPackageFromDeployer.isPresent()) { + // TODO: Remove after introducing new application version + if (!options.deployCurrentVersion && !canDownloadReportedApplicationVersion(application)) { if (application.change().application().isPresent()) { - application = application.withDeploying(application.change().with(applicationVersion)); + application = application.withChange(application.change().with(applicationVersion)); } - if (!canDeployDirectlyTo(zone, options) && jobType.isPresent()) { + if (!canDeployDirectlyTo(zone, options) && job.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, jobType.get()); - application = application.withJobTriggering(jobType.get(), + JobStatus.JobRun triggering = getOrCreateTriggering(application, version, job.get()); + application = application.withJobTriggering(job.get(), application.change(), triggering.at(), version, @@ -416,7 +426,8 @@ public class ApplicationController { Log logEntry = new Log(); logEntry.level = "WARNING"; logEntry.time = clock.instant().toEpochMilli(); - logEntry.message = "Ignoring deployment of " + get(applicationId) + " to " + zone + " as a deployment is not currently expected"; + logEntry.message = "Ignoring deployment of " + require(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()); @@ -698,6 +709,20 @@ 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 e744df0da68..4b6213733a0 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,8 +60,11 @@ public class LockedApplication extends Application { return new LockedApplication(new Builder(this).with(deploymentJobs().with(issueId))); } - public LockedApplication withJobCompletion(DeploymentJobs.JobReport report, Instant notificationTime, Controller controller) { - return new LockedApplication(new Builder(this).with(deploymentJobs().withCompletion(report, notificationTime, controller))); + 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 withJobTriggering(JobType type, Change change, Instant triggerTime, @@ -119,8 +122,8 @@ public class LockedApplication extends Application { return new LockedApplication(new Builder(this).with(validationOverrides)); } - public LockedApplication withDeploying(Change deploying) { - return new LockedApplication(new Builder(this).withDeploying(deploying)); + public LockedApplication withChange(Change change) { + return new LockedApplication(new Builder(this).withChange(change)); } public LockedApplication withOutstandingChange(boolean outstandingChange) { @@ -146,6 +149,17 @@ 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()); @@ -205,7 +219,7 @@ public class LockedApplication extends Application { return this; } - private Builder withDeploying(Change deploying) { + private Builder withChange(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 304d82b2bec..31ee5fa6d44 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,18 +11,19 @@ import java.util.Optional; * @author bratseth * @author mpolden */ -public class ApplicationVersion { +public class ApplicationVersion implements Comparable<ApplicationVersion> { - // TODO: Remove the need for this + /** + * Used in cases where application version cannot be determined, such as manual deployments (e.g. in dev + * environment) + */ public static final ApplicationVersion unknown = new ApplicationVersion(); - // Never changes. Only used to create a valid version number for the bundle + // This never changes and is only used to create a valid semantic version number, as required by application bundles private static final String majorVersion = "1.0"; - // TODO: Remove after introducing new application version + // TODO: Remove after 2018-03-01 private final Optional<String> applicationPackageHash; - - // TODO: Make mandatory private final Optional<SourceRevision> source; private final Optional<Long> buildNumber; @@ -37,11 +38,11 @@ public class ApplicationVersion { Objects.requireNonNull(applicationPackageHash, "applicationPackageHash cannot be null"); Objects.requireNonNull(source, "source cannot be null"); Objects.requireNonNull(buildNumber, "buildNumber cannot be null"); - if (buildNumber.isPresent() && !source.isPresent()) { - throw new IllegalArgumentException("both buildNumber and source must be set if buildNumber is set"); + if (!applicationPackageHash.isPresent() && source.isPresent() != buildNumber.isPresent()) { + throw new IllegalArgumentException("both buildNumber and source must be set together"); } - if ( ! buildNumber.isPresent() && ! applicationPackageHash.isPresent()) { - throw new IllegalArgumentException("applicationPackageHash must be given if buildNumber is unset"); + if (buildNumber.isPresent() && buildNumber.get() <= 0) { + throw new IllegalArgumentException("buildNumber must be > 0"); } this.applicationPackageHash = applicationPackageHash; this.source = source; @@ -68,9 +69,14 @@ public class ApplicationVersion { if (applicationPackageHash.isPresent()) { return applicationPackageHash.get(); } - return String.format("%s.%d-%s", majorVersion, buildNumber.get(), abbreviateCommit(source.get().commit())); + return source.map(sourceRevision -> String.format("%s.%d-%s", majorVersion, buildNumber.get(), + abbreviateCommit(source.get().commit()))) + .orElse("unknown"); } + /** 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) @@ -80,23 +86,29 @@ public class 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 int hashCode() { return applicationPackageHash.hashCode(); } + 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); + } @Override - public boolean equals(Object other) { - if (this == other) return true; - if ( ! (other instanceof ApplicationVersion)) return false; - return this.applicationPackageHash.equals(((ApplicationVersion)other).applicationPackageHash); + public int hashCode() { + return Objects.hash(applicationPackageHash, source, buildNumber); } @Override public String toString() { - if (buildNumber.isPresent()) { - return "Application package version: " + abbreviateCommit(source.get().commit()) + "-" + buildNumber.get(); - } - return "Application package revision '" + applicationPackageHash + "'" + - (source.isPresent() ? " with " + source.get() : ""); + return "Application package version: " + id() + source.map(s -> ", " + s.toString()).orElse(""); } /** Abbreviate given commit hash to 9 characters */ @@ -104,4 +116,11 @@ public class 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 bb7b39eed0f..932777e690d 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,11 +55,13 @@ public class DeploymentJobs { } /** Return a new instance with the given completion */ - public DeploymentJobs withCompletion(JobReport report, Instant notificationTime, Controller controller) { + public DeploymentJobs withCompletion(JobReport report, ApplicationVersion applicationVersion, + 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(), report.jobError(), notificationTime, controller); + return job.withCompletion(report.buildNumber(), applicationVersion, report.jobError(), notificationTime, + controller); }); return new DeploymentJobs(Optional.of(report.projectId()), status, issueId); } @@ -254,11 +256,16 @@ 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; // TODO: Require non-empty source revision if jobType == component + this.sourceRevision = sourceRevision; 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 41060a7af4c..fb9ab8735d8 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() == ApplicationVersion.unknown) return true; // Indicates the component job, which is always an application change. + 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/application/JobStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java index e165d3c9fe5..e963fde8f94 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,27 +57,31 @@ 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) { + 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) { 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(); @@ -197,7 +201,7 @@ public class JobStatus { /** Returns the Vespa version used on this run */ public Version version() { return version; } - /** Returns the application version used for this run, or empty when not known */ + /** Returns the application version used in this run */ 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 1beab1307c1..6560423f3b9 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,7 +78,8 @@ public class DeploymentTrigger { */ public void triggerFromCompletion(JobReport report) { applications().lockOrThrow(report.applicationId(), application -> { - application = application.withJobCompletion(report, clock.instant(), controller); + application = application.withJobCompletion(report, applicationVersionFrom(report), clock.instant(), + controller); application = application.withProjectId(report.projectId()); // Handle successful starting and ending @@ -89,8 +90,9 @@ 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.withDeploying(Change.of(applicationVersion)); + applicationVersion = ApplicationVersion.from(report.sourceRevision().get(), + report.buildNumber()); + application = application.withChange(Change.of(applicationVersion)); } } else { // postpone @@ -100,7 +102,7 @@ public class DeploymentTrigger { } else if (deploymentComplete(application)) { // change completed - application = application.withDeploying(Change.empty()); + application = application.withChange(Change.empty()); } } @@ -139,7 +141,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() == ApplicationVersion.unknown) return false; + if (change.application().get().isUnknown()) return false; if ( ! change.application().get().equals(deployment.applicationVersion())) return false; } } @@ -260,7 +262,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.withDeploying(change); + application = application.withChange(change); if (change.application().isPresent()) application = application.withOutstandingChange(false); application = trigger(JobType.systemTest, application, false, change.toString()); @@ -276,7 +278,7 @@ public class DeploymentTrigger { public void cancelChange(ApplicationId applicationId) { applications().lockOrThrow(applicationId, application -> { buildSystem.removeJobs(application.id()); - applications().store(application.withDeploying(Change.empty())); + applications().store(application.withChange(Change.empty())); }); } @@ -355,10 +357,17 @@ 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 652f95a2d13..4b7993b3b7d 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,7 +6,6 @@ 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; @@ -15,6 +14,7 @@ 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,6 +61,7 @@ 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"; @@ -198,9 +199,15 @@ public class ApplicationSerializer { } private void toSlime(ApplicationVersion applicationVersion, Cursor object) { - object.setString(applicationPackageHashField, applicationVersion.id()); - if (applicationVersion.source().isPresent()) + 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)); + } + } } private void toSlime(SourceRevision sourceRevision, Cursor object) { @@ -236,7 +243,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() != ApplicationVersion.unknown) + if ( ! jobRun.get().applicationVersion().isUnknown()) toSlime(jobRun.get().applicationVersion(), object.setObject(revisionField)); object.setBool(upgradeField, jobRun.get().upgrade()); object.setString(reasonField, jobRun.get().reason()); @@ -249,7 +256,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() != ApplicationVersion.unknown) + if (deploying.application().isPresent() && !deploying.application().get().isUnknown()) toSlime(deploying.application().get(), object); } @@ -342,10 +349,17 @@ public class ApplicationSerializer { private ApplicationVersion applicationVersionFromSlime(Inspector object) { if ( ! object.valid()) return ApplicationVersion.unknown; - String applicationPackageHash = object.field(applicationPackageHashField).asString(); + Optional<String> applicationPackageHash = optionalString(object.field(applicationPackageHashField)); + Optional<Long> applicationBuildNumber = optionalLong(object.field(applicationBuildNumberField)); Optional<SourceRevision> sourceRevision = sourceRevisionFromSlime(object.field(sourceRevisionField)); - return sourceRevision.isPresent() ? ApplicationVersion.from(applicationPackageHash, sourceRevision.get()) - : ApplicationVersion.from(applicationPackageHash); + 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()); } private Optional<SourceRevision> sourceRevisionFromSlime(Inspector object) { @@ -369,7 +383,8 @@ public class ApplicationSerializer { Change change = Change.empty(); if (versionFieldValue.valid()) change = Change.of(Version.fromString(versionFieldValue.asString())); - if (object.field(applicationPackageHashField).valid()) + if (object.field(applicationBuildNumberField).valid() || + object.field(applicationPackageHashField).valid()) // TODO: Remove after 2018-03-01 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 9b2174b881d..77e0fb1ef05 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,9 +478,10 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private void toSlime(ApplicationVersion applicationVersion, Cursor object) { - object.setString("hash", applicationVersion.id()); - if (applicationVersion.source().isPresent()) + if (!applicationVersion.isUnknown()) { + object.setString("hash", applicationVersion.id()); sourceRevisionToSlime(applicationVersion.source(), object.setObject("source")); + } } private void sourceRevisionToSlime(Optional<SourceRevision> revision, Cursor object) { @@ -970,7 +971,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() != ApplicationVersion.unknown) + if (!jobRun.applicationVersion().isUnknown()) toSlime(jobRun.applicationVersion(), object.setObject("revision")); object.setString("reason", jobRun.reason()); object.setLong("at", jobRun.at().toEpochMilli()); 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 efbc10e8deb..cfe60cd3e96 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 @@ -14,11 +14,22 @@ import java.util.Objects; */ public class ArtifactRepositoryMock implements ArtifactRepository { - private final Map<Integer, byte[]> repository = new HashMap<>(); + private final Map<Integer, Artifact> repository = new HashMap<>(); public ArtifactRepositoryMock put(ApplicationId applicationId, ApplicationPackage applicationPackage, String applicationVersion) { - repository.put(artifactHash(applicationId, applicationVersion), applicationPackage.zippedContent()); + repository.put(artifactHash(applicationId, applicationVersion), + new Artifact(applicationPackage.zippedContent())); + return this; + } + + public int hits(ApplicationId applicationId, String applicationVersion) { + Artifact artifact = repository.get(artifactHash(applicationId, applicationVersion)); + return artifact == null ? 0 : artifact.hits; + } + + public ArtifactRepository resetHits() { + repository.values().forEach(Artifact::resetHits); return this; } @@ -29,11 +40,32 @@ public class ArtifactRepositoryMock implements ArtifactRepository { throw new IllegalArgumentException("No application package found for " + applicationId + " with version " + applicationVersion); } - return repository.get(artifactHash); + Artifact artifact = repository.get(artifactHash); + artifact.recordHit(); + return artifact.data; } private static int artifactHash(ApplicationId applicationId, String applicationVersion) { return Objects.hash(applicationId, applicationVersion); } + private class Artifact { + + private final byte[] data; + private int hits = 0; + + private Artifact(byte[] data) { + this.data = data; + } + + private void recordHit() { + 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 8d03b4f7121..97bbaec6427 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 @@ -11,6 +11,7 @@ import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.athenz.api.AthenzDomain; +import com.yahoo.vespa.athenz.api.NToken; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.api.Tenant; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; @@ -21,13 +22,13 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; 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.athenz.api.NToken; 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.application.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; +import com.yahoo.vespa.hosted.controller.application.Deployment; 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; @@ -201,129 +202,60 @@ public class ControllerTest { assertNull("Deployment job was removed", applications.require(app1.id()).deploymentJobs().jobStatus().get(productionCorpUsEast1)); } - // TODO: Replace above test with this one after introducing new application version number @Test - public void testDeploymentWithApplicationVersion() { - // Setup system + public void testDeploymentApplicationVersion() { DeploymentTester tester = new DeploymentTester(); - ApplicationController applications = tester.controller().applications(); - Version version1 = Version.fromString("6.1"); // Set in config server mock - Application app1 = tester.createApplication("app1", "tenant1", 1, 11L); - - // Component runs, uploads artifact and notifies completion + 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", "branch", "deadbeef"); - String expectedVersionString = "1.0.37-deadbeef"; - tester.artifactRepository().put(app1.id(), applicationPackage, expectedVersionString); - tester.notifyJobCompletion(component, app1, Optional.empty(), Optional.of(source), 37); - ApplicationVersion expectedVersion = ApplicationVersion.from(source, 37); - assertEquals(expectedVersionString, tester.controller().applications() - .require(app1.id()) - .change().application().get().id()); - - // Deploy without application package - tester.deployAndNotify(app1, true, systemTest); - tester.deployAndNotify(app1, true, stagingTest); - assertEquals(4, applications.require(app1.id()).deploymentJobs().jobStatus().size()); - assertStatus(JobStatus.initial(stagingTest) - .withTriggering(version1, expectedVersion, false, "", 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, expectedVersion, false, "", tester.clock().instant()), app1.id(), tester.controller()); - tester.clock().advance(Duration.ofSeconds(1)); - - // production job (failing) - tester.deployAndNotify(app1, false, productionCorpUsEast1); - assertEquals(4, applications.require(app1.id()).deploymentJobs().jobStatus().size()); - - JobStatus expectedJobStatus = JobStatus.initial(productionCorpUsEast1) - .withTriggering(version1, expectedVersion, false, "", tester.clock().instant()) - .withCompletion(42, Optional.of(JobError.unknown), tester.clock().instant(), tester.controller()); - - assertStatus(expectedJobStatus, app1.id(), tester.controller()); - - // Simulate restart - tester.restartController(); - applications = tester.controller().applications(); - - assertNotNull(tester.controller().tenants().tenant(new TenantId("tenant1"))); - assertNotNull(applications.get(ApplicationId.from(TenantName.from("tenant1"), - ApplicationName.from("application1"), - InstanceName.from("default")))); - assertEquals(4, applications.require(app1.id()).deploymentJobs().jobStatus().size()); - + SourceRevision source = new SourceRevision("repo", "master", "commit1"); - tester.clock().advance(Duration.ofHours(1)); - - tester.notifyJobCompletion(productionCorpUsEast1, app1, false); // Need to complete the job, or new jobs won't start. - - // Component is triggered again - tester.artifactRepository().put(app1.id(), applicationPackage, "1.0.38-deadbeef"); - tester.notifyJobCompletion(component, app1, Optional.empty(), Optional.of(source), 38); - tester.deployAndNotify(app1, Optional.empty(), true, false, systemTest); - expectedVersion = ApplicationVersion.from(source, 38); - assertStatus(JobStatus.initial(systemTest) - .withTriggering(version1, expectedVersion, false, "", tester.clock().instant().minus(Duration.ofMillis(1))) - .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller()); - tester.deployAndNotify(app1, Optional.empty(), true, true, stagingTest); - - // production job succeeding now - tester.deployAndNotify(app1, Optional.empty(), true, true, productionCorpUsEast1); - expectedJobStatus = expectedJobStatus - .withTriggering(version1, expectedVersion, false, "", 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, expectedVersion, false, "", tester.clock().instant()), - app1.id(), tester.controller()); - tester.deployAndNotify(app1, Optional.empty(), true, true, productionUsEast3); - - assertEquals(5, applications.get(app1.id()).get().deploymentJobs().jobStatus().size()); - - // prod zone removal is not allowed - applicationPackage = new ApplicationPackageBuilder() - .environment(Environment.prod) - .region("us-east-3") - .build(); - tester.artifactRepository().put(app1.id(), applicationPackage, "1.0.56-cafed00d"); - source = new SourceRevision("repo", "branch", "cafed00d"); - tester.notifyJobCompletion(component, app1, Optional.empty(), Optional.of(source), 56); - try { - tester.deploy(systemTest, app1, Optional.empty(), false); - fail("Expected exception due to unallowed production deployment removal"); - } - catch (IllegalArgumentException e) { - assertEquals("deployment-removal: application 'tenant1.app1' is deployed in corp-us-east-1, but does not include this zone in deployment.xml", e.getMessage()); - } - assertNotNull("Zone was not removed", - applications.require(app1.id()).deployments().get(productionCorpUsEast1.zone(SystemName.main).get())); - JobStatus jobStatus = applications.require(app1.id()).deploymentJobs().jobStatus().get(productionCorpUsEast1); - assertNotNull("Deployment job was not removed", jobStatus); - assertEquals(42, jobStatus.lastCompleted().get().id()); - assertEquals("staging-test completed", jobStatus.lastCompleted().get().reason()); + ApplicationVersion applicationVersion = ApplicationVersion.from(source, 101); + tester.artifactRepository().put(app.id(), applicationPackage, applicationVersion.id()); + runDeployment(tester, app.id(), applicationVersion, applicationPackage, Optional.of(source), 101); + assertEquals("Artifact is downloaded twice in staging and once for other zones", 5, + tester.artifactRepository().hits(app.id(), applicationVersion.id())); + } - // prod zone removal is allowed with override - applicationPackage = new ApplicationPackageBuilder() - .allow(ValidationId.deploymentRemoval) - .upgradePolicy("default") + @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(); - tester.artifactRepository().put(app1.id(), applicationPackage, "1.0.103-c00ffefe"); - source = new SourceRevision("repo", "branch", "c00ffefe"); - tester.notifyJobCompletion(component, app1, Optional.empty(), Optional.of(source), 103); - tester.deployAndNotify(app1, Optional.empty(), true, true, systemTest); - assertNull("Zone was removed", - applications.require(app1.id()).deployments().get(productionCorpUsEast1.zone(SystemName.main).get())); - assertNull("Deployment job was removed", applications.require(app1.id()).deploymentJobs().jobStatus().get(productionCorpUsEast1)); + 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, 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, 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, applicationPackage, 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 @@ -662,19 +594,20 @@ public class ControllerTest { @Test public void testDeployUntestedChangeFails() { - ControllerTester tester = new ControllerTester(); + DeploymentTester tester = new DeploymentTester(); ApplicationController applications = tester.controller().applications(); - TenantId tenant = tester.createTenant("tenant1", "domain1", 11L); - Application app = tester.createApplication(tenant, "app1", "default", 1); + TenantId tenant = tester.controllerTester().createTenant("tenant1", "domain1", 11L); + Application app = tester.controllerTester().createApplication(tenant, "app1", "default", 1); + tester.deployCompletely(app, applicationPackage); tester.controller().applications().lockOrThrow(app.id(), application -> { - application = application.withDeploying(Change.of(Version.fromString("6.3"))); + application = application.withChange(Change.of(Version.fromString("6.3"))); applications.store(application); try { - tester.deploy(app, ZoneId.from("prod", "us-east-3")); + tester.controllerTester().deploy(app, ZoneId.from("prod", "corp-us-east-1"), applicationPackage); fail("Expected exception"); } catch (IllegalArgumentException e) { - assertEquals("Rejecting deployment of application 'tenant1.app1' to zone prod.us-east-3 as upgrade to 6.3 is not tested", e.getMessage()); + assertEquals("Rejecting deployment of application 'tenant1.app1' to zone prod.corp-us-east-1 as upgrade to 6.3 is not tested", e.getMessage()); } }); } @@ -915,4 +848,51 @@ public class ControllerTest { } + private void runDeployment(DeploymentTester tester, ApplicationId application, + ApplicationVersion expectedVersion, + ApplicationPackage applicationPackage, Optional<SourceRevision> sourceRevision, + long initialBuildNumber) { + Version vespaVersion = Version.fromString("6.1"); // Set in config server mock + Application app = tester.applications().require(application); + + // Component notifies completion + tester.notifyJobCompletion(component, app, Optional.empty(), sourceRevision, initialBuildNumber); + ApplicationVersion change = + sourceRevision.map(sr -> ApplicationVersion.from(sr, initialBuildNumber)) + .orElse(ApplicationVersion.unknown); + assertEquals(change.id(), tester.controller().applications() + .require(application) + .change().application().get().id()); + + // Deploy in test + tester.deployAndNotify(app, applicationPackage, true, systemTest); + tester.deployAndNotify(app, applicationPackage, true, stagingTest); + //assertEquals(4, applications.require(application).deploymentJobs().jobStatus().size()); + assertStatus(JobStatus.initial(stagingTest) + .withTriggering(vespaVersion, expectedVersion, false, "", + tester.clock().instant().minus(Duration.ofMillis(1))) + .withCompletion(42, Optional.empty(), tester.clock().instant(), + tester.controller()), application, tester.controller()); + + // Deploy in production + tester.deployAndNotify(app, applicationPackage, true, productionCorpUsEast1); + assertStatus(JobStatus.initial(productionCorpUsEast1) + .withTriggering(vespaVersion, expectedVersion, false, "", + tester.clock().instant().minus(Duration.ofMillis(1))) + .withCompletion(42, Optional.empty(), tester.clock().instant(), + tester.controller()), application, tester.controller()); + tester.deployAndNotify(app, applicationPackage, true, true, productionUsEast3); + assertStatus(JobStatus.initial(productionUsEast3) + .withTriggering(vespaVersion, expectedVersion, false, "", + tester.clock().instant().minus(Duration.ofMillis(1))) + .withCompletion(42, Optional.empty(), tester.clock().instant(), + tester.controller()), application, tester.controller()); + + // Verify deployed version + app = tester.controller().applications().require(app.id()); + for (Deployment deployment : app.productionDeployments().values()) { + assertEquals(expectedVersion, deployment.applicationVersion()); + } + } + } 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 d0dfe825558..58f2a21fa6a 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 @@ -253,10 +253,6 @@ public class DeploymentTester { deployCurrentVersion)); } - public void deployAndNotify(Application application, boolean success, JobType... job) { - deployAndNotify(application, Optional.empty(), success, true, job); - } - public void deployAndNotify(Application application, String upgradePolicy, boolean success, JobType... jobs) { deployAndNotify(application, applicationPackage(upgradePolicy), success, true, jobs); } @@ -276,6 +272,10 @@ public class DeploymentTester { consumeJobs(application, expectOnlyTheseJobs, jobs); for (JobType job : jobs) { if (success) { + // Staging deploys twice, once with current version and once with new version + if (job == JobType.stagingTest) { + deploy(job, application, applicationPackage, true); + } deploy(job, application, applicationPackage, false); } notifyJobCompletion(job, application, success); 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 5c61e43f9cf..029eb335d82 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 @@ -320,7 +320,7 @@ public class DeploymentTriggerTest { new JobControl(tester.controllerTester().curator())); LockedApplication app = (LockedApplication)tester.createAndDeploy("default0", 3, "default"); // Store that we are upgrading but don't start the system-tests job - tester.controller().applications().store(app.withDeploying(Change.of(Version.fromString("6.2")))); + tester.controller().applications().store(app.withChange(Change.of(Version.fromString("6.2")))); assertEquals(0, tester.buildSystem().jobs().size()); readyJobsTrigger.run(); assertEquals(1, tester.buildSystem().jobs().size()); 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 9a945281789..35bd63745c4 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 @@ -62,9 +62,9 @@ public class ApplicationSerializerTest { "</validation-overrides>"); List<Deployment> deployments = new ArrayList<>(); - ApplicationVersion applicationVersion1 = ApplicationVersion.from("appHash1"); + ApplicationVersion applicationVersion1 = ApplicationVersion.from(new SourceRevision("repo1", "branch1", "commit1"), 31); ApplicationVersion applicationVersion2 = ApplicationVersion - .from("appHash2", new SourceRevision("repo1", "branch1", "commit1")); + .from(new SourceRevision("repo1", "branch1", "commit1"), 32); deployments.add(new Deployment(zone1, applicationVersion1, Version.fromString("1.2.3"), Instant.ofEpochMilli(3))); // One deployment without cluster info and utils 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))); @@ -148,20 +148,19 @@ public class ApplicationSerializerTest { assertEquals(6, serialized.deployments().get(zone2).metrics().writeLatencyMillis(), Double.MIN_VALUE); { // test more deployment serialization cases - Application original2 = writable(original).withDeploying(Change.of(ApplicationVersion.from("hash1"))); + Application original2 = writable(original).withChange(Change.of(ApplicationVersion.from("hash1"))); 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).withDeploying(Change.of(ApplicationVersion.from("hash1", - new SourceRevision("a", "b", "c")))); + Application original3 = writable(original).withChange(Change.of(ApplicationVersion.from("hash1", + new SourceRevision("a", "b", "c")))); Application serialized3 = applicationSerializer.fromSlime(applicationSerializer.toSlime(original3)); - assertEquals(original3.change(), serialized2.change()); + assertEquals(original3.change(), serialized3.change()); assertEquals(serialized3.change().application().get().source(), original3.change().application().get().source()); - - Application original4 = writable(original).withDeploying(Change.empty()); + Application original4 = writable(original).withChange(Change.empty()); Application serialized4 = applicationSerializer.fromSlime(applicationSerializer.toSlime(original4)); assertEquals(original4.change(), serialized4.change()); } |