diff options
14 files changed, 285 insertions, 40 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 4635070fee8..db85fd47a43 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -445,7 +445,8 @@ "public static com.yahoo.config.application.api.DeploymentSpec fromXml(java.lang.String, boolean)", "public static java.lang.String toMessageString(java.lang.Throwable)", "public boolean equals(java.lang.Object)", - "public int hashCode()" + "public int hashCode()", + "public int deployableHashCode()" ], "fields": [ "public static final com.yahoo.config.application.api.DeploymentSpec empty" diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java index e701f3903cb..677933f3b85 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java @@ -9,14 +9,21 @@ import com.yahoo.config.provision.RegionName; import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.yahoo.config.provision.Environment.prod; +import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; + /** * The deployment spec for an application instance * @@ -224,6 +231,19 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { return Objects.hash(globalServiceId, upgradePolicy, revisionTarget, upgradeRollout, changeBlockers, steps(), athenzService, notifications, endpoints); } + int deployableHashCode() { + List<DeploymentSpec.DeclaredZone> zones = zones().stream().filter(zone -> zone.concerns(prod)).collect(toList()); + Object[] toHash = new Object[zones.size() + 3]; + int i = 0; + toHash[i++] = name; + toHash[i++] = endpoints; + toHash[i++] = globalServiceId; + for (DeploymentSpec.DeclaredZone zone : zones) + toHash[i++] = Objects.hash(zone, zone.athenzService()); + + return Arrays.hashCode(toHash); + } + @Override public String toString() { return "instance '" + name + "'"; diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java index 96e3ba75a38..9135e9f49ff 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java @@ -11,6 +11,7 @@ import com.yahoo.config.provision.RegionName; import java.io.Reader; import java.time.Duration; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.List; @@ -285,6 +286,19 @@ public class DeploymentSpec { return Objects.hash(majorVersion, steps, xmlForm); } + /** Computes a hash of all fields that influence what is deployed with this spec, i.e., not orchestration. */ + public int deployableHashCode() { + Object[] toHash = new Object[instances().size() + 3]; + int i = 0; + toHash[i++] = athenzDomain; + toHash[i++] = athenzService; + toHash[i++] = endpoints; + for (DeploymentInstanceSpec instance : instances()) + toHash[i++] = instance.deployableHashCode(); + + return Arrays.hashCode(toHash); + } + /** A deployment step */ public abstract static class Step { @@ -395,7 +409,7 @@ public class DeploymentSpec { public int hashCode() { return Objects.hash(environment, region); } - + @Override public boolean equals(Object o) { if (o == this) return true; @@ -405,7 +419,7 @@ public class DeploymentSpec { if ( ! this.region.equals(other.region())) return false; return true; } - + @Override public String toString() { return environment + (region.map(regionName -> "." + regionName).orElse("")); diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java index 86a2ede85ab..5efa49b3656 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java @@ -25,6 +25,7 @@ import static com.yahoo.config.application.api.Notifications.When.failing; import static com.yahoo.config.application.api.Notifications.When.failingCommit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -1337,6 +1338,131 @@ public class DeploymentSpecTest { } } + @Test + public void testDeployableHash() { + assertEquals(DeploymentSpec.fromXml("<deployment>\n" + + " <instance id='default' />\n" + + "</deployment>").deployableHashCode(), + DeploymentSpec.fromXml("<deployment major-version='9'>\n" + + " <instance id='default'>\n" + + " <test />\n" + + " <staging tester-flavor='2-8-50' />\n" + + " <block-change days='mon' />\n" + + " <upgrade policy='canary' revision-target='next' revision-change='when-clear' rollout='simultaneous' />\n" + + " <prod />\n" + + " <notifications>\n" + + " <email role='author' />\n" + + " <email address='dev@duff' />\n" + + " </notifications>\n" + + " </instance>\n" + + "</deployment>").deployableHashCode()); + + assertEquals(DeploymentSpec.fromXml("<deployment>\n" + + " <parallel>\n" + + " <instance id='one'>\n" + + " <prod>\n" + + " <region>name</region>\n" + + " </prod>\n" + + " </instance>\n" + + " <instance id='two' />" + + " </parallel>\n" + + "</deployment>").deployableHashCode(), + DeploymentSpec.fromXml("<deployment>\n" + + " <instance id='one'>\n" + + " <prod>\n" + + " <steps>\n" + + " <region>name</region>\n" + + " <delay hours='3' />\n" + + " <test>name</test>\n" + + " </steps>\n" + + " </prod>\n" + + " </instance>\n" + + " <instance id='two' />" + + "</deployment>").deployableHashCode()); + + String referenceSpec = "<deployment>\n" + + " <instance id='default'>" + + " <prod>" + + " <region>name</region>\n" + + " </prod>\n" + + " </instance>\n" + + "</deployment>"; + + assertNotEquals(DeploymentSpec.fromXml(referenceSpec).deployableHashCode(), + DeploymentSpec.fromXml("<deployment />").deployableHashCode()); + + assertNotEquals(DeploymentSpec.fromXml(referenceSpec).deployableHashCode(), + DeploymentSpec.fromXml("<deployment>\n" + + " <instance id='default' />\n" + + "</deployment>").deployableHashCode()); + + assertNotEquals(DeploymentSpec.fromXml(referenceSpec).deployableHashCode(), + DeploymentSpec.fromXml("<deployment>\n" + + " <instance id='custom'>" + + " <prod>" + + " <region>name</region>\n" + + " </prod>\n" + + " </instance>\n" + + "</deployment>").deployableHashCode()); + + assertNotEquals(DeploymentSpec.fromXml(referenceSpec).deployableHashCode(), + DeploymentSpec.fromXml("<deployment>\n" + + " <instance id='custom'>" + + " <prod>" + + " <region>other</region>\n" + + " </prod>\n" + + " </instance>\n" + + "</deployment>").deployableHashCode()); + + assertNotEquals(DeploymentSpec.fromXml(referenceSpec).deployableHashCode(), + DeploymentSpec.fromXml("<deployment athenz-domain='domain' athenz-service='service'>\n" + + " <instance id='default'>" + + " <prod>" + + " <region>name</region>\n" + + " </prod>\n" + + " </instance>\n" + + "</deployment>").deployableHashCode()); + + assertNotEquals(DeploymentSpec.fromXml(referenceSpec).deployableHashCode(), + DeploymentSpec.fromXml("<deployment athenz-domain='domain'>\n" + + " <instance id='default' athenz-service='service'>" + + " <prod>" + + " <region>name</region>\n" + + " </prod>\n" + + " </instance>\n" + + "</deployment>").deployableHashCode()); + + assertNotEquals(DeploymentSpec.fromXml(referenceSpec).deployableHashCode(), + DeploymentSpec.fromXml("<deployment athenz-domain='domain'>\n" + + " <instance id='default'>" + + " <prod athenz-service='prod'>" + + " <region>name</region>\n" + + " </prod>\n" + + " </instance>\n" + + "</deployment>").deployableHashCode()); + + assertNotEquals(DeploymentSpec.fromXml(referenceSpec).deployableHashCode(), + DeploymentSpec.fromXml("<deployment>\n" + + " <instance id='default'>" + + " <prod global-service-id='service'>" + + " <region>name</region>\n" + + " </prod>\n" + + " </instance>\n" + + "</deployment>").deployableHashCode()); + + assertNotEquals(DeploymentSpec.fromXml(referenceSpec).deployableHashCode(), + DeploymentSpec.fromXml("<deployment>\n" + + " <instance id='default'>" + + " <prod>" + + " <region>name</region>\n" + + " </prod>\n" + + " <endpoints>" + + " <endpoint container-id=\"quux\" />" + + " </endpoints>" + + " </instance>\n" + + "</deployment>").deployableHashCode()); + } + private static void assertInvalid(String deploymentSpec, String errorMessagePart) { assertInvalid(deploymentSpec, errorMessagePart, new ManualClock()); } 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 3ef38715c5d..1a784af84e5 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 @@ -19,9 +19,12 @@ import com.yahoo.vespa.hosted.controller.tenant.Tenant; import java.security.PublicKey; import java.time.Instant; +import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.Deque; import java.util.List; import java.util.Map; import java.util.Objects; @@ -118,6 +121,20 @@ public class Application { return versions; } + /** Returns the currently deployed versions of the application */ + public Collection<ApplicationVersion> deployableVersions(boolean ascending) { + Deque<ApplicationVersion> versions = new ArrayDeque<>(); + String previousHash = ""; + for (ApplicationVersion version : versions()) { + if (previousHash.equals(previousHash = version.bundleHash().orElse("")) && version.bundleHash().isPresent()) + continue; + + if (ascending) versions.addLast(version); + else versions.addFirst(version); + } + return versions; + } + /** * Returns the last deployed validation overrides of this application, * or the empty validation overrides if it has never been deployed 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 2b88d6ded9e..88df49ad3ab 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 @@ -150,7 +150,7 @@ public class ApplicationController { for (TenantAndApplicationId id : curator.readApplicationIds()) { lockApplicationIfPresent(id, application -> { for (InstanceName instance : application.get().deploymentSpec().instanceNames()) - if (!application.get().instances().containsKey(instance)) + if ( ! application.get().instances().containsKey(instance)) application = withNewInstance(application, id.instance(instance)); store(application); }); @@ -416,7 +416,7 @@ public class ApplicationController { } /** Stores the deployment spec and validation overrides from the application package, and runs cleanup. */ - public LockedApplication storeWithUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) { + public void storeWithUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) { applicationPackageValidator.validate(application.get(), applicationPackage, clock.instant()); application = application.with(applicationPackage.deploymentSpec()); @@ -464,7 +464,6 @@ public class ApplicationController { } store(application); - return application; } /** Deploy a system application to given zone */ @@ -770,14 +769,14 @@ public class ApplicationController { public void verifyApplicationIdentityConfiguration(TenantName tenantName, Optional<InstanceName> instanceName, Optional<ZoneId> zoneId, ApplicationPackage applicationPackage, Optional<Principal> deployer) { Optional<AthenzDomain> identityDomain = applicationPackage.deploymentSpec().athenzDomain() .map(domain -> new AthenzDomain(domain.value())); - if(identityDomain.isEmpty()) { + if (identityDomain.isEmpty()) { // If there is no domain configured in deployment.xml there is nothing to do. return; } // Verify that the system supports launching services. // Consider adding a capability to the system. - if(! (accessControl instanceof AthenzFacade)) { + if ( ! (accessControl instanceof AthenzFacade)) { throw new IllegalArgumentException("Athenz domain and service specified in deployment.xml, but not supported by system."); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java index da62175089d..34e2f1350f0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java @@ -17,6 +17,7 @@ import com.yahoo.security.X509CertificateUtils; import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; +import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.deployment.ZipBuilder; import com.yahoo.yolean.Exceptions; @@ -66,7 +67,7 @@ public class ApplicationPackage { private static final String servicesFile = "services.xml"; private final String contentHash; - private final String bundleHash; // Hash of all files except deployment.xml and build-meta + private final String bundleHash; private final byte[] zippedContent; private final DeploymentSpec deploymentSpec; private final ValidationOverrides validationOverrides; @@ -125,6 +126,7 @@ public class ApplicationPackage { /** Returns a hash of the content of this package */ public String hash() { return contentHash; } + /** Hash of all files and settings that influence what is deployed to config servers. */ public String bundleHash() { return bundleHash; } @@ -133,8 +135,9 @@ public class ApplicationPackage { public byte[] zippedContent() { return zippedContent; } /** - * Returns the deployment spec from the deployment.xml file of the package content. - * This is the DeploymentSpec.empty instance if this package does not contain a deployment.xml file. + * Returns the deployment spec from the deployment.xml file of the package content.<br> + * This is the DeploymentSpec.empty instance if this package does not contain a deployment.xml file.<br> + * <em>NB: <strong>Always</strong> read deployment spec from the {@link Application}, for deployment orchestration.</em> */ public DeploymentSpec deploymentSpec() { return deploymentSpec; } @@ -220,15 +223,18 @@ public class ApplicationPackage { return ValidationOverrides.fromXml(validationOverridesContents.toString()); } - // Hashes all files that require a deployment to be forwarded to configservers + // Hashes all files and settings that require a deployment to be forwarded to configservers private String calculateBundleHash() { - Predicate<String> entryMatcher = name -> !name.endsWith(deploymentFile) && !name.endsWith(buildMetaFile); + Predicate<String> entryMatcher = name -> ! name.endsWith(deploymentFile) && ! name.endsWith(buildMetaFile); SortedMap<String, Long> entryCRCs = ZipStreamReader.getEntryCRCs(new ByteArrayInputStream(zippedContent), entryMatcher); Funnel<SortedMap<String, Long>> funnel = (from, into) -> from.entrySet().forEach(entry -> { into.putBytes(entry.getKey().getBytes()); into.putLong(entry.getValue()); }); - return Hashing.sha1().hashObject(entryCRCs, funnel).toString(); + return Hashing.sha1().newHasher() + .putObject(entryCRCs, funnel) + .putInt(deploymentSpec.deployableHashCode()) + .hash().toString(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java index 0eb7f3de3cf..174ac4cb8b0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java @@ -66,10 +66,9 @@ public class ZipStreamReader { public static SortedMap<String, Long> getEntryCRCs(InputStream in, Predicate<String> entryNameMatcher) { SortedMap<String, Long> entryCRCs = new TreeMap<>(); byte[] buffer = new byte[2048]; - try (ZipInputStream zipIn = new ZipInputStream(in); - OutputStream os = new ByteArrayOutputStream()) { + try (ZipInputStream zipIn = new ZipInputStream(in)) { for (ZipEntry entry = zipIn.getNextEntry(); entry != null; entry = zipIn.getNextEntry()) { - if (!entryNameMatcher.test(entry.getName())) + if ( ! entryNameMatcher.test(entry.getName())) continue; // CRC is not set until entry is read while ( -1 != zipIn.read(buffer)){} 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 d05a56a3ce0..4f546081c07 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 @@ -257,8 +257,7 @@ public class DeploymentStatus { */ public Change outstandingChange(InstanceName instance) { return Optional.ofNullable(instanceSteps().get(instance)) - .flatMap(instanceStatus -> application.versions().stream() - .sorted(application.deploymentSpec().requireInstance(instance).revisionTarget() == next ? naturalOrder() : reverseOrder()) + .flatMap(instanceStatus -> application.deployableVersions(application.deploymentSpec().requireInstance(instance).revisionTarget() == next).stream() .filter(version -> instanceStatus.dependenciesCompletedAt(Change.of(version), Optional.empty()).map(at -> ! at.isAfter(now)).orElse(false)) .filter(version -> application.productionDeployments().getOrDefault(instance, List.of()).stream() .noneMatch(deployment -> deployment.applicationVersion().compareTo(version) > 0)) @@ -269,14 +268,6 @@ public class DeploymentStatus { .orElse(Change.empty()); } - private Stream<InstanceStatus> allDependencies(StepStatus step) { - return step.dependencies.stream() - .flatMap(dep -> Stream.concat(Stream.of(dep), allDependencies(dep))) - .filter(InstanceStatus.class::isInstance) - .map(InstanceStatus.class::cast) - .distinct(); - } - /** Earliest instant when job was triggered with given versions, or both system and staging tests were successful. */ public Optional<Instant> verifiedAt(JobId job, Versions versions) { Optional<Instant> triggeredAt = allJobs.get(job) 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 fccb0326b6f..5f9f0aeb64c 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 @@ -3,7 +3,11 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.google.common.collect.ImmutableSortedMap; import com.yahoo.component.Version; +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.config.provision.RegionName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; @@ -45,10 +49,13 @@ import java.util.TreeMap; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.UnaryOperator; import java.util.logging.Level; +import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.yahoo.config.provision.Environment.prod; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.reset; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.running; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; @@ -61,6 +68,7 @@ import static java.util.Comparator.naturalOrder; import static java.util.function.Predicate.not; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; +import static java.util.stream.Collectors.toSet; import static java.util.stream.Collectors.toUnmodifiableList; /** @@ -440,22 +448,20 @@ public class JobController { byte[] testPackageBytes) { AtomicReference<ApplicationVersion> version = new AtomicReference<>(); controller.applications().lockApplicationOrThrow(id, application -> { - long run = 1 + application.get().latestVersion() - .map(latestVersion -> latestVersion.buildNumber().getAsLong()) - .orElse(0L); - version.set(ApplicationVersion.from(revision, run, authorEmail, + Optional<ApplicationVersion> previousVersion = application.get().latestVersion(); + Optional<ApplicationPackage> previousPackage = previousVersion.flatMap(previous -> controller.applications().applicationStore().find(id.tenant(), id.application(), previous.buildNumber().getAsLong())) + .map(ApplicationPackage::new); + long previousBuild = previousVersion.map(latestVersion -> latestVersion.buildNumber().getAsLong()).orElse(0L); + version.set(ApplicationVersion.from(revision, 1 + previousBuild, authorEmail, applicationPackage.compileVersion(), applicationPackage.buildTime(), sourceUrl, revision.map(SourceRevision::commit), false, Optional.of(applicationPackage.bundleHash()))); - byte[] diff = application.get().latestVersion() - .map(v -> v.buildNumber().getAsLong()) - .flatMap(prevBuild -> controller.applications().applicationStore().find(id.tenant(), id.application(), prevBuild)) - .map(prevApplication -> ApplicationPackageDiff.diff(new ApplicationPackage(prevApplication), applicationPackage)) - .orElseGet(() -> ApplicationPackageDiff.diffAgainstEmpty(applicationPackage)); + byte[] diff = previousPackage.map(previous -> ApplicationPackageDiff.diff(previous, applicationPackage)) + .orElseGet(() -> ApplicationPackageDiff.diffAgainstEmpty(applicationPackage)); controller.applications().applicationStore().put(id.tenant(), id.application(), version.get(), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java index cf0b46dfba2..9a831b27cb3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java @@ -114,16 +114,21 @@ public class ApplicationPackageTest { @Test public void testBundleHashesAreSameWithDifferentDeploymentXml() throws Exception { var originalPackage = getApplicationZip("original.zip"); - var changedDeploymentXml = getApplicationZip("changed-deployment-xml.zip"); var changedServices = getApplicationZip("changed-services-xml.zip"); + var changedDeploymentXml = getApplicationZip("changed-deployment-xml.zip"); + var similarDeploymentXml = getApplicationZip("similar-deployment-xml.zip"); // services.xml is changed -> different bundle hash assertNotEquals(originalPackage.bundleHash(), changedServices.bundleHash()); assertNotEquals(originalPackage.hash(), changedServices.hash()); - // deployment.xml is changed -> same bundle hash - assertEquals(originalPackage.bundleHash(), changedDeploymentXml.bundleHash()); + // deployment.xml is changed, with real changes -> different bundle hash + assertNotEquals(originalPackage.bundleHash(), changedDeploymentXml.bundleHash()); assertNotEquals(originalPackage.hash(), changedDeploymentXml.hash()); + + // deployment.xml is changed, but only deployment orchestration settings -> same bundle hash + assertEquals(originalPackage.bundleHash(), similarDeploymentXml.bundleHash()); + assertNotEquals(originalPackage.hash(), similarDeploymentXml.hash()); } private static Map<String, String> unzip(byte[] zip) { 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 9c5f7e3376a..1e41421de53 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 @@ -32,6 +32,7 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; +import com.yahoo.vespa.hosted.controller.application.pkg.ZipStreamReader; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.maintenance.JobRunner; import com.yahoo.vespa.hosted.controller.maintenance.NameServiceDispatcher; @@ -40,6 +41,8 @@ import com.yahoo.vespa.hosted.controller.routing.RoutingPolicy; import com.yahoo.vespa.hosted.controller.routing.RoutingPolicyId; import javax.security.auth.x500.X500Principal; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.math.BigInteger; import java.security.KeyPair; import java.security.cert.X509Certificate; @@ -52,6 +55,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.concurrent.atomic.AtomicLong; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.failed; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; @@ -76,6 +80,8 @@ import static org.junit.Assert.assertTrue; */ public class DeploymentContext { + private final AtomicLong salt = new AtomicLong(); + // Application packages are expensive to construct, and a given test typically only needs to the test in the context // of a single system. Construct them lazily. private static final Supplier<ApplicationPackage> applicationPackage = Suppliers.memoize(() -> new ApplicationPackageBuilder() @@ -255,13 +261,28 @@ public class DeploymentContext { } /** Submit given application package for deployment */ + public DeploymentContext submit(ApplicationPackage applicationPackage, long salt) { + return submit(applicationPackage, Optional.of(defaultSourceRevision), salt); + } + + /** Submit given application package for deployment */ public DeploymentContext submit(ApplicationPackage applicationPackage, Optional<SourceRevision> sourceRevision) { + return submit(applicationPackage, sourceRevision, salt.getAndIncrement()); + } + + /** Submit given application package for deployment */ + public DeploymentContext submit(ApplicationPackage applicationPackage, Optional<SourceRevision> sourceRevision, long salt) { + var buffer = new ByteArrayOutputStream(); + ZipStreamReader.transferAndWrite(buffer, + new ByteArrayInputStream(applicationPackage.zippedContent()), + "salt", + new byte[]{ (byte) (salt >> 56), (byte) (salt >> 48), (byte) (salt >> 40), (byte) (salt >> 32), (byte) (salt >> 24), (byte) (salt >> 16), (byte) (salt >> 8), (byte) salt }); var projectId = tester.controller().applications() .requireApplication(applicationId) .projectId() .orElse(1000); // These are really set through submission, so just pick one if it hasn't been set. lastSubmission = jobs.submit(applicationId, sourceRevision, Optional.of("a@b"), Optional.empty(), - projectId, applicationPackage, new byte[0]); + projectId, new ApplicationPackage(buffer.toByteArray()), new byte[0]); return this; } 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 d0462fd6182..a4f9ce01e97 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 @@ -203,6 +203,46 @@ public class DeploymentTriggerTest { } @Test + public void similarDeploymentSpecsAreNotRolledOut() { + ApplicationPackage firstPackage = new ApplicationPackageBuilder() + .region("us-east-3") + .build(); + + DeploymentContext app = tester.newDeploymentContext().submit(firstPackage, 5417); + var version = app.lastSubmission(); + assertEquals(version, app.instance().change().application()); + app.runJob(systemTest) + .runJob(stagingTest) + .runJob(productionUsEast3); + assertEquals(Change.empty(), app.instance().change()); + + // A similar application package is submitted. Since a new job is added, the original revision is again a target. + ApplicationPackage secondPackage = new ApplicationPackageBuilder() + .systemTest() + .stagingTest() + .region("us-east-3") + .delay(Duration.ofHours(1)) + .test("us-east-3") + .build(); + + app.submit(secondPackage, 5417); + app.triggerJobs(); + assertEquals(List.of(), tester.jobs().active()); + assertEquals(version, app.instance().change().application()); + + tester.clock().advance(Duration.ofHours(1)); + app.runJob(testUsEast3); + assertEquals(List.of(), tester.jobs().active()); + assertEquals(Change.empty(), app.instance().change()); + + // The original application package is submitted again. No new jobs are added, so no change needs to roll out now. + app.submit(firstPackage, 5417); + app.triggerJobs(); + assertEquals(List.of(), tester.jobs().active()); + assertEquals(Change.empty(), app.instance().change()); + } + + @Test public void deploymentSpecWithDelays() { ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .systemTest() diff --git a/controller-server/src/test/resources/application-packages/changed-deployment-xml.zip b/controller-server/src/test/resources/application-packages/changed-deployment-xml.zip Binary files differindex 90d52524fb3..05e75f1d24a 100644 --- a/controller-server/src/test/resources/application-packages/changed-deployment-xml.zip +++ b/controller-server/src/test/resources/application-packages/changed-deployment-xml.zip |