diff options
author | Martin Polden <mpolden@mpolden.no> | 2018-06-27 14:27:04 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-06-27 14:27:04 +0200 |
commit | 4079da9c4ee89197a367138a8a46a386540d19b9 (patch) | |
tree | 8d798d6f0726e06a10718796ddbd2d1eaadecc30 | |
parent | f13c54ccc443d30fa9895c46e71ded8b144a194f (diff) | |
parent | e70ff30af3a0452302b6556ffd2f85905e5c36b6 (diff) |
Merge pull request #6299 from vespa-engine/mpolden/system-version-change-requires-config-convergence
Ensure that config has converged before changing system version
9 files changed, 73 insertions, 35 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java index 98ec05e563a..05ac311c514 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java @@ -3,7 +3,12 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.RegionName; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; +import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import java.util.Arrays; import java.util.List; @@ -49,6 +54,20 @@ public enum SystemApplication { return nodeType == NodeType.proxy; } + /** Returns whether config for this application has converged in given zone */ + public boolean configConvergedIn(ZoneId zone, Controller controller) { + if (!hasApplicationPackage()) { + return true; + } + // TODO: Remove this hack once Docker hosts are removed from zone-application. + if (isAws(zone.region())) { + return true; // Skip checking config convergence on AWS as Docker hosts do not have cloud config + } + return controller.configServer().serviceConvergence(new DeploymentId(id(), zone)) + .map(ServiceConvergence::converged) + .orElse(false); + } + /** All known system applications */ public static List<SystemApplication> all() { return Arrays.asList(values()); @@ -64,4 +83,8 @@ public enum SystemApplication { return String.format("system application %s of type %s", id, nodeType); } + private static boolean isAws(RegionName region) { + return region.value().startsWith("cd-aws-") || region.value().startsWith("aws-"); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java index e9dd093fae8..953e95e25c8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java @@ -3,9 +3,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; -import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; @@ -71,21 +69,11 @@ public class SystemUpgrader extends Maintainer { if (convergedOn(target, application.dependencies(), zone)) { deploy(target, application, zone); } - converged &= convergedOn(target, application, zone) & configConverged(application, zone); + converged &= convergedOn(target, application, zone) & application.configConvergedIn(zone, controller()); } return converged; } - /** Returns whether config for given application has converged */ - private boolean configConverged(SystemApplication application, ZoneId zone) { - if (!application.hasApplicationPackage()) { - return true; - } - return controller().configServer().serviceConvergence(new DeploymentId(application.id(), zone)) - .map(ServiceConvergence::converged) - .orElse(false); - } - /** Deploy application on given version idempotently */ private void deploy(Version target, SystemApplication application, ZoneId zone) { if (!wantedVersion(zone, application, target).equals(target)) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java index dea991bc653..565acf6ebd5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java @@ -1,10 +1,11 @@ // 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.log.LogLevel; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; +import com.yahoo.yolean.Exceptions; -import java.io.UncheckedIOException; import java.time.Duration; /** @@ -25,8 +26,9 @@ public class VersionStatusUpdater extends Maintainer { try { VersionStatus newStatus = VersionStatus.compute(controller()); controller().updateVersionStatus(newStatus); - } catch (UncheckedIOException e) { - log.warning("Failed to compute version status. This is likely a transient error: " + e.getMessage()); + } catch (Exception e) { + log.log(LogLevel.WARNING, "Failed to compute version status: " + Exceptions.toMessageString(e) + + ". Retrying in " + maintenanceInterval()); } } 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 dcb7947728c..9b3423e9f10 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 @@ -130,6 +130,9 @@ public class VersionStatus { ListMap<Version, HostName> versions = new ListMap<>(); for (ZoneId zone : zones) { for (SystemApplication application : SystemApplication.all()) { + if (!application.configConvergedIn(zone, controller)) { + throw new IllegalStateException("Config for " + application.id() + " in " + zone + " has not converged"); + } for (Node node : controller.configServer().nodeRepository().list(zone, application.id(), SystemApplication.activeStates())) { versions.put(node.currentVersion(), node.hostname()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java index a023abd1a1d..ab33d6901a5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java @@ -75,6 +75,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer )) .collect(Collectors.toList()); nodeRepository().add(zone, nodes); + convergeServices(application.id(), zone); } } } @@ -103,12 +104,10 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer } /** Set version for system applications in given zone */ - public void setVersion(Version version, ZoneId zone, List<SystemApplication> applications) { - for (SystemApplication application : applications) { - for (Node node : nodeRepository().list(zone, application.id())) { - nodeRepository().add(zone, new Node(node.hostname(), node.state(), node.type(), node.owner(), - version, version)); - } + public void setVersion(ApplicationId application, ZoneId zone, Version version) { + for (Node node : nodeRepository().list(zone, application)) { + nodeRepository().add(zone, new Node(node.hostname(), node.state(), node.type(), node.owner(), + version, version)); } } @@ -174,6 +173,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer node.currentVersion(), application.version().get())); } + serviceStatus.remove(deployment); // Deployment is no longer converging after new deployment PrepareResponse prepareResponse = new PrepareResponse(); prepareResponse.message = "foo"; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index e6ca68c90e1..04e149327b6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -30,15 +30,17 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock; import com.yahoo.vespa.hosted.controller.integration.MockMetricsService; +import com.yahoo.vespa.hosted.controller.maintenance.JobControl; +import com.yahoo.vespa.hosted.controller.maintenance.VersionStatusUpdater; import com.yahoo.vespa.hosted.controller.persistence.ApplicationSerializer; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import com.yahoo.vespa.hosted.controller.routing.MockRoutingGenerator; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; -import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; +import java.time.Duration; import java.util.Optional; import java.util.OptionalLong; import java.util.function.Supplier; @@ -283,7 +285,8 @@ public final class ControllerTester { buildService, new MockLogStore(), () -> "test-controller"); - controller.updateVersionStatus(VersionStatus.compute(controller)); + // Calculate initial versions + new VersionStatusUpdater(controller, Duration.ofDays(1), new JobControl(curator)).run(); return controller; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index bae1f667566..2aad2353c23 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -12,18 +12,18 @@ import com.yahoo.vespa.hosted.controller.ArtifactRepositoryMock; import com.yahoo.vespa.hosted.controller.ConfigServerMock; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.ControllerTester; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockBuildService; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.maintenance.JobControl; import com.yahoo.vespa.hosted.controller.maintenance.ReadyJobsTrigger; import com.yahoo.vespa.hosted.controller.maintenance.SystemUpgrader; import com.yahoo.vespa.hosted.controller.maintenance.Upgrader; -import com.yahoo.vespa.hosted.controller.versions.VersionStatus; +import com.yahoo.vespa.hosted.controller.maintenance.VersionStatusUpdater; import java.time.Duration; import java.util.List; @@ -50,6 +50,7 @@ public class DeploymentTester { private final Upgrader upgrader; private final SystemUpgrader systemUpgrader; private final ReadyJobsTrigger readyJobTrigger; + private final VersionStatusUpdater versionStatusUpdater; public DeploymentTester() { this(new ControllerTester()); @@ -58,11 +59,12 @@ public class DeploymentTester { public DeploymentTester(ControllerTester tester) { this.tester = tester; tester.curator().writeUpgradesPerMinute(100); - this.upgrader = new Upgrader(tester.controller(), maintenanceInterval, new JobControl(tester.curator()), - tester.curator()); - this.systemUpgrader = new SystemUpgrader(tester.controller(), maintenanceInterval, new JobControl(tester.curator())); - this.readyJobTrigger = new ReadyJobsTrigger(tester.controller(), maintenanceInterval, - new JobControl(tester.curator())); + + JobControl jobControl = new JobControl(tester.curator()); + this.upgrader = new Upgrader(tester.controller(), maintenanceInterval, jobControl, tester.curator()); + this.systemUpgrader = new SystemUpgrader(tester.controller(), maintenanceInterval, jobControl); + this.readyJobTrigger = new ReadyJobsTrigger(tester.controller(), maintenanceInterval, jobControl); + this.versionStatusUpdater = new VersionStatusUpdater(tester.controller(), maintenanceInterval, jobControl); } public SystemUpgrader systemUpgrader() { @@ -99,7 +101,7 @@ public class DeploymentTester { /** Re-compute and write version status */ public void computeVersionStatus() { - controller().updateVersionStatus(VersionStatus.compute(controller())); + versionStatusUpdater.run(); } /** Upgrade controller to given version */ @@ -111,7 +113,10 @@ public class DeploymentTester { /** Upgrade system applications in all zones to given version */ public void upgradeSystemApplications(Version version) { for (ZoneId zone : tester.zoneRegistry().zones().all().ids()) { - tester.configServer().setVersion(version, zone, SystemApplication.all()); + for (SystemApplication application : SystemApplication.all()) { + tester.configServer().setVersion(application.id(), zone, version); + tester.configServer().convergeServices(application.id(), zone); + } } computeVersionStatus(); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java index cddc0f5603e..0e9518a63e9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java @@ -8,6 +8,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.zone.UpgradePolicy; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; +import org.junit.Before; import org.junit.Test; import java.util.Arrays; @@ -27,7 +28,12 @@ public class SystemUpgraderTest { private static final ZoneId zone3 = ZoneId.from("prod", "us-central-1"); private static final ZoneId zone4 = ZoneId.from("prod", "us-east-3"); - private final DeploymentTester tester = new DeploymentTester(); + private DeploymentTester tester; + + @Before + public void before() { + tester = new DeploymentTester(); + } @Test public void upgrade_system() { @@ -115,6 +121,11 @@ public class SystemUpgraderTest { tester.systemUpgrader().maintain(); assertWantedVersion(SystemApplication.zone, version2, zone4); completeUpgrade(SystemApplication.zone, version2, zone4); + + // zone 4: System version remains unchanged until config converges + tester.computeVersionStatus(); + assertEquals(version1, tester.controller().versionStatus().systemVersion().get().versionNumber()); + convergeServices(SystemApplication.zone, zone4); tester.computeVersionStatus(); assertEquals(version2, tester.controller().versionStatus().systemVersion().get().versionNumber()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java index c72edf7e403..b5a79eebcfd 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java @@ -57,7 +57,10 @@ public class ContainerTester { public void upgradeSystem(Version version) { controller().curator().writeControllerVersion(controller().hostname(), version); for (ZoneId zone : controller().zoneRegistry().zones().all().ids()) { - configServer().setVersion(version, zone, SystemApplication.all()); + for (SystemApplication application : SystemApplication.all()) { + configServer().setVersion(application.id(), zone, version); + configServer().convergeServices(application.id(), zone); + } } computeVersionStatus(); } |