From 87a36ac961c402e24db67be5bb6b0dac0efef1b5 Mon Sep 17 00:00:00 2001 From: jonmv Date: Wed, 7 Sep 2022 11:44:47 +0200 Subject: Ensure comp.platform for all changes of change, and do not override present ones --- .../controller/deployment/DeploymentStatus.java | 16 ++++++------- .../controller/deployment/DeploymentTrigger.java | 9 ++++---- .../deployment/DeploymentTriggerTest.java | 26 ++++++++++++++++++++++ 3 files changed, 38 insertions(+), 13 deletions(-) (limited to 'controller-server') 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 50492077e20..8dbe85bedb5 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 @@ -178,18 +178,18 @@ public class DeploymentStatus { .map(application.revisions()::get) .flatMap(ApplicationVersion::compileVersion); - // If the revision requires a certain platform for compatibility, add that here. + // If the revision requires a certain platform for compatibility, add that here, unless we're already deploying a compatible platform. VersionCompatibility compatibility = versionCompatibility.apply(instance); Predicate compatibleWithCompileVersion = version -> compileVersion.map(compiled -> compatibility.accept(version, compiled)).orElse(true); + if (change.platform().map(compatibleWithCompileVersion::test).orElse(false)) + return change; + if ( application.productionDeployments().isEmpty() // TODO: replace with adding this for test jobs when needed || application.productionDeployments().getOrDefault(instance, List.of()).stream() - .anyMatch(deployment -> ! compatibleWithCompileVersion.test(deployment.version()))) { - return targetsForPolicy(versionStatus, systemVersion, application.deploymentSpec().requireInstance(instance).upgradePolicy()) - .stream() // Pick the latest platform with appropriate confidence, which is compatible with the compile version. - .filter(compatibleWithCompileVersion) - .findFirst() - .map(platform -> change.withoutPin().with(platform)) - .orElse(change); + .anyMatch(deployment -> ! compatibleWithCompileVersion.test(deployment.version()))) { + for (Version platform : targetsForPolicy(versionStatus, systemVersion, application.deploymentSpec().requireInstance(instance).upgradePolicy())) + if (compatibleWithCompileVersion.test(platform)) + return change.withoutPin().with(platform); } return change; } 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 7c0a4a83e4f..c1032bb5920 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 @@ -91,10 +91,8 @@ public class DeploymentTrigger { && acceptNewRevision(status, instanceName, outstanding.revision().get()); application = application.with(instanceName, instance -> withRemainingChange(instance, - status.withCompatibilityPlatform((deployOutstanding ? outstanding - : Change.empty()) - .onTopOf(instance.change()), - instanceName), + deployOutstanding ? outstanding.onTopOf(instance.change()) + : instance.change(), status)); } applications().store(application); @@ -430,7 +428,8 @@ public class DeploymentTrigger { remaining = remaining.withoutPlatform(); if (status.hasCompleted(instance.name(), change.withoutPlatform())) remaining = remaining.withoutApplication(); - return instance.withChange(remaining); + + return instance.withChange(status.withCompatibilityPlatform(remaining, instance.name())); } // ---------- Version and job helpers ---------- 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 3b8d0374a45..6f7c8403a2b 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 @@ -2222,6 +2222,32 @@ public class DeploymentTriggerTest { newApp.deploy(); assertEquals(version2, tester.jobs().last(newApp.instanceId(), productionUsEast3).get().versions().targetPlatform()); assertEquals(version2, newApp.application().revisions().get(tester.jobs().last(newApp.instanceId(), productionUsEast3).get().versions().targetRevision()).compileVersion().get()); + + // New app compiles against old major, and downgrades when a pin is also applied. + newApp.submit(new ApplicationPackageBuilder().compileVersion(version1) + .systemTest() + .region("us-east-3") + .build()); + newRevision = newApp.lastSubmission().get(); + + assertEquals(Change.of(newRevision).with(version1), newApp.instance().change()); + tester.triggerJobs(); + newApp.assertNotRunning(systemTest); // Without a pin, the platform won't downgrade, and 8 is incompatible with compiled 7. + + tester.outstandingChangeDeployer().run(); + assertEquals(Change.of(newRevision).with(version1), newApp.instance().change()); + tester.upgrader().run(); + assertEquals(Change.of(newRevision).with(version1), newApp.instance().change()); + + tester.deploymentTrigger().forceChange(newApp.instanceId(), newApp.instance().change().withPin()); + tester.outstandingChangeDeployer().run(); + assertEquals(Change.of(newRevision).with(version1).withPin(), newApp.instance().change()); + tester.upgrader().run(); + assertEquals(Change.of(newRevision).with(version1).withPin(), newApp.instance().change()); + + newApp.deploy(); + assertEquals(version1, tester.jobs().last(newApp.instanceId(), productionUsEast3).get().versions().targetPlatform()); + assertEquals(version1, newApp.application().revisions().get(tester.jobs().last(newApp.instanceId(), productionUsEast3).get().versions().targetRevision()).compileVersion().get()); } @Test -- cgit v1.2.3