diff options
51 files changed, 1284 insertions, 943 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java b/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java index 1263b9bed6e..c42da6dcd19 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/ml/ConvertedModel.java @@ -172,13 +172,16 @@ public class ConvertedModel { ExpressionFunction expression = expressions.get(arguments.toName()); if (expression != null) return expression; - if ( ! arguments.signature().isPresent()) { + expression = expressions.get("default." + arguments.toName()); + if (expression != null) return expression; + + if (arguments.signature().isEmpty()) { if (expressions.size() > 1) throw new IllegalArgumentException("Multiple candidate expressions " + missingExpressionMessageSuffix()); return expressions.values().iterator().next(); } - if ( ! arguments.output().isPresent()) { + if (arguments.output().isEmpty()) { List<Map.Entry<String, ExpressionFunction>> entriesWithTheRightPrefix = expressions.entrySet().stream().filter(entry -> entry.getKey().startsWith(arguments.signature().get() + ".")).collect(Collectors.toList()); if (entriesWithTheRightPrefix.size() < 1) diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java index 754f161c70b..4387d19c474 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java @@ -128,6 +128,20 @@ public class RankingExpressionWithOnnxTestCase { } @Test + public void testOnnxReferenceWithSpecifiedOutput() { + RankProfileSearchFixture search = fixtureWith("tensor<float>(d0[2],d1[784])(0.0)", + "onnx('mnist_softmax.onnx', 'add')"); + search.assertFirstPhaseExpression(vespaExpression, "my_profile"); + } + + @Test + public void testOnnxReferenceWithSpecifiedOutputAndSignature() { + RankProfileSearchFixture search = fixtureWith("tensor<float>(d0[2],d1[784])(0.0)", + "onnx('mnist_softmax.onnx', 'default.add')"); + search.assertFirstPhaseExpression(vespaExpression, "my_profile"); + } + + @Test public void testOnnxReferenceMissingFunction() throws ParseException { try { RankProfileSearchFixture search = new RankProfileSearchFixture( @@ -171,7 +185,7 @@ public class RankingExpressionWithOnnxTestCase { @Test public void testOnnxReferenceSpecifyingNonExistingOutput() { try { - RankProfileSearchFixture search = fixtureWith("tensor(d0[2],d1[784])(0.0)", + RankProfileSearchFixture search = fixtureWith("tensor<float>(d0[2],d1[784])(0.0)", "onnx('mnist_softmax.onnx', 'y')"); search.assertFirstPhaseExpression(vespaExpression, "my_profile"); fail("Expecting exception"); 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 c75f3102772..e37d6accd89 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 @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedMap; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec; @@ -45,8 +46,6 @@ public class Application { private final ValidationOverrides validationOverrides; private final Optional<ApplicationVersion> latestVersion; private final OptionalLong projectId; - private final Change change; - private final Change outstandingChange; private final Optional<IssueId> deploymentIssueId; private final Optional<IssueId> ownershipIssueId; private final Optional<User> owner; @@ -57,22 +56,20 @@ public class Application { /** Creates an empty application. */ public Application(TenantAndApplicationId id, Instant now) { - this(id, now, DeploymentSpec.empty, ValidationOverrides.empty, Change.empty(), Change.empty(), + this(id, now, DeploymentSpec.empty, ValidationOverrides.empty, Optional.empty(), Optional.empty(), Optional.empty(), OptionalInt.empty(), new ApplicationMetrics(0, 0), Set.of(), OptionalLong.empty(), Optional.empty(), List.of()); } // DO NOT USE! For serialization purposes, only. public Application(TenantAndApplicationId id, Instant createdAt, DeploymentSpec deploymentSpec, ValidationOverrides validationOverrides, - Change change, Change outstandingChange, Optional<IssueId> deploymentIssueId, Optional<IssueId> ownershipIssueId, Optional<User> owner, + Optional<IssueId> deploymentIssueId, Optional<IssueId> ownershipIssueId, Optional<User> owner, OptionalInt majorVersion, ApplicationMetrics metrics, Set<PublicKey> deployKeys, OptionalLong projectId, Optional<ApplicationVersion> latestVersion, Collection<Instance> instances) { this.id = Objects.requireNonNull(id, "id cannot be null"); this.createdAt = Objects.requireNonNull(createdAt, "instant of creation cannot be null"); this.deploymentSpec = Objects.requireNonNull(deploymentSpec, "deploymentSpec cannot be null"); this.validationOverrides = Objects.requireNonNull(validationOverrides, "validationOverrides cannot be null"); - this.change = Objects.requireNonNull(change, "change cannot be null"); - this.outstandingChange = Objects.requireNonNull(outstandingChange, "outstandingChange cannot be null"); this.deploymentIssueId = Objects.requireNonNull(deploymentIssueId, "deploymentIssueId cannot be null"); this.ownershipIssueId = Objects.requireNonNull(ownershipIssueId, "ownershipIssueId cannot be null"); this.owner = Objects.requireNonNull(owner, "owner cannot be null"); @@ -118,18 +115,6 @@ public class Application { return get(instance).orElseThrow(() -> new IllegalArgumentException("Unknown instance '" + instance + "' in '" + id + "'")); } - /** - * Returns base change for this application, i.e., the change that is deployed outside block windows. - * This is empty when no change is currently under deployment. - */ - public Change change() { return change; } - - /** - * Returns whether this has an outstanding change (in the source repository), which - * has currently not started deploying (because a deployment is (or was) already in progress - */ - public Change outstandingChange() { return outstandingChange; } - /** Returns ID of any open deployment issue filed for this */ public Optional<IssueId> deploymentIssueId() { return deploymentIssueId; 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 ecea7b09782..8339e1b8fdd 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 @@ -393,7 +393,7 @@ public class ApplicationController { applicationPackage = getApplicationPackage(instanceId, applicationVersion); applicationPackage = withTesterCertificate(applicationPackage, instanceId, jobType); - validateRun(application.get(), instance, zone, platformVersion, applicationVersion); + validateRun(application.get().require(instance), zone, platformVersion, applicationVersion); } if (controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) { @@ -887,14 +887,14 @@ public class ApplicationController { } /** Verify that we don't downgrade an existing production deployment. */ - private void validateRun(Application application, InstanceName instance, ZoneId zone, Version platformVersion, ApplicationVersion applicationVersion) { - Deployment deployment = application.require(instance).deployments().get(zone); + private void validateRun(Instance instance, ZoneId zone, Version platformVersion, ApplicationVersion applicationVersion) { + Deployment deployment = instance.deployments().get(zone); if ( zone.environment().isProduction() && deployment != null - && ( platformVersion.compareTo(deployment.version()) < 0 && ! application.change().isPinned() + && ( platformVersion.compareTo(deployment.version()) < 0 && ! instance.change().isPinned() || applicationVersion.compareTo(deployment.applicationVersion()) < 0)) throw new IllegalArgumentException(String.format("Rejecting deployment of application %s to %s, as the requested versions (platform: %s, application: %s)" + " are older than the currently deployed (platform: %s, application: %s).", - application.id().instance(instance), zone, platformVersion, applicationVersion, deployment.version(), deployment.applicationVersion())); + instance.id(), zone, platformVersion, applicationVersion, deployment.version(), deployment.applicationVersion())); } /** Returns the rotation repository, used for managing global rotation assignments */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java index d60102658cc..a510275b98e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java @@ -12,6 +12,7 @@ import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.AssignedRotation; +import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.ClusterInfo; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; @@ -46,21 +47,23 @@ public class Instance { private final List<AssignedRotation> rotations; private final RotationStatus rotationStatus; private final Map<JobType, Instant> jobPauses; + private final Change change; /** Creates an empty instance */ public Instance(ApplicationId id) { - this(id, Set.of(), Map.of(), List.of(), RotationStatus.EMPTY); + this(id, Set.of(), Map.of(), List.of(), RotationStatus.EMPTY, Change.empty()); } /** Creates an empty instance*/ public Instance(ApplicationId id, Collection<Deployment> deployments, Map<JobType, Instant> jobPauses, - List<AssignedRotation> rotations, RotationStatus rotationStatus) { + List<AssignedRotation> rotations, RotationStatus rotationStatus, Change change) { this.id = Objects.requireNonNull(id, "id cannot be null"); this.deployments = ImmutableMap.copyOf(Objects.requireNonNull(deployments, "deployments cannot be null").stream() .collect(Collectors.toMap(Deployment::zone, Function.identity()))); this.jobPauses = Map.copyOf(Objects.requireNonNull(jobPauses, "deploymentJobs cannot be null")); this.rotations = List.copyOf(Objects.requireNonNull(rotations, "rotations cannot be null")); this.rotationStatus = Objects.requireNonNull(rotationStatus, "rotationStatus cannot be null"); + this.change = Objects.requireNonNull(change, "change cannot be null"); } public Instance withNewDeployment(ZoneId zone, ApplicationVersion applicationVersion, Version version, @@ -82,7 +85,7 @@ public class Instance { else jobPauses.remove(jobType); - return new Instance(id, deployments.values(), jobPauses, rotations, rotationStatus); + return new Instance(id, deployments.values(), jobPauses, rotations, rotationStatus, change); } public Instance withClusterInfo(ZoneId zone, Map<ClusterSpec.Id, ClusterInfo> clusterInfo) { @@ -110,11 +113,15 @@ public class Instance { } public Instance with(List<AssignedRotation> assignedRotations) { - return new Instance(id, deployments.values(), jobPauses, assignedRotations, rotationStatus); + return new Instance(id, deployments.values(), jobPauses, assignedRotations, rotationStatus, change); } public Instance with(RotationStatus rotationStatus) { - return new Instance(id, deployments.values(), jobPauses, rotations, rotationStatus); + return new Instance(id, deployments.values(), jobPauses, rotations, rotationStatus, change); + } + + public Instance withChange(Change change) { + return new Instance(id, deployments.values(), jobPauses, rotations, rotationStatus, change); } private Instance with(Deployment deployment) { @@ -124,7 +131,7 @@ public class Instance { } private Instance with(Map<ZoneId, Deployment> deployments) { - return new Instance(id, deployments.values(), jobPauses, rotations, rotationStatus); + return new Instance(id, deployments.values(), jobPauses, rotations, rotationStatus, change); } public ApplicationId id() { return id; } @@ -178,6 +185,11 @@ public class Instance { return rotationStatus; } + /** Returns the currently deploying change for this instance. */ + public Change change() { + return change; + } + @Override public boolean equals(Object o) { if (this == o) return true; 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 fa81a990c70..3c5bc3b98ad 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 @@ -36,8 +36,6 @@ public class LockedApplication { private final Instant createdAt; private final DeploymentSpec deploymentSpec; private final ValidationOverrides validationOverrides; - private final Change change; - private final Change outstandingChange; private final Optional<IssueId> deploymentIssueId; private final Optional<IssueId> ownershipIssueId; private final Optional<User> owner; @@ -56,14 +54,14 @@ public class LockedApplication { */ LockedApplication(Application application, Lock lock) { this(Objects.requireNonNull(lock, "lock cannot be null"), application.id(), application.createdAt(), - application.deploymentSpec(), application.validationOverrides(), application.change(), - application.outstandingChange(), application.deploymentIssueId(), application.ownershipIssueId(), + application.deploymentSpec(), application.validationOverrides(), + application.deploymentIssueId(), application.ownershipIssueId(), application.owner(), application.majorVersion(), application.metrics(), application.deployKeys(), application.projectId(), application.latestVersion(), application.instances()); } private LockedApplication(Lock lock, TenantAndApplicationId id, Instant createdAt, DeploymentSpec deploymentSpec, - ValidationOverrides validationOverrides, Change change, Change outstandingChange, + ValidationOverrides validationOverrides, Optional<IssueId> deploymentIssueId, Optional<IssueId> ownershipIssueId, Optional<User> owner, OptionalInt majorVersion, ApplicationMetrics metrics, Set<PublicKey> deployKeys, OptionalLong projectId, Optional<ApplicationVersion> latestVersion, @@ -73,8 +71,6 @@ public class LockedApplication { this.createdAt = createdAt; this.deploymentSpec = deploymentSpec; this.validationOverrides = validationOverrides; - this.change = change; - this.outstandingChange = outstandingChange; this.deploymentIssueId = deploymentIssueId; this.ownershipIssueId = ownershipIssueId; this.owner = owner; @@ -88,7 +84,7 @@ public class LockedApplication { /** Returns a read-only copy of this */ public Application get() { - return new Application(id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new Application(id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances.values()); } @@ -96,7 +92,7 @@ public class LockedApplication { public LockedApplication withNewInstance(InstanceName instance) { var instances = new HashMap<>(this.instances); instances.put(instance, new Instance(id.instance(instance))); - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } @@ -104,7 +100,7 @@ public class LockedApplication { public LockedApplication with(InstanceName instance, UnaryOperator<Instance> modification) { var instances = new HashMap<>(this.instances); instances.put(instance, modification.apply(instances.get(instance))); - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } @@ -112,75 +108,63 @@ public class LockedApplication { public LockedApplication without(InstanceName instance) { var instances = new HashMap<>(this.instances); instances.remove(instance); - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } public LockedApplication withNewSubmission(ApplicationVersion latestVersion) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, Optional.of(latestVersion), instances); } public LockedApplication withProjectId(OptionalLong projectId) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } public LockedApplication withDeploymentIssueId(IssueId issueId) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, Optional.ofNullable(issueId), ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } public LockedApplication with(DeploymentSpec deploymentSpec) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } public LockedApplication with(ValidationOverrides validationOverrides) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, - deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, latestVersion, instances); - } - - public LockedApplication withChange(Change change) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, - deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, - projectId, latestVersion, instances); - } - - public LockedApplication withOutstandingChange(Change outstandingChange) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } public LockedApplication withOwnershipIssueId(IssueId issueId) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, Optional.of(issueId), owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } public LockedApplication withOwner(User owner) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, Optional.of(owner), majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } /** Set a major version for this, or set to null to remove any major version override */ public LockedApplication withMajorVersion(Integer majorVersion) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion == null ? OptionalInt.empty() : OptionalInt.of(majorVersion), metrics, deployKeys, projectId, latestVersion, instances); } public LockedApplication with(ApplicationMetrics metrics) { - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } @@ -188,7 +172,7 @@ public class LockedApplication { public LockedApplication withDeployKey(PublicKey pemDeployKey) { Set<PublicKey> keys = new LinkedHashSet<>(deployKeys); keys.add(pemDeployKey); - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, keys, projectId, latestVersion, instances); } @@ -196,7 +180,7 @@ public class LockedApplication { public LockedApplication withoutDeployKey(PublicKey pemDeployKey) { Set<PublicKey> keys = new LinkedHashSet<>(deployKeys); keys.remove(pemDeployKey); - return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, change, outstandingChange, + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, keys, projectId, latestVersion, instances); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java index 7926531d1fa..10f99395170 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java @@ -49,16 +49,6 @@ public class ApplicationList extends AbstractFilteringList<Application, Applicat // ----------------------------------- Filters - /** Returns the subset of applications which have changes left to deploy; blocked, or deploying */ - public ApplicationList withChanges() { - return matching(application -> application.change().hasTargets() || application.outstandingChange().hasTargets()); - } - - /** Returns the subset of applications which are currently deploying a change */ - public ApplicationList deploying() { - return matching(application -> application.change().hasTargets()); - } - /** Returns the subset of applications which have at least one production deployment */ public ApplicationList withProductionDeployment() { return matching(application -> application.instances().values().stream() diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java index af63e8e5da7..8d082222721 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java @@ -8,6 +8,8 @@ import java.util.Objects; import java.util.Optional; import java.util.StringJoiner; +import static java.util.Objects.requireNonNull; + /** * The changes to an application we currently wish to complete deploying. * A goal of the system is to deploy platform and application versions separately. @@ -28,17 +30,17 @@ public final class Change { /** The application version we are changing to, or empty if none */ private final Optional<ApplicationVersion> application; + /** Whether this change is a pin to its contained Vespa version, or to the application's current. */ private final boolean pinned; private Change(Optional<Version> platform, Optional<ApplicationVersion> application, boolean pinned) { - Objects.requireNonNull(platform, "platform cannot be null"); - Objects.requireNonNull(application, "application cannot be null"); + this.platform = requireNonNull(platform, "platform cannot be null"); + this.application = requireNonNull(application, "application cannot be null"); if (application.isPresent() && application.get().isUnknown()) { throw new IllegalArgumentException("Application version to deploy must be a known version"); } - this.platform = platform; - this.application = application; this.pinned = pinned; + } public Change withoutPlatform() { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java new file mode 100644 index 00000000000..de3f76d50cd --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java @@ -0,0 +1,144 @@ +package com.yahoo.vespa.hosted.controller.application; + +import com.google.common.collect.ImmutableMap; +import com.yahoo.collections.AbstractFilteringList; +import com.yahoo.component.Version; +import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.InstanceName; +import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.Instance; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatusList; +import com.yahoo.vespa.hosted.controller.deployment.RunStatus; + +import java.time.Instant; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static java.util.Comparator.comparing; +import static java.util.Comparator.naturalOrder; + +public class InstanceList extends AbstractFilteringList<ApplicationId, InstanceList> { + + private final Map<ApplicationId, DeploymentStatus> statuses; + + private InstanceList(Collection<? extends ApplicationId> items, boolean negate, Map<ApplicationId, DeploymentStatus> statuses) { + super(items, negate, (i, n) -> new InstanceList(i, n, statuses)); + this.statuses = statuses; + } + + public static InstanceList from(DeploymentStatusList statuses) { + ImmutableMap.Builder<ApplicationId, DeploymentStatus> builder = ImmutableMap.builder(); + for (DeploymentStatus status : statuses.asList()) + for (InstanceName instance : status.application().deploymentSpec().instanceNames()) + builder.put(status.application().id().instance(instance), status); + Map<ApplicationId, DeploymentStatus> map = builder.build(); + return new InstanceList(map.keySet(), false, map); + } + + /** + * Returns the subset of instances that aren't pinned to an an earlier major version than the given one. + * + * @param targetMajorVersion the target major version which applications returned allows upgrading to + * @param defaultMajorVersion the default major version to assume for applications not specifying one + */ + public InstanceList allowMajorVersion(int targetMajorVersion, int defaultMajorVersion) { + return matching(id -> targetMajorVersion <= application(id).deploymentSpec().majorVersion() + .orElse(application(id).majorVersion() + .orElse(defaultMajorVersion))); + } + + /** Returns the subset of instances that are allowed to upgrade to the given version at the given time */ + public InstanceList canUpgradeAt(Version version, Instant instant) { + return matching(id -> statuses.get(id).instanceSteps().get(id.instance()) + .readyAt(Change.of(version)) + .map(readyAt -> ! readyAt.isAfter(instant)).orElse(false)); + } + + /** Returns the subset of instances which have at least one productiog deployment */ + public InstanceList withProductionDeployment() { + return matching(id -> instance(id).productionDeployments().size() > 0); + } + + /** Returns the subset of instances which have at least one deployment on a lower version than the given one */ + public InstanceList onLowerVersionThan(Version version) { + return matching(id -> instance(id).productionDeployments().values().stream() + .anyMatch(deployment -> deployment.version().isBefore(version))); + } + + /** Returns the subset of instances which have changes left to deploy; blocked, or deploying */ + public InstanceList withChanges() { + return matching(id -> instance(id).change().hasTargets() || statuses.get(id).outstandingChange(id.instance()).hasTargets()); + } + + /** Returns the subset of instances which are currently deploying a change */ + public InstanceList deploying() { + return matching(id -> instance(id).change().hasTargets()); + } + + /** Returns the subset of instances which currently have failing jobs on the given version */ + public InstanceList failingOn(Version version) { + return matching(id -> ! statuses.get(id).instanceJobs().get(id).failing().lastCompleted().on(version).isEmpty()); + } + + /** Returns the subset of instances which are not pinned to a certain Vespa version. */ + public InstanceList unpinned() { + return matching(id -> ! instance(id).change().isPinned()); + } + + /** Returns the subset of instances which are not currently failing any jobs. */ + public InstanceList failing() { + return matching(id -> ! statuses.get(id).instanceJobs().get(id).failing().not().withStatus(RunStatus.outOfCapacity).isEmpty()); + } + + /** Returns the subset of instances which are currently failing an upgrade. */ + public InstanceList failingUpgrade() { + return matching(id -> ! statuses.get(id).instanceJobs().get(id).failing().not().failingApplicationChange().isEmpty()); + } + + /** Returns the subset of instances which are upgrading (to any version), not considering block windows. */ + public InstanceList upgrading() { + return matching(id -> instance(id).change().platform().isPresent()); + } + + /** Returns the subset of instances which are currently upgrading to the given version */ + public InstanceList upgradingTo(Version version) { + return upgradingTo(List.of(version)); + } + + + /** Returns the subset of instances which are currently upgrading to the given version */ + public InstanceList upgradingTo(Collection<Version> versions) { + return matching(id -> versions.stream().anyMatch(version -> instance(id).change().platform().equals(Optional.of(version)))); + } + + public InstanceList with(DeploymentSpec.UpgradePolicy policy) { + return matching(id -> application(id).deploymentSpec().requireInstance(id.instance()).upgradePolicy() == policy); + } + + /** Returns the subset of instances which started failing on the given version */ + public InstanceList startedFailingOn(Version version) { + return matching(id -> ! statuses.get(id).instanceJobs().get(id).firstFailing().on(version).isEmpty()); + } + + /** Returns this list sorted by increasing oldest production deployment version. Applications without any deployments are ordered first. */ + public InstanceList byIncreasingDeployedVersion() { + return sortedBy(comparing(id -> instance(id).productionDeployments().values().stream() + .map(Deployment::version) + .min(naturalOrder()) + .orElse(Version.emptyVersion))); + } + + private Application application(ApplicationId id) { + return statuses.get(id).application(); + } + + private Instance instance(ApplicationId id) { + return application(id).require(id.instance()); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index e574b994870..3a60c480100 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -12,15 +12,17 @@ import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; +import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import java.time.Duration; import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.ArrayList; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -40,6 +42,7 @@ import static java.util.stream.Collectors.collectingAndThen; import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toMap; import static java.util.stream.Collectors.toUnmodifiableList; +import static java.util.stream.Collectors.toUnmodifiableMap; /** * Status of the deployment jobs of an {@link Application}. @@ -77,33 +80,40 @@ public class DeploymentStatus { private final JobList allJobs; private final SystemName system; private final Version systemVersion; + private final Instant now; private final Map<JobId, StepStatus> jobSteps; private final List<StepStatus> allSteps; - public DeploymentStatus(Application application, Map<JobId, JobStatus> allJobs, SystemName system, Version systemVersion) { + public DeploymentStatus(Application application, Map<JobId, JobStatus> allJobs, SystemName system, + Version systemVersion, Instant now) { this.application = requireNonNull(application); this.allJobs = JobList.from(allJobs.values()); - this.system = system; - this.systemVersion = systemVersion; + this.system = requireNonNull(system); + this.systemVersion = requireNonNull(systemVersion); + this.now = requireNonNull(now); List<StepStatus> allSteps = new ArrayList<>(); this.jobSteps = jobDependencies(application.deploymentSpec(), allSteps); this.allSteps = List.copyOf(allSteps); } + /** The application this deployment status concerns. */ public Application application() { return application; } + /** A filterable list of the status of all jobs for this application. */ public JobList jobs() { return allJobs; } + /** Whether any jobs of this application are failing with other errors than lack of capacity in a test zone. */ public boolean hasFailures() { return ! allJobs.failing() .not().withStatus(RunStatus.outOfCapacity) .isEmpty(); } + /** All job statuses, by job type, for the given instance. */ public Map<JobType, JobStatus> instanceJobs(InstanceName instance) { return allJobs.asList().stream() .filter(job -> job.id().application().equals(application.id().instance(instance))) @@ -111,22 +121,28 @@ public class DeploymentStatus { job -> job)); } + /** Filterable job status lists for each instance of this application. */ public Map<ApplicationId, JobList> instanceJobs() { return allJobs.asList().stream() .collect(groupingBy(job -> job.id().application(), collectingAndThen(toUnmodifiableList(), JobList::from))); } - /** Returns the set of jobs that need to run for the application's current change to be considered complete. */ + /** + * The set of jobs that need to run for the changes of each instance of the application to be considered complete, + * and any test jobs for any oustanding change, which will likely be needed to lated deploy this change. + */ public Map<JobId, List<Versions>> jobsToRun() { - Map<JobId, List<Versions>> jobs = jobsToRun(application().change()); - if (application.outstandingChange().isEmpty()) - return jobs; + Map<InstanceName, Change> changes = new LinkedHashMap<>(); + for (InstanceName instance : application.deploymentSpec().instanceNames()) + changes.put(instance, application.require(instance).change()); + Map<JobId, List<Versions>> jobs = jobsToRun(changes); // Add test jobs for any outstanding change. - var testJobs = jobsToRun(application.outstandingChange().onTopOf(application.change())) - .entrySet().stream() - .filter(entry -> ! entry.getKey().type().isProduction()); + for (InstanceName instance : application.deploymentSpec().instanceNames()) + changes.put(instance, outstandingChange(instance).onTopOf(application.require(instance).change())); + var testJobs = jobsToRun(changes).entrySet().stream() + .filter(entry -> ! entry.getKey().type().isProduction()); return Stream.concat(jobs.entrySet().stream(), testJobs) .collect(collectingAndThen(toMap(Map.Entry::getKey, @@ -136,17 +152,28 @@ public class DeploymentStatus { ImmutableMap::copyOf)); } - /** Returns the set of jobs that need to run for the given change to be considered complete. */ - public Map<JobId, List<Versions>> jobsToRun(Change change) { - Map<JobId, Versions> productionJobs = productionJobs(change); + /** The set of jobs that need to run for the given change to be considered complete. */ + public Map<JobId, List<Versions>> jobsToRun(Map<InstanceName, Change> changes) { + Map<JobId, Versions> productionJobs = new LinkedHashMap<>(); + changes.forEach((instance, change) -> productionJobs.putAll(productionJobs(instance, change))); Map<JobId, List<Versions>> testJobs = testJobs(productionJobs); Map<JobId, List<Versions>> jobs = new LinkedHashMap<>(testJobs); productionJobs.forEach((job, versions) -> jobs.put(job, List.of(versions))); return ImmutableMap.copyOf(jobs); } + /** The step status for all steps in the deployment spec of this, which are jobs, in the same order as in the deployment spec. */ public Map<JobId, StepStatus> jobSteps() { return jobSteps; } + public Map<InstanceName, StepStatus> instanceSteps() { + ImmutableMap.Builder<InstanceName, StepStatus> instances = ImmutableMap.builder(); + for (StepStatus status : allSteps) + if (status instanceof InstanceStatus) + instances.put(status.instance(), status); + return instances.build(); + } + + /** The step status for all steps in the deployment spec of this, in the same order as in the deployment spec. */ public List<StepStatus> allSteps() { return allSteps; } public Optional<Deployment> deploymentFor(JobId job) { @@ -155,33 +182,48 @@ public class DeploymentStatus { } /** + * The change of this application's latest submission, if this upgrades any of its production deployments, + * and has not yet started rolling out, due to some other change or a block window being present at the time of submission. + */ + public Change outstandingChange(InstanceName instance) { + return application.latestVersion().map(Change::of) + .filter(change -> application.require(instance).change().application().map(change::upgrades).orElse(true)) + .filter(change -> ! jobsToRun(Map.of(instance, change)).isEmpty()) + .orElse(Change.empty()); + } + + /** * True if the job has already been triggered on the given versions, or if all test types (systemTest, stagingTest), * restricted to the job's instance if declared in that instance, have successful runs on the given versions. */ - public boolean isTested(JobId job, Versions versions) { + public boolean isTested(JobId job, Change change) { + Versions versions = Versions.from(change, application, deploymentFor(job), systemVersion); return allJobs.triggeredOn(versions).get(job).isPresent() - || Stream.of(systemTest, stagingTest) - .noneMatch(testType -> declaredTest(job.application(), testType).map(__ -> allJobs.instance(job.application().instance())) - .orElse(allJobs) - .type(testType) - .successOn(versions).isEmpty()); + || Stream.of(systemTest, stagingTest) + .noneMatch(testType -> declaredTest(job.application(), testType).map(__ -> allJobs.instance(job.application().instance())) + .orElse(allJobs) + .type(testType) + .successOn(versions).isEmpty()); } - public Map<JobId, Versions> productionJobs(Change change) { - Map<JobId, Versions> jobs = new LinkedHashMap<>(); + /** The production jobs that need to run to complete roll-out of the given change to production. */ + public Map<JobId, Versions> productionJobs(InstanceName instance, Change change) { + ImmutableMap.Builder<JobId, Versions> jobs = ImmutableMap.builder(); jobSteps.forEach((job, step) -> { - Versions versions = Versions.from(change, application, deploymentFor(job), systemVersion); - if (job.type().isProduction() && step.completedAt(change, versions).isEmpty()) - jobs.put(job, versions); + if ( job.application().instance().equals(instance) + && job.type().isProduction() + && step.completedAt(change).isEmpty()) + jobs.put(job, Versions.from(change, application, deploymentFor(job), systemVersion)); }); - return ImmutableMap.copyOf(jobs); + return jobs.build(); } + /** The test jobs that need to run prior to the given production deployment jobs. */ public Map<JobId, List<Versions>> testJobs(Map<JobId, Versions> jobs) { Map<JobId, List<Versions>> testJobs = new LinkedHashMap<>(); for (JobType testType : List.of(systemTest, stagingTest)) { jobs.forEach((job, versions) -> { - if (job.type().isDeployment()) { + if (job.type().isProduction() && job.type().isDeployment()) { declaredTest(job.application(), testType).ifPresent(testJob -> { if (allJobs.successOn(versions).get(testJob).isEmpty()) testJobs.merge(testJob, List.of(versions), DeploymentStatus::union); @@ -189,7 +231,7 @@ public class DeploymentStatus { } }); jobs.forEach((job, versions) -> { - if ( job.type().isDeployment() + if ( job.type().isProduction() && job.type().isDeployment() && allJobs.successOn(versions).type(testType).isEmpty() && testJobs.keySet().stream() .noneMatch(test -> test.type() == testType @@ -200,12 +242,13 @@ public class DeploymentStatus { return ImmutableMap.copyOf(testJobs); } + /** JobId of any declared test of the given type, for the given instance. */ private Optional<JobId> declaredTest(ApplicationId instanceId, JobType testJob) { JobId jobId = new JobId(instanceId, testJob); return jobSteps.get(jobId).isDeclared() ? Optional.of(jobId) : Optional.empty(); } - /** Returns a DAG of the dependencies between the primitive steps in the spec, with iteration order equal to declaration order. */ + /** A DAG of the dependencies between the primitive steps in the spec, with iteration order equal to declaration order. */ private Map<JobId, StepStatus> jobDependencies(DeploymentSpec spec, List<StepStatus> allSteps) { if (DeploymentSpec.empty.equals(spec)) return Map.of(); @@ -213,7 +256,7 @@ public class DeploymentStatus { Map<JobId, StepStatus> dependencies = new LinkedHashMap<>(); List<StepStatus> previous = List.of(); for (DeploymentSpec.Step step : spec.steps()) - previous = fillStep(dependencies, allSteps, step, previous, spec.instanceNames().get(0)); + previous = fillStep(dependencies, allSteps, step, previous, null); return ImmutableMap.copyOf(dependencies); } @@ -222,8 +265,11 @@ public class DeploymentStatus { private List<StepStatus> fillStep(Map<JobId, StepStatus> dependencies, List<StepStatus> allSteps, DeploymentSpec.Step step, List<StepStatus> previous, InstanceName instance) { if (step.steps().isEmpty()) { + if (instance == null) + return previous; // Ignore test and staging outside all instances. + if ( ! step.delay().isZero()) { - StepStatus stepStatus = new DelayStatus((DeploymentSpec.Delay) step, previous); + StepStatus stepStatus = new DelayStatus((DeploymentSpec.Delay) step, previous, instance); allSteps.add(stepStatus); return List.of(stepStatus); } @@ -252,22 +298,26 @@ public class DeploymentStatus { previous = List.of(stepStatus); } else return previous; // Empty container steps end up here, and are simply ignored. + JobId jobId = new JobId(application.id().instance(instance), jobType); + allSteps.removeIf(existing -> existing.job().equals(Optional.of(jobId))); // Replace implicit tests with explicit ones. allSteps.add(stepStatus); - dependencies.put(new JobId(application.id().instance(instance), jobType), stepStatus); + dependencies.put(jobId, stepStatus); return previous; } - // TODO jonmv: Make instance status as well, including block-change and upgrade policy, to keep track of change; - // set it equal to application's when dependencies are completed. if (step instanceof DeploymentInstanceSpec) { - instance = ((DeploymentInstanceSpec) step).name(); + DeploymentInstanceSpec spec = ((DeploymentInstanceSpec) step); + StepStatus instanceStatus = new InstanceStatus(spec, previous, now, application.require(spec.name())); + instance = spec.name(); + allSteps.add(instanceStatus); + previous = List.of(instanceStatus); for (JobType test : List.of(systemTest, stagingTest)) { JobId job = new JobId(application.id().instance(instance), test); if ( ! dependencies.containsKey(job)) { - var stepStatus = JobStepStatus.ofTestDeployment(new DeclaredZone(test.environment()), List.of(), + var testStatus = JobStepStatus.ofTestDeployment(new DeclaredZone(test.environment()), List.of(), this, job.application().instance(), test, false); - dependencies.put(job, stepStatus); - allSteps.add(stepStatus); + dependencies.put(job, testStatus); + allSteps.add(testStatus); } } } @@ -317,17 +367,13 @@ public class DeploymentStatus { private final StepType type; private final DeploymentSpec.Step step; private final List<StepStatus> dependencies; - private final Optional<InstanceName> instance; - - private StepStatus(StepType type, DeploymentSpec.Step step, List<StepStatus> dependencies) { - this(type, step, dependencies, null); - } + private final InstanceName instance; private StepStatus(StepType type, DeploymentSpec.Step step, List<StepStatus> dependencies, InstanceName instance) { this.type = requireNonNull(type); this.step = requireNonNull(step); this.dependencies = List.copyOf(dependencies); - this.instance = Optional.ofNullable(instance); + this.instance = instance; } /** The type of step this is. */ @@ -339,43 +385,48 @@ public class DeploymentStatus { /** The list of steps that need to be complete before this may start. */ public final List<StepStatus> dependencies() { return dependencies; } - /** The instance of this, if any. */ - public final Optional<InstanceName> instance() { return instance; } + /** The instance of this. */ + public final InstanceName instance() { return instance; } /** The id of the job this corresponds to, if any. */ public Optional<JobId> job() { return Optional.empty(); } /** The time at which this is, or was, complete on the given change and / or versions. */ - public abstract Optional<Instant> completedAt(Change change, Versions versions); + public Optional<Instant> completedAt(Change change) { return completedAt(change, Optional.empty()); } + + /** The time at which this is, or was, complete on the given change and / or versions. */ + abstract Optional<Instant> completedAt(Change change, Optional<JobId> dependent); + + /** The time at which this step is ready to run the specified change and / or versions. */ + public Optional<Instant> readyAt(Change change) { return readyAt(change, Optional.empty()); } /** The time at which this step is ready to run the specified change and / or versions. */ - public Optional<Instant> readyAt(Change change, Versions versions) { - return dependenciesCompletedAt(change, versions) + Optional<Instant> readyAt(Change change, Optional<JobId> dependent) { + return dependenciesCompletedAt(change, dependent) .map(ready -> Stream.of(blockedUntil(change), pausedUntil(), - coolingDownUntil(versions)) + coolingDownUntil(change)) .flatMap(Optional::stream) .reduce(ready, maxBy(naturalOrder()))); } /** The time at which all dependencies completed on the given change and / or versions. */ - public Optional<Instant> dependenciesCompletedAt(Change change, Versions versions) { - return dependencies.stream().allMatch(step -> step.completedAt(change, versions).isPresent()) - ? dependencies.stream().map(step -> step.completedAt(change, versions).get()) + Optional<Instant> dependenciesCompletedAt(Change change, Optional<JobId> dependent) { + return dependencies.stream().allMatch(step -> step.completedAt(change, dependent).isPresent()) + ? dependencies.stream().map(step -> step.completedAt(change, dependent).get()) .max(naturalOrder()) .or(() -> Optional.of(Instant.EPOCH)) : Optional.empty(); } /** The time until which this step is blocked by a change blocker. */ - // TODO jonmv: readyAt for instance can be delayed by block window. Upgrade policy / confidence is something different. public Optional<Instant> blockedUntil(Change change) { return Optional.empty(); } /** The time until which this step is paused by user intervention. */ public Optional<Instant> pausedUntil() { return Optional.empty(); } /** The time until which this step is cooling down, due to consecutive failures. */ - public Optional<Instant> coolingDownUntil(Versions versions) { return Optional.empty(); } + public Optional<Instant> coolingDownUntil(Change change) { return Optional.empty(); } /** Whether this step is declared in the deployment spec, or is an implicit step. */ public boolean isDeclared() { return true; } @@ -385,13 +436,58 @@ public class DeploymentStatus { private static class DelayStatus extends StepStatus { - private DelayStatus(DeploymentSpec.Delay step, List<StepStatus> dependencies) { - super(StepType.delay, step, dependencies); + private DelayStatus(DeploymentSpec.Delay step, List<StepStatus> dependencies, InstanceName instance) { + super(StepType.delay, step, dependencies, instance); } @Override - public Optional<Instant> completedAt(Change change, Versions versions) { - return readyAt(change, versions).map(completion -> completion.plus(step().delay())); + public Optional<Instant> completedAt(Change change, Optional<JobId> dependent) { + return readyAt(change, dependent).map(completion -> completion.plus(step().delay())); + } + + } + + + private static class InstanceStatus extends StepStatus { + + private final DeploymentInstanceSpec spec; + private final Instant now; + private final Instance instance; + + private InstanceStatus(DeploymentInstanceSpec spec, List<StepStatus> dependencies, Instant now, + Instance instance) { + super(StepType.instance, spec, dependencies, spec.name()); + this.spec = spec; + this.now = now; + this.instance = instance; + } + + /** Time of completion of its dependencies, if all parts of the given change are contained in the change for this instance. */ + @Override + public Optional<Instant> completedAt(Change change, Optional<JobId> dependent) { + return (change.platform().isEmpty() || change.platform().equals(instance.change().platform())) + && (change.application().isEmpty() || change.application().equals(instance.change().application())) + ? dependenciesCompletedAt(change, dependent) + : Optional.empty(); + } + + @Override + public Optional<Instant> blockedUntil(Change change) { + for (Instant current = now; now.plus(Duration.ofDays(7)).isAfter(current); ) { + boolean blocked = false; + for (DeploymentSpec.ChangeBlocker blocker : spec.changeBlocker()) { + while ( blocker.window().includes(current) + && now.plus(Duration.ofDays(7)).isAfter(current) + && ( change.platform().isPresent() && blocker.blocksVersions() + || change.application().isPresent() && blocker.blocksRevisions())) { + blocked = true; + current = current.plus(Duration.ofHours(1)).truncatedTo(ChronoUnit.HOURS); + } + } + if ( ! blocked) + return current == now ? Optional.empty() : Optional.of(current); + } + return Optional.of(Instant.MAX); } } @@ -418,11 +514,13 @@ public class DeploymentStatus { } @Override - public Optional<Instant> coolingDownUntil(Versions versions) { + public Optional<Instant> coolingDownUntil(Change change) { if (job.lastTriggered().isEmpty()) return Optional.empty(); if (job.lastCompleted().isEmpty()) return Optional.empty(); if (job.firstFailing().isEmpty()) return Optional.empty(); - if ( ! versions.targetsMatch(job.lastCompleted().get().versions())) return Optional.empty(); + Versions lastVersions = job.lastCompleted().get().versions(); + if (change.platform().isPresent() && ! change.platform().get().equals(lastVersions.targetPlatform())) return Optional.empty(); + if (change.application().isPresent() && ! change.application().get().equals(lastVersions.targetApplication())) return Optional.empty(); if (status.application.deploymentSpec().requireInstance(job.id().application().instance()).upgradePolicy() == DeploymentSpec.UpgradePolicy.canary) return Optional.empty(); if (job.id().type().environment().isTest() && job.isOutOfCapacity()) return Optional.empty(); @@ -445,20 +543,20 @@ public class DeploymentStatus { return new JobStepStatus(StepType.deployment, step, dependencies, job, status) { @Override - public Optional<Instant> readyAt(Change change, Versions versions) { - return super.readyAt(change, versions) - .filter(__ -> status.isTested(job.id(), versions)); + public Optional<Instant> readyAt(Change change, Optional<JobId> dependent) { + return super.readyAt(change, Optional.of(job.id())) + .filter(__ -> status.isTested(job.id(), change)); } /** Complete if deployment is on pinned version, and last successful deployment, or if given versions is strictly a downgrade, and this isn't forced by a pin. */ @Override - public Optional<Instant> completedAt(Change change, Versions versions) { + public Optional<Instant> completedAt(Change change, Optional<JobId> dependent) { if ( change.isPinned() && change.platform().isPresent() && ! existingDeployment.map(Deployment::version).equals(change.platform())) return Optional.empty(); - Change fullChange = status.application().change(); + Change fullChange = status.application().require(instance).change(); if (existingDeployment.map(deployment -> ! (change.upgrades(deployment.version()) || change.upgrades(deployment.applicationVersion())) && (fullChange.downgrades(deployment.version()) || fullChange.downgrades(deployment.applicationVersion()))) .orElse(false)) @@ -473,14 +571,15 @@ public class DeploymentStatus { } private static JobStepStatus ofProductionTest(DeclaredTest step, List<StepStatus> dependencies, - DeploymentStatus status, InstanceName instance, JobType testType, JobType jobType) { + DeploymentStatus status, InstanceName instance, JobType testType, JobType prodType) { JobStatus job = status.instanceJobs(instance).get(testType); return new JobStepStatus(StepType.test, step, dependencies, job, status) { @Override - public Optional<Instant> completedAt(Change change, Versions versions) { + public Optional<Instant> completedAt(Change change, Optional<JobId> dependent) { + Versions versions = Versions.from(change, status.application, status.deploymentFor(job.id()), status.systemVersion); return job.lastSuccess() .filter(run -> versions.targetsMatch(run.versions())) - .filter(run -> status.instanceJobs(instance).get(jobType).lastCompleted() + .filter(run -> status.instanceJobs(instance).get(prodType).lastCompleted() .map(last -> ! last.end().get().isAfter(run.start())).orElse(false)) .map(run -> run.end().get()); } @@ -493,9 +592,14 @@ public class DeploymentStatus { JobStatus job = status.instanceJobs(instance).get(jobType); return new JobStepStatus(StepType.test, step, dependencies, job, status) { @Override - public Optional<Instant> completedAt(Change change, Versions versions) { + public Optional<Instant> completedAt(Change change, Optional<JobId> dependent) { return RunList.from(job) - .on(versions) + .matching(run -> change.platform().map(run.versions().targetPlatform()::equals).orElse(true)) + .matching(run -> change.application().map(run.versions().targetApplication()::equals).orElse(true)) + .matching(run -> dependent.flatMap(status::deploymentFor) + .map(deployment -> Versions.from(change, deployment)) + .map(run.versions()::targetsMatch) + .orElse(true)) .status(RunStatus.success) .asList().stream() .map(run -> run.end().get()) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatusList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatusList.java index 6f4d947a3c5..c14493a0b72 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatusList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatusList.java @@ -28,63 +28,10 @@ public class DeploymentStatusList extends AbstractFilteringList<DeploymentStatus return new DeploymentStatusList(status, false); } - public ApplicationList asApplicationList() { - return ApplicationList.from(mapToList(DeploymentStatus::application)); - } - - public DeploymentStatusList forApplications(Collection<TenantAndApplicationId> applications) { - return matching(status -> applications.contains(status.application().id())); - } - - public DeploymentStatusList withProductionDeployment() { - return matching(status -> status.application().productionDeployments().values().stream() - .anyMatch(deployments -> ! deployments.isEmpty())); - } - - public DeploymentStatusList failing() { - return matching(DeploymentStatus::hasFailures); - } - - public DeploymentStatusList failingUpgrade() { - return matching(status -> status.instanceJobs().values().stream() - .anyMatch(jobs -> ! jobs.failing().not().failingApplicationChange().isEmpty())); - } - - /** Returns the subset of applications which are upgrading (to any version), not considering block windows. */ - public DeploymentStatusList upgrading() { - return matching(status -> status.application().change().platform().isPresent()); - } - - /** Returns the subset of applications which are currently upgrading to the given version */ - public DeploymentStatusList upgradingTo(Version version) { - return upgradingTo(List.of(version)); - } - - /** Returns the subset of applications which are currently upgrading to the given version */ - public DeploymentStatusList upgradingTo(Collection<Version> versions) { - return matching(status -> versions.stream().anyMatch(version -> status.application().change().platform().equals(Optional.of(version)))); - } - - /** Returns the subset of applications which are not pinned to a certain Vespa version. */ - public DeploymentStatusList unpinned() { - return matching(status -> ! status.application().change().isPinned()); - } - - /** Returns the subset of applications which has the given upgrade policy */ - // TODO jonmv: Make this instance based when instances are orchestrated, and deployments reported per instance. - public DeploymentStatusList with(DeploymentSpec.UpgradePolicy policy) { - return matching(status -> status.application().deploymentSpec().instances().stream() - .anyMatch(instance -> instance.upgradePolicy() == policy)); - } - /** Returns the subset of applications which have changes left to deploy; blocked, or deploying */ public DeploymentStatusList withChanges() { - return matching(status -> status.application().change().hasTargets() || status.application().outstandingChange().hasTargets()); - } - - /** Returns the subset of applications which are currently deploying a change */ - public DeploymentStatusList deploying() { - return matching(status -> status.application().change().hasTargets()); + return matching(status -> status.application().instances().values().stream() + .anyMatch(instance -> instance.change().hasTargets() || status.outstandingChange(instance.name()).hasTargets())); } /** Returns the subset of applications which have been failing an upgrade to the given version since the given instant */ @@ -99,24 +46,6 @@ public class DeploymentStatusList extends AbstractFilteringList<DeploymentStatus .anyMatch(jobs -> failingApplicationChangeSince(jobs, threshold))); } - /** Returns the subset of applications which currently have failing jobs on the given version */ - public DeploymentStatusList failingOn(Version version) { - return matching(status -> status.instanceJobs().values().stream() - .anyMatch(jobs -> failingOn(jobs, version))); - } - - /** Returns the subset of applications which started failing on the given version */ - public DeploymentStatusList startedFailingOn(Version version) { - return matching(status -> status.instanceJobs().values().stream() - .anyMatch(jobs -> ! jobs.firstFailing().on(version).isEmpty())); - } - - private static boolean failingOn(JobList jobs, Version version) { - return ! jobs.failing() - .lastCompleted().on(version) - .isEmpty(); - } - private static boolean failingUpgradeToVersionSince(JobList jobs, Version version, Instant threshold) { return ! jobs.not().failingApplicationChange() .firstFailing().endedBefore(threshold) 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 1e02c9751d4..51841396d6a 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 @@ -4,8 +4,7 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.config.provision.InstanceName; import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; @@ -18,6 +17,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.ApplicationList; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; +import com.yahoo.vespa.hosted.controller.application.InstanceList; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import java.time.Clock; @@ -31,13 +31,9 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.OptionalLong; -import java.util.function.Predicate; import java.util.function.Supplier; import java.util.logging.Logger; -import java.util.stream.Stream; -import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest; -import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; import static java.util.Comparator.comparing; import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.partitioningBy; @@ -45,7 +41,7 @@ import static java.util.stream.Collectors.toList; /** * Responsible for scheduling deployment jobs in a build system and keeping - * {@link Application#change()} in sync with what is scheduled. + * {@link Instance#change()} in sync with what is scheduled. * * This class is multi-thread safe. * @@ -80,19 +76,39 @@ public class DeploymentTrigger { } applications().lockApplicationOrThrow(id, application -> { - if (acceptNewApplicationVersion(application.get())) { - application = application.withChange(application.get().change().with(version)) - .withOutstandingChange(Change.empty()); - for (Run run : jobs.active(id)) - if ( ! run.id().type().environment().isManuallyDeployed()) - jobs.abort(run.id()); - } - else - application = application.withOutstandingChange(Change.of(version)); - application = application.withProjectId(OptionalLong.of(projectId)); application = application.withNewSubmission(version); - applications().store(application.withChange(remainingChange(application.get()))); + applications().store(application); + }); + triggerNewRevision(id); + } + + /** + * Propagates the latest revision to ready instances. + * Ready instances are those whose dependencies are complete, and which aren't blocked, and, additionally, + * which aren't upgrading, or are already deploying an application change, or failing upgrade. + */ + public void triggerNewRevision(TenantAndApplicationId id) { + applications().lockApplicationIfPresent(id, application -> { + DeploymentStatus status = jobs.deploymentStatus(application.get()); + for (InstanceName instanceName : application.get().deploymentSpec().instanceNames()) { + Change outstanding = status.outstandingChange(instanceName); + if ( outstanding.hasTargets() + && status.instanceSteps().get(instanceName) + .readyAt(outstanding) + .map(readyAt -> ! readyAt.isAfter(clock.instant())).orElse(false) + && acceptNewApplicationVersion(status, instanceName)) { + application = application.with(instanceName, + instance -> { + instance = instance.withChange(instance.change().with(outstanding.application().get())); + return instance.withChange(remainingChange(instance, status)); + }); + for (Run run : jobs.active(application.get().id().instance(instanceName))) + if ( ! run.id().type().environment().isManuallyDeployed()) + jobs.abort(run.id()); + } + } + applications().store(application); }); } @@ -107,7 +123,8 @@ public class DeploymentTrigger { } applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> - applications().store(application.withChange(remainingChange(application.get())))); + applications().store(application.with(id.instance(), + instance -> instance.withChange(remainingChange(instance, jobs.deploymentStatus(application.get())))))); } /** @@ -156,16 +173,15 @@ public class DeploymentTrigger { public List<JobId> forceTrigger(ApplicationId applicationId, JobType jobType, String user, boolean requireTests) { Application application = applications().requireApplication(TenantAndApplicationId.from(applicationId)); Instance instance = application.require(applicationId.instance()); - Versions versions = Versions.from(application.change(), application, deploymentFor(instance, jobType), - controller.systemVersion()); - String reason = "Job triggered manually by " + user; DeploymentStatus status = jobs.deploymentStatus(application); JobId job = new JobId(instance.id(), jobType); + Versions versions = Versions.from(instance.change(), application, status.deploymentFor(job), controller.systemVersion()); + String reason = "Job triggered manually by " + user; Map<JobId, List<Versions>> jobs = status.testJobs(Map.of(job, versions)); - if (jobs.isEmpty() || ! requireTests || ! jobType.isProduction()) + if (jobs.isEmpty() || ! requireTests) jobs = Map.of(job, List.of(versions)); jobs.forEach((jobId, versionsList) -> { - trigger(deploymentJob(instance, versionsList.get(0), application.change(), jobId.type(), status.jobs().get(jobId).get(), reason, clock.instant())); + trigger(deploymentJob(instance, versionsList.get(0), jobId.type(), status.jobs().get(jobId).get(), reason, clock.instant())); }); return List.copyOf(jobs.keySet()); } @@ -181,35 +197,35 @@ public class DeploymentTrigger { } /** Triggers a change of this application, unless it already has a change. */ - public void triggerChange(TenantAndApplicationId applicationId, Change change) { - applications().lockApplicationOrThrow(applicationId, application -> { - if ( ! application.get().change().hasTargets()) - forceChange(applicationId, change); + public void triggerChange(ApplicationId instanceId, Change change) { + applications().lockApplicationOrThrow(TenantAndApplicationId.from(instanceId), application -> { + if ( ! application.get().require(instanceId.instance()).change().hasTargets()) + forceChange(instanceId, change); }); } - /** Overrides the given application's platform and application changes with any contained in the given change. */ - public void forceChange(TenantAndApplicationId applicationId, Change change) { - applications().lockApplicationOrThrow(applicationId, application -> { - if (change.application().isPresent()) - application = application.withOutstandingChange(Change.empty()); - applications().store(application.withChange(change.onTopOf(application.get().change()))); + /** Overrides the given instance's platform and application changes with any contained in the given change. */ + public void forceChange(ApplicationId instanceId, Change change) { + applications().lockApplicationOrThrow(TenantAndApplicationId.from(instanceId), application -> { + applications().store(application.with(instanceId.instance(), + instance -> instance.withChange(change.onTopOf(application.get().require(instanceId.instance()).change())))); }); } /** Cancels the indicated part of the given application's change. */ - public void cancelChange(TenantAndApplicationId applicationId, ChangesToCancel cancellation) { - applications().lockApplicationOrThrow(applicationId, application -> { + public void cancelChange(ApplicationId instanceId, ChangesToCancel cancellation) { + applications().lockApplicationOrThrow(TenantAndApplicationId.from(instanceId), application -> { Change change; switch (cancellation) { case ALL: change = Change.empty(); break; case VERSIONS: change = Change.empty().withPin(); break; - case PLATFORM: change = application.get().change().withoutPlatform(); break; - case APPLICATION: change = application.get().change().withoutApplication(); break; - case PIN: change = application.get().change().withoutPin(); break; + case PLATFORM: change = application.get().require(instanceId.instance()).change().withoutPlatform(); break; + case APPLICATION: change = application.get().require(instanceId.instance()).change().withoutApplication(); break; + case PIN: change = application.get().require(instanceId.instance()).change().withoutPin(); break; default: throw new IllegalArgumentException("Unknown cancellation choice '" + cancellation + "'!"); } - applications().store(application.withChange(change)); + applications().store(application.with(instanceId.instance(), + instance -> instance.withChange(change))); }); } @@ -229,49 +245,44 @@ public class DeploymentTrigger { /** Returns the set of all jobs which have changes to propagate from the upstream steps. */ private List<Job> computeReadyJobs() { - return ApplicationList.from(applications().asList()) - .withProjectId() // Need to keep this, as we have applications with deployment spec that shouldn't be orchestrated. - .withChanges() - .withDeploymentSpec() - .idList().stream() - .map(this::computeReadyJobs) - .flatMap(Collection::stream) - .collect(toList()); + return jobs.deploymentStatuses(ApplicationList.from(applications().asList()) + .withProjectId() // Need to keep this, as we have applications with deployment spec that shouldn't be orchestrated. + .withDeploymentSpec()) + .withChanges() + .asList().stream() + .map(this::computeReadyJobs) + .flatMap(Collection::stream) + .collect(toList()); } /** * Finds the next step to trigger for the given application, if any, and returns these as a list. */ - private List<Job> computeReadyJobs(TenantAndApplicationId id) { + private List<Job> computeReadyJobs(DeploymentStatus status) { List<Job> jobs = new ArrayList<>(); - applications().getApplication(id).map(controller.jobController()::deploymentStatus).ifPresent(status -> { - status.jobsToRun().forEach((job, versionsList) -> { - for (Versions versions : versionsList) - status.jobSteps().get(job).readyAt(status.application().change(), versions) - .filter(readyAt -> ! clock.instant().isBefore(readyAt)) - .ifPresent(readyAt -> { - if ( ! ( isSuspendedInAnotherZone(status.application().require(job.application().instance()), - job.type().zone(controller.system())) - && job.type().environment() == Environment.prod) - && ! status.jobs().get(job).get().isRunning()) - jobs.add(deploymentJob(status.application().require(job.application().instance()), - versions, - status.application().change(), - job.type(), - status.instanceJobs(job.application().instance()).get(job.type()), - "unknown reason", - readyAt)); - }); - }); + status.jobsToRun().forEach((job, versionsList) -> { + for (Versions versions : versionsList) + status.jobSteps().get(job).readyAt(status.application().require(job.application().instance()).change()) + .filter(readyAt -> ! clock.instant().isBefore(readyAt)) + .filter(__ -> ! status.jobs().get(job).get().isRunning()) + .filter(__ -> ! (job.type().isProduction() && isSuspendedInAnotherZone(status.application(), job))) + .ifPresent(readyAt -> { + jobs.add(deploymentJob(status.application().require(job.application().instance()), + versions, + job.type(), + status.instanceJobs(job.application().instance()).get(job.type()), + "unknown reason", + readyAt)); + }); }); return Collections.unmodifiableList(jobs); } /** Returns whether given job should be triggered */ - private boolean isSuspendedInAnotherZone(Instance instance, ZoneId zone) { - for (Deployment deployment : instance.productionDeployments().values()) { - if ( ! deployment.zone().equals(zone) - && controller.applications().isSuspended(new DeploymentId(instance.id(), deployment.zone()))) + private boolean isSuspendedInAnotherZone(Application application, JobId job) { + for (Deployment deployment : application.require(job.application().instance()).productionDeployments().values()) { + if ( ! deployment.zone().equals(job.type().zone(controller.system())) + && controller.applications().isSuspended(new DeploymentId(job.application(), deployment.zone()))) return true; } return false; @@ -346,28 +357,25 @@ public class DeploymentTrigger { // ---------- Change management o_O ---------- - private boolean acceptNewApplicationVersion(Application application) { - if ( ! application.deploymentSpec().instances().stream() - .allMatch(instance -> instance.canChangeRevisionAt(clock.instant()))) return false; - if (application.change().application().isPresent()) return true; // Replacing a previous application change is ok. - if (jobs.deploymentStatus(application).hasFailures()) return true; // Allow changes to fix upgrade problems. - return application.change().platform().isEmpty(); + private boolean acceptNewApplicationVersion(DeploymentStatus status, InstanceName instance) { + if (status.application().require(instance).change().application().isPresent()) return true; // Replacing a previous application change is ok. + if (status.hasFailures()) return true; // Allow changes to fix upgrade problems. + return status.application().require(instance).change().platform().isEmpty(); } - private Change remainingChange(Application application) { - Change change = application.change(); - DeploymentStatus status = jobs.deploymentStatus(application); - if (status.jobsToRun(status.application().change().withoutApplication()).isEmpty()) + private Change remainingChange(Instance instance, DeploymentStatus status) { + Change change = instance.change(); + if (status.jobsToRun(Map.of(instance.name(), instance.change().withoutApplication())).isEmpty()) change = change.withoutPlatform(); - if (status.jobsToRun(status.application().change().withoutPlatform()).isEmpty()) + if (status.jobsToRun(Map.of(instance.name(), instance.change().withoutPlatform())).isEmpty()) change = change.withoutApplication(); return change; } // ---------- Version and job helpers ---------- - private Job deploymentJob(Instance instance, Versions versions, Change change, JobType jobType, JobStatus jobStatus, String reason, Instant availableSince) { - return new Job(instance, versions, jobType, availableSince, jobStatus.isOutOfCapacity(), change.application().isPresent()); + private Job deploymentJob(Instance instance, Versions versions, JobType jobType, JobStatus jobStatus, String reason, Instant availableSince) { + return new Job(instance, versions, jobType, availableSince, jobStatus.isOutOfCapacity(), instance.change().application().isPresent()); } // ---------- Data containers ---------- diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 5798a11758d..20c64751335 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -588,7 +588,7 @@ public class InternalStepRunner implements StepRunner { private void sendNotification(Run run, DualLogger logger) { Application application = controller.applications().requireApplication(TenantAndApplicationId.from(run.id().application())); Notifications notifications = application.deploymentSpec().requireInstance(run.id().application().instance()).notifications(); - boolean newCommit = application.change().application() + boolean newCommit = application.require(run.id().application().instance()).change().application() .map(run.versions().targetApplication()::equals) .orElse(false); When when = newCommit ? failingCommit : failing; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 61b86df6190..f9ad2ebf553 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -270,7 +270,7 @@ public class JobController { .collect(toUnmodifiableList()); } - /** Returns a list of all active runs for the given instance. */ + /** Returns a list of all active runs for the given application. */ public List<Run> active(TenantAndApplicationId id) { return copyOf(controller.applications().requireApplication(id).instances().keySet().stream() .flatMap(name -> Stream.of(JobType.values()) @@ -280,6 +280,15 @@ public class JobController { .iterator()); } + /** Returns a list of all active runs for the given instance. */ + public List<Run> active(ApplicationId id) { + return copyOf(Stream.of(JobType.values()) + .map(type -> last(id, type)) + .flatMap(Optional::stream) + .filter(run -> ! run.hasEnded()) + .iterator()); + } + /** Returns the job status of the given job, possibly empty. */ public JobStatus jobStatus(JobId id) { return new JobStatus(id, runs(id)); @@ -292,7 +301,8 @@ public class JobController { .collect(toUnmodifiableMap(job -> job, job -> jobStatus(job))), controller.system(), - controller.systemVersion()); + controller.systemVersion(), + controller.clock().instant()); } /** Adds deployment status to each of the given applications. */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java index 87d4f3f0b9a..c4135120e3d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java @@ -96,11 +96,6 @@ public class Versions { targetApplication.id()); } - /** Create versions using change contained in application */ - public static Versions from(Application application, Version defaultPlatformVersion) { - return from(application.change(), application, Optional.empty(), defaultPlatformVersion); - } - /** Create versions using given change and application */ public static Versions from(Change change, Application application, Optional<Deployment> deployment, Version defaultPlatformVersion) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java index e43877cb7e5..ec209c5ed98 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java @@ -60,8 +60,8 @@ public class ControllerMaintenance extends AbstractComponent { deploymentExpirer = new DeploymentExpirer(controller, maintenanceInterval, jobControl); deploymentIssueReporter = new DeploymentIssueReporter(controller, controller.serviceRegistry().deploymentIssues(), maintenanceInterval, jobControl); metricsReporter = new MetricsReporter(controller, metric, jobControl); - outstandingChangeDeployer = new OutstandingChangeDeployer(controller, Duration.ofMinutes(1), jobControl); - versionStatusUpdater = new VersionStatusUpdater(controller, Duration.ofMinutes(1), jobControl); + outstandingChangeDeployer = new OutstandingChangeDeployer(controller, Duration.ofMinutes(3), jobControl); + versionStatusUpdater = new VersionStatusUpdater(controller, Duration.ofMinutes(3), jobControl); upgrader = new Upgrader(controller, maintenanceInterval, jobControl, curator); readyJobsTrigger = new ReadyJobsTrigger(controller, Duration.ofMinutes(1), jobControl); clusterInfoMaintainer = new ClusterInfoMaintainer(controller, Duration.ofHours(2), jobControl); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java index b130f7107dd..37ecef8b41e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java @@ -1,9 +1,18 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; +import com.yahoo.config.application.api.DeploymentInstanceSpec; +import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.InstanceName; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.application.ApplicationList; +import com.yahoo.vespa.hosted.controller.application.Change; +import com.yahoo.vespa.hosted.controller.application.InstanceList; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatusList; import java.time.Duration; @@ -20,14 +29,11 @@ public class OutstandingChangeDeployer extends Maintainer { @Override protected void maintain() { - for (Application application : controller().applications().asList()) { - if ( application.outstandingChange().hasTargets() - && application.deploymentSpec().instances().stream() - .allMatch(instance -> instance.canChangeRevisionAt(controller().clock().instant()))) { - controller().applications().deploymentTrigger().triggerChange(application.id(), - application.outstandingChange()); - } - } + for (Application application : ApplicationList.from(controller().applications().asList()) + .withProductionDeployment() + .withDeploymentSpec() + .asList()) + controller().applications().deploymentTrigger().triggerNewRevision(application.id()); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java index 49a505475c6..da24d758320 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java @@ -3,19 +3,18 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.curator.Lock; -import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.application.ApplicationList; import com.yahoo.vespa.hosted.controller.application.Change; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatusList; +import com.yahoo.vespa.hosted.controller.application.InstanceList; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import java.time.Duration; import java.util.Collection; -import java.util.Comparator; import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; @@ -26,6 +25,8 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.PLATFORM; +import static java.util.Comparator.comparing; +import static java.util.Comparator.naturalOrder; /** * Maintenance job which schedules applications for Vespa version upgrade @@ -57,35 +58,35 @@ public class Upgrader extends Maintainer { // Cancel upgrades to broken targets (let other ongoing upgrades complete to avoid starvation) for (VespaVersion version : controller().versionStatus().versions()) { if (version.confidence() == Confidence.broken) - cancelUpgradesOf(applications().upgradingTo(version.versionNumber()) - .not().with(UpgradePolicy.canary), + cancelUpgradesOf(instances().upgradingTo(version.versionNumber()) + .not().with(UpgradePolicy.canary), version.versionNumber() + " is broken"); } // Canaries should always try the canary target - cancelUpgradesOf(applications().upgrading() - .not().upgradingTo(canaryTarget) - .with(UpgradePolicy.canary), + cancelUpgradesOf(instances().upgrading() + .not().upgradingTo(canaryTarget) + .with(UpgradePolicy.canary), "Outdated target version for Canaries"); // Cancel *failed* upgrades to earlier versions, as the new version may fix it String reason = "Failing on outdated version"; - cancelUpgradesOf(applications().upgrading() - .failing() - .not().upgradingTo(defaultTargets) - .with(UpgradePolicy.defaultPolicy), + cancelUpgradesOf(instances().upgrading() + .failing() + .not().upgradingTo(defaultTargets) + .with(UpgradePolicy.defaultPolicy), reason); - cancelUpgradesOf(applications().upgrading() - .failing() - .not().upgradingTo(conservativeTargets) - .with(UpgradePolicy.conservative), + cancelUpgradesOf(instances().upgrading() + .failing() + .not().upgradingTo(conservativeTargets) + .with(UpgradePolicy.conservative), reason); // Schedule the right upgrades - DeploymentStatusList applications = applications(); - upgrade(applications.with(UpgradePolicy.canary), canaryTarget, applications.size()); - defaultTargets.forEach(target -> upgrade(applications.with(UpgradePolicy.defaultPolicy), target, numberOfApplicationsToUpgrade())); - conservativeTargets.forEach(target -> upgrade(applications.with(UpgradePolicy.conservative), target, numberOfApplicationsToUpgrade())); + InstanceList instances = instances(); + upgrade(instances.with(UpgradePolicy.canary), canaryTarget, instances.size()); + defaultTargets.forEach(target -> upgrade(instances.with(UpgradePolicy.defaultPolicy), target, numberOfApplicationsToUpgrade())); + conservativeTargets.forEach(target -> upgrade(instances.with(UpgradePolicy.conservative), target, numberOfApplicationsToUpgrade())); } /** Returns the target versions for given confidence, one per major version in the system */ @@ -97,33 +98,34 @@ public class Upgrader extends Maintainer { .map(VespaVersion::versionNumber) .collect(Collectors.toMap(Version::getMajor, // Key on major version Function.identity(), // Use version as value - BinaryOperator.<Version>maxBy(Comparator.naturalOrder()))) // Pick highest version when merging versions within this major + BinaryOperator.<Version>maxBy(naturalOrder()))) // Pick highest version when merging versions within this major .values(); } - /** Returns a list of all applications, except those which are pinned — these should not be manipulated by the Upgrader */ - private DeploymentStatusList applications() { - return controller().jobController().deploymentStatuses(ApplicationList.from(controller().applications().asList())).unpinned(); + /** Returns a list of all production application instances, except those which are pinned, which we should not manipulate here. */ + private InstanceList instances() { + return InstanceList.from(controller().jobController().deploymentStatuses(ApplicationList.from(controller().applications().asList()))) + .withProductionDeployment() + .unpinned(); } - private void upgrade(DeploymentStatusList statuses, Version version, int numberToUpgrade) { - statuses.withProductionDeployment() - .not().deploying() // wait with applications deploying an application change or already upgrading - .not().failingOn(version) // try to upgrade only if it hasn't failed on this version - .asApplicationList() - .onLowerVersionThan(version) - .allowMajorVersion(version.getMajor(), targetMajorVersion().orElse(version.getMajor())) - .canUpgradeAt(controller().clock().instant()) // wait with applications that are currently blocking upgrades - .byIncreasingDeployedVersion() // start with lowest versions - .first(numberToUpgrade) - .asList().forEach(application -> controller().applications().deploymentTrigger().triggerChange(application.id(), Change.of(version))); + private void upgrade(InstanceList instances, Version version, int numberToUpgrade) { + instances.not().failingOn(version) + .allowMajorVersion(version.getMajor(), targetMajorVersion().orElse(version.getMajor())) + .not().deploying() + .onLowerVersionThan(version) + .canUpgradeAt(version, controller().clock().instant()) + .byIncreasingDeployedVersion() + .first(numberToUpgrade).asList() + .forEach(instance -> controller().applications().deploymentTrigger().triggerChange(instance, Change.of(version))); } - private void cancelUpgradesOf(DeploymentStatusList applications, String reason) { - if (applications.isEmpty()) return; - log.info("Cancelling upgrading of " + applications.asList().size() + " applications: " + reason); - for (Application application : applications.asApplicationList().asList()) - controller().applications().deploymentTrigger().cancelChange(application.id(), PLATFORM); + private void cancelUpgradesOf(InstanceList instances, String reason) { + instances = instances.unpinned(); + if (instances.isEmpty()) return; + log.info("Cancelling upgrading of " + instances.asList().size() + " instances: " + reason); + for (ApplicationId instance : instances.asList()) + controller().applications().deploymentTrigger().cancelChange(instance, PLATFORM); } /** Returns the number of applications to upgrade in this run */ 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 3758fb8476d..acbae53ad2c 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 @@ -82,9 +82,7 @@ public class ApplicationSerializer { private static final String deployingField = "deployingField"; private static final String projectIdField = "projectId"; private static final String latestVersionField = "latestVersion"; - private static final String builtInternallyField = "builtInternally"; private static final String pinnedField = "pinned"; - private static final String outstandingChangeField = "outstandingChangeField"; private static final String deploymentIssueField = "deploymentIssueId"; private static final String ownershipIssueIdField = "ownershipIssueId"; private static final String ownerField = "confirmedOwner"; @@ -175,9 +173,6 @@ public class ApplicationSerializer { application.projectId().ifPresent(projectId -> root.setLong(projectIdField, projectId)); application.deploymentIssueId().ifPresent(jiraIssueId -> root.setString(deploymentIssueField, jiraIssueId.value())); application.ownershipIssueId().ifPresent(issueId -> root.setString(ownershipIssueIdField, issueId.value())); - root.setBool(builtInternallyField, true); // TODO jonmv: remove when the change with this comment has deployed. - toSlime(application.change(), root, deployingField); - toSlime(application.outstandingChange(), root, outstandingChangeField); application.owner().ifPresent(owner -> root.setString(ownerField, owner.username())); application.majorVersion().ifPresent(majorVersion -> root.setLong(majorVersionField, majorVersion)); root.setDouble(queryQualityField, application.metrics().queryServiceQuality()); @@ -196,6 +191,7 @@ public class ApplicationSerializer { toSlime(instance.jobPauses(), instanceObject.setObject(deploymentJobsField)); assignedRotationsToSlime(instance.rotations(), instanceObject, assignedRotationsField); toSlime(instance.rotationStatus(), instanceObject.setArray(rotationStatusField)); + toSlime(instance.change(), instanceObject, deployingField); } } @@ -339,8 +335,6 @@ public class ApplicationSerializer { Instant createdAt = Instant.ofEpochMilli(root.field(createdAtField).asLong()); DeploymentSpec deploymentSpec = DeploymentSpec.fromXml(root.field(deploymentSpecField).asString(), false); ValidationOverrides validationOverrides = ValidationOverrides.fromXml(root.field(validationOverridesField).asString()); - Change deploying = changeFromSlime(root.field(deployingField)); - Change outstandingChange = changeFromSlime(root.field(outstandingChangeField)); Optional<IssueId> deploymentIssueId = Serializers.optionalString(root.field(deploymentIssueField)).map(IssueId::from); Optional<IssueId> ownershipIssueId = Serializers.optionalString(root.field(ownershipIssueIdField)).map(IssueId::from); Optional<User> owner = Serializers.optionalString(root.field(ownerField)).map(User::from); @@ -352,7 +346,7 @@ public class ApplicationSerializer { OptionalLong projectId = Serializers.optionalLong(root.field(projectIdField)); Optional<ApplicationVersion> latestVersion = latestVersionFromSlime(root.field(latestVersionField)); - return new Application(id, createdAt, deploymentSpec, validationOverrides, deploying, outstandingChange, + return new Application(id, createdAt, deploymentSpec, validationOverrides, deploymentIssueId, ownershipIssueId, owner, majorVersion, metrics, deployKeys, projectId, latestVersion, instances); } @@ -372,11 +366,13 @@ public class ApplicationSerializer { Map<JobType, Instant> jobPauses = jobPausesFromSlime(object.field(deploymentJobsField)); List<AssignedRotation> assignedRotations = assignedRotationsFromSlime(deploymentSpec, instanceName, object); RotationStatus rotationStatus = rotationStatusFromSlime(object); + Change change = changeFromSlime(object.field(deployingField)); instances.add(new Instance(id.instance(instanceName), deployments, jobPauses, assignedRotations, - rotationStatus)); + rotationStatus, + change)); }); return instances; } 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 0f92ecde133..0445b6c6230 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 @@ -214,13 +214,13 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return application(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deployment")) return JobControllerApiHandlerHelper.overviewResponse(controller, TenantAndApplicationId.from(path.get("tenant"), path.get("application")), request.getUri()); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/package")) return applicationPackage(path.get("tenant"), path.get("application"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return deploying(path.get("tenant"), path.get("application"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deploying(path.get("tenant"), path.get("application"), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return deploying(path.get("tenant"), path.get("application"), "default", request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deploying(path.get("tenant"), path.get("application"), "default", request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/metering")) return metering(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance")) return applications(path.get("tenant"), Optional.of(path.get("application")), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}")) return instance(path.get("tenant"), path.get("application"), path.get("instance"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying")) return deploying(path.get("tenant"), path.get("application"), request); // TODO jonmv: remove - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/pin")) return deploying(path.get("tenant"), path.get("application"), request); // TODO jonmv: remove + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying")) return deploying(path.get("tenant"), path.get("application"), path.get("instance"), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/pin")) return deploying(path.get("tenant"), path.get("application"), path.get("instance"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job")) return JobControllerApiHandlerHelper.jobTypeResponse(controller, appIdFromPath(path), request.getUri()); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return JobControllerApiHandlerHelper.runResponse(controller.jobController().runs(appIdFromPath(path), jobTypeFromPath(path)), request.getUri()); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/test-config")) return testConfig(appIdFromPath(path), jobTypeFromPath(path)); @@ -257,16 +257,16 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}")) return createTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/key")) return addDeveloperKey(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return createApplication(path.get("tenant"), path.get("application"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/platform")) return deployPlatform(path.get("tenant"), path.get("application"), false, request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), true, request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/platform")) return deployPlatform(path.get("tenant"), path.get("application"), "default", false, request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), "default", true, request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"), "default", request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/key")) return addDeployKey(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/submit")) return submit(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}")) return createInstance(path.get("tenant"), path.get("application"), path.get("instance"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploy/{jobtype}")) return jobDeploy(appIdFromPath(path), jobTypeFromPath(path), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/platform")) return deployPlatform(path.get("tenant"), path.get("application"), false, request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), true, request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/platform")) return deployPlatform(path.get("tenant"), path.get("application"), path.get("instance"), false, request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), path.get("instance"), true, request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"), path.get("instance"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/submit")) return submit(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return trigger(appIdFromPath(path), jobTypeFromPath(path), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/pause")) return pause(appIdFromPath(path), jobTypeFromPath(path)); @@ -289,12 +289,12 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}")) return deleteTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/key")) return removeDeveloperKey(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return deleteApplication(path.get("tenant"), path.get("application"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return cancelDeploy(path.get("tenant"), path.get("application"), "all"); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/{choice}")) return cancelDeploy(path.get("tenant"), path.get("application"), path.get("choice")); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return cancelDeploy(path.get("tenant"), path.get("application"), "default", "all"); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/{choice}")) return cancelDeploy(path.get("tenant"), path.get("application"), "default", path.get("choice")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/key")) return removeDeployKey(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}")) return deleteInstance(path.get("tenant"), path.get("application"), path.get("instance"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying")) return cancelDeploy(path.get("tenant"), path.get("application"), "all"); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/{choice}")) return cancelDeploy(path.get("tenant"), path.get("application"), path.get("choice")); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying")) return cancelDeploy(path.get("tenant"), path.get("application"), path.get("instance"), "all"); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/{choice}")) return cancelDeploy(path.get("tenant"), path.get("application"), path.get("instance"), path.get("choice")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return JobControllerApiHandlerHelper.abortJobResponse(controller.jobController(), appIdFromPath(path), jobTypeFromPath(path)); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}")) return deactivate(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/global-rotation/override")) return setGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), true, request); @@ -699,24 +699,27 @@ public class ApplicationApiHandler extends LoggingRequestHandler { "/job/", request.getUri()).toString()); + DeploymentStatus status = controller.jobController().deploymentStatus(application); application.latestVersion().ifPresent(version -> toSlime(version, object.setObject("latestVersion"))); application.projectId().ifPresent(id -> object.setLong("projectId", id)); - // Currently deploying change - if ( ! application.change().isEmpty()) - toSlime(object.setObject("deploying"), application.change()); + // TODO jonmv: Remove this when users are updated. + application.instances().values().stream().findFirst().ifPresent(instance -> { + // Currently deploying change + if ( ! instance.change().isEmpty()) + toSlime(object.setObject("deploying"), instance.change()); - // Outstanding change - if ( ! application.outstandingChange().isEmpty()) - toSlime(object.setObject("outstandingChange"), application.outstandingChange()); + // Outstanding change + if ( ! status.outstandingChange(instance.name()).isEmpty()) + toSlime(object.setObject("outstandingChange"), status.outstandingChange(instance.name())); + }); // Compile version. The version that should be used when building an application object.setString("compileVersion", compileVersion(application.id()).toFullString()); application.majorVersion().ifPresent(majorVersion -> object.setLong("majorVersion", majorVersion)); - DeploymentStatus status = controller.jobController().deploymentStatus(application); Cursor instancesArray = object.setArray("instances"); for (Instance instance : application.instances().values()) toSlime(instancesArray.addObject(), status, instance, application.deploymentSpec(), request); @@ -749,6 +752,12 @@ public class ApplicationApiHandler extends LoggingRequestHandler { .steps(deploymentSpec.requireInstance(instance.name())) .sortedJobs(status.instanceJobs(instance.name()).values()); + if ( ! instance.change().isEmpty()) + toSlime(object.setObject("deploying"), instance.change()); + + // Outstanding change + if ( ! status.outstandingChange(instance.name()).isEmpty()) + toSlime(object.setObject("outstandingChange"), status.outstandingChange(instance.name())); Cursor deploymentJobsArray = object.setArray("deploymentJobs"); for (JobStatus job : jobStatus) { @@ -849,22 +858,19 @@ public class ApplicationApiHandler extends LoggingRequestHandler { application.projectId().ifPresent(id -> object.setLong("projectId", id)); - // Currently deploying change - if ( ! application.change().isEmpty()) { - toSlime(object.setObject("deploying"), application.change()); - } - - // Outstanding change - if ( ! application.outstandingChange().isEmpty()) { - toSlime(object.setObject("outstandingChange"), application.outstandingChange()); - } - if (application.deploymentSpec().instance(instance.name()).isPresent()) { // Jobs sorted according to deployment spec List<JobStatus> jobStatus = controller.applications().deploymentTrigger() .steps(application.deploymentSpec().requireInstance(instance.name())) .sortedJobs(status.instanceJobs(instance.name()).values()); + if ( ! instance.change().isEmpty()) + toSlime(object.setObject("deploying"), instance.change()); + + // Outstanding change + if ( ! status.outstandingChange(instance.name()).isEmpty()) + toSlime(object.setObject("outstandingChange"), status.outstandingChange(instance.name())); + Cursor deploymentsArray = object.setArray("deploymentJobs"); for (JobStatus job : jobStatus) { Cursor jobObject = deploymentsArray.addObject(); @@ -1251,14 +1257,14 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return new SlimeJsonResponse(slime); } - private HttpResponse deploying(String tenant, String application, HttpRequest request) { - Application app = controller.applications().requireApplication(TenantAndApplicationId.from(tenant, application)); + private HttpResponse deploying(String tenantName, String applicationName, String instanceName, HttpRequest request) { + Instance instance = controller.applications().requireInstance(ApplicationId.from(tenantName, applicationName, instanceName)); Slime slime = new Slime(); Cursor root = slime.setObject(); - if ( ! app.change().isEmpty()) { - app.change().platform().ifPresent(version -> root.setString("platform", version.toString())); - app.change().application().ifPresent(applicationVersion -> root.setString("application", applicationVersion.id())); - root.setBool("pinned", app.change().isPinned()); + if ( ! instance.change().isEmpty()) { + instance.change().platform().ifPresent(version -> root.setString("platform", version.toString())); + instance.change().application().ifPresent(applicationVersion -> root.setString("application", applicationVersion.id())); + root.setBool("pinned", instance.change().isPinned()); } return new SlimeJsonResponse(slime); } @@ -1357,12 +1363,12 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } /** Trigger deployment of the given Vespa version if a valid one is given, e.g., "7.8.9". */ - private HttpResponse deployPlatform(String tenantName, String applicationName, boolean pin, HttpRequest request) { + private HttpResponse deployPlatform(String tenantName, String applicationName, String instanceName, boolean pin, HttpRequest request) { request = controller.auditLogger().log(request); String versionString = readToString(request.getData()); - TenantAndApplicationId id = TenantAndApplicationId.from(tenantName, applicationName); + ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName); StringBuilder response = new StringBuilder(); - controller.applications().lockApplicationOrThrow(id, application -> { + controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { Version version = Version.fromString(versionString); if (version.equals(Version.emptyVersion)) version = controller.systemVersion(); @@ -1385,11 +1391,11 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } /** Trigger deployment to the last known application package for the given application. */ - private HttpResponse deployApplication(String tenantName, String applicationName, HttpRequest request) { + private HttpResponse deployApplication(String tenantName, String applicationName, String instanceName, HttpRequest request) { controller.auditLogger().log(request); - TenantAndApplicationId id = TenantAndApplicationId.from(tenantName, applicationName); + ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName); StringBuilder response = new StringBuilder(); - controller.applications().lockApplicationOrThrow(id, application -> { + controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { Change change = Change.of(application.get().latestVersion().get()); controller.applications().deploymentTrigger().forceChange(id, change); response.append("Triggered " + change + " for " + id); @@ -1398,20 +1404,20 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } /** Cancel ongoing change for given application, e.g., everything with {"cancel":"all"} */ - private HttpResponse cancelDeploy(String tenantName, String applicationName, String choice) { - TenantAndApplicationId id = TenantAndApplicationId.from(tenantName, applicationName); + private HttpResponse cancelDeploy(String tenantName, String applicationName, String instanceName, String choice) { + ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName); StringBuilder response = new StringBuilder(); - controller.applications().lockApplicationOrThrow(id, application -> { - Change change = application.get().change(); + controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { + Change change = application.get().require(id.instance()).change(); if (change.isEmpty()) { - response.append("No deployment in progress for " + application + " at this time"); + response.append("No deployment in progress for " + id + " at this time"); return; } ChangesToCancel cancel = ChangesToCancel.valueOf(choice.toUpperCase()); controller.applications().deploymentTrigger().cancelChange(id, cancel); response.append("Changed deployment from '" + change + "' to '" + - controller.applications().requireApplication(id).change() + "' for " + application); + controller.applications().requireInstance(id).change() + "' for " + id); }); return new MessageResponse(response.toString()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 6e22acd65a1..33038e3ea69 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -80,7 +80,7 @@ class JobControllerApiHandlerHelper { Application application = controller.applications().requireApplication(TenantAndApplicationId.from(id)); DeploymentStatus deploymentStatus = controller.jobController().deploymentStatus(application); Instance instance = application.require(id.instance()); - Change change = application.change(); + Change change = instance.change(); Optional<DeploymentInstanceSpec> spec = application.deploymentSpec().instance(id.instance()); Optional<DeploymentSteps> steps = spec.map(s -> new DeploymentSteps(s, controller::system)); List<JobType> jobs = deploymentStatus.jobSteps().keySet().stream() @@ -210,9 +210,9 @@ class JobControllerApiHandlerHelper { } else lastPlatformObject.setString("pending", - application.change().isEmpty() + instance.change().isEmpty() ? "Waiting for upgrade slot" - : "Waiting for " + application.change() + " to complete"); + : "Waiting for " + instance.change() + " to complete"); } private static void lastApplicationToSlime(Cursor lastApplicationObject, Application application, Instance instance, Map<JobType, JobStatus> status, Change change, List<JobType> productionJobs, Controller controller) { @@ -320,7 +320,7 @@ class JobControllerApiHandlerHelper { break; for (JobType stepType : steps.toJobs(step)) { if (pendingProduction.containsKey(stepType)) { - Versions jobVersions = Versions.from(application.change(), + Versions jobVersions = Versions.from(instance.change(), application, Optional.ofNullable(instance.deployments().get(stepType.zone(controller.system()))), controller.systemVersion()); @@ -524,17 +524,17 @@ class JobControllerApiHandlerHelper { responseObject.setString("tenant", id.tenant().value()); responseObject.setString("application", id.application().value()); - Change change = status.application().change(); Map<JobId, List<Versions>> jobsToRun = status.jobsToRun(); Cursor stepsArray = responseObject.setArray("steps"); for (DeploymentStatus.StepStatus stepStatus : status.allSteps()) { + Change change = status.application().require(stepStatus.instance()).change(); Cursor stepObject = stepsArray.addObject(); stepObject.setString("type", stepStatus.type().name()); stepStatus.dependencies().stream() .map(status.allSteps()::indexOf) .forEach(stepObject.setArray("dependencies")::addLong); stepObject.setBool("declared", stepStatus.isDeclared()); - stepStatus.instance().ifPresent(instance -> stepObject.setString("instance", instance.value())); + stepObject.setString("instance", stepStatus.instance().value()); stepStatus.job().ifPresent(job -> { stepObject.setString("jobName", job.type().jobName()); @@ -565,12 +565,12 @@ class JobControllerApiHandlerHelper { Cursor runObject = toRunArray.addObject(); toSlime(runObject.setObject("versions"), versions); - stepStatus.readyAt(change, versions).ifPresent(ready -> runObject.setLong("readyAt", ready.toEpochMilli())); - stepStatus.readyAt(change, versions) + stepStatus.readyAt(change).ifPresent(ready -> runObject.setLong("readyAt", ready.toEpochMilli())); + stepStatus.readyAt(change) .filter(controller.clock().instant()::isBefore) .ifPresent(until -> runObject.setLong("delayedUntil", until.toEpochMilli())); stepStatus.pausedUntil().ifPresent(until -> runObject.setLong("pausedUntil", until.toEpochMilli())); - stepStatus.coolingDownUntil(versions).ifPresent(until -> runObject.setLong("coolingDownUntil", until.toEpochMilli())); + stepStatus.coolingDownUntil(change).ifPresent(until -> runObject.setLong("coolingDownUntil", until.toEpochMilli())); stepStatus.blockedUntil(change).ifPresent(until -> runObject.setLong("blockedUntil", until.toEpochMilli())); } @@ -602,6 +602,7 @@ class JobControllerApiHandlerHelper { version.buildNumber().ifPresent(id -> versionObject.setLong("id", id)); version.source().ifPresent(source -> versionObject.setString("commit", source.commit())); version.source().flatMap(source -> toUrl(source)).ifPresent(source -> versionObject.setString("source", source.toString())); + version.compileVersion().ifPresent(platform -> versionObject.setString("compileVersion", platform.toFullString())); } private static void toSlime(Cursor versionsObject, Versions versions) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java index 2b523bdc12a..1f1dcbb8001 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java @@ -86,6 +86,10 @@ public class DeploymentApiHandler extends LoggingRequestHandler { Slime slime = new Slime(); Cursor root = slime.setObject(); Cursor platformArray = root.setArray("versions"); + Map<ApplicationId, JobList> jobs = controller.jobController().deploymentStatuses(ApplicationList.from(controller.applications().asList())) + .asList().stream() + .flatMap(status -> status.instanceJobs().entrySet().stream()) + .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); for (VespaVersion version : controller.versionStatus().versions()) { Cursor versionObject = platformArray.addObject(); versionObject.setString("version", version.versionNumber().toString()); @@ -101,10 +105,6 @@ public class DeploymentApiHandler extends LoggingRequestHandler { configServerObject.setString("hostname", hostname.value()); } - Map<ApplicationId, JobList> jobs = controller.jobController().deploymentStatuses(ApplicationList.from(controller.applications().asList())) - .asList().stream() - .flatMap(status -> status.instanceJobs().entrySet().stream()) - .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); Cursor failingArray = versionObject.setArray("failingApplications"); for (ApplicationId id : version.statistics().failing()) { if (jobs.containsKey(id)) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java index 1c4fe391a35..1b2928c57b1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java @@ -195,7 +195,7 @@ public class VersionStatus { versionMap.put(infrastructureVersion, DeploymentStatistics.empty(infrastructureVersion)); } - for (DeploymentStatus status : statuses.withProductionDeployment().asList()) { + for (DeploymentStatus status : statuses.asList()) { for (Instance instance : status.application().instances().values()) for (Deployment deployment : instance.productionDeployments().values()) versionMap.computeIfAbsent(deployment.version(), DeploymentStatistics::empty); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java index bc6fd093e2f..f722eb4f6bb 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java @@ -5,6 +5,7 @@ import com.yahoo.component.Version; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.application.ApplicationList; +import com.yahoo.vespa.hosted.controller.application.InstanceList; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatusList; @@ -48,13 +49,13 @@ public class VespaVersion implements Comparable<VespaVersion> { } public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller) { - DeploymentStatusList all = controller.jobController().deploymentStatuses(ApplicationList.from(controller.applications().asList())) - .withProductionDeployment(); + InstanceList all = InstanceList.from(controller.jobController().deploymentStatuses(ApplicationList.from(controller.applications().asList()))) + .withProductionDeployment(); // 'production on this': All deployment jobs upgrading to this version have completed without failure - DeploymentStatusList productionOnThis = all.forApplications(statistics.production().stream().map(TenantAndApplicationId::from).collect(Collectors.toList())) + InstanceList productionOnThis = all.matching(statistics.production()::contains) .not().failingUpgrade() .not().upgradingTo(statistics.version()); - DeploymentStatusList failingOnThis = all.forApplications(statistics.failing().stream().map(TenantAndApplicationId::from).collect(Collectors.toList())); + InstanceList failingOnThis = all.matching(statistics.failing()::contains); // 'broken' if any Canary fails if ( ! failingOnThis.with(UpgradePolicy.canary).isEmpty()) @@ -166,10 +167,10 @@ public class VespaVersion implements Comparable<VespaVersion> { } private static boolean nonCanaryApplicationsBroken(Version version, - DeploymentStatusList failingOnThis, - DeploymentStatusList productionOnThis) { - DeploymentStatusList failingNonCanaries = failingOnThis.startedFailingOn(version).not().with(UpgradePolicy.canary); - DeploymentStatusList productionNonCanaries = productionOnThis.not().with(UpgradePolicy.canary); + InstanceList failingOnThis, + InstanceList productionOnThis) { + InstanceList failingNonCanaries = failingOnThis.startedFailingOn(version).not().with(UpgradePolicy.canary); + InstanceList productionNonCanaries = productionOnThis.not().with(UpgradePolicy.canary); if (productionNonCanaries.size() + failingNonCanaries.size() == 0) return false; 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 35e4b018eed..bc98775c00e 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 @@ -80,11 +80,11 @@ public class ControllerTest { context.submit(applicationPackage); assertEquals("Application version is known from completion of initial job", ApplicationVersion.from(DeploymentContext.defaultSourceRevision, 1, "a@b", new Version("6.1"), Instant.ofEpochSecond(1)), - context.application().change().application().get()); + context.instance().change().application().get()); context.runJob(systemTest); context.runJob(stagingTest); - ApplicationVersion applicationVersion = context.application().change().application().get(); + ApplicationVersion applicationVersion = context.instance().change().application().get(); assertFalse("Application version has been set during deployment", applicationVersion.isUnknown()); tester.triggerJobs(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java index 309af7e450b..e4b5e77b377 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java @@ -188,10 +188,6 @@ public class ApplicationPackageBuilder { xml.append(athenzIdentityAttributes); } xml.append(">\n"); - if (explicitSystemTest) - xml.append(" <test />\n"); - if (explicitStagingTest) - xml.append(" <staging />\n"); xml.append(" <instance id='").append(instances).append("'>\n"); if (upgradePolicy != null) { xml.append(" <upgrade policy='"); @@ -200,6 +196,10 @@ public class ApplicationPackageBuilder { } xml.append(notifications); xml.append(blockChange); + if (explicitSystemTest) + xml.append(" <test />\n"); + if (explicitStagingTest) + xml.append(" <staging />\n"); xml.append(" <"); xml.append(environment.value()); if (globalServiceId != null) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java index fcf2ef97b49..120b9bad468 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java @@ -92,11 +92,8 @@ public class DeploymentContext { private final TesterId testerId; private final JobController jobs; private final RoutingGeneratorMock routing; - private final MockTesterCloud cloud; private final JobRunner runner; - private final ControllerTester tester; - private final ReadyJobsTrigger readyJobsTrigger; - private final NameServiceDispatcher nameServiceDispatcher;; + private final DeploymentTester tester; private ApplicationVersion lastSubmission = null; private boolean deferDnsUpdates = false; @@ -108,18 +105,15 @@ public class DeploymentContext { this.testerId = TesterId.of(instanceId); this.jobs = tester.controller().jobController(); this.runner = tester.runner(); - this.tester = tester.controllerTester(); - this.routing = this.tester.serviceRegistry().routingGeneratorMock(); - this.cloud = this.tester.serviceRegistry().testerCloud(); - this.readyJobsTrigger = tester.readyJobsTrigger(); - this.nameServiceDispatcher = tester.nameServiceDispatcher(); + this.tester = tester; + this.routing = tester.controllerTester().serviceRegistry().routingGeneratorMock(); createTenantAndApplication(); } private void createTenantAndApplication() { try { - var tenant = tester.createTenant(instanceId.tenant().value()); - tester.createApplication(tenant.value(), instanceId.application().value(), instanceId.instance().value()); + var tenant = tester.controllerTester().createTenant(instanceId.tenant().value()); + tester.controllerTester().createApplication(tenant.value(), instanceId.application().value(), instanceId.instance().value()); } catch (IllegalArgumentException ignored) { } // Tenant and or application may already exist with custom setup. } @@ -156,30 +150,30 @@ public class DeploymentContext { /** Completely deploy the latest change */ public DeploymentContext deploy() { - assertNotNull("Application package submitted", lastSubmission); + assertTrue("Application package submitted", application().latestVersion().isPresent()); assertFalse("Submission is not already deployed", application().instances().values().stream() .anyMatch(instance -> instance.deployments().values().stream() .anyMatch(deployment -> deployment.applicationVersion().equals(lastSubmission)))); - assertEquals(lastSubmission, application().change().application().get()); + assertEquals(application().latestVersion(), instance().change().application()); completeRollout(); - assertFalse(application().change().hasTargets()); + assertFalse(instance().change().hasTargets()); return this; } /** Upgrade platform of this to given version */ public DeploymentContext deployPlatform(Version version) { - assertEquals(application().change().platform().get(), version); + assertEquals(instance().change().platform().get(), version); assertFalse(application().instances().values().stream() .anyMatch(instance -> instance.deployments().values().stream() .anyMatch(deployment -> deployment.version().equals(version)))); - assertEquals(version, application().change().platform().get()); - assertFalse(application().change().application().isPresent()); + assertEquals(version, instance().change().platform().get()); + assertFalse(instance().change().application().isPresent()); completeRollout(); assertTrue(application().productionDeployments().values().stream() - .allMatch(deployments -> deployments.stream() - .allMatch(deployment -> deployment.version().equals(version)))); + .allMatch(deployments -> deployments.stream() + .allMatch(deployment -> deployment.version().equals(version)))); for (var spec : application().deploymentSpec().instances()) for (JobType type : new DeploymentSteps(spec, tester.controller()::system).productionJobs()) @@ -187,7 +181,7 @@ public class DeploymentContext { .list(type.zone(tester.controller().system()), applicationId.defaultInstance()).stream() // TODO jonmv: support more .allMatch(node -> node.currentVersion().equals(version))); - assertFalse(application().change().hasTargets()); + assertFalse(instance().change().hasTargets()); return this; } @@ -200,7 +194,7 @@ public class DeploymentContext { /** Flush all pending DNS updates */ public DeploymentContext flushDnsUpdates() { - nameServiceDispatcher.run(); + tester.nameServiceDispatcher().run(); assertTrue("All name service requests dispatched", tester.controller().curator().readNameServiceQueue().requests().isEmpty()); return this; @@ -279,7 +273,7 @@ public class DeploymentContext { else throw new AssertionError("Job '" + run.id() + "' was run twice"); - assertFalse("Change should have no targets, but was " + application().change(), application().change().hasTargets()); + assertFalse("Change should have no targets, but was " + instance().change(), instance().change().hasTargets()); if (!deferDnsUpdates) { flushDnsUpdates(); } @@ -370,12 +364,12 @@ public class DeploymentContext { /** Deploy default application package, start a run for that change and return its ID */ public RunId newRun(JobType type) { submit(); - readyJobsTrigger.maintain(); + tester.readyJobsTrigger().maintain(); if (type.isProduction()) { runJob(JobType.systemTest); runJob(JobType.stagingTest); - readyJobsTrigger.maintain(); + tester.readyJobsTrigger().maintain(); } Run run = jobs.active().stream() @@ -523,7 +517,7 @@ public class DeploymentContext { assertEquals(Step.Status.succeeded, jobs.run(id).get().stepStatuses().get(Step.startTests)); assertEquals(unfinished, jobs.run(id).get().stepStatuses().get(Step.endTests)); - cloud.set(TesterCloud.Status.SUCCESS); + tester.cloud().set(TesterCloud.Status.SUCCESS); runner.advance(currentRun(job)); assertTrue(jobs.run(id).get().hasEnded()); assertFalse(jobs.run(id).get().hasFailed()); 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 c3739f037c9..656afda149d 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 @@ -126,7 +126,7 @@ public class DeploymentTriggerTest { .collect(Collectors.toCollection(() -> EnumSet.noneOf(JobType.class)))); app.deploy(); - assertEquals(Change.empty(), app.application().change()); + assertEquals(Change.empty(), app.instance().change()); tester.controllerTester().upgradeSystem(new Version("8.9")); tester.upgrader().maintain(); @@ -264,7 +264,7 @@ public class DeploymentTriggerTest { tester.configServer().setSuspended(application.deploymentIdIn(ZoneId.from("prod", "us-central-1")), false); tester.triggerJobs(); application.runJob(productionUsWest1).runJob(productionUsEast3); - assertEquals(Change.empty(), application.application().change()); + assertEquals(Change.empty(), application.instance().change()); } @Test @@ -292,11 +292,11 @@ public class DeploymentTriggerTest { assertEquals(0, tester.jobs().active().size()); app.submit(applicationPackage); - assertTrue(app.application().outstandingChange().hasTargets()); + assertTrue(app.deploymentStatus().outstandingChange(app.instance().name()).hasTargets()); app.runJob(systemTest).runJob(stagingTest); tester.outstandingChangeDeployer().run(); - assertTrue(app.application().outstandingChange().hasTargets()); + assertTrue(app.deploymentStatus().outstandingChange(app.instance().name()).hasTargets()); tester.triggerJobs(); assertEquals(emptyList(), tester.jobs().active()); @@ -304,7 +304,7 @@ public class DeploymentTriggerTest { tester.clock().advance(Duration.ofHours(2)); // ---------------- Exit block window: 20:30 tester.outstandingChangeDeployer().run(); - assertFalse(app.application().outstandingChange().hasTargets()); + assertFalse(app.deploymentStatus().outstandingChange(app.instance().name()).hasTargets()); tester.triggerJobs(); // Tests already run for the blocked production job. app.assertRunning(productionUsWest1); @@ -337,7 +337,7 @@ public class DeploymentTriggerTest { tester.outstandingChangeDeployer().run(); app.runJob(productionUsWest1); assertEquals(1, app.instanceJobs().get(productionUsWest1).lastSuccess().get().versions().targetApplication().buildNumber().getAsLong()); - assertEquals(2, app.application().outstandingChange().application().get().buildNumber().getAsLong()); + assertEquals(2, app.deploymentStatus().outstandingChange(app.instance().name()).application().get().buildNumber().getAsLong()); tester.triggerJobs(); // Platform upgrade keeps rolling, since it began before block window, and tests for the new revision have also started. @@ -346,18 +346,18 @@ public class DeploymentTriggerTest { assertEquals(2, tester.jobs().active().size()); // Upgrade is done, and outstanding change rolls out when block window ends. - assertEquals(Change.empty(), app.application().change()); - assertTrue(app.application().outstandingChange().hasTargets()); + assertEquals(Change.empty(), app.instance().change()); + assertTrue(app.deploymentStatus().outstandingChange(app.instance().name()).hasTargets()); app.runJob(stagingTest).runJob(systemTest); tester.clock().advance(Duration.ofHours(1)); tester.outstandingChangeDeployer().run(); - assertTrue(app.application().change().hasTargets()); - assertFalse(app.application().outstandingChange().hasTargets()); + assertTrue(app.instance().change().hasTargets()); + assertFalse(app.deploymentStatus().outstandingChange(app.instance().name()).hasTargets()); app.runJob(productionUsWest1).runJob(productionUsEast3); - assertFalse(app.application().change().hasTargets()); + assertFalse(app.instance().change().hasTargets()); } @Test @@ -420,12 +420,12 @@ public class DeploymentTriggerTest { assertEquals(appVersion1, app.deployment(ZoneId.from("prod.us-central-1")).applicationVersion()); // Verify the application change is not removed when platform change is cancelled. - tester.deploymentTrigger().cancelChange(app.application().id(), PLATFORM); - assertEquals(Change.of(appVersion1), app.application().change()); + tester.deploymentTrigger().cancelChange(app.instanceId(), PLATFORM); + assertEquals(Change.of(appVersion1), app.instance().change()); // Now cancel the change as is done through the web API. - tester.deploymentTrigger().cancelChange(app.application().id(), ALL); - assertEquals(Change.empty(), app.application().change()); + tester.deploymentTrigger().cancelChange(app.instanceId(), ALL); + assertEquals(Change.empty(), app.instance().change()); // A new version is released, which should now deploy the currently deployed application version to avoid downgrades. Version version1 = new Version("6.2"); @@ -464,7 +464,7 @@ public class DeploymentTriggerTest { tester.upgrader().maintain(); // Deploy application2 to keep this version present in the system app2.deployPlatform(version1); - tester.deploymentTrigger().cancelChange(app1.application().id(), ALL); + tester.deploymentTrigger().cancelChange(app1.instanceId(), ALL); // version2 is released and application1 starts upgrading Version version2 = Version.fromString("7.2"); @@ -480,7 +480,7 @@ public class DeploymentTriggerTest { tester.upgrader().overrideConfidence(version2, VespaVersion.Confidence.broken); tester.controllerTester().computeVersionStatus(); tester.upgrader().maintain(); // Cancel upgrades to broken version - assertEquals("Change becomes latest non-broken version", Change.of(version1), app1.application().change()); + assertEquals("Change becomes latest non-broken version", Change.of(version1), app1.instance().change()); // version1 proceeds 'til the last job, where it fails; us-central-1 is skipped, as current change is strictly dominated by what's deployed there. app1.failDeployment(productionEuWest1); @@ -491,7 +491,7 @@ public class DeploymentTriggerTest { app1.submit(applicationPackage); ApplicationVersion revision2 = app1.lastSubmission().get(); app1.runJob(systemTest).runJob(stagingTest); - assertEquals(Change.of(version1).with(revision2), app1.application().change()); + assertEquals(Change.of(version1).with(revision2), app1.instance().change()); tester.triggerJobs(); app1.assertRunning(productionUsCentral1); assertEquals(version2, app1.instance().deployments().get(productionUsCentral1.zone(main)).version()); @@ -511,7 +511,7 @@ public class DeploymentTriggerTest { // Last job has a different deployment target, so tests need to run again. app1.runJob(systemTest).runJob(stagingTest).runJob(productionEuWest1); - assertFalse(app1.application().change().hasTargets()); + assertFalse(app1.instance().change().hasTargets()); assertFalse(app1.instanceJobs().get(productionUsCentral1).isSuccess()); } @@ -532,7 +532,7 @@ public class DeploymentTriggerTest { tester.upgrader().maintain(); app.runJob(systemTest).runJob(stagingTest); app.timeOutConvergence(productionEuWest1); - tester.deploymentTrigger().cancelChange(app.application().id(), PLATFORM); + tester.deploymentTrigger().cancelChange(app.instanceId(), PLATFORM); assertEquals(v2, app.deployment(productionEuWest1.zone(main)).version()); assertEquals(v1, app.deployment(productionUsEast3.zone(main)).version()); @@ -558,7 +558,7 @@ public class DeploymentTriggerTest { // Both jobs fail again, and must be re-triggered -- this is ok, as they are both already triggered on their current targets. app.failDeployment(productionEuWest1).failDeployment(productionUsEast3) .runJob(productionEuWest1).runJob(productionUsEast3); - assertFalse(app.application().change().hasTargets()); + assertFalse(app.instance().change().hasTargets()); assertEquals(2, app.instanceJobs().get(productionEuWest1).lastSuccess().get().versions().targetApplication().buildNumber().getAsLong()); assertEquals(2, app.instanceJobs().get(productionUsEast3).lastSuccess().get().versions().targetApplication().buildNumber().getAsLong()); } @@ -647,7 +647,7 @@ public class DeploymentTriggerTest { assertEquals("Application change preserves version, and new region gets oldest version too", version1, app1.application().oldestDeployedPlatform().get()); assertEquals(version1, tester.configServer().lastPrepareVersion().get()); - assertFalse("Change deployed", app1.application().change().hasTargets()); + assertFalse("Change deployed", app1.instance().change().hasTargets()); tester.upgrader().maintain(); app1.deployPlatform(version2); @@ -737,9 +737,9 @@ public class DeploymentTriggerTest { app3.abortJob(stagingTest); assertEquals(0, tester.jobs().active().size()); - assertTrue(app1.application().change().application().isPresent()); - assertFalse(app2.application().change().application().isPresent()); - assertFalse(app3.application().change().application().isPresent()); + assertTrue(app1.instance().change().application().isPresent()); + assertFalse(app2.instance().change().application().isPresent()); + assertFalse(app3.instance().change().application().isPresent()); tester.readyJobsTrigger().maintain(); app1.assertRunning(stagingTest); @@ -801,7 +801,7 @@ public class DeploymentTriggerTest { app.runJob(testUsEast3) .runJob(productionUsWest1).runJob(productionUsCentral1) .runJob(testUsCentral1).runJob(testUsWest1); - assertEquals(Change.empty(), app.application().change()); + assertEquals(Change.empty(), app.instance().change()); // Application starts upgrade, but is confidence is broken cancelled after first zone. Tests won't run. Version version0 = app.application().oldestDeployedPlatform().get(); @@ -820,25 +820,25 @@ public class DeploymentTriggerTest { tester.upgrader().maintain(); app.failDeployment(testUsEast3); app.assertNotRunning(testUsEast3); - assertEquals(Change.empty(), app.application().change()); + assertEquals(Change.empty(), app.instance().change()); // Application is pinned to previous version, and downgrades to that. Tests are re-run. - tester.deploymentTrigger().triggerChange(app.application().id(), Change.of(version0).withPin()); + tester.deploymentTrigger().triggerChange(app.instanceId(), Change.of(version0).withPin()); app.runJob(stagingTest).runJob(productionUsEast3); tester.clock().advance(Duration.ofMinutes(1)); app.failDeployment(testUsEast3); tester.clock().advance(Duration.ofMinutes(11)); // Job is cooling down after consecutive failures. app.runJob(testUsEast3); - assertEquals(Change.empty().withPin(), app.application().change()); + assertEquals(Change.empty().withPin(), app.instance().change()); } @Test public void testDeployComplicatedDeploymentSpec() { String complicatedDeploymentSpec = "<deployment version='1.0' athenz-domain='domain' athenz-service='service'>\n" + - " <staging />\n" + " <parallel>\n" + " <instance id='instance' athenz-service='in-service'>\n" + + " <staging />\n" + " <prod>\n" + " <parallel>\n" + " <region active='true'>us-west-1</region>\n" + @@ -875,11 +875,9 @@ public class DeploymentTriggerTest { " </endpoints>\n" + " </instance>\n" + " <instance id='other'>\n" + - // TODO jonmv: support change per instance — this instance will be upgraded later, but for now this is ignored. " <upgrade policy='conservative' />\n" + " <test />\n" + - // TODO jonmv: test this when change per instance. - // " <block-change days='sat' hours='10' time-zone='CET' />\n" + + " <block-change revision='true' version='false' days='sat' hours='0-23' time-zone='CET' />\n" + " <prod>\n" + " <region active='true'>eu-west-1</region>\n" + " <test>eu-west-1</test>\n" + @@ -891,12 +889,20 @@ public class DeploymentTriggerTest { " </notifications>\n" + " </instance>\n" + " </parallel>\n" + + " <instance id='last'>\n" + + " <upgrade policy='conservative' />\n" + + " <prod>\n" + + " <region active='true'>eu-west-1</region>\n" + + " </prod>\n" + + " </instance>\n" + "</deployment>\n"; ApplicationPackage applicationPackage = ApplicationPackageBuilder.fromDeploymentXml(complicatedDeploymentSpec); var app1 = tester.newDeploymentContext("t", "a", "instance").submit(applicationPackage); - var app2 = tester.newDeploymentContext("t", "a", "other").submit(applicationPackage); + var app2 = tester.newDeploymentContext("t", "a", "other"); + var app3 = tester.newDeploymentContext("t", "a", "last"); + // Verify that the first submission rolls out as per the spec. tester.triggerJobs(); assertEquals(2, tester.jobs().active().size()); app1.runJob(stagingTest); @@ -954,7 +960,92 @@ public class DeploymentTriggerTest { assertEquals(1, tester.jobs().active().size()); app1.runJob(productionApSoutheast1); tester.triggerJobs(); + assertEquals(1, tester.jobs().active().size()); + app3.runJob(productionEuWest1); + tester.triggerJobs(); + assertEquals(List.of(), tester.jobs().active()); + + tester.atMondayMorning().clock().advance(Duration.ofDays(5)); // Inside block window for second instance. + Version version = Version.fromString("8.1"); + tester.controllerTester().upgradeSystem(version); + tester.upgrader().maintain(); + assertEquals(Change.of(version), app1.instance().change()); + assertEquals(Change.empty(), app2.instance().change()); + assertEquals(Change.empty(), app3.instance().change()); + + // Upgrade instance 1; a failure allows an application change to accompany the upgrade. + // The new platform won't roll out to the conservative instance until the normal one is upgraded. + app1.failDeployment(systemTest); + app1.submit(applicationPackage); + assertEquals(Change.of(version).with(app1.application().latestVersion().get()), app1.instance().change()); + app1.runJob(systemTest) + .jobAborted(stagingTest) + .runJob(stagingTest) + .runJob(productionUsWest1) + .runJob(productionUsEast3); + tester.clock().advance(Duration.ofHours(2)); + app1.runJob(productionEuWest1); + tester.clock().advance(Duration.ofHours(1)); + app1.runJob(productionAwsUsEast1a); + tester.triggerJobs(); + app1.runJob(testAwsUsEast1a); + app1.runJob(productionApNortheast2); + app1.runJob(productionApNortheast1); + tester.clock().advance(Duration.ofHours(1)); + app1.runJob(testApNortheast1); + app1.runJob(testApNortheast2); + app1.runJob(testUsEast3); + app1.runJob(productionApSoutheast1); + + app2.runJob(systemTest) // Testing outstanding revision. + .runJob(stagingTest); // Testing outstanding revision. + + tester.controllerTester().computeVersionStatus(); + tester.upgrader().maintain(); + tester.outstandingChangeDeployer().run(); + tester.triggerJobs(); + assertEquals(2, tester.jobs().active().size()); + assertEquals(Change.empty(), app1.instance().change()); + assertEquals(Change.of(version), app2.instance().change()); + assertEquals(Change.empty(), app3.instance().change()); + + app2.runJob(systemTest) // Explicitly defined for this instance. + .runJob(stagingTest) // Never completed successfully with just the upgrade. + .runJob(productionEuWest1) + .failDeployment(testEuWest1) + .runJob(systemTest); // Testing outstanding revision with currently deployed (upgraded) platform. + + tester.clock().advance(Duration.ofDays(1)); // Leave block window for revisions. + app2.abortJob(testEuWest1); + tester.upgrader().maintain(); + tester.outstandingChangeDeployer().run(); + assertEquals(0, tester.jobs().active().size()); + tester.triggerJobs(); + assertEquals(1, tester.jobs().active().size()); + assertEquals(Change.empty(), app1.instance().change()); + assertEquals(Change.of(version).with(app1.application().latestVersion().get()), app2.instance().change()); + + app2.runJob(productionEuWest1) + .runJob(testEuWest1); + assertEquals(Change.empty(), app2.instance().change()); + assertEquals(Change.empty(), app3.instance().change()); + + // Two first instances upgraded and with new revision — last instance gets change from whatever maintainer runs first. + tester.upgrader().maintain(); + tester.outstandingChangeDeployer().run(); + assertEquals(Change.of(version), app3.instance().change()); + + tester.deploymentTrigger().cancelChange(app3.instanceId(), ALL); + tester.outstandingChangeDeployer().run(); + tester.upgrader().maintain(); + assertEquals(Change.of(app1.application().latestVersion().get()), app3.instance().change()); + + app3.runJob(productionEuWest1); + tester.upgrader().maintain(); + app3.runJob(productionEuWest1); + tester.triggerJobs(); assertEquals(List.of(), tester.jobs().active()); + assertEquals(Change.empty(), app3.instance().change()); } @Test @@ -968,12 +1059,12 @@ public class DeploymentTriggerTest { app.submit(); tester.triggerJobs(); tester.outstandingChangeDeployer().run(); - assertEquals(Change.of(version), app.application().change()); + assertEquals(Change.of(version), app.instance().change()); app.runJob(productionUsEast3).runJob(productionUsWest1); tester.triggerJobs(); tester.outstandingChangeDeployer().run(); - assertEquals(Change.of(app.lastSubmission().get()), app.application().change()); + assertEquals(Change.of(app.lastSubmission().get()), app.instance().change()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java index 0afe7377d40..ea946e132a9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java @@ -144,7 +144,7 @@ public class DeploymentIssueReporterTest { // app2 is changed to be a canary app2.submit(canaryPackage).deploy(); assertEquals(canary, app2.application().deploymentSpec().requireInstance("default").upgradePolicy()); - assertEquals(Change.empty(), app2.application().change()); + assertEquals(Change.empty(), app2.instance().change()); // Bump system version to upgrade canary app2. Version version = Version.fromString("6.3"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java index a949625baf4..99eb4f049b6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java @@ -139,7 +139,7 @@ public class MetricsReporterTest { // Application change completes context.deploy(); - assertFalse("Change deployed", context.application().change().hasTargets()); + assertFalse("Change deployed", context.instance().change().hasTargets()); // New versions is released and upgrade fails in test environments Version version = Version.fromString("7.1"); @@ -159,7 +159,7 @@ public class MetricsReporterTest { // Upgrade eventually succeeds context.runJob(productionUsWest1); - assertFalse("Upgrade deployed", context.application().change().hasTargets()); + assertFalse("Upgrade deployed", context.instance().change().hasTargets()); reporter.maintain(); assertEquals(0, getDeploymentsFailingUpgrade(context.instanceId())); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java index 11c77304dd0..442a2fd1853 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java @@ -40,17 +40,17 @@ public class OutstandingChangeDeployerTest { var app1 = tester.newDeploymentContext("tenant", "app1", "default").submit(applicationPackage).deploy(); Version version = new Version(6, 2); - tester.deploymentTrigger().triggerChange(app1.application().id(), Change.of(version)); + tester.deploymentTrigger().triggerChange(app1.instanceId(), Change.of(version)); tester.deploymentTrigger().triggerReadyJobs(); - assertEquals(Change.of(version), app1.application().change()); - assertFalse(app1.application().outstandingChange().hasTargets()); + assertEquals(Change.of(version), app1.instance().change()); + assertFalse(app1.deploymentStatus().outstandingChange(app1.instance().name()).hasTargets()); assertEquals(1, app1.application().latestVersion().get().buildNumber().getAsLong()); app1.submit(applicationPackage, new SourceRevision("repository1", "master", "cafed00d")); - assertTrue(app1.application().outstandingChange().hasTargets()); - assertEquals("1.0.2-cafed00d", app1.application().outstandingChange().application().get().id()); + assertTrue(app1.deploymentStatus().outstandingChange(app1.instance().name()).hasTargets()); + assertEquals("1.0.2-cafed00d", app1.deploymentStatus().outstandingChange(app1.instance().name()).application().get().id()); app1.assertRunning(JobType.systemTest); app1.assertRunning(JobType.stagingTest); assertEquals(2, tester.jobs().active().size()); @@ -58,7 +58,7 @@ public class OutstandingChangeDeployerTest { deployer.maintain(); tester.triggerJobs(); assertEquals("No effect as job is in progress", 2, tester.jobs().active().size()); - assertEquals("1.0.2-cafed00d", app1.application().outstandingChange().application().get().id()); + assertEquals("1.0.2-cafed00d", app1.deploymentStatus().outstandingChange(app1.instance().name()).application().get().id()); app1.runJob(JobType.systemTest).runJob(JobType.stagingTest).runJob(JobType.productionUsWest1) .runJob(JobType.stagingTest).runJob(JobType.systemTest); @@ -66,11 +66,11 @@ public class OutstandingChangeDeployerTest { deployer.maintain(); tester.triggerJobs(); - assertEquals("1.0.2-cafed00d", app1.application().change().application().get().id()); + assertEquals("1.0.2-cafed00d", app1.instance().change().application().get().id()); List<Run> runs = tester.jobs().active(); assertEquals(1, runs.size()); app1.assertRunning(JobType.productionUsWest1); - assertFalse(app1.application().outstandingChange().hasTargets()); + assertFalse(app1.deploymentStatus().outstandingChange(app1.instance().name()).hasTargets()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java index c4a3a34a4f6..249c2644170 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java @@ -190,7 +190,7 @@ public class UpgraderTest { assertEquals("Upgrade of defaults are scheduled", 10, tester.jobs().active().size()); for (var context : List.of(default0, default1, default2, default3, default4)) - assertEquals(version4, context.application().change().platform().get()); + assertEquals(version4, context.instance().change().platform().get()); default0.deployPlatform(version4); @@ -207,9 +207,9 @@ public class UpgraderTest { tester.triggerJobs(); assertEquals("Upgrade of defaults are scheduled", 10, tester.jobs().active().size()); - assertEquals(version5, default0.application().change().platform().get()); + assertEquals(version5, default0.instance().change().platform().get()); for (var context : List.of(default1, default2, default3, default4)) - assertEquals(version4, context.application().change().platform().get()); + assertEquals(version4, context.instance().change().platform().get()); default1.deployPlatform(version4); default2.deployPlatform(version4); @@ -240,7 +240,7 @@ public class UpgraderTest { tester.upgrader().maintain(); - assertEquals(version4, default3.application().change().platform().get()); + assertEquals(version4, default3.instance().change().platform().get()); } @Test @@ -361,10 +361,10 @@ public class UpgraderTest { // We "manually" cancel upgrades to V1 so that we can use the applications to make V2 fail instead // But we keep one (default4) to avoid V1 being garbage collected - tester.deploymentTrigger().cancelChange(default0.application().id(), ALL); - tester.deploymentTrigger().cancelChange(default1.application().id(), ALL); - tester.deploymentTrigger().cancelChange(default2.application().id(), ALL); - tester.deploymentTrigger().cancelChange(default3.application().id(), ALL); + tester.deploymentTrigger().cancelChange(default0.instanceId(), ALL); + tester.deploymentTrigger().cancelChange(default1.instanceId(), ALL); + tester.deploymentTrigger().cancelChange(default2.instanceId(), ALL); + tester.deploymentTrigger().cancelChange(default3.instanceId(), ALL); default0.abortJob(systemTest).abortJob(stagingTest); default1.abortJob(systemTest).abortJob(stagingTest); default2.abortJob(systemTest).abortJob(stagingTest); @@ -374,7 +374,7 @@ public class UpgraderTest { tester.upgrader().maintain(); tester.triggerJobs(); assertEquals("Upgrade scheduled for remaining apps", 10, tester.jobs().active().size()); - assertEquals("default4 is still upgrading to 6.3", v1, default4.application().change().platform().get()); + assertEquals("default4 is still upgrading to 6.3", v1, default4.instance().change().platform().get()); // 4/5 applications fail (in the last prod zone) and lowers confidence default0.runJob(systemTest).runJob(stagingTest).runJob(productionUsWest1).failDeployment(productionUsEast3); @@ -399,7 +399,7 @@ public class UpgraderTest { assertEquals(v2, default0.deployment(ZoneId.from("prod.us-west-1")).version()); assertEquals("Last zone is upgraded to v1", v1, default0.deployment(ZoneId.from("prod.us-east-3")).version()); - assertFalse(default0.application().change().hasTargets()); + assertFalse(default0.instance().change().hasTargets()); } @Test @@ -788,8 +788,8 @@ public class UpgraderTest { // Application change recorded together with ongoing upgrade assertTrue("Change contains both upgrade and application change", - app.application().change().platform().get().equals(version) && - app.application().change().application().get().id().equals(applicationVersion)); + app.instance().change().platform().get().equals(version) && + app.instance().change().application().get().id().equals(applicationVersion)); // Deployment completes app.runJob(systemTest).runJob(stagingTest).runJob(productionUsWest1).runJob(productionUsEast3); @@ -879,13 +879,13 @@ public class UpgraderTest { assertEquals(List.of(), tester.jobs().active()); // No jobs left. tester.outstandingChangeDeployer().run(); - assertFalse(app.application().change().hasTargets()); + assertFalse(app.instance().change().hasTargets()); tester.clock().advance(Duration.ofHours(2)); tester.outstandingChangeDeployer().run(); - assertTrue(app.application().change().hasTargets()); + assertTrue(app.instance().change().hasTargets()); app.runJob(productionUsWest1).runJob(productionUsCentral1).runJob(productionUsEast3); - assertFalse(app.application().change().hasTargets()); + assertFalse(app.instance().change().hasTargets()); } @Test @@ -895,50 +895,50 @@ public class UpgraderTest { // Create an application with pinned platform version. var context = tester.newDeploymentContext(); - tester.deploymentTrigger().forceChange(context.application().id(), Change.empty().withPin()); + tester.deploymentTrigger().forceChange(context.instanceId(), Change.empty().withPin()); context.submit().deploy(); - assertFalse(context.application().change().hasTargets()); - assertTrue(context.application().change().isPinned()); + assertFalse(context.instance().change().hasTargets()); + assertTrue(context.instance().change().isPinned()); assertEquals(3, context.instance().deployments().size()); // Application does not upgrade. Version version1 = Version.fromString("6.3"); tester.controllerTester().upgradeSystem(version1); tester.upgrader().maintain(); - assertFalse(context.application().change().hasTargets()); - assertTrue(context.application().change().isPinned()); + assertFalse(context.instance().change().hasTargets()); + assertTrue(context.instance().change().isPinned()); // New application package is deployed. context.submit().deploy(); - assertFalse(context.application().change().hasTargets()); - assertTrue(context.application().change().isPinned()); + assertFalse(context.instance().change().hasTargets()); + assertTrue(context.instance().change().isPinned()); // Application upgrades to new version when pin is removed. - tester.deploymentTrigger().cancelChange(context.application().id(), PIN); + tester.deploymentTrigger().cancelChange(context.instanceId(), PIN); tester.upgrader().maintain(); - assertTrue(context.application().change().hasTargets()); - assertFalse(context.application().change().isPinned()); + assertTrue(context.instance().change().hasTargets()); + assertFalse(context.instance().change().isPinned()); // Application is pinned to new version, and upgrade is therefore not cancelled, even though confidence is broken. - tester.deploymentTrigger().forceChange(context.application().id(), Change.empty().withPin()); + tester.deploymentTrigger().forceChange(context.instanceId(), Change.empty().withPin()); tester.upgrader().maintain(); tester.triggerJobs(); - assertEquals(version1, context.application().change().platform().get()); + assertEquals(version1, context.instance().change().platform().get()); // Application fails upgrade after one zone is complete, and is pinned again to the old version. context.runJob(systemTest).runJob(stagingTest).runJob(productionUsCentral1) .timeOutUpgrade(productionUsWest1); - tester.deploymentTrigger().cancelChange(context.application().id(), ALL); - tester.deploymentTrigger().forceChange(context.application().id(), Change.of(version0).withPin()); - assertEquals(version0, context.application().change().platform().get()); + tester.deploymentTrigger().cancelChange(context.instanceId(), ALL); + tester.deploymentTrigger().forceChange(context.instanceId(), Change.of(version0).withPin()); + assertEquals(version0, context.instance().change().platform().get()); // Application downgrades to pinned version. tester.abortAll(); context.runJob(stagingTest).runJob(productionUsCentral1); - assertTrue(context.application().change().hasTargets()); + assertTrue(context.instance().change().hasTargets()); context.runJob(productionUsWest1); // us-east-3 never upgraded, so no downgrade is needed. - assertFalse(context.application().change().hasTargets()); + assertFalse(context.instance().change().hasTargets()); } @Test @@ -955,7 +955,8 @@ public class UpgraderTest { // Keep app 1 on current version tester.controller().applications().lockApplicationIfPresent(app1.application().id(), app -> - tester.controller().applications().store(app.withChange(app.get().change().withPin()))); + tester.controller().applications().store(app.with(app1.instance().name(), + instance -> instance.withChange(instance.change().withPin())))); // New version is released Version version1 = Version.fromString("6.2"); @@ -972,14 +973,15 @@ public class UpgraderTest { // App 2 is allowed on new major and upgrades tester.controller().applications().lockApplicationIfPresent(app2.application().id(), app -> tester.applications().store(app.withMajorVersion(7))); tester.upgrader().maintain(); - assertEquals(version2, app2.application().change().platform().orElseThrow()); + assertEquals(version2, app2.instance().change().platform().orElseThrow()); // App 1 is unpinned and upgrades to latest 6 tester.controller().applications().lockApplicationIfPresent(app1.application().id(), app -> - tester.controller().applications().store(app.withChange(app.get().change().withoutPin()))); + tester.controller().applications().store(app.with(app1.instance().name(), + instance -> instance.withChange(instance.change().withoutPin())))); tester.upgrader().maintain(); assertEquals("Application upgrades to latest allowed major", version1, - app1.application().change().platform().orElseThrow()); + app1.instance().change().platform().orElseThrow()); } @Test @@ -998,7 +1000,7 @@ public class UpgraderTest { Version v2 = Version.fromString("6.2"); tester.controllerTester().upgradeSystem(v2); tester.upgrader().maintain(); - assertEquals(Change.of(v2), application.application().change()); + assertEquals(Change.of(v2), application.instance().change()); application.runJob(systemTest).runJob(stagingTest).runJob(productionUsCentral1); tester.triggerJobs(); @@ -1007,13 +1009,13 @@ public class UpgraderTest { tester.controllerTester().computeVersionStatus(); tester.upgrader().maintain(); application.runJob(productionUsWest1); - assertTrue(application.application().change().isEmpty()); + assertTrue(application.instance().change().isEmpty()); // Next version is released Version v3 = Version.fromString("6.3"); tester.controllerTester().upgradeSystem(v3); tester.upgrader().maintain(); - assertEquals(Change.of(v3), application.application().change()); + assertEquals(Change.of(v3), application.instance().change()); application.runJob(systemTest).runJob(stagingTest); // First deployment starts upgrading @@ -1038,7 +1040,7 @@ public class UpgraderTest { // Upgrade completes application.runJob(productionUsEast3); - assertTrue("Upgrade complete", application.application().change().isEmpty()); + assertTrue("Upgrade complete", application.instance().change().isEmpty()); } @Test 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 4f396321544..e485e60815b 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 @@ -110,19 +110,19 @@ public class ApplicationSerializerTest { deployments, Map.of(JobType.systemTest, Instant.ofEpochMilli(333)), List.of(AssignedRotation.fromStrings("foo", "default", "my-rotation", Set.of("us-west-1"))), - rotationStatus), + rotationStatus, + Change.of(new Version("6.1"))), new Instance(id3, List.of(), Map.of(), List.of(), - RotationStatus.EMPTY)); + RotationStatus.EMPTY, + Change.of(Version.fromString("6.7")).withPin())); Application original = new Application(TenantAndApplicationId.from(id1), Instant.now().truncatedTo(ChronoUnit.MILLIS), deploymentSpec, validationOverrides, - Change.of(Version.fromString("6.7")).withPin(), - Change.of(ApplicationVersion.from(new SourceRevision("repo", "master", "deadcafe"), 42)), Optional.of(IssueId.from("4321")), Optional.of(IssueId.from("1234")), Optional.of(User.from("by-username")), @@ -164,17 +164,17 @@ public class ApplicationSerializerTest { assertEquals(original.require(id1.instance()).jobPause(JobType.stagingTest), serialized.require(id1.instance()).jobPause(JobType.stagingTest)); - assertEquals(original.outstandingChange(), serialized.outstandingChange()); - assertEquals(original.ownershipIssueId(), serialized.ownershipIssueId()); assertEquals(original.owner(), serialized.owner()); assertEquals(original.majorVersion(), serialized.majorVersion()); - assertEquals(original.change(), serialized.change()); assertEquals(original.deployKeys(), serialized.deployKeys()); assertEquals(original.require(id1.instance()).rotations(), serialized.require(id1.instance()).rotations()); assertEquals(original.require(id1.instance()).rotationStatus(), serialized.require(id1.instance()).rotationStatus()); + assertEquals(original.require(id1.instance()).change(), serialized.require(id1.instance()).change()); + assertEquals(original.require(id3.instance()).change(), serialized.require(id3.instance()).change()); + // Test cluster info assertEquals(3, serialized.require(id1.instance()).deployments().get(zone2).clusterInfo().size()); assertEquals(10, serialized.require(id1.instance()).deployments().get(zone2).clusterInfo().get(ClusterSpec.Id.from("id2")).getFlavorCost()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index cbc7a5477c4..42d15c23f23 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -340,7 +340,7 @@ public class ApplicationApiTest extends ControllerContainerTest { id2.application()); // Trigger upgrade and then application change - deploymentTester.applications().deploymentTrigger().triggerChange(TenantAndApplicationId.from(id2), Change.of(Version.fromString("7.0"))); + deploymentTester.applications().deploymentTrigger().triggerChange(id2, Change.of(Version.fromString("7.0"))); // POST an application package and a test jar, submitting a new application for production deployment. tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2/submit", POST) @@ -467,19 +467,19 @@ public class ApplicationApiTest extends ControllerContainerTest { // DELETE (cancel) ongoing change tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", DELETE) .userIdentity(HOSTED_VESPA_OPERATOR), - "{\"message\":\"Changed deployment from 'application change to 1.0.1-commit1' to 'no change' for application 'tenant1.application1'\"}"); + "{\"message\":\"Changed deployment from 'application change to 1.0.1-commit1' to 'no change' for tenant1.application1.instance1\"}"); // DELETE (cancel) again is a no-op tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", DELETE) .userIdentity(USER_ID) .data("{\"cancel\":\"all\"}"), - "{\"message\":\"No deployment in progress for application 'tenant1.application1' at this time\"}"); + "{\"message\":\"No deployment in progress for tenant1.application1.instance1 at this time\"}"); // POST pinning to a given version to an application tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin", POST) .userIdentity(USER_ID) .data("6.1.0"), - "{\"message\":\"Triggered pin to 6.1 for tenant1.application1\"}"); + "{\"message\":\"Triggered pin to 6.1 for tenant1.application1.instance1\"}"); assertTrue("Action is logged to audit log", tester.controller().auditLogger().readLog().entries().stream() .anyMatch(entry -> entry.resource().equals("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin"))); @@ -491,7 +491,7 @@ public class ApplicationApiTest extends ControllerContainerTest { // DELETE only the pin to a given version tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin", DELETE) .userIdentity(USER_ID), - "{\"message\":\"Changed deployment from 'pin to 6.1' to 'upgrade to 6.1' for application 'tenant1.application1'\"}"); + "{\"message\":\"Changed deployment from 'pin to 6.1' to 'upgrade to 6.1' for tenant1.application1.instance1\"}"); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":false}"); @@ -499,21 +499,21 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin", POST) .userIdentity(USER_ID) .data("6.1"), - "{\"message\":\"Triggered pin to 6.1 for tenant1.application1\"}"); + "{\"message\":\"Triggered pin to 6.1 for tenant1.application1.instance1\"}"); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":true}"); // DELETE only the version, but leave the pin tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/platform", DELETE) .userIdentity(USER_ID), - "{\"message\":\"Changed deployment from 'pin to 6.1' to 'pin to current platform' for application 'tenant1.application1'\"}"); + "{\"message\":\"Changed deployment from 'pin to 6.1' to 'pin to current platform' for tenant1.application1.instance1\"}"); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) .userIdentity(USER_ID), "{\"pinned\":true}"); // DELETE also the pin to a given version tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin", DELETE) .userIdentity(USER_ID), - "{\"message\":\"Changed deployment from 'pin to current platform' to 'no change' for application 'tenant1.application1'\"}"); + "{\"message\":\"Changed deployment from 'pin to current platform' to 'no change' for tenant1.application1.instance1\"}"); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) .userIdentity(USER_ID), "{}"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json index 4c0781fc034..16a5529a2ad 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json @@ -12,20 +12,6 @@ } }, "projectId": 1000, - "deploying": { - "version": "7" - }, - "outstandingChange": { - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - } - } - }, "compileVersion": "6.1.0", "majorVersion": 7, "instances": [ @@ -36,6 +22,20 @@ }, { "instance": "instance1", + "deploying": { + "version": "7" + }, + "outstandingChange": { + "revision": { + "buildNumber": 1, + "hash": "1.0.1-commit1", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + } + } + }, "deploymentJobs": [ { "type": "system-test", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json index e5900f57e03..6382824175d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json @@ -12,20 +12,6 @@ } }, "projectId": 1000, - "deploying": { - "version": "7" - }, - "outstandingChange": { - "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - } - } - }, "compileVersion": "6.1.0", "instances": [ { @@ -35,6 +21,20 @@ }, { "instance": "instance1", + "deploying": { + "version": "7" + }, + "outstandingChange": { + "revision": { + "buildNumber": 1, + "hash": "1.0.1-commit1", + "source": { + "gitRepository": "repository1", + "gitBranch": "master", + "gitCommit": "commit1" + } + } + }, "deploymentJobs": [ { "type": "system-test", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json index 59b200753fe..8f68bde1a21 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json @@ -3,83 +3,63 @@ "steps": [ { "declared": true, - "jobName": "staging-test", - "environment": "staging", "instance": "default", - "toRun": [ - { - "readyAt": 752000, - "coolingDownUntil": 752000, - "versions": { - "targetApplication": { - "commit": "commit1", - "id": 3, - "source": "repository1/tree/commit1" - }, - "sourcePlatform": "6.1.0", - "targetPlatform": "6.1.0", - "sourceApplication": { - "commit": "commit1", - "id": 1, - "source": "repository1/tree/commit1" - } - }, - "delayedUntil": 752000 - } - ], + "type": "instance", + "dependencies": [] + }, + { + "declared": false, + "jobName": "system-test", + "environment": "test", + "instance": "default", + "toRun": [], "type": "test", - "region": "staging.us-east-3", + "region": "test.us-east-1", "runs": [ { "versions": { "targetApplication": { "commit": "commit1", "id": 3, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", - "id": 1, - "source": "repository1/tree/commit1" + "id": 2, + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, - "start": 102000, - "end": 102000, - "id": 5, + "start": 2000, + "end": 2000, + "id": 3, "steps": [ { "name": "deployTester", "status": "succeeded" }, { - "name": "deployInitialReal", - "status": "succeeded" - }, - { - "name": "installInitialReal", - "status": "failed" - }, - { "name": "deployReal", - "status": "unfinished" + "status": "succeeded" }, { "name": "installTester", - "status": "unfinished" + "status": "succeeded" }, { "name": "installReal", - "status": "unfinished" + "status": "succeeded" }, { "name": "startTests", - "status": "unfinished" + "status": "succeeded" }, { "name": "endTests", - "status": "unfinished" + "status": "succeeded" }, { "name": "copyVespaLogs", @@ -98,59 +78,53 @@ "status": "succeeded" } ], - "url": "https://some.url:43/instance/default/job/staging-test/run/run 5 of staging-test for tenant.application", - "status": "installationFailed" + "url": "https://some.url:43/instance/default/job/system-test/run/run 3 of system-test for tenant.application", + "status": "success" }, { "versions": { "targetApplication": { "commit": "commit1", - "id": 3, - "source": "repository1/tree/commit1" + "id": 2, + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, - "start": 2000, - "end": 2000, - "id": 4, + "start": 1000, + "end": 1000, + "id": 2, "steps": [ { "name": "deployTester", "status": "succeeded" }, { - "name": "deployInitialReal", - "status": "succeeded" - }, - { - "name": "installInitialReal", - "status": "failed" - }, - { "name": "deployReal", - "status": "unfinished" + "status": "succeeded" }, { "name": "installTester", - "status": "unfinished" + "status": "succeeded" }, { "name": "installReal", - "status": "unfinished" + "status": "succeeded" }, { "name": "startTests", - "status": "unfinished" + "status": "succeeded" }, { "name": "endTests", - "status": "unfinished" + "status": "succeeded" }, { "name": "copyVespaLogs", @@ -169,41 +143,28 @@ "status": "succeeded" } ], - "url": "https://some.url:43/instance/default/job/staging-test/run/run 4 of staging-test for tenant.application", - "status": "installationFailed" + "url": "https://some.url:43/instance/default/job/system-test/run/run 2 of system-test for tenant.application", + "status": "success" }, { "versions": { "targetApplication": { "commit": "commit1", - "id": 3, - "source": "repository1/tree/commit1" + "id": 1, + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, - "sourcePlatform": "6.1.0", - "targetPlatform": "6.1.0", - "sourceApplication": { - "commit": "commit1", - "id": 2, - "source": "repository1/tree/commit1" - } + "targetPlatform": "6.1.0" }, - "start": 2000, - "end": 2000, - "id": 3, + "start": 0, + "end": 0, + "id": 1, "steps": [ { "name": "deployTester", "status": "succeeded" }, { - "name": "deployInitialReal", - "status": "succeeded" - }, - { - "name": "installInitialReal", - "status": "succeeded" - }, - { "name": "deployReal", "status": "succeeded" }, @@ -240,27 +201,64 @@ "status": "succeeded" } ], - "url": "https://some.url:43/instance/default/job/staging-test/run/run 3 of staging-test for tenant.application", + "url": "https://some.url:43/instance/default/job/system-test/run/run 1 of system-test for tenant.application", "status": "success" - }, + } + ], + "url": "https://some.url:43/instance/default/job/system-test", + "dependencies": [] + }, + { + "declared": true, + "jobName": "staging-test", + "environment": "staging", + "instance": "default", + "toRun": [ { + "readyAt": 752000, + "coolingDownUntil": 752000, "versions": { "targetApplication": { "commit": "commit1", - "id": 2, - "source": "repository1/tree/commit1" + "id": 3, + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, - "start": 1000, - "end": 1000, - "id": 2, + "delayedUntil": 752000 + } + ], + "type": "test", + "region": "staging.us-east-3", + "runs": [ + { + "versions": { + "targetApplication": { + "commit": "commit1", + "id": 3, + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" + }, + "sourcePlatform": "6.1.0", + "targetPlatform": "6.1.0", + "sourceApplication": { + "commit": "commit1", + "id": 1, + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" + } + }, + "start": 102000, + "end": 102000, + "id": 5, "steps": [ { "name": "deployTester", @@ -272,27 +270,27 @@ }, { "name": "installInitialReal", - "status": "succeeded" + "status": "failed" }, { "name": "deployReal", - "status": "succeeded" + "status": "unfinished" }, { "name": "installTester", - "status": "succeeded" + "status": "unfinished" }, { "name": "installReal", - "status": "succeeded" + "status": "unfinished" }, { "name": "startTests", - "status": "succeeded" + "status": "unfinished" }, { "name": "endTests", - "status": "succeeded" + "status": "unfinished" }, { "name": "copyVespaLogs", @@ -311,21 +309,29 @@ "status": "succeeded" } ], - "url": "https://some.url:43/instance/default/job/staging-test/run/run 2 of staging-test for tenant.application", - "status": "success" + "url": "https://some.url:43/instance/default/job/staging-test/run/run 5 of staging-test for tenant.application", + "status": "installationFailed" }, { "versions": { "targetApplication": { "commit": "commit1", - "id": 1, - "source": "repository1/tree/commit1" + "id": 3, + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, - "targetPlatform": "6.1.0" + "sourcePlatform": "6.1.0", + "targetPlatform": "6.1.0", + "sourceApplication": { + "commit": "commit1", + "id": 1, + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" + } }, - "start": 0, - "end": 0, - "id": 1, + "start": 2000, + "end": 2000, + "id": 4, "steps": [ { "name": "deployTester", @@ -337,27 +343,27 @@ }, { "name": "installInitialReal", - "status": "succeeded" + "status": "failed" }, { "name": "deployReal", - "status": "succeeded" + "status": "unfinished" }, { "name": "installTester", - "status": "succeeded" + "status": "unfinished" }, { "name": "installReal", - "status": "succeeded" + "status": "unfinished" }, { "name": "startTests", - "status": "succeeded" + "status": "unfinished" }, { "name": "endTests", - "status": "succeeded" + "status": "unfinished" }, { "name": "copyVespaLogs", @@ -376,35 +382,24 @@ "status": "succeeded" } ], - "url": "https://some.url:43/instance/default/job/staging-test/run/run 1 of staging-test for tenant.application", - "status": "success" - } - ], - "url": "https://some.url:43/instance/default/job/staging-test", - "dependencies": [] - }, - { - "declared": false, - "jobName": "system-test", - "environment": "test", - "instance": "default", - "toRun": [], - "type": "test", - "region": "test.us-east-1", - "runs": [ + "url": "https://some.url:43/instance/default/job/staging-test/run/run 4 of staging-test for tenant.application", + "status": "installationFailed" + }, { "versions": { "targetApplication": { "commit": "commit1", "id": 3, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 2, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "start": 2000, @@ -416,6 +411,14 @@ "status": "succeeded" }, { + "name": "deployInitialReal", + "status": "succeeded" + }, + { + "name": "installInitialReal", + "status": "succeeded" + }, + { "name": "deployReal", "status": "succeeded" }, @@ -452,7 +455,7 @@ "status": "succeeded" } ], - "url": "https://some.url:43/instance/default/job/system-test/run/run 3 of system-test for tenant.application", + "url": "https://some.url:43/instance/default/job/staging-test/run/run 3 of staging-test for tenant.application", "status": "success" }, { @@ -460,14 +463,16 @@ "targetApplication": { "commit": "commit1", "id": 2, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "start": 1000, @@ -479,6 +484,14 @@ "status": "succeeded" }, { + "name": "deployInitialReal", + "status": "succeeded" + }, + { + "name": "installInitialReal", + "status": "succeeded" + }, + { "name": "deployReal", "status": "succeeded" }, @@ -515,7 +528,7 @@ "status": "succeeded" } ], - "url": "https://some.url:43/instance/default/job/system-test/run/run 2 of system-test for tenant.application", + "url": "https://some.url:43/instance/default/job/staging-test/run/run 2 of staging-test for tenant.application", "status": "success" }, { @@ -523,7 +536,8 @@ "targetApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "targetPlatform": "6.1.0" }, @@ -536,6 +550,14 @@ "status": "succeeded" }, { + "name": "deployInitialReal", + "status": "succeeded" + }, + { + "name": "installInitialReal", + "status": "succeeded" + }, + { "name": "deployReal", "status": "succeeded" }, @@ -572,11 +594,11 @@ "status": "succeeded" } ], - "url": "https://some.url:43/instance/default/job/system-test/run/run 1 of system-test for tenant.application", + "url": "https://some.url:43/instance/default/job/staging-test/run/run 1 of staging-test for tenant.application", "status": "success" } ], - "url": "https://some.url:43/instance/default/job/system-test", + "url": "https://some.url:43/instance/default/job/staging-test", "dependencies": [] }, { @@ -589,7 +611,8 @@ "currentApplication": { "commit": "commit1", "id": 3, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "type": "deployment", "region": "prod.us-central-1", @@ -599,14 +622,16 @@ "targetApplication": { "commit": "commit1", "id": 3, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 2, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "start": 2000, @@ -653,14 +678,16 @@ "targetApplication": { "commit": "commit1", "id": 2, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "start": 1000, @@ -708,7 +735,8 @@ "targetApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "targetPlatform": "6.1.0" }, @@ -755,7 +783,8 @@ ], "url": "https://some.url:43/instance/default/job/production-us-central-1", "dependencies": [ - 0 + 0, + 2 ] }, { @@ -769,14 +798,16 @@ "targetApplication": { "commit": "commit1", "id": 3, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 3, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } } } @@ -789,14 +820,16 @@ "targetApplication": { "commit": "commit1", "id": 2, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 2, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "start": 1000, @@ -836,14 +869,16 @@ "targetApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "start": 0, @@ -881,7 +916,7 @@ ], "url": "https://some.url:43/instance/default/job/test-us-central-1", "dependencies": [ - 2 + 3 ] }, { @@ -896,14 +931,16 @@ "targetApplication": { "commit": "commit1", "id": 3, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 2, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } } } @@ -911,7 +948,8 @@ "currentApplication": { "commit": "commit1", "id": 2, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "type": "deployment", "region": "prod.us-west-1", @@ -921,14 +959,16 @@ "targetApplication": { "commit": "commit1", "id": 2, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "start": 1000, @@ -976,7 +1016,8 @@ "targetApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "targetPlatform": "6.1.0" }, @@ -1023,7 +1064,7 @@ ], "url": "https://some.url:43/instance/default/job/production-us-west-1", "dependencies": [ - 3 + 4 ] }, { @@ -1038,14 +1079,16 @@ "targetApplication": { "commit": "commit1", "id": 3, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } } } @@ -1053,7 +1096,8 @@ "currentApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "type": "deployment", "region": "prod.us-east-3", @@ -1063,14 +1107,16 @@ "targetApplication": { "commit": "commit1", "id": 2, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "sourcePlatform": "6.1.0", "targetPlatform": "6.1.0", "sourceApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "start": 1000, @@ -1118,7 +1164,8 @@ "targetApplication": { "commit": "commit1", "id": 1, - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" }, "targetPlatform": "6.1.0" }, @@ -1165,7 +1212,7 @@ ], "url": "https://some.url:43/instance/default/job/production-us-east-3", "dependencies": [ - 3 + 4 ] } ], diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json index df04b2a165a..ba41468be24 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json @@ -3,6 +3,12 @@ "application": "application1", "steps": [ { + "type": "instance", + "dependencies": [], + "declared": true, + "instance": "instance1" + }, + { "type": "test", "dependencies": [], "declared": false, @@ -23,7 +29,8 @@ "targetApplication": { "id": 4, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "steps": [ @@ -80,7 +87,8 @@ "targetApplication": { "id": 1, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "steps": [ @@ -149,7 +157,8 @@ "targetApplication": { "id": 4, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "steps": [ @@ -214,7 +223,8 @@ "targetApplication": { "id": 1, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "steps": [ @@ -272,7 +282,9 @@ }, { "type": "deployment", - "dependencies": [], + "dependencies": [ + 0 + ], "declared": true, "instance": "instance1", "jobName": "production-us-central-1", @@ -286,7 +298,8 @@ "targetApplication": { "id": 4, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } } } @@ -303,7 +316,8 @@ "targetApplication": { "id": 1, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "steps": [ @@ -346,7 +360,7 @@ { "type": "deployment", "dependencies": [ - 2 + 3 ], "declared": true, "instance": "instance1", @@ -361,7 +375,8 @@ "targetApplication": { "id": 4, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } } } @@ -420,7 +435,7 @@ { "type": "deployment", "dependencies": [ - 2 + 3 ], "declared": true, "instance": "instance1", @@ -435,7 +450,8 @@ "targetApplication": { "id": 4, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } } } @@ -451,7 +467,8 @@ "targetApplication": { "id": 1, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } }, "steps": [ @@ -492,6 +509,15 @@ ] }, { + "type": "instance", + "dependencies": [ + 4, + 5 + ], + "declared": true, + "instance": "instance2" + }, + { "type": "test", "dependencies": [], "declared": false, @@ -518,8 +544,7 @@ { "type": "deployment", "dependencies": [ - 3, - 4 + 6 ], "declared": true, "instance": "instance2", @@ -534,7 +559,8 @@ "targetApplication": { "id": 4, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } } } @@ -544,7 +570,7 @@ { "type": "deployment", "dependencies": [ - 7 + 9 ], "declared": true, "instance": "instance2", @@ -559,7 +585,8 @@ "targetApplication": { "id": 4, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } } } @@ -569,7 +596,7 @@ { "type": "deployment", "dependencies": [ - 7 + 9 ], "declared": true, "instance": "instance2", @@ -584,7 +611,8 @@ "targetApplication": { "id": 4, "commit": "commit1", - "source": "repository1/tree/commit1" + "source": "repository1/tree/commit1", + "compileVersion": "6.1.0" } } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview-user-instance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview-user-instance.json index 1af3abeea91..6d5a1f3812b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview-user-instance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/overview-user-instance.json @@ -39,7 +39,7 @@ }, "hash": "1.0.3-commit1" }, - "deploying": "0 of 0 complete" + "completed": "0 of 0 complete" }, "platform": { "at": 0, @@ -47,16 +47,7 @@ "platform": "7.1" } }, - "deploying": { - "application": { - "build": 3, - "source": { - "gitRepository": "repository1", - "gitCommit": "commit1", - "gitBranch": "master" - }, - "hash": "1.0.3-commit1" - } - }, + "deploying": {}, "jobs": {} } + diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java index b3cb74e9501..6df2b00c9e5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.restapi.deployment; import com.yahoo.component.Version; +import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.ControllerTester; @@ -52,9 +53,9 @@ public class DeploymentApiTest extends ControllerContainerTest { productionApp.submit(multiInstancePackage).runJob(JobType.systemTest).runJob(JobType.stagingTest).runJob(JobType.productionUsWest1); otherProductionApp.runJob(JobType.productionUsWest1); - // Deploy once so that job information is stored, then remove the deployment + // Deploy once so that job information is stored, then remove the deployment by submitting an empty deployment spec. appWithoutDeployments.submit(applicationPackage).deploy(); - deploymentTester.applications().deactivate(appWithoutDeployments.instanceId(), ZoneId.from("prod", "us-west-1")); + appWithoutDeployments.submit(new ApplicationPackageBuilder().allow(ValidationId.deploymentRemoval).build()); // New version released version = Version.fromString("5.1"); @@ -65,6 +66,7 @@ public class DeploymentApiTest extends ControllerContainerTest { deploymentTester.triggerJobs(); productionApp.runJob(JobType.systemTest).runJob(JobType.stagingTest).runJob(JobType.productionUsWest1); failingApp.runJob(JobType.systemTest).failDeployment(JobType.stagingTest); + deploymentTester.upgrader().maintain(); deploymentTester.triggerJobs(); tester.controller().updateVersionStatus(censorConfigServers(VersionStatus.compute(tester.controller()))); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java index db79e3164fc..dca5c092ad2 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java @@ -108,7 +108,7 @@ public class NodeFailer extends Maintainer { continue; } String reason = entry.getValue(); - nodeRepository().fail(node.hostname(), Agent.system, reason); + nodeRepository().fail(node.hostname(), Agent.NodeFailer, reason); } } @@ -144,7 +144,7 @@ public class NodeFailer extends Maintainer { if (! node.history().hasEventAfter(History.Event.Type.requested, lastLocalRequest.get())) { History updatedHistory = node.history() - .with(new History.Event(History.Event.Type.requested, Agent.system, lastLocalRequest.get())); + .with(new History.Event(History.Event.Type.requested, Agent.NodeFailer, lastLocalRequest.get())); nodeRepository().write(node.with(updatedHistory), lock); } } @@ -357,12 +357,12 @@ public class NodeFailer extends Maintainer { if (failingTenantNode.state() == Node.State.active) { allTenantNodesFailedOutSuccessfully &= failActive(failingTenantNode, reasonForChildFailure); } else { - nodeRepository().fail(failingTenantNode.hostname(), Agent.system, reasonForChildFailure); + nodeRepository().fail(failingTenantNode.hostname(), Agent.NodeFailer, reasonForChildFailure); } } if (! allTenantNodesFailedOutSuccessfully) return false; - node = nodeRepository().fail(node.hostname(), Agent.system, reason); + node = nodeRepository().fail(node.hostname(), Agent.NodeFailer, reason); try { deployment.get().activate(); return true; @@ -373,7 +373,7 @@ public class NodeFailer extends Maintainer { } catch (RuntimeException e) { // The expected reason for deployment to fail here is that there is no capacity available to redeploy. // In that case we should leave the node in the active state to avoid failing additional nodes. - nodeRepository().reactivate(node.hostname(), Agent.system, + nodeRepository().reactivate(node.hostname(), Agent.NodeFailer, "Failed to redeploy after being failed by NodeFailer"); log.log(Level.WARNING, "Attempted to fail " + node + " for " + node.allocation().get().owner() + ", but redeploying without the node failed", e); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java index feab5ed1ed8..7ed05432e01 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java @@ -203,7 +203,7 @@ class NodesResponse extends HttpResponse { Cursor object = array.addObject(); object.setString("event", event.type().name()); object.setLong("at", event.at().toEpochMilli()); - object.setString("agent", normalizedAgentUntilV6IsGone(event.agent()).name()); + object.setString("agent", event.agent().name()); } } @@ -232,12 +232,6 @@ class NodesResponse extends HttpResponse { return nodeRepository.dockerImage(nodeType.isDockerHost() ? nodeType.childNodeType() : nodeType); } - - /** maven-vespa-plugin @ v6 needs to deserialize nodes w/history. */ - private Agent normalizedAgentUntilV6IsGone(Agent agent) { - return agent == Agent.NodeFailer ? Agent.system : agent; - } - private void ipAddressesToSlime(Set<String> ipAddresses, Cursor array) { ipAddresses.forEach(array::addString); } diff --git a/searchlib/src/tests/attribute/posting_list_merger/posting_list_merger_test.cpp b/searchlib/src/tests/attribute/posting_list_merger/posting_list_merger_test.cpp index a9fde8c8dd3..6ee3eeaccba 100644 --- a/searchlib/src/tests/attribute/posting_list_merger/posting_list_merger_test.cpp +++ b/searchlib/src/tests/attribute/posting_list_merger/posting_list_merger_test.cpp @@ -3,6 +3,7 @@ #include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/attribute/posting_list_merger.h> #include <vespa/vespalib/test/insertion_operators.h> +#include <algorithm> using search::btree::BTreeNoLeafData; using search::attribute::PostingListMerger; diff --git a/searchlib/src/tests/common/bitvector/bitvector_test.cpp b/searchlib/src/tests/common/bitvector/bitvector_test.cpp index e61c21bee1c..4cbe96c74b5 100644 --- a/searchlib/src/tests/common/bitvector/bitvector_test.cpp +++ b/searchlib/src/tests/common/bitvector/bitvector_test.cpp @@ -9,6 +9,7 @@ #include <vespa/searchlib/fef/termfieldmatchdata.h> #include <vespa/searchlib/fef/termfieldmatchdataarray.h> #include <vespa/searchlib/util/rand48.h> +#include <algorithm> using namespace search; diff --git a/searchlib/src/tests/docstore/document_store/document_store_test.cpp b/searchlib/src/tests/docstore/document_store/document_store_test.cpp index 86c6d3ad883..f950377be4b 100644 --- a/searchlib/src/tests/docstore/document_store/document_store_test.cpp +++ b/searchlib/src/tests/docstore/document_store/document_store_test.cpp @@ -105,9 +105,9 @@ void verifyValue(vespalib::stringref s, const Value & v) { TEST("require that Value and cache entries have expected size") { using pair = std::pair<DocumentIdT, Value>; using Node = vespalib::hash_node<pair>; - EXPECT_EQUAL(64ul, sizeof(Value)); - EXPECT_EQUAL(72ul, sizeof(pair)); - EXPECT_EQUAL(80ul, sizeof(Node)); + EXPECT_EQUAL(48ul, sizeof(Value)); + EXPECT_EQUAL(56ul, sizeof(pair)); + EXPECT_EQUAL(64ul, sizeof(Node)); } TEST("require that Value can store uncompressed data") { @@ -144,7 +144,7 @@ TEST("require that Value can store zstd compressed data") { TEST("require that Value can detect if output not equal to input") { Value v = createValue(S1, CompressionConfig::NONE); - static_cast<uint8_t *>(v.get())[8] ^= 0xff; + const_cast<uint8_t *>(static_cast<const uint8_t *>(v.get()))[8] ^= 0xff; EXPECT_FALSE(v.decompressed().second); } diff --git a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp index f92f5fd9581..c383358db9e 100644 --- a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp +++ b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp @@ -626,7 +626,7 @@ TEST("test that the integrated visit cache works.") { for (size_t i(1); i <= 100; i++) { vcs.verifyRead(i); } - constexpr size_t BASE_SZ = 22174; + constexpr size_t BASE_SZ = 20594; TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 100, 100, BASE_SZ)); for (size_t i(1); i <= 100; i++) { vcs.verifyRead(i); @@ -648,16 +648,16 @@ TEST("test that the integrated visit cache works.") { vcs.verifyVisit({7,9,17,19,67,88,89}, true); TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 103, 99, BASE_SZ+180)); vcs.rewrite(17); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 103, 97, BASE_SZ-680)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 103, 97, BASE_SZ-671)); vcs.verifyVisit({7,9,17,19,67,88,89}, true); TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 98, BASE_SZ-20)); vcs.remove(17); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 97, BASE_SZ-680)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 97, BASE_SZ-671)); vcs.verifyVisit({7,9,17,19,67,88,89}, {7,9,19,67,88,89}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 105, 98, BASE_SZ-90)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 105, 98, BASE_SZ-89)); vcs.verifyVisit({41, 42}, true); - TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 106, 99, BASE_SZ+210)); + TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 106, 99, BASE_SZ+215)); vcs.verifyVisit({43, 44}, true); TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 107, 100, BASE_SZ+520)); vcs.verifyVisit({41, 42, 43, 44}, true); diff --git a/searchlib/src/vespa/searchlib/attribute/posting_list_merger.cpp b/searchlib/src/vespa/searchlib/attribute/posting_list_merger.cpp index 39ca733eae7..b1ad8665441 100644 --- a/searchlib/src/vespa/searchlib/attribute/posting_list_merger.cpp +++ b/searchlib/src/vespa/searchlib/attribute/posting_list_merger.cpp @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "posting_list_merger.h" +#include <algorithm> namespace search::attribute { diff --git a/searchlib/src/vespa/searchlib/docstore/value.cpp b/searchlib/src/vespa/searchlib/docstore/value.cpp index 68bb060683d..ea29c894cba 100644 --- a/searchlib/src/vespa/searchlib/docstore/value.cpp +++ b/searchlib/src/vespa/searchlib/docstore/value.cpp @@ -12,29 +12,29 @@ namespace search::docstore { Value::Value() : _syncToken(0), + _uncompressedCrc(0), _compressedSize(0), _uncompressedSize(0), - _uncompressedCrc(0), + _buf(), _compression(CompressionConfig::NONE) {} Value::Value(uint64_t syncToken) : _syncToken(syncToken), + _uncompressedCrc(0), _compressedSize(0), _uncompressedSize(0), - _uncompressedCrc(0), + _buf(), _compression(CompressionConfig::NONE) {} -Value::Value(const Value &rhs) - : _syncToken(rhs._syncToken), - _compressedSize(rhs._compressedSize), - _uncompressedSize(rhs._uncompressedSize), - _uncompressedCrc(rhs._uncompressedCrc), - _compression(rhs._compression), - _buf(Alloc::alloc(rhs.size())) -{ - memcpy(get(), rhs.get(), size()); +Value::Value(const Value &rhs) = default; + +Value::~Value() = default; + +const void * +Value::get() const { + return _buf ? _buf->get() : nullptr; } void @@ -44,16 +44,18 @@ Value::set(vespalib::DataBuffer &&buf, ssize_t len) { void Value::set(vespalib::DataBuffer &&buf, ssize_t len, const CompressionConfig &compression) { + assert(len < std::numeric_limits<uint32_t>::max()); //Underlying buffer must be identical to allow swap. vespalib::DataBuffer compressed(buf.getData(), 0u); vespalib::ConstBufferRef input(buf.getData(), len); CompressionConfig::Type type = compress(compression, input, compressed, true); _compressedSize = compressed.getDataLen(); + if (buf.getData() == compressed.getData()) { // Uncompressed so we can just steal the underlying buffer. - buf.stealBuffer().swap(_buf); + _buf = std::make_shared<Alloc>(buf.stealBuffer()); } else { - compressed.stealBuffer().swap(_buf); + _buf = std::make_shared<Alloc>(compressed.stealBuffer()); } assert(((type == CompressionConfig::NONE) && (len == ssize_t(_compressedSize))) || diff --git a/searchlib/src/vespa/searchlib/docstore/value.h b/searchlib/src/vespa/searchlib/docstore/value.h index 426bcaf0e31..f58d96e3e77 100644 --- a/searchlib/src/vespa/searchlib/docstore/value.h +++ b/searchlib/src/vespa/searchlib/docstore/value.h @@ -25,6 +25,7 @@ public: Value &operator=(Value &&rhs) = default; Value(const Value &rhs); + ~Value(); uint64_t getSyncToken() const { return _syncToken; } CompressionConfig::Type getCompression() const { return _compression; } @@ -42,16 +43,15 @@ public: size_t size() const { return _compressedSize; } bool empty() const { return size() == 0; } - operator const void *() const { return _buf.get(); } - const void *get() const { return _buf.get(); } - void *get() { return _buf.get(); } + operator const void *() const { return get(); } + const void *get() const; private: - uint64_t _syncToken; - size_t _compressedSize; - size_t _uncompressedSize; - uint64_t _uncompressedCrc; + uint64_t _syncToken; + uint64_t _uncompressedCrc; + uint32_t _compressedSize; + uint32_t _uncompressedSize; + std::shared_ptr<Alloc> _buf; CompressionConfig::Type _compression; - Alloc _buf; }; } diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessorTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessorTest.java index 0c636ba804e..9753a180618 100644 --- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessorTest.java +++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessorTest.java @@ -11,14 +11,11 @@ import com.yahoo.vespa.http.client.core.EndpointResult; import org.junit.Test; import java.util.ArrayDeque; -import java.util.Optional; import java.util.Queue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; @@ -79,46 +76,45 @@ public class OperationProcessorTest { //check a, b, c, d Result aggregated = queue.poll(); - assertThat(aggregated.getDocumentId(), equalTo("id:a:type::b")); - assertThat(aggregated.getDetails().size(), is(4)); - assertThat(aggregated.getDetails().get(0).getEndpoint().getHostname(), equalTo("a")); - assertThat(aggregated.getDetails().get(1).getEndpoint().getHostname(), equalTo("b")); - assertThat(aggregated.getDetails().get(2).getEndpoint().getHostname(), equalTo("c")); - assertThat(aggregated.getDetails().get(3).getEndpoint().getHostname(), equalTo("d")); - assertThat(aggregated.getDocumentDataAsCharSequence().toString(), is("data doc 1")); - - assertThat(queue.size(), is(0)); + assertEquals("id:a:type::b", aggregated.getDocumentId()); + assertEquals(4, aggregated.getDetails().size()); + assertEquals("a", aggregated.getDetails().get(0).getEndpoint().getHostname()); + assertEquals("b", aggregated.getDetails().get(1).getEndpoint().getHostname()); + assertEquals("c", aggregated.getDetails().get(2).getEndpoint().getHostname()); + assertEquals("d", aggregated.getDetails().get(3).getEndpoint().getHostname()); + assertEquals("data doc 1", aggregated.getDocumentDataAsCharSequence().toString()); + assertEquals(0, queue.size()); q.sendDocument(doc2); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc2.getOperationId(), new Result.Detail(Endpoint.create("a"))), 0); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc2.getOperationId(), new Result.Detail(Endpoint.create("b"))), 1); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc2.getOperationId(), new Result.Detail(Endpoint.create("c"))), 2); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc2.getOperationId(), new Result.Detail(Endpoint.create("d"))), 3); - assertThat(queue.size(), is(1)); + assertEquals(1, queue.size()); q.resultReceived(new EndpointResult(doc2.getOperationId(), new Result.Detail(Endpoint.create("e"))), 0); - assertThat(queue.size(), is(1)); + assertEquals(1, queue.size()); - //check a, b, c, d + // check a, b, c, d aggregated = queue.poll(); - assertThat(aggregated.getDocumentId(), equalTo("id:a:type::b2")); - assertThat(aggregated.getDetails().size(), is(4)); - assertThat(aggregated.getDetails().get(0).getEndpoint().getHostname(), equalTo("a")); - assertThat(aggregated.getDetails().get(1).getEndpoint().getHostname(), equalTo("b")); - assertThat(aggregated.getDetails().get(2).getEndpoint().getHostname(), equalTo("c")); - assertThat(aggregated.getDetails().get(3).getEndpoint().getHostname(), equalTo("d")); - assertThat(aggregated.getDocumentDataAsCharSequence().toString(), is("data doc 2")); - - assertThat(queue.size(), is(0)); + assertEquals("id:a:type::b2", aggregated.getDocumentId()); + assertEquals(4, aggregated.getDetails().size()); + assertEquals("a", aggregated.getDetails().get(0).getEndpoint().getHostname()); + assertEquals("b", aggregated.getDetails().get(1).getEndpoint().getHostname()); + assertEquals("c", aggregated.getDetails().get(2).getEndpoint().getHostname()); + assertEquals("d", aggregated.getDetails().get(3).getEndpoint().getHostname()); + assertEquals("data doc 2", aggregated.getDocumentDataAsCharSequence().toString()); + + assertEquals(0, queue.size()); } @Test @@ -136,27 +132,27 @@ public class OperationProcessorTest { operationProcessor.sendDocument(doc1); operationProcessor.sendDocument(doc1b); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); // Only one operations should be in flight. - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(1)); + assertEquals(1, operationProcessor.getIncompleteResultQueueSize()); operationProcessor.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("host"))), 0); - assertThat(queue.size(), is(0)); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(1)); + assertEquals(0, queue.size()); + assertEquals(1, operationProcessor.getIncompleteResultQueueSize()); operationProcessor.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("host"))), 1); - assertThat(queue.size(), is(1)); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(1)); + assertEquals(1, queue.size()); + assertEquals(1, operationProcessor.getIncompleteResultQueueSize()); operationProcessor.resultReceived(new EndpointResult(doc1b.getOperationId(), new Result.Detail(Endpoint.create("host"))), 0); - assertThat(queue.size(), is(1)); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(1)); + assertEquals(1, queue.size()); + assertEquals(1, operationProcessor.getIncompleteResultQueueSize()); operationProcessor.resultReceived(new EndpointResult(doc1b.getOperationId(), new Result.Detail(Endpoint.create("host"))), 1); - assertThat(queue.size(), is(2)); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(0)); + assertEquals(2, queue.size()); + assertEquals(0, operationProcessor.getIncompleteResultQueueSize()); // This should have no effect. operationProcessor.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("host"))), 0); operationProcessor.resultReceived(new EndpointResult(doc1b.getOperationId(), new Result.Detail(Endpoint.create("host"))), 0); operationProcessor.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("host"))), 1); operationProcessor.resultReceived(new EndpointResult(doc1b.getOperationId(), new Result.Detail(Endpoint.create("host"))), 1); - assertThat(queue.size(), is(2)); + assertEquals(2, queue.size()); } @Test @@ -174,22 +170,22 @@ public class OperationProcessorTest { operationProcessor.sendDocument(doc1); operationProcessor.sendDocument(doc1b); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); // Only one operations should be in flight. - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(1)); - assertThat(operationProcessor.oldestIncompleteResultId(), is(Optional.of(doc1.getOperationId()))); + assertEquals(1, operationProcessor.getIncompleteResultQueueSize()); + assertEquals(doc1.getOperationId(), operationProcessor.oldestIncompleteResultId().get()); operationProcessor.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("host"))), 0); - assertThat(queue.size(), is(1)); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(1)); - assertThat(operationProcessor.oldestIncompleteResultId(), is(Optional.of(doc1b.getOperationId()))); + assertEquals(1, queue.size()); + assertEquals(1, operationProcessor.getIncompleteResultQueueSize()); + assertEquals(doc1b.getOperationId(), operationProcessor.oldestIncompleteResultId().get()); operationProcessor.resultReceived(new EndpointResult(doc1b.getOperationId(), new Result.Detail(Endpoint.create("host"))), 0); - assertThat(queue.size(), is(2)); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(0)); - assertThat(operationProcessor.oldestIncompleteResultId(), is(Optional.empty())); + assertEquals(2, queue.size()); + assertEquals(0, operationProcessor.getIncompleteResultQueueSize()); + assertFalse(operationProcessor.oldestIncompleteResultId().isPresent()); // This should have no effect. operationProcessor.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("host"))), 0); operationProcessor.resultReceived(new EndpointResult(doc1b.getOperationId(), new Result.Detail(Endpoint.create("host"))), 0); - assertThat(queue.size(), is(2)); + assertEquals(2, queue.size()); } @Test @@ -212,16 +208,16 @@ public class OperationProcessorTest { } for (int x = 0; x < 100; x++) { - assertThat(queue.size(), is(x)); + assertEquals(x, queue.size()); // Only one operations should be in flight. - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(1)); + assertEquals(1, operationProcessor.getIncompleteResultQueueSize()); Document document = documentQueue.poll(); operationProcessor.resultReceived(new EndpointResult(document.getOperationId(), new Result.Detail(Endpoint.create("host"))), 0); - assertThat(queue.size(), is(x + 1)); + assertEquals(x+1, queue.size()); if (x < 99) { - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(1)); + assertEquals(1, operationProcessor.getIncompleteResultQueueSize()); } else { - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(0)); + assertEquals(0, operationProcessor.getIncompleteResultQueueSize()); } } } @@ -244,26 +240,26 @@ public class OperationProcessorTest { operationProcessor.sendDocument(doc2); operationProcessor.sendDocument(doc3); - assertThat(queue.size(), is(0)); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(3)); - assertThat(operationProcessor.oldestIncompleteResultId(), is(Optional.of(doc1.getOperationId()))); + assertEquals(0, queue.size()); + assertEquals(3, operationProcessor.getIncompleteResultQueueSize()); + assertEquals(doc1.getOperationId(), operationProcessor.oldestIncompleteResultId().get()); // This should have no effect since it should not be sent. operationProcessor.resultReceived(new EndpointResult(doc1b.getOperationId(), new Result.Detail(endpoint)), 0); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(3)); - assertThat(operationProcessor.oldestIncompleteResultId(), is(Optional.of(doc1.getOperationId()))); + assertEquals(3, operationProcessor.getIncompleteResultQueueSize()); + assertEquals(doc1.getOperationId(), operationProcessor.oldestIncompleteResultId().get()); operationProcessor.resultReceived(new EndpointResult(doc3.getOperationId(), new Result.Detail(endpoint)), 0); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(2)); - assertThat(operationProcessor.oldestIncompleteResultId(), is(Optional.of(doc1.getOperationId()))); + assertEquals(2, operationProcessor.getIncompleteResultQueueSize()); + assertEquals(doc1.getOperationId(), operationProcessor.oldestIncompleteResultId().get()); operationProcessor.resultReceived(new EndpointResult(doc2.getOperationId(), new Result.Detail(endpoint)), 0); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(1)); - assertThat(operationProcessor.oldestIncompleteResultId(), is(Optional.of(doc1.getOperationId()))); + assertEquals(1, operationProcessor.getIncompleteResultQueueSize()); + assertEquals(doc1.getOperationId(), operationProcessor.oldestIncompleteResultId().get()); operationProcessor.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(endpoint)), 0); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(1)); - assertThat(operationProcessor.oldestIncompleteResultId(), is(Optional.of(doc1b.getOperationId()))); + assertEquals(1, operationProcessor.getIncompleteResultQueueSize()); + assertEquals(doc1b.getOperationId(), operationProcessor.oldestIncompleteResultId().get()); operationProcessor.resultReceived(new EndpointResult(doc1b.getOperationId(), new Result.Detail(endpoint)), 0); - assertThat(operationProcessor.getIncompleteResultQueueSize(), is(0)); - assertThat(operationProcessor.oldestIncompleteResultId(), is(Optional.empty())); + assertEquals(0, operationProcessor.getIncompleteResultQueueSize()); + assertFalse(operationProcessor.oldestIncompleteResultId().isPresent()); } @Test @@ -280,16 +276,16 @@ public class OperationProcessorTest { sessionParams, null); q.sendDocument(doc1); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("a"))), 0); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("b"))), 0); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("c"))), 0); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); } @Test @@ -306,52 +302,51 @@ public class OperationProcessorTest { sessionParams, null); q.sendDocument(doc1); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.sendDocument(doc2); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.sendDocument(doc3); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("a"))), 0); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("a"))), 0); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("b"))), 1); - assertThat(queue.size(), is(0)); + assertEquals(0, queue.size()); q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("c"))), 2); - assertThat(queue.size(), is(1)); + assertEquals(1, queue.size()); q.resultReceived(new EndpointResult(doc3.getOperationId(), new Result.Detail(Endpoint.create("a"))), 0); - assertThat(queue.size(), is(1)); + assertEquals(1, queue.size()); q.resultReceived(new EndpointResult(doc2.getOperationId(), new Result.Detail(Endpoint.create("a"))), 0); - assertThat(queue.size(), is(1)); + assertEquals(1, queue.size()); q.resultReceived(new EndpointResult(doc2.getOperationId(), new Result.Detail(Endpoint.create("b"))), 1); - assertThat(queue.size(), is(1)); + assertEquals(1, queue.size()); q.resultReceived(new EndpointResult(doc2.getOperationId(), new Result.Detail(Endpoint.create("c"))), 2); - assertThat(queue.size(), is(2)); + assertEquals(2, queue.size()); q.resultReceived(new EndpointResult(doc3.getOperationId(), new Result.Detail(Endpoint.create("c"))), 2); - assertThat(queue.size(), is(2)); + assertEquals(2, queue.size()); q.resultReceived(new EndpointResult(doc3.getOperationId(), new Result.Detail(Endpoint.create("c"))), 2); - assertThat(queue.size(), is(2)); + assertEquals(2, queue.size()); q.resultReceived(new EndpointResult(doc3.getOperationId(), new Result.Detail(Endpoint.create("b"))), 1); - assertThat(queue.size(), is(3)); + assertEquals(3, queue.size()); q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("b"))), 1); - assertThat(queue.size(), is(3)); - assertThat(queue.remove().getDocumentDataAsCharSequence().toString(), is("data doc 1")); - assertThat(queue.remove().getDocumentDataAsCharSequence().toString(), is("data doc 2")); - assertThat(queue.remove().getDocumentDataAsCharSequence().toString(), is("data doc 3")); - + assertEquals(3, queue.size()); + assertEquals("data doc 1", queue.remove().getDocumentDataAsCharSequence().toString()); + assertEquals("data doc 2", queue.remove().getDocumentDataAsCharSequence().toString()); + assertEquals("data doc 3", queue.remove().getDocumentDataAsCharSequence().toString()); } @Test @@ -425,7 +420,7 @@ public class OperationProcessorTest { CountDownLatch countDownLatch = new CountDownLatch(3); - OperationProcessor operationProcessor = new OperationProcessor( + new OperationProcessor( new IncompleteResultsThrottler(19, 19, null, null), (docId, documentResult) -> { countDownLatch.countDown(); diff --git a/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java b/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java index 95e3f7fcf39..b7c6322d951 100644 --- a/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java +++ b/vespajlib/src/main/java/com/yahoo/collections/AbstractFilteringList.java @@ -16,7 +16,7 @@ import static java.util.stream.Collectors.reducing; import static java.util.stream.Collectors.toUnmodifiableList; /** - * Abstract, immutable list for subclassing with concrete types, and domain specific filters. + * Abstract, immutable list for subclassing with concrete types and domain specific filters. * * @author jonmv */ @@ -52,7 +52,7 @@ public abstract class AbstractFilteringList<Type, ListType extends AbstractFilte return constructor.apply(items.subList(negate ? n : 0, negate ? items.size() : n), false); } - /** Returns the first item in this list, or empty if there is none. */ + /** Returns the first item in this list, or empty if there are none. */ public Optional<Type> first() { return items.stream().findFirst(); } |