diff options
25 files changed, 450 insertions, 187 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Cloud.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Cloud.java index b1df63dae44..d79b00c62c7 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Cloud.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Cloud.java @@ -15,8 +15,8 @@ public class Cloud { private final boolean reprovisionToUpgradeOs; private final boolean requireAccessControl; - protected Cloud(CloudName name, boolean dynamicProvisioning, boolean allowHostSharing, boolean reprovisionToUpgradeOs, - boolean requireAccessControl) { + public Cloud(CloudName name, boolean dynamicProvisioning, boolean allowHostSharing, boolean reprovisionToUpgradeOs, + boolean requireAccessControl) { this.name = name; this.dynamicProvisioning = dynamicProvisioning; this.allowHostSharing = allowHostSharing; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java index b5d49df7e9c..aebfab7cbff 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepository.java @@ -14,6 +14,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeMemb import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState; +import java.time.Duration; import java.time.Instant; import java.util.Collection; import java.util.List; @@ -76,7 +77,7 @@ public interface NodeRepository { void upgrade(ZoneId zone, NodeType type, Version version); /** Upgrade OS for all nodes of given type to a new version */ - void upgradeOs(ZoneId zone, NodeType type, Version version); + void upgradeOs(ZoneId zone, NodeType type, Version version, Optional<Duration> upgradeBudget); /** Get target versions for upgrades in given zone */ TargetVersions targetVersionsOf(ZoneId zone); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeUpgrade.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeUpgrade.java index 21c1d23ba3a..b8e2a626c72 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeUpgrade.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeUpgrade.java @@ -22,12 +22,16 @@ public class NodeUpgrade { @JsonProperty("force") private final boolean force; + @JsonProperty("upgradeBudget") + private final String upgradeBudget; + @JsonCreator public NodeUpgrade(@JsonProperty("version") String version, @JsonProperty("osVersion") String osVersion, - @JsonProperty("force") boolean force) { + @JsonProperty("force") boolean force, @JsonProperty("upgradeBudget") String upgradeBudget) { this.version = version; this.osVersion = osVersion; this.force = force; + this.upgradeBudget = upgradeBudget; } public String getVersion() { @@ -42,4 +46,8 @@ public class NodeUpgrade { return force; } + public String getUpgradeBudget() { + return upgradeBudget; + } + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java index b9ee696431b..b7da96a5501 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.zone; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.AthenzDomain; +import com.yahoo.config.provision.Cloud; import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeType; @@ -94,4 +95,7 @@ public interface ZoneRegistry { /** Returns a URL to the controller's api endpoint */ URI apiUrl(); + /** Get cloud by name */ + Cloud cloud(CloudName name); + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index 297f3fcf218..12a7520129a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -6,6 +6,7 @@ import com.yahoo.component.AbstractComponent; import com.yahoo.component.Version; import com.yahoo.component.Vtag; import com.yahoo.concurrent.maintenance.JobControl; +import com.yahoo.config.provision.Cloud; import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.SystemName; @@ -26,12 +27,14 @@ import com.yahoo.vespa.hosted.controller.security.AccessControl; import com.yahoo.vespa.hosted.controller.versions.ControllerVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; +import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; import com.yahoo.vespa.serviceview.bindings.ApplicationView; import java.time.Clock; +import java.time.Duration; import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; @@ -188,33 +191,37 @@ public class Controller extends AbstractComponent { /** Returns the target OS version for infrastructure in this system. The controller will drive infrastructure OS * upgrades to this version */ - public Optional<OsVersion> osVersion(CloudName cloud) { - return osVersions().stream().filter(osVersion -> osVersion.cloud().equals(cloud)).findFirst(); + public Optional<OsVersionTarget> osVersionTarget(CloudName cloud) { + return osVersionTargets().stream().filter(target -> target.osVersion().cloud().equals(cloud)).findFirst(); } /** Returns all target OS versions in this system */ - public Set<OsVersion> osVersions() { - return curator.readOsVersions(); + public Set<OsVersionTarget> osVersionTargets() { + return curator.readOsVersionTargets(); } /** Set the target OS version for infrastructure on cloud in this system */ - public void upgradeOsIn(CloudName cloud, Version version, boolean force) { + public void upgradeOsIn(CloudName cloudName, Version version, Optional<Duration> upgradeBudget, boolean force) { if (version.isEmpty()) { throw new IllegalArgumentException("Invalid version '" + version.toFullString() + "'"); } - if (!clouds().contains(cloud)) { - throw new IllegalArgumentException("Cloud '" + cloud.value() + "' does not exist in this system"); + Cloud cloud = zoneRegistry.cloud(cloudName); + if (cloud == null) { + throw new IllegalArgumentException("Cloud '" + cloudName + "' does not exist in this system"); + } + if (cloud.reprovisionToUpgradeOs() && upgradeBudget.isEmpty()) { + throw new IllegalArgumentException("Cloud '" + cloudName.value() + "' requires a time budget for OS upgrades"); } try (Lock lock = curator.lockOsVersions()) { - Set<OsVersion> versions = new TreeSet<>(curator.readOsVersions()); - if (!force && versions.stream().anyMatch(osVersion -> osVersion.cloud().equals(cloud) && - osVersion.version().isAfter(version))) { - throw new IllegalArgumentException("Cannot downgrade cloud '" + cloud.value() + "' to version " + + Set<OsVersionTarget> targets = new TreeSet<>(curator.readOsVersionTargets()); + if (!force && targets.stream().anyMatch(target -> target.osVersion().cloud().equals(cloudName) && + target.osVersion().version().isAfter(version))) { + throw new IllegalArgumentException("Cannot downgrade cloud '" + cloudName.value() + "' to version " + version.toFullString()); } - versions.removeIf(osVersion -> osVersion.cloud().equals(cloud)); // Only allow a single target per cloud - versions.add(new OsVersion(version, cloud)); - curator.writeOsVersions(versions); + targets.removeIf(target -> target.osVersion().cloud().equals(cloudName)); // Only allow a single target per cloud + targets.add(new OsVersionTarget(new OsVersion(version, cloudName), upgradeBudget)); + curator.writeOsVersionTargets(targets); } } 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 66015c76b06..27a97f1ef01 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,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.Cloud; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.Controller; @@ -63,8 +64,11 @@ public enum SystemApplication { .orElse(false); } - /** Returns whether this should receive OS upgrades */ - public boolean shouldUpgradeOs() { + /** Returns whether this should receive OS upgrades in given cloud */ + public boolean shouldUpgradeOsIn(Cloud cloud) { + if (cloud.reprovisionToUpgradeOs()) { + return nodeType == NodeType.host; // TODO(mpolden): Remove once all node types are supported + } return nodeType.isDockerHost(); } 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 b92da2ce740..d20be628078 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 @@ -108,7 +108,7 @@ public class ControllerMaintenance extends AbstractComponent { .map(ZoneApi::getCloudName) .distinct() .sorted() - .map(cloud -> new OsUpgrader(controller, Duration.ofMinutes(1), cloud)) + .map(cloud -> new OsUpgrader(controller, Duration.ofMinutes(1), controller.zoneRegistry().cloud(cloud))) .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java index 40072df48b5..7006458538d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java @@ -23,11 +23,11 @@ import java.util.logging.Logger; * * @author mpolden */ -public abstract class InfrastructureUpgrader extends ControllerMaintainer { +public abstract class InfrastructureUpgrader<VERSION> extends ControllerMaintainer { private static final Logger log = Logger.getLogger(InfrastructureUpgrader.class.getName()); - private final UpgradePolicy upgradePolicy; + protected final UpgradePolicy upgradePolicy; public InfrastructureUpgrader(Controller controller, Duration interval, UpgradePolicy upgradePolicy, String name) { super(controller, interval, name, EnumSet.allOf(SystemName.class)); @@ -40,7 +40,7 @@ public abstract class InfrastructureUpgrader extends ControllerMaintainer { } /** Deploy a list of system applications until they converge on the given version */ - private void upgradeAll(Version target, List<SystemApplication> applications) { + private void upgradeAll(VERSION target, List<SystemApplication> applications) { for (List<ZoneApi> zones : upgradePolicy.asList()) { boolean converged = true; for (ZoneApi zone : zones) { @@ -63,11 +63,11 @@ public abstract class InfrastructureUpgrader extends ControllerMaintainer { } /** Returns whether all applications have converged to the target version in zone */ - private boolean upgradeAll(Version target, List<SystemApplication> applications, ZoneApi zone) { + private boolean upgradeAll(VERSION target, List<SystemApplication> applications, ZoneApi zone) { boolean converged = true; for (SystemApplication application : applications) { if (convergedOn(target, application.dependencies(), zone)) { - if (shouldUpgrade(target, application, zone)) { + if (changeTargetTo(target, application, zone)) { upgrade(target, application, zone); } converged &= convergedOn(target, application, zone); @@ -76,24 +76,24 @@ public abstract class InfrastructureUpgrader extends ControllerMaintainer { return converged; } - private boolean convergedOn(Version target, List<SystemApplication> applications, ZoneApi zone) { + private boolean convergedOn(VERSION target, List<SystemApplication> applications, ZoneApi zone) { return applications.stream().allMatch(application -> convergedOn(target, application, zone)); } - /** Returns whether application in zone should be told to upgrade to given target */ - protected abstract boolean shouldUpgrade(Version target, SystemApplication application, ZoneApi zone); + /** Returns whether target version for application in zone should be changed */ + protected abstract boolean changeTargetTo(VERSION target, SystemApplication application, ZoneApi zone); /** Upgrade component to target version. Implementation should be idempotent */ - protected abstract void upgrade(Version target, SystemApplication application, ZoneApi zone); + protected abstract void upgrade(VERSION target, SystemApplication application, ZoneApi zone); /** Returns whether application has converged to target version in zone */ - protected abstract boolean convergedOn(Version target, SystemApplication application, ZoneApi zone); + protected abstract boolean convergedOn(VERSION target, SystemApplication application, ZoneApi zone); /** Returns the target version for the component upgraded by this, if any */ - protected abstract Optional<Version> targetVersion(); + protected abstract Optional<VERSION> targetVersion(); - /** Returns whether the upgrader should require given node to upgrade */ - protected abstract boolean requireUpgradeOf(Node node, SystemApplication application, ZoneApi zone); + /** Returns whether the upgrader should expect given node to upgrade */ + protected abstract boolean expectUpgradeOf(Node node, SystemApplication application, ZoneApi zone); /** Find the minimum value of a version field in a zone by comparing all nodes */ protected final Optional<Version> minVersion(ZoneApi zone, SystemApplication application, Function<Node, Version> versionField) { @@ -102,7 +102,7 @@ public abstract class InfrastructureUpgrader extends ControllerMaintainer { .nodeRepository() .list(zone.getId(), application.id()) .stream() - .filter(node -> requireUpgradeOf(node, application, zone)) + .filter(node -> expectUpgradeOf(node, application, zone)) .map(versionField) .min(Comparator.naturalOrder()); } catch (Exception e) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java index 8a97f8f1a9d..817ec9c08e8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java @@ -2,12 +2,14 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; +import com.yahoo.config.provision.Cloud; import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; import com.yahoo.vespa.hosted.controller.application.SystemApplication; -import com.yahoo.vespa.hosted.controller.versions.OsVersion; +import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget; import java.time.Duration; import java.util.Optional; @@ -19,7 +21,7 @@ import java.util.logging.Logger; * * @author mpolden */ -public class OsUpgrader extends InfrastructureUpgrader { +public class OsUpgrader extends InfrastructureUpgrader<OsVersionTarget> { private static final Logger log = Logger.getLogger(OsUpgrader.class.getName()); @@ -29,56 +31,72 @@ public class OsUpgrader extends InfrastructureUpgrader { Node.State.reserved ); - private final CloudName cloud; + private final Cloud cloud; - public OsUpgrader(Controller controller, Duration interval, CloudName cloud) { - super(controller, interval, controller.zoneRegistry().osUpgradePolicy(cloud), name(cloud)); + public OsUpgrader(Controller controller, Duration interval, Cloud cloud) { + super(controller, interval, controller.zoneRegistry().osUpgradePolicy(cloud.name()), name(cloud.name())); this.cloud = cloud; } @Override - protected void upgrade(Version target, SystemApplication application, ZoneApi zone) { - log.info(String.format("Upgrading OS of %s to version %s in %s in cloud %s", application.id(), target, zone.getId(), zone.getCloudName())); - controller().serviceRegistry().configServer().nodeRepository().upgradeOs(zone.getId(), application.nodeType(), target); + protected void upgrade(OsVersionTarget target, SystemApplication application, ZoneApi zone) { + Optional<Duration> zoneUpgradeBudget = target.upgradeBudget() + .map(totalBudget -> zoneBudgetOf(totalBudget, zone)); + log.info(String.format("Upgrading OS of %s to version %s in %s in cloud %s%s", application.id(), target, + zone.getId(), zone.getCloudName(), + zoneUpgradeBudget.map(d -> " with time budget " + d).orElse(""))); + controller().serviceRegistry().configServer().nodeRepository().upgradeOs(zone.getId(), application.nodeType(), + target.osVersion().version(), + zoneUpgradeBudget); } @Override - protected boolean convergedOn(Version target, SystemApplication application, ZoneApi zone) { - return currentVersion(zone, application, target).equals(target); + protected boolean convergedOn(OsVersionTarget target, SystemApplication application, ZoneApi zone) { + return currentVersion(zone, application, target.osVersion().version()).equals(target.osVersion().version()); } @Override - protected boolean requireUpgradeOf(Node node, SystemApplication application, ZoneApi zone) { - return cloud.equals(zone.getCloudName()) && eligibleForUpgrade(node, application); + protected boolean expectUpgradeOf(Node node, SystemApplication application, ZoneApi zone) { + return cloud.name().equals(zone.getCloudName()) && // Cloud is managed by this upgrader + application.shouldUpgradeOsIn(cloud) && // Application should upgrade in this cloud + canUpgrade(node); // Node is in an upgradable state } @Override - protected Optional<Version> targetVersion() { + protected Optional<OsVersionTarget> targetVersion() { // Return target if we have nodes in this cloud on a lower version - return controller().osVersion(cloud) - .filter(target -> controller().osVersionStatus().nodesIn(cloud).stream() - .anyMatch(node -> node.currentVersion().isBefore(target.version()))) - .map(OsVersion::version); + return controller().osVersionTarget(cloud.name()) + .filter(target -> controller().osVersionStatus().nodesIn(cloud.name()).stream() + .anyMatch(node -> node.currentVersion().isBefore(target.osVersion().version()))); } @Override - protected boolean shouldUpgrade(Version target, SystemApplication application, ZoneApi zone) { - if (!application.shouldUpgradeOs()) return false; // Never upgrade + protected boolean changeTargetTo(OsVersionTarget target, SystemApplication application, ZoneApi zone) { + if (!application.shouldUpgradeOsIn(cloud)) return false; return controller().serviceRegistry().configServer().nodeRepository() .targetVersionsOf(zone.getId()) .osVersion(application.nodeType()) - .map(target::isAfter) // Upgrade if target is after current - .orElse(true); // Upgrade if target is unset + .map(currentTarget -> target.osVersion().version().isAfter(currentTarget)) + .orElse(true); } private Version currentVersion(ZoneApi zone, SystemApplication application, Version defaultVersion) { return minVersion(zone, application, Node::currentOsVersion).orElse(defaultVersion); } - /** Returns whether node in application should be upgraded by this */ - public static boolean eligibleForUpgrade(Node node, SystemApplication application) { - return upgradableNodeStates.contains(node.state()) && - application.shouldUpgradeOs(); + /** Returns the available upgrade budget for given zone */ + private Duration zoneBudgetOf(Duration totalBudget, ZoneApi zone) { + if (!zone.getEnvironment().isProduction()) return Duration.ZERO; + long consecutiveProductionZones = upgradePolicy.asList().stream() + .filter(parallelZones -> parallelZones.stream().map(ZoneApi::getEnvironment) + .anyMatch(Environment::isProduction)) + .count(); + return totalBudget.dividedBy(consecutiveProductionZones); + } + + /** Returns whether node is in a state where it can be upgraded */ + public static boolean canUpgrade(Node node) { + return upgradableNodeStates.contains(node.state()); } private static String name(CloudName cloud) { 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 be87b5f9223..bf44c796f34 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 @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; -import com.yahoo.concurrent.maintenance.JobControl; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; @@ -19,7 +18,7 @@ import java.util.logging.Logger; * * @author mpolden */ -public class SystemUpgrader extends InfrastructureUpgrader { +public class SystemUpgrader extends InfrastructureUpgrader<Version> { private static final Logger log = Logger.getLogger(SystemUpgrader.class.getName()); @@ -46,7 +45,7 @@ public class SystemUpgrader extends InfrastructureUpgrader { } @Override - protected boolean requireUpgradeOf(Node node, SystemApplication application, ZoneApi zone) { + protected boolean expectUpgradeOf(Node node, SystemApplication application, ZoneApi zone) { return eligibleForUpgrade(node); } @@ -59,7 +58,7 @@ public class SystemUpgrader extends InfrastructureUpgrader { } @Override - protected boolean shouldUpgrade(Version target, SystemApplication application, ZoneApi zone) { + protected boolean changeTargetTo(Version target, SystemApplication application, ZoneApi zone) { if (application.hasApplicationPackage()) { // For applications with package we do not have a zone-wide version target. This means that we must check // the wanted version of each node. diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index ebd4921ccd7..3058037ccc0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -31,8 +31,8 @@ import com.yahoo.vespa.hosted.controller.routing.RoutingPolicyId; import com.yahoo.vespa.hosted.controller.routing.ZoneRoutingPolicy; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.versions.ControllerVersion; -import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; +import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; @@ -98,6 +98,7 @@ public class CuratorDb implements JobControl.Db { private final ApplicationSerializer applicationSerializer = new ApplicationSerializer(); private final RunSerializer runSerializer = new RunSerializer(); private final OsVersionSerializer osVersionSerializer = new OsVersionSerializer(); + private final OsVersionTargetSerializer osVersionTargetSerializer = new OsVersionTargetSerializer(osVersionSerializer); private final OsVersionStatusSerializer osVersionStatusSerializer = new OsVersionStatusSerializer(osVersionSerializer, nodeVersionSerializer); private final RoutingPolicySerializer routingPolicySerializer = new RoutingPolicySerializer(); private final ZoneRoutingPolicySerializer zoneRoutingPolicySerializer = new ZoneRoutingPolicySerializer(routingPolicySerializer); @@ -297,12 +298,12 @@ public class CuratorDb implements JobControl.Db { // Infrastructure upgrades - public void writeOsVersions(Set<OsVersion> versions) { - curator.set(osTargetVersionPath(), asJson(osVersionSerializer.toSlime(versions))); + public void writeOsVersionTargets(Set<OsVersionTarget> versions) { + curator.set(osVersionTargetsPath(), asJson(osVersionTargetSerializer.toSlime(versions))); } - public Set<OsVersion> readOsVersions() { - return readSlime(osTargetVersionPath()).map(osVersionSerializer::fromSlime).orElseGet(Collections::emptySet); + public Set<OsVersionTarget> readOsVersionTargets() { + return readSlime(osVersionTargetsPath()).map(osVersionTargetSerializer::fromSlime).orElseGet(Collections::emptySet); } public void writeOsVersionStatus(OsVersionStatus status) { @@ -598,7 +599,7 @@ public class CuratorDb implements JobControl.Db { return root.append("upgrader").append("confidenceOverrides"); } - private static Path osTargetVersionPath() { + private static Path osVersionTargetsPath() { return root.append("osUpgrader").append("targetVersion"); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionTargetSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionTargetSerializer.java new file mode 100644 index 00000000000..627ad9f709c --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionTargetSerializer.java @@ -0,0 +1,57 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.persistence; + +import com.yahoo.slime.ArrayTraverser; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.hosted.controller.versions.OsVersion; +import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget; + +import java.time.Duration; +import java.util.Collections; +import java.util.Optional; +import java.util.Set; +import java.util.TreeSet; + +/** + * Serializer for {@link com.yahoo.vespa.hosted.controller.versions.OsVersionTarget}. + * + * @author mpolden + */ +public class OsVersionTargetSerializer { + + private final OsVersionSerializer osVersionSerializer; + + private static final String versionsField = "versions"; + private static final String upgradeBudgetField = "upgradeBudget"; + + public OsVersionTargetSerializer(OsVersionSerializer osVersionSerializer) { + this.osVersionSerializer = osVersionSerializer; + } + + public Slime toSlime(Set<OsVersionTarget> osVersionTargets) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + Cursor array = root.setArray(versionsField); + osVersionTargets.forEach(target -> toSlime(target, array.addObject())); + return slime; + } + + public Set<OsVersionTarget> fromSlime(Slime slime) { + Inspector array = slime.get().field(versionsField); + Set<OsVersionTarget> osVersionTargets = new TreeSet<>(); + array.traverse((ArrayTraverser) (i, inspector) -> { + OsVersion osVersion = osVersionSerializer.fromSlime(inspector); + Optional<Duration> upgradeBudget = Serializers.optionalDuration(inspector.field(upgradeBudgetField)); + osVersionTargets.add(new OsVersionTarget(osVersion, upgradeBudget)); + }); + return Collections.unmodifiableSet(osVersionTargets); + } + + private void toSlime(OsVersionTarget target, Cursor object) { + osVersionSerializer.toSlime(target.osVersion(), object); + target.upgradeBudget().ifPresent(d -> object.setLong(upgradeBudgetField, d.toMillis())); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/Serializers.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/Serializers.java index e5adccc850c..b254732f324 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/Serializers.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/Serializers.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.yahoo.slime.Inspector; import com.yahoo.slime.SlimeUtils; +import java.time.Duration; import java.time.Instant; import java.util.Optional; import java.util.OptionalDouble; @@ -40,4 +41,9 @@ public class Serializers { return value.isPresent() ? Optional.of(Instant.ofEpochMilli(value.getAsLong())) : Optional.empty(); } + public static Optional<Duration> optionalDuration(Inspector field) { + var value = optionalLong(field); + return value.isPresent() ? Optional.of(Duration.ofMillis(value.getAsLong())) : Optional.empty(); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java index 7d8c5922c3f..ac63f0cbcaf 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java @@ -6,28 +6,30 @@ import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.zone.ZoneApi; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.config.provision.zone.ZoneList; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.io.IOUtils; +import com.yahoo.restapi.ErrorResponse; +import com.yahoo.restapi.MessageResponse; import com.yahoo.restapi.Path; +import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.config.provision.zone.ZoneList; import com.yahoo.vespa.hosted.controller.auditlog.AuditLoggingRequestHandler; -import com.yahoo.restapi.ErrorResponse; -import com.yahoo.restapi.MessageResponse; -import com.yahoo.restapi.SlimeJsonResponse; -import com.yahoo.vespa.hosted.controller.versions.OsVersion; +import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget; import com.yahoo.yolean.Exceptions; import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; +import java.time.Duration; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.StringJoiner; import java.util.logging.Level; @@ -133,6 +135,7 @@ public class OsApiHandler extends AuditLoggingRequestHandler { Inspector root = requestData.get(); Inspector versionField = root.field("version"); Inspector cloudField = root.field("cloud"); + Inspector upgradeBudgetField = root.field("upgradeBudget"); boolean force = root.field("force").asBool(); if (!versionField.valid() || !cloudField.valid()) { throw new IllegalArgumentException("Fields 'version' and 'cloud' are required"); @@ -145,25 +148,37 @@ public class OsApiHandler extends AuditLoggingRequestHandler { } catch (IllegalArgumentException e) { throw new IllegalArgumentException("Invalid version '" + versionField.asString() + "'", e); } - - controller.upgradeOsIn(cloud, target, force); + Optional<Duration> upgradeBudget = Optional.of(upgradeBudgetField) + .filter(Inspector::valid) + .map(Inspector::asString).map(s -> { + try { + return Duration.parse(s); + } catch (Exception e2) { + throw new IllegalArgumentException("Invalid duration '" + s + "'", e2); + } + }); + + controller.upgradeOsIn(cloud, target, upgradeBudget, force); Slime response = new Slime(); Cursor cursor = response.setObject(); cursor.setString("message", "Set target OS version for cloud '" + cloud.value() + "' to " + - target.toFullString()); + target.toFullString() + upgradeBudget.map(d -> " with upgrade budget " + d) + .orElse("")); return response; } private Slime osVersions() { Slime slime = new Slime(); Cursor root = slime.setObject(); - Set<OsVersion> osVersions = controller.osVersions(); + Set<OsVersionTarget> targets = controller.osVersionTargets(); Cursor versions = root.setArray("versions"); controller.osVersionStatus().versions().forEach((osVersion, nodeVersions) -> { Cursor currentVersionObject = versions.addObject(); currentVersionObject.setString("version", osVersion.version().toFullString()); - currentVersionObject.setBool("targetVersion", osVersions.contains(osVersion)); + Optional<OsVersionTarget> target = targets.stream().filter(t -> t.osVersion().equals(osVersion)).findFirst(); + currentVersionObject.setBool("targetVersion", target.isPresent()); + target.flatMap(OsVersionTarget::upgradeBudget).ifPresent(budget -> currentVersionObject.setString("upgradeBudget", budget.toString())); currentVersionObject.setString("cloud", osVersion.cloud().value()); Cursor nodesArray = currentVersionObject.setArray("nodes"); nodeVersions.asMap().values().forEach(nodeVersion -> { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java index 1dffd1383bd..ad8a6611e48 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionStatus.java @@ -59,18 +59,19 @@ public class OsVersionStatus { /** Compute the current OS versions in this system. This is expensive and should be called infrequently */ public static OsVersionStatus compute(Controller controller) { var osVersions = new HashMap<OsVersion, List<NodeVersion>>(); - controller.osVersions().forEach(osVersion -> osVersions.put(osVersion, new ArrayList<>())); + controller.osVersionTargets().forEach(target -> osVersions.put(target.osVersion(), new ArrayList<>())); for (var application : SystemApplication.all()) { - if (!application.shouldUpgradeOs()) continue; for (var zone : zonesToUpgrade(controller)) { + var cloud = controller.zoneRegistry().cloud(zone.getCloudName()); + if (!application.shouldUpgradeOsIn(cloud)) continue; var targetOsVersion = controller.serviceRegistry().configServer().nodeRepository() .targetVersionsOf(zone.getId()) .osVersion(application.nodeType()) .orElse(Version.emptyVersion); for (var node : controller.serviceRegistry().configServer().nodeRepository().list(zone.getId(), application.id())) { - if (!OsUpgrader.eligibleForUpgrade(node, application)) continue; + if (!OsUpgrader.canUpgrade(node)) continue; var suspendedAt = node.suspendedSince(); var nodeVersion = new NodeVersion(node.hostname(), zone.getId(), node.currentOsVersion(), targetOsVersion, suspendedAt); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionTarget.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionTarget.java new file mode 100644 index 00000000000..bacd2ada298 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/OsVersionTarget.java @@ -0,0 +1,68 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.versions; + +import org.jetbrains.annotations.NotNull; + +import java.time.Duration; +import java.util.Objects; +import java.util.Optional; + +/** + * An {@link OsVersion} and its upgrade budget. + * + * @author mpolden + */ +public class OsVersionTarget implements Comparable<OsVersionTarget> { + + // WARNING: Since there are multiple servers in a ZooKeeper cluster and they upgrade one by one + // (and rewrite all nodes on startup), changes to the serialized format must be made + // such that what is serialized on version N+1 can be read by version N: + // - ADDING FIELDS: Always ok + // - REMOVING FIELDS: Stop reading the field first. Stop writing it on a later version. + // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. + + private final OsVersion osVersion; + private final Optional<Duration> upgradeBudget; + + public OsVersionTarget(OsVersion osVersion, Optional<Duration> upgradeBudget) { + this.osVersion = Objects.requireNonNull(osVersion); + this.upgradeBudget = requireNotNegative(upgradeBudget); + } + + /** The OS version contained in this target */ + public OsVersion osVersion() { + return osVersion; + } + + /** The total time budget across all zones for applying target, if any */ + public Optional<Duration> upgradeBudget() { + return upgradeBudget; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + OsVersionTarget that = (OsVersionTarget) o; + return osVersion.equals(that.osVersion) && + upgradeBudget.equals(that.upgradeBudget); + } + + @Override + public int hashCode() { + return Objects.hash(osVersion, upgradeBudget); + } + + private static Optional<Duration> requireNotNegative(Optional<Duration> duration) { + Objects.requireNonNull(duration); + if (duration.isEmpty()) return duration; + if (duration.get().isNegative()) throw new IllegalArgumentException("Duration cannot be negative"); + return duration; + } + + @Override + public int compareTo(@NotNull OsVersionTarget o) { + return osVersion.compareTo(o.osVersion); + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java index 76a30c289b8..90276b6b590 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java @@ -16,6 +16,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeList import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState; +import java.time.Duration; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -37,6 +38,7 @@ public class NodeRepositoryMock implements NodeRepository { private final Map<ZoneId, Map<HostName, Node>> nodeRepository = new HashMap<>(); private final Map<ZoneId, Map<ApplicationId, Application>> applications = new HashMap<>(); private final Map<ZoneId, TargetVersions> targetVersions = new HashMap<>(); + private final Map<Integer, Duration> osUpgradeBudgets = new HashMap<>(); /** Add or update given nodes in zone */ public void putNodes(ZoneId zone, List<Node> nodes) { @@ -190,7 +192,8 @@ public class NodeRepositoryMock implements NodeRepository { } @Override - public void upgradeOs(ZoneId zone, NodeType type, Version version) { + public void upgradeOs(ZoneId zone, NodeType type, Version version, Optional<Duration> upgradeBudget) { + upgradeBudget.ifPresent(d -> this.osUpgradeBudgets.put(Objects.hash(zone, type, version), d)); this.targetVersions.compute(zone, (ignored, targetVersions) -> { if (targetVersions == null) { targetVersions = TargetVersions.EMPTY; @@ -223,6 +226,10 @@ public class NodeRepositoryMock implements NodeRepository { nodeRepository.get(zoneId).remove(HostName.from(hostName)); } + public Optional<Duration> osUpgradeBudget(ZoneId zone, NodeType type, Version version) { + return Optional.ofNullable(osUpgradeBudgets.get(Objects.hash(zone, type, version))); + } + public void doUpgrade(DeploymentId deployment, Optional<HostName> hostName, Version version) { modifyNodes(deployment, hostName, node -> { assert node.wantedVersion().equals(version); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java index efc875b06f5..a453a990855 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.integration; import com.yahoo.component.AbstractComponent; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.AthenzDomain; +import com.yahoo.config.provision.Cloud; import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeType; @@ -36,6 +37,7 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry private final Map<Environment, RegionName> defaultRegionForEnvironment = new HashMap<>(); private final Map<CloudName, UpgradePolicy> osUpgradePolicies = new HashMap<>(); private final Map<ZoneApi, List<RoutingMethod>> zoneRoutingMethods = new HashMap<>(); + private final Map<CloudName, Cloud> clouds = new HashMap<>(); private List<? extends ZoneApi> zones; private SystemName system; @@ -60,6 +62,8 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry ZoneApiMock.fromId("prod.us-west-1"), ZoneApiMock.fromId("prod.us-central-1"), ZoneApiMock.fromId("prod.eu-west-1")); + var cloud = Cloud.defaultCloud(); + this.clouds.put(cloud.name(), cloud); // All zones use a shared routing method by default setRoutingMethod(this.zones, RoutingMethod.shared); } @@ -98,6 +102,13 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry return this; } + public ZoneRegistryMock addCloud(Cloud... clouds) { + for (var cloud : clouds) { + this.clouds.put(cloud.name(), cloud); + } + return this; + } + public ZoneRegistryMock exclusiveRoutingIn(ZoneApi... zones) { return exclusiveRoutingIn(List.of(zones)); } @@ -199,6 +210,11 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry } @Override + public Cloud cloud(CloudName name) { + return clouds.get(name); + } + + @Override public boolean hasZone(ZoneId zoneId) { return zones.stream().anyMatch(zone -> zone.getId().equals(zoneId)); } 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 8e7f04f6ecc..b37fb8123fa 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 @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.Cloud; import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; @@ -22,6 +23,7 @@ import org.junit.Test; import java.time.Duration; import java.util.Comparator; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.function.UnaryOperator; import java.util.stream.Collectors; @@ -290,15 +292,14 @@ public class MetricsReporterTest { var zone = ZoneId.from("prod.eu-west-1"); var cloud = CloudName.defaultName(); tester.zoneRegistry().setOsUpgradePolicy(cloud, UpgradePolicy.create().upgrade(ZoneApiMock.from(zone))); - var osUpgrader = new OsUpgrader(tester.controller(), Duration.ofDays(1), - CloudName.defaultName()); + var osUpgrader = new OsUpgrader(tester.controller(), Duration.ofDays(1), Cloud.defaultCloud()); var statusUpdater = new OsVersionStatusUpdater(tester.controller(), Duration.ofDays(1) ); tester.configServer().bootstrap(List.of(zone), SystemApplication.configServerHost, SystemApplication.tenantHost); // All nodes upgrade to initial OS version var version0 = Version.fromString("8.0"); - tester.controller().upgradeOsIn(cloud, version0, false); + tester.controller().upgradeOsIn(cloud, version0, Optional.empty(), false); osUpgrader.maintain(); tester.configServer().setOsVersion(version0, SystemApplication.tenantHost.id(), zone); tester.configServer().setOsVersion(version0, SystemApplication.configServerHost.id(), zone); @@ -312,7 +313,7 @@ public class MetricsReporterTest { var currentVersion = i == 0 ? version0 : targets.get(i - 1); var version = targets.get(i); // System starts upgrading to next OS version - tester.controller().upgradeOsIn(cloud, version, false); + tester.controller().upgradeOsIn(cloud, version, Optional.empty(), false); runAll(osUpgrader, statusUpdater, reporter); assertOsChangeDuration(Duration.ZERO, hosts, currentVersion); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java index a5a7304398b..c8319efa348 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; +import com.yahoo.config.provision.Cloud; import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.UpgradePolicy; @@ -16,43 +17,45 @@ import com.yahoo.vespa.hosted.controller.versions.NodeVersion; import org.junit.Test; import java.time.Duration; +import java.util.Collection; import java.util.List; +import java.util.Optional; import java.util.function.Function; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author mpolden */ public class OsUpgraderTest { - private static final ZoneApi zone1 = ZoneApiMock.newBuilder().withId("prod.eu-west-1").build(); - private static final ZoneApi zone2 = ZoneApiMock.newBuilder().withId("prod.us-west-1").build(); - private static final ZoneApi zone3 = ZoneApiMock.newBuilder().withId("prod.us-central-1").build(); - private static final ZoneApi zone4 = ZoneApiMock.newBuilder().withId("prod.us-east-3").build(); - private static final ZoneApi zone5 = ZoneApiMock.newBuilder().withId("prod.us-north-1").withCloud("other").build(); - private final ControllerTester tester = new ControllerTester(); - private final OsVersionStatusUpdater statusUpdater = new OsVersionStatusUpdater(tester.controller(), Duration.ofDays(1) - ); + private final OsVersionStatusUpdater statusUpdater = new OsVersionStatusUpdater(tester.controller(), Duration.ofDays(1)); + @Test public void upgrade_os() { - OsUpgrader osUpgrader = osUpgrader( - UpgradePolicy.create() - .upgrade(zone1) - .upgradeInParallel(zone2, zone3) - .upgrade(zone5) // Belongs to a different cloud and is ignored by this upgrader - .upgrade(zone4), - SystemName.cd - ); + Cloud cloud1 = new Cloud(CloudName.from("c1"), false, true, false, true); + Cloud cloud2 = new Cloud(CloudName.from("c2"), false, true, false, true); + ZoneApi zone1 = zone("prod.eu-west-1", cloud1); + ZoneApi zone2 = zone("prod.us-west-1", cloud1); + ZoneApi zone3 = zone("prod.us-central-1", cloud1); + ZoneApi zone4 = zone("prod.us-east-3", cloud1); + ZoneApi zone5 = zone("prod.us-north-1", cloud2); + UpgradePolicy upgradePolicy = UpgradePolicy.create() + .upgrade(zone1) + .upgradeInParallel(zone2, zone3) + .upgrade(zone5) // Belongs to a different cloud and is ignored by this upgrader + .upgrade(zone4); + OsUpgrader osUpgrader = osUpgrader(upgradePolicy, SystemName.cd, cloud1, cloud2); // Bootstrap system tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId(), zone5.getId()), List.of(SystemApplication.tenantHost)); - // Add system applications that exist in a real system, but are currently not upgraded + // Add system applications that exist in a real system, but isn't upgraded tester.configServer().addNodes(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId(), zone5.getId()), List.of(SystemApplication.configServer)); @@ -62,10 +65,9 @@ public class OsUpgraderTest { // New OS version released Version version1 = Version.fromString("7.1"); - CloudName cloud = CloudName.defaultName(); - tester.controller().upgradeOsIn(cloud, Version.fromString("7.0"), false); - tester.controller().upgradeOsIn(cloud, version1, false); - assertEquals(1, tester.controller().osVersions().size()); // Only allows one version per cloud + tester.controller().upgradeOsIn(cloud1.name(), Version.fromString("7.0"), Optional.empty(), false); + tester.controller().upgradeOsIn(cloud1.name(), version1, Optional.empty(), false); + assertEquals(1, tester.controller().osVersionTargets().size()); // Only allows one version per cloud statusUpdater.maintain(); // zone 1: begins upgrading @@ -102,10 +104,74 @@ public class OsUpgraderTest { osUpgrader.maintain(); assertWanted(version1, SystemApplication.tenantHost, zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()); statusUpdater.maintain(); - assertTrue("All nodes on target version", tester.controller().osVersionStatus().nodesIn(cloud).stream() + assertTrue("All nodes on target version", tester.controller().osVersionStatus().nodesIn(cloud1.name()).stream() .allMatch(node -> node.currentVersion().equals(version1))); } + @Test + public void upgrade_os_with_budget() { + Cloud cloud = new Cloud(CloudName.from("cloud"), false, true, true, true); + ZoneApi zone1 = zone("dev.us-east-1", cloud); + ZoneApi zone2 = zone("prod.us-west-1", cloud); + ZoneApi zone3 = zone("prod.us-central-1", cloud); + ZoneApi zone4 = zone("prod.eu-west-1", cloud); + UpgradePolicy upgradePolicy = UpgradePolicy.create() + .upgrade(zone1) + .upgradeInParallel(zone2, zone3) + .upgrade(zone4); + OsUpgrader osUpgrader = osUpgrader(upgradePolicy, SystemName.cd, cloud); + + // Bootstrap system + tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()), + List.of(SystemApplication.tenantHost)); + tester.configServer().addNodes(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()), + List.of(SystemApplication.configServerHost)); // Not supported yet + + // Upgrade without budget fails + Version version = Version.fromString("7.1"); + try { + tester.controller().upgradeOsIn(cloud.name(), version, Optional.empty(), false); + fail("Expected exception"); + } catch (IllegalArgumentException ignored) {} + + // Upgrade with budget + tester.controller().upgradeOsIn(cloud.name(), version, Optional.of(Duration.ofHours(12)), false); + assertEquals(Duration.ofHours(12), tester.controller().osVersionTarget(cloud.name()).get().upgradeBudget().get()); + statusUpdater.maintain(); + osUpgrader.maintain(); + + // First zone upgrades + assertWanted(Version.emptyVersion, SystemApplication.configServerHost, zone1.getId()); + assertEquals("Dev zone gets a zero budget", Duration.ZERO, upgradeBudget(zone1.getId(), SystemApplication.tenantHost, version)); + completeUpgrade(version, SystemApplication.tenantHost, zone1.getId()); + + // Next set of zones upgrade + osUpgrader.maintain(); + for (var zone : List.of(zone2.getId(), zone3.getId())) { + assertEquals("Parallel prod zones share the budget of a single zone", Duration.ofHours(6), + upgradeBudget(zone, SystemApplication.tenantHost, version)); + completeUpgrade(version, SystemApplication.tenantHost, zone); + } + + // Last zone upgrades + osUpgrader.maintain(); + assertEquals("Last prod zone gets the budget of a single zone", Duration.ofHours(6), + upgradeBudget(zone4.getId(), SystemApplication.tenantHost, version)); + completeUpgrade(version, SystemApplication.tenantHost, zone4.getId()); + + // All host applications upgraded + statusUpdater.maintain(); + assertTrue("All nodes on target version", tester.controller().osVersionStatus().nodesIn(cloud.name()).stream() + .allMatch(node -> node.currentVersion().equals(version))); + } + + private Duration upgradeBudget(ZoneId zone, SystemApplication application, Version version) { + var upgradeBudget = tester.configServer().nodeRepository().osUpgradeBudget(zone, application.nodeType(), version); + assertTrue("Expected budget for upgrade to " + version + " of " + application.id() + " in " + zone, + upgradeBudget.isPresent()); + return upgradeBudget.get(); + } + private List<NodeVersion> nodesOn(Version version) { return tester.controller().osVersionStatus().versions().entrySet().stream() .filter(entry -> entry.getKey().version().equals(version)) @@ -136,7 +202,7 @@ public class OsUpgraderTest { private List<Node> nodesRequiredToUpgrade(ZoneId zone, SystemApplication application) { return nodeRepository().list(zone, application.id()) .stream() - .filter(node -> OsUpgrader.eligibleForUpgrade(node, application)) + .filter(OsUpgrader::canUpgrade) .collect(Collectors.toList()); } @@ -164,13 +230,19 @@ public class OsUpgraderTest { return tester.configServer().nodeRepository(); } - private OsUpgrader osUpgrader(UpgradePolicy upgradePolicy, SystemName system) { + private OsUpgrader osUpgrader(UpgradePolicy upgradePolicy, SystemName system, Cloud managedCloud, Cloud... otherClouds) { + var zones = upgradePolicy.asList().stream().flatMap(Collection::stream).collect(Collectors.toList()); tester.zoneRegistry() - .setZones(zone1, zone2, zone3, zone4, zone5) + .setZones(zones) + .addCloud(managedCloud) + .addCloud(otherClouds) .setSystemName(system) - .setOsUpgradePolicy(CloudName.defaultName(), upgradePolicy); - return new OsUpgrader(tester.controller(), Duration.ofDays(1), - CloudName.defaultName()); + .setOsUpgradePolicy(managedCloud.name(), upgradePolicy); + return new OsUpgrader(tester.controller(), Duration.ofDays(1), managedCloud); + } + + private static ZoneApi zone(String id, Cloud cloud) { + return ZoneApiMock.newBuilder().withId(id).withCloud(cloud.name().value()).build(); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java index 5ddd2064c32..e9a1adcfe88 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java @@ -11,6 +11,7 @@ import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import org.junit.Test; import java.time.Duration; +import java.util.Optional; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -40,7 +41,7 @@ public class OsVersionStatusUpdaterTest { // Setting a new target adds it to current status Version version1 = Version.fromString("7.1"); CloudName cloud = CloudName.defaultName(); - tester.controller().upgradeOsIn(cloud, version1, false); + tester.controller().upgradeOsIn(cloud, version1, Optional.empty(), false); statusUpdater.maintain(); var osVersions = tester.controller().osVersionStatus().versions(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionTargetSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionTargetSerializerTest.java new file mode 100644 index 00000000000..dea09998952 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionTargetSerializerTest.java @@ -0,0 +1,33 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.persistence; + +import com.google.common.collect.ImmutableSet; +import com.yahoo.component.Version; +import com.yahoo.config.provision.CloudName; +import com.yahoo.vespa.hosted.controller.versions.OsVersion; +import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget; +import org.junit.Test; + +import java.time.Duration; +import java.util.Optional; +import java.util.Set; + +import static org.junit.Assert.assertEquals; + +/** + * @author mpolden + */ +public class OsVersionTargetSerializerTest { + + @Test + public void serialization() { + OsVersionTargetSerializer serializer = new OsVersionTargetSerializer(new OsVersionSerializer()); + Set<OsVersionTarget> targets = ImmutableSet.of( + new OsVersionTarget(new OsVersion(Version.fromString("7.1"), CloudName.defaultName()), Optional.empty()), + new OsVersionTarget(new OsVersion(Version.fromString("7.1"), CloudName.from("foo")), Optional.of(Duration.ofDays(1))) + ); + Set<OsVersionTarget> serialized = serializer.fromSlime(serializer.toSlime(targets)); + assertEquals(targets, serialized); + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java index 66493e6e226..5534b6937b2 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.restapi.os; import com.yahoo.application.container.handler.Request; +import com.yahoo.config.provision.Cloud; import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.UpgradePolicy; @@ -36,11 +37,11 @@ public class OsApiTest extends ControllerContainerTest { private static final String responses = "src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/"; private static final AthenzIdentity operator = AthenzUser.fromUserId("operatorUser"); - private static final CloudName cloud1 = CloudName.from("cloud1"); - private static final CloudName cloud2 = CloudName.from("cloud2"); - private static final ZoneApi zone1 = ZoneApiMock.newBuilder().withId("prod.us-east-3").with(cloud1).build(); - private static final ZoneApi zone2 = ZoneApiMock.newBuilder().withId("prod.us-west-1").with(cloud1).build(); - private static final ZoneApi zone3 = ZoneApiMock.newBuilder().withId("prod.eu-west-1").with(cloud2).build(); + private static final Cloud cloud1 = new Cloud(CloudName.from("cloud1"), false, true, false, false); + private static final Cloud cloud2 = new Cloud(CloudName.from("cloud2"), true, false, true, true); + private static final ZoneApi zone1 = ZoneApiMock.newBuilder().withId("prod.us-east-3").with(cloud1.name()).build(); + private static final ZoneApi zone2 = ZoneApiMock.newBuilder().withId("prod.us-west-1").with(cloud1.name()).build(); + private static final ZoneApi zone3 = ZoneApiMock.newBuilder().withId("prod.eu-west-1").with(cloud2.name()).build(); private ContainerTester tester; private List<OsUpgrader> osUpgraders; @@ -51,8 +52,9 @@ public class OsApiTest extends ControllerContainerTest { addUserToHostedOperatorRole(operator); zoneRegistryMock().setSystemName(SystemName.cd) .setZones(zone1, zone2, zone3) - .setOsUpgradePolicy(cloud1, UpgradePolicy.create().upgrade(zone1).upgrade(zone2)) - .setOsUpgradePolicy(cloud2, UpgradePolicy.create().upgrade(zone3)); + .addCloud(cloud1, cloud2) + .setOsUpgradePolicy(cloud1.name(), UpgradePolicy.create().upgrade(zone1).upgrade(zone2)) + .setOsUpgradePolicy(cloud2.name(), UpgradePolicy.create().upgrade(zone3)); osUpgraders = List.of( new OsUpgrader(tester.controller(), Duration.ofDays(1), cloud1), @@ -70,8 +72,8 @@ public class OsApiTest extends ControllerContainerTest { // Upgrade OS to a different version in each cloud assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"7.5.2\", \"cloud\": \"cloud1\"}", Request.Method.PATCH), "{\"message\":\"Set target OS version for cloud 'cloud1' to 7.5.2\"}", 200); - assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"8.2.1\", \"cloud\": \"cloud2\"}", Request.Method.PATCH), - "{\"message\":\"Set target OS version for cloud 'cloud2' to 8.2.1\"}", 200); + assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"8.2.1\", \"cloud\": \"cloud2\", \"upgradeBudget\": \"PT24H\"}", Request.Method.PATCH), + "{\"message\":\"Set target OS version for cloud 'cloud2' to 8.2.1 with upgrade budget PT24H\"}", 200); // Status is updated after some zones are upgraded upgradeAndUpdateStatus(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json index 01af1bd70dd..5834e4cef4a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json @@ -100,39 +100,10 @@ { "version": "8.2.1", "targetVersion": true, + "upgradeBudget": "PT24H", "cloud": "cloud2", "nodes": [ { - "hostname": "node-1-configserver-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { - "hostname": "node-2-configserver-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { - "hostname": "node-3-configserver-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { - "hostname": "node-1-proxy-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { - "hostname": "node-3-proxy-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { - "hostname": "node-2-proxy-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { "hostname": "node-1-tenant-host-prod.eu-west-1", "environment": "prod", "region": "eu-west-1" diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-partially-upgraded.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-partially-upgraded.json index 2b907c1156c..c8833fea100 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-partially-upgraded.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-partially-upgraded.json @@ -110,36 +110,6 @@ "cloud": "cloud2", "nodes": [ { - "hostname": "node-1-configserver-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { - "hostname": "node-2-configserver-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { - "hostname": "node-3-configserver-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { - "hostname": "node-1-proxy-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { - "hostname": "node-3-proxy-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { - "hostname": "node-2-proxy-host-prod.eu-west-1", - "environment": "prod", - "region": "eu-west-1" - }, - { "hostname": "node-1-tenant-host-prod.eu-west-1", "environment": "prod", "region": "eu-west-1" @@ -159,6 +129,7 @@ { "version": "8.2.1", "targetVersion": true, + "upgradeBudget": "PT24H", "cloud": "cloud2", "nodes": [] } |