From d6be7f2e2696d2cc00518cd1593d8381692439c2 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 4 Oct 2019 12:48:01 +0200 Subject: Extract NodeVersionSerializer --- .../persistence/NodeVersionSerializer.java | 61 ++++++++++++++++++++++ .../persistence/VersionStatusSerializer.java | 24 +++------ 2 files changed, 68 insertions(+), 17 deletions(-) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java new file mode 100644 index 00000000000..088808f5942 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java @@ -0,0 +1,61 @@ +// Copyright 2019 Oath Inc. 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.component.Version; +import com.yahoo.config.provision.HostName; +import com.yahoo.slime.ArrayTraverser; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.vespa.hosted.controller.versions.NodeVersion; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Optional; + +/** + * Serializer for {@link com.yahoo.vespa.hosted.controller.versions.NodeVersion}. + * + * @author mpolden + */ +public class NodeVersionSerializer { + + // 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 static final String hostnameField = "hostname"; + private static final String currentVersionField = "currentVersion"; + private static final String wantedVersionField = "wantedVersion"; + private static final String changedAtField = "changedAt"; + + public void nodeVersionsToSlime(Collection nodeVersions, Cursor array, boolean writeCurrentVersion) { + for (var nodeVersion : nodeVersions) { + var nodeVersionObject = array.addObject(); + nodeVersionObject.setString(hostnameField, nodeVersion.hostname().value()); + if (writeCurrentVersion) { + nodeVersionObject.setString(currentVersionField, nodeVersion.currentVersion().toFullString()); + } + nodeVersionObject.setString(wantedVersionField, nodeVersion.wantedVersion().toFullString()); + nodeVersionObject.setLong(changedAtField, nodeVersion.changedAt().toEpochMilli()); + } + } + + public List nodeVersionsFromSlime(Inspector object, Optional version) { + var nodeVersions = new ArrayList(); + object.traverse((ArrayTraverser) (i, entry) -> { + var hostname = HostName.from(entry.field(hostnameField).asString()); + var currentVersion = version.orElseGet(() -> Version.fromString(entry.field(currentVersionField).asString())); + var wantedVersion = Version.fromString(entry.field(wantedVersionField).asString()); + var changedAt = Instant.ofEpochMilli(entry.field(changedAtField).asLong()); + nodeVersions.add(new NodeVersion(hostname, currentVersion, wantedVersion, changedAt)); + }); + return Collections.unmodifiableList(nodeVersions); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java index 5061f32da68..62fca7a419b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java @@ -21,6 +21,7 @@ import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; +import java.util.Optional; import java.util.Set; /** @@ -53,17 +54,14 @@ public class VersionStatusSerializer { // NodeVersions fields private static final String nodeVersionsField = "nodeVersions"; - // NodeVersion fields - private static final String hostnameField = "hostname"; - private static final String wantedVersionField = "wantedVersion"; - private static final String changedAtField = "changedAt"; - // DeploymentStatistics fields private static final String versionField = "version"; private static final String failingField = "failing"; private static final String productionField = "production"; private static final String deployingField = "deploying"; + private final NodeVersionSerializer nodeVersionSerializer = new NodeVersionSerializer(); + public Slime toSlime(VersionStatus status) { Slime slime = new Slime(); Cursor root = slime.setObject(); @@ -93,12 +91,7 @@ public class VersionStatusSerializer { } private void nodeVersionsToSlime(NodeVersions nodeVersions, Cursor array) { - for (NodeVersion nodeVersion : nodeVersions.asMap().values()) { - var nodeVersionObject = array.addObject(); - nodeVersionObject.setString(hostnameField, nodeVersion.hostname().value()); - nodeVersionObject.setString(wantedVersionField, nodeVersion.wantedVersion().toFullString()); - nodeVersionObject.setLong(changedAtField, nodeVersion.changedAt().toEpochMilli()); - } + nodeVersionSerializer.nodeVersionsToSlime(nodeVersions.asMap().values(), array, false); } // TODO(mpolden): Remove after October 2019 @@ -140,12 +133,9 @@ public class VersionStatusSerializer { var nodeVersions = ImmutableMap.builder(); var nodeVersionsRoot = root.field(nodeVersionsField); if (nodeVersionsRoot.valid()) { - nodeVersionsRoot.traverse((ArrayTraverser) (i, entry) -> { - var hostname = HostName.from(entry.field(hostnameField).asString()); - var wantedVersion = Version.fromString(entry.field(wantedVersionField).asString()); - var changedAt = Instant.ofEpochMilli(entry.field(changedAtField).asLong()); - nodeVersions.put(hostname, new NodeVersion(hostname, version, wantedVersion, changedAt)); - }); + for (var nodeVersion : nodeVersionSerializer.nodeVersionsFromSlime(nodeVersionsRoot, Optional.of(version))) { + nodeVersions.put(nodeVersion.hostname(), nodeVersion); + } } else { // TODO(mpolden): Remove after October 2019 var configServerHostnames = configServersFromSlime(root.field(configServersField)); -- cgit v1.2.3 From e8b367d8ff559d3cabf9fe3dee6a52d200af4d96 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 4 Oct 2019 13:17:47 +0200 Subject: Add zone field to NodeVersion --- .../controller/persistence/NodeVersionSerializer.java | 9 ++++++++- .../vespa/hosted/controller/versions/NodeVersion.java | 16 ++++++++++++---- .../vespa/hosted/controller/versions/VersionStatus.java | 2 +- .../persistence/VersionStatusSerializerTest.java | 3 ++- .../controller/restapi/deployment/DeploymentApiTest.java | 4 ++-- 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java index 088808f5942..d0e785198b1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.yahoo.component.Version; import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; @@ -30,6 +31,7 @@ public class NodeVersionSerializer { // - CHANGING THE FORMAT OF A FIELD: Don't do it bro. private static final String hostnameField = "hostname"; + private static final String zoneField = "zone"; private static final String currentVersionField = "currentVersion"; private static final String wantedVersionField = "wantedVersion"; private static final String changedAtField = "changedAt"; @@ -38,6 +40,7 @@ public class NodeVersionSerializer { for (var nodeVersion : nodeVersions) { var nodeVersionObject = array.addObject(); nodeVersionObject.setString(hostnameField, nodeVersion.hostname().value()); + nodeVersionObject.setString(zoneField, nodeVersion.zone().value()); if (writeCurrentVersion) { nodeVersionObject.setString(currentVersionField, nodeVersion.currentVersion().toFullString()); } @@ -50,10 +53,14 @@ public class NodeVersionSerializer { var nodeVersions = new ArrayList(); object.traverse((ArrayTraverser) (i, entry) -> { var hostname = HostName.from(entry.field(hostnameField).asString()); + // TODO(mpolden): Make non-optional after September 2019 + var zone = Serializers.optionalString(entry.field(zoneField)) + .map(ZoneId::from) + .orElseGet(ZoneId::defaultId); var currentVersion = version.orElseGet(() -> Version.fromString(entry.field(currentVersionField).asString())); var wantedVersion = Version.fromString(entry.field(wantedVersionField).asString()); var changedAt = Instant.ofEpochMilli(entry.field(changedAtField).asLong()); - nodeVersions.add(new NodeVersion(hostname, currentVersion, wantedVersion, changedAt)); + nodeVersions.add(new NodeVersion(hostname, zone, currentVersion, wantedVersion, changedAt)); }); return Collections.unmodifiableList(nodeVersions); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java index 0a690b90410..5e463e04322 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.versions; import com.yahoo.component.Version; import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.zone.ZoneId; import java.time.Instant; import java.util.Objects; @@ -17,12 +18,14 @@ import java.util.Objects; public class NodeVersion { private final HostName hostname; + private final ZoneId zone; private final Version currentVersion; private final Version wantedVersion; private final Instant changedAt; - public NodeVersion(HostName hostname, Version currentVersion, Version wantedVersion, Instant changedAt) { + public NodeVersion(HostName hostname, ZoneId zone, Version currentVersion, Version wantedVersion, Instant changedAt) { this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null"); + this.zone = Objects.requireNonNull(zone, "zone must be non-null"); this.currentVersion = Objects.requireNonNull(currentVersion, "version must be non-null"); this.wantedVersion = Objects.requireNonNull(wantedVersion, "wantedVersion must be non-null"); this.changedAt = Objects.requireNonNull(changedAt, "changedAt must be non-null"); @@ -33,6 +36,11 @@ public class NodeVersion { return hostname; } + /** Zone of this */ + public ZoneId zone() { + return zone; + } + /** Current version of this */ public Version currentVersion() { return currentVersion; @@ -56,13 +64,13 @@ public class NodeVersion { /** Returns a copy of this with current version set to given version */ public NodeVersion withCurrentVersion(Version version, Instant changedAt) { if (currentVersion.equals(version)) return this; - return new NodeVersion(hostname, version, wantedVersion, changedAt); + return new NodeVersion(hostname, zone, version, wantedVersion, changedAt); } /** Returns a copy of this with wanted version set to given version */ public NodeVersion withWantedVersion(Version version) { if (wantedVersion.equals(version)) return this; - return new NodeVersion(hostname, currentVersion, version, changedAt); + return new NodeVersion(hostname, zone, currentVersion, version, changedAt); } @Override @@ -87,7 +95,7 @@ public class NodeVersion { } public static NodeVersion empty(HostName hostname) { - return new NodeVersion(hostname, Version.emptyVersion, Version.emptyVersion, Instant.EPOCH); + return new NodeVersion(hostname, ZoneId.defaultId(), Version.emptyVersion, Version.emptyVersion, Instant.EPOCH); } } 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 bb43ec20234..ab445de5a7f 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 @@ -172,7 +172,7 @@ public class VersionStatus { for (var node : nodes) { // Only use current node version if config has converged Version version = configConverged ? node.currentVersion() : controller.systemVersion(); - newNodeVersions.add(new NodeVersion(node.hostname(), version, node.wantedVersion(), now)); + newNodeVersions.add(new NodeVersion(node.hostname(), zone.getId(), version, node.wantedVersion(), now)); } } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java index 5d65cf0381e..dd66f52561f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.versions.DeploymentStatistics; import com.yahoo.vespa.hosted.controller.versions.NodeVersion; @@ -97,7 +98,7 @@ public class VersionStatusSerializerTest { private static NodeVersions nodeVersions(Version version, Version wantedVersion, Instant changedAt, String... hostnames) { var nodeVersions = new ArrayList(); for (var hostname : hostnames) { - nodeVersions.add(new NodeVersion(HostName.from(hostname), version, wantedVersion, changedAt)); + nodeVersions.add(new NodeVersion(HostName.from(hostname), ZoneId.from("prod", "us-north-1"), version, wantedVersion, changedAt)); } return NodeVersions.EMPTY.with(nodeVersions); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java index 0a4d046e318..bb1e6b6256a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java @@ -76,8 +76,8 @@ public class DeploymentApiTest extends ControllerContainerTest { version.isControllerVersion(), version.isSystemVersion(), version.isReleased(), - NodeVersions.EMPTY.with(List.of(new NodeVersion(HostName.from("config1.test"), version.versionNumber(), version.versionNumber(), Instant.EPOCH), - new NodeVersion(HostName.from("config2.test"), version.versionNumber(), version.versionNumber(), Instant.EPOCH))), + NodeVersions.EMPTY.with(List.of(new NodeVersion(HostName.from("config1.test"), ZoneId.defaultId(), version.versionNumber(), version.versionNumber(), Instant.EPOCH), + new NodeVersion(HostName.from("config2.test"), ZoneId.defaultId(), version.versionNumber(), version.versionNumber(), Instant.EPOCH))), VespaVersion.confidenceFrom(version.statistics(), controller) ); } -- cgit v1.2.3 From c92653dace5115cfa3d180342b3fd9400904a0d6 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 4 Oct 2019 15:10:11 +0200 Subject: Stop reading legacy configServerHostnames field --- .../hosted/controller/persistence/CuratorDb.java | 4 +-- .../persistence/VersionStatusSerializer.java | 30 ++++++++-------------- .../persistence/VersionStatusSerializerTest.java | 13 +++++++--- .../testdata/version-status-legacy-format.json | 20 ++++++++++++--- 4 files changed, 37 insertions(+), 30 deletions(-) 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 357dbb37b27..b22b8845c24 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 @@ -6,7 +6,6 @@ import com.google.inject.Inject; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.path.Path; @@ -82,7 +81,8 @@ public class CuratorDb { private static final Path applicationCertificateRoot = root.append("applicationCertificates"); private final StringSetSerializer stringSetSerializer = new StringSetSerializer(); - private final VersionStatusSerializer versionStatusSerializer = new VersionStatusSerializer(); + private final NodeVersionSerializer nodeVersionSerializer = new NodeVersionSerializer(); + private final VersionStatusSerializer versionStatusSerializer = new VersionStatusSerializer(nodeVersionSerializer); private final ControllerVersionSerializer controllerVersionSerializer = new ControllerVersionSerializer(); private final ConfidenceOverrideSerializer confidenceOverrideSerializer = new ConfidenceOverrideSerializer(); private final TenantSerializer tenantSerializer = new TenantSerializer(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java index 62fca7a419b..fd02546cf58 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java @@ -21,6 +21,7 @@ import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -60,7 +61,11 @@ public class VersionStatusSerializer { private static final String productionField = "production"; private static final String deployingField = "deploying"; - private final NodeVersionSerializer nodeVersionSerializer = new NodeVersionSerializer(); + private final NodeVersionSerializer nodeVersionSerializer; + + public VersionStatusSerializer(NodeVersionSerializer nodeVersionSerializer) { + this.nodeVersionSerializer = Objects.requireNonNull(nodeVersionSerializer, "nodeVersionSerializer must be non-null"); + } public Slime toSlime(VersionStatus status) { Slime slime = new Slime(); @@ -86,7 +91,6 @@ public class VersionStatusSerializer { object.setBool(isReleasedField, version.isReleased()); deploymentStatisticsToSlime(version.statistics(), object.setObject(deploymentStatisticsField)); object.setString(confidenceField, version.confidence().name()); - configServersToSlime(version.nodeVersions().hostnames(), object.setArray(configServersField)); nodeVersionsToSlime(version.nodeVersions(), object.setArray(nodeVersionsField)); } @@ -94,11 +98,6 @@ public class VersionStatusSerializer { nodeVersionSerializer.nodeVersionsToSlime(nodeVersions.asMap().values(), array, false); } - // TODO(mpolden): Remove after October 2019 - private void configServersToSlime(Set configServerHostnames, Cursor array) { - configServerHostnames.stream().map(HostName::value).forEach(array::addString); - } - private void deploymentStatisticsToSlime(DeploymentStatistics statistics, Cursor object) { object.setString(versionField, statistics.version().toString()); applicationsToSlime(statistics.failing(), object.setArray(failingField)); @@ -124,24 +123,15 @@ public class VersionStatusSerializer { object.field(isControllerVersionField).asBool(), object.field(isSystemVersionField).asBool(), object.field(isReleasedField).asBool(), - nodeVersionsFromSlime(object, deploymentStatistics.version()), + nodeVersionsFromSlime(object.field(nodeVersionsField), deploymentStatistics.version()), VespaVersion.Confidence.valueOf(object.field(confidenceField).asString()) ); } - private NodeVersions nodeVersionsFromSlime(Inspector root, Version version) { + private NodeVersions nodeVersionsFromSlime(Inspector object, Version version) { var nodeVersions = ImmutableMap.builder(); - var nodeVersionsRoot = root.field(nodeVersionsField); - if (nodeVersionsRoot.valid()) { - for (var nodeVersion : nodeVersionSerializer.nodeVersionsFromSlime(nodeVersionsRoot, Optional.of(version))) { - nodeVersions.put(nodeVersion.hostname(), nodeVersion); - } - } else { - // TODO(mpolden): Remove after October 2019 - var configServerHostnames = configServersFromSlime(root.field(configServersField)); - for (var hostname : configServerHostnames) { - nodeVersions.put(hostname, NodeVersion.empty(hostname)); - } + for (var nodeVersion : nodeVersionSerializer.nodeVersionsFromSlime(object, Optional.of(version))) { + nodeVersions.put(nodeVersion.hostname(), nodeVersion); } return new NodeVersions(nodeVersions.build()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java index dd66f52561f..a80dcc118dc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java @@ -46,7 +46,7 @@ public class VersionStatusSerializerTest { false, nodeVersions(Version.fromString("5.0"), Version.fromString("5.1"), Instant.ofEpochMilli(456), "cfg1", "cfg2", "cfg3"), VespaVersion.Confidence.normal)); VersionStatus status = new VersionStatus(vespaVersions); - VersionStatusSerializer serializer = new VersionStatusSerializer(); + VersionStatusSerializer serializer = new VersionStatusSerializer(new NodeVersionSerializer()); VersionStatus deserialized = serializer.fromSlime(serializer.toSlime(status)); assertEquals(status.versions().size(), deserialized.versions().size()); @@ -68,7 +68,7 @@ public class VersionStatusSerializerTest { @Test public void testLegacySerialization() throws Exception { var data = Files.readAllBytes(Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/version-status-legacy-format.json")); - var serializer = new VersionStatusSerializer(); + var serializer = new VersionStatusSerializer(new NodeVersionSerializer()); var deserializedStatus = serializer.fromSlime(SlimeUtils.jsonToSlime(data)); var statistics = new DeploymentStatistics( @@ -77,11 +77,16 @@ public class VersionStatusSerializerTest { List.of(), List.of() ); + var nodeVersions = List.of(new NodeVersion(HostName.from("cfg1"), ZoneId.defaultId(), Version.fromString("7.0"), + Version.fromString("7.1"), Instant.ofEpochMilli(1111)), + new NodeVersion(HostName.from("cfg2"), ZoneId.defaultId(), Version.fromString("7.0"), + Version.fromString("7.1"), Instant.ofEpochMilli(2222)), + new NodeVersion(HostName.from("cfg3"), ZoneId.defaultId(), Version.fromString("7.0"), + Version.fromString("7.1"), Instant.ofEpochMilli(3333))); var vespaVersion = new VespaVersion(statistics, "badc0ffee", Instant.ofEpochMilli(123), true, true, true, - nodeVersions(Version.emptyVersion, Version.emptyVersion, - Instant.EPOCH, "cfg1", "cfg2", "cfg3"), + NodeVersions.EMPTY.with(nodeVersions), VespaVersion.Confidence.normal); VespaVersion deserialized = deserializedStatus.versions().get(0); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/version-status-legacy-format.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/version-status-legacy-format.json index 96ca22e1c1a..08463ed7cb4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/version-status-legacy-format.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/version-status-legacy-format.json @@ -13,10 +13,22 @@ "deploying": [] }, "confidence": "normal", - "configServerHostnames": [ - "cfg1", - "cfg2", - "cfg3" + "nodeVersions": [ + { + "hostname": "cfg1", + "wantedVersion": "7.1", + "changedAt": 1111 + }, + { + "hostname": "cfg2", + "wantedVersion": "7.1", + "changedAt": 2222 + }, + { + "hostname": "cfg3", + "wantedVersion": "7.1", + "changedAt": 3333 + } ] } ] -- cgit v1.2.3 From dd2739ec3abce6ca0ab3341b0ef6968f48f26492 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Mon, 7 Oct 2019 09:41:43 +0200 Subject: Replace OsVersion.Node with NodeVersion --- .../hosted/controller/maintenance/OsUpgrader.java | 2 +- .../hosted/controller/persistence/CuratorDb.java | 2 +- .../persistence/NodeVersionSerializer.java | 56 ++++++---- .../persistence/OsVersionStatusSerializer.java | 60 +++++----- .../persistence/VersionStatusSerializer.java | 15 +-- .../hosted/controller/restapi/os/OsApiHandler.java | 10 +- .../controller/versions/OsVersionStatus.java | 122 ++++++++------------- .../controller/maintenance/OsUpgraderTest.java | 8 +- .../maintenance/OsVersionStatusUpdaterTest.java | 11 +- .../persistence/OsVersionStatusSerializerTest.java | 56 +++++++--- .../testdata/os-version-status-legacy-format.json | 22 ++++ 11 files changed, 187 insertions(+), 177 deletions(-) create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/os-version-status-legacy-format.json 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 60bc3d15ec6..93d1dac7382 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 @@ -61,7 +61,7 @@ public class OsUpgrader extends InfrastructureUpgrader { // 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.version().isBefore(target.version()))) + .anyMatch(node -> node.currentVersion().isBefore(target.version()))) .map(OsVersion::version); } 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 b22b8845c24..dbd52fc6d02 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 @@ -89,7 +89,7 @@ public class CuratorDb { private final ApplicationSerializer applicationSerializer = new ApplicationSerializer(); private final RunSerializer runSerializer = new RunSerializer(); private final OsVersionSerializer osVersionSerializer = new OsVersionSerializer(); - private final OsVersionStatusSerializer osVersionStatusSerializer = new OsVersionStatusSerializer(osVersionSerializer); + private final OsVersionStatusSerializer osVersionStatusSerializer = new OsVersionStatusSerializer(osVersionSerializer, nodeVersionSerializer); private final RoutingPolicySerializer routingPolicySerializer = new RoutingPolicySerializer(); private final AuditLogSerializer auditLogSerializer = new AuditLogSerializer(); private final NameServiceQueueSerializer nameServiceQueueSerializer = new NameServiceQueueSerializer(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java index d0e785198b1..4b6e997241d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NodeVersionSerializer.java @@ -1,6 +1,7 @@ // Copyright 2019 Oath Inc. 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.ImmutableMap; import com.yahoo.component.Version; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.zone.ZoneId; @@ -8,13 +9,9 @@ import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; import com.yahoo.vespa.hosted.controller.versions.NodeVersion; +import com.yahoo.vespa.hosted.controller.versions.NodeVersions; import java.time.Instant; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Optional; /** * Serializer for {@link com.yahoo.vespa.hosted.controller.versions.NodeVersion}. @@ -32,37 +29,50 @@ public class NodeVersionSerializer { private static final String hostnameField = "hostname"; private static final String zoneField = "zone"; - private static final String currentVersionField = "currentVersion"; private static final String wantedVersionField = "wantedVersion"; private static final String changedAtField = "changedAt"; - public void nodeVersionsToSlime(Collection nodeVersions, Cursor array, boolean writeCurrentVersion) { - for (var nodeVersion : nodeVersions) { + // Legacy fields + private static final String environmentField = "environment"; + private static final String regionField = "region"; + + public void nodeVersionsToSlime(NodeVersions nodeVersions, Cursor array) { + for (var nodeVersion : nodeVersions.asMap().values()) { var nodeVersionObject = array.addObject(); nodeVersionObject.setString(hostnameField, nodeVersion.hostname().value()); nodeVersionObject.setString(zoneField, nodeVersion.zone().value()); - if (writeCurrentVersion) { - nodeVersionObject.setString(currentVersionField, nodeVersion.currentVersion().toFullString()); - } nodeVersionObject.setString(wantedVersionField, nodeVersion.wantedVersion().toFullString()); nodeVersionObject.setLong(changedAtField, nodeVersion.changedAt().toEpochMilli()); } } - public List nodeVersionsFromSlime(Inspector object, Optional version) { - var nodeVersions = new ArrayList(); - object.traverse((ArrayTraverser) (i, entry) -> { + public NodeVersions nodeVersionsFromSlime(Inspector array, Version version) { + var nodeVersions = ImmutableMap.builder(); + array.traverse((ArrayTraverser) (i, entry) -> { var hostname = HostName.from(entry.field(hostnameField).asString()); - // TODO(mpolden): Make non-optional after September 2019 - var zone = Serializers.optionalString(entry.field(zoneField)) - .map(ZoneId::from) - .orElseGet(ZoneId::defaultId); - var currentVersion = version.orElseGet(() -> Version.fromString(entry.field(currentVersionField).asString())); - var wantedVersion = Version.fromString(entry.field(wantedVersionField).asString()); - var changedAt = Instant.ofEpochMilli(entry.field(changedAtField).asLong()); - nodeVersions.add(new NodeVersion(hostname, zone, currentVersion, wantedVersion, changedAt)); + var zone = zoneFromSlime(entry); + // TODO(mpolden): Make the following fields non-optional after September 2019 + var wantedVersion = Serializers.optionalString(entry.field(wantedVersionField)) + .map(Version::fromString) + .orElse(Version.emptyVersion); + var changedAt = Serializers.optionalInstant(entry.field(changedAtField)).orElse(Instant.EPOCH); + nodeVersions.put(hostname, new NodeVersion(hostname, zone, version, wantedVersion, changedAt)); }); - return Collections.unmodifiableList(nodeVersions); + return new NodeVersions(nodeVersions.build()); + } + + // TODO(mpolden): Simplify and in-line after September 2019 + private ZoneId zoneFromSlime(Inspector object) { + var zoneInspector = object.field(zoneField); + if (zoneInspector.valid()) { + return ZoneId.from(zoneInspector.asString()); + } + var regionInspector = object.field(regionField); + var environmentInspector = object.field(environmentField); + if (regionInspector.valid() && environmentInspector.valid()) { + return ZoneId.from(environmentInspector.asString(), regionInspector.asString()); + } + return ZoneId.defaultId(); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java index 88805f54d65..fa29969f166 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializer.java @@ -1,23 +1,19 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.persistence; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSortedMap; import com.yahoo.component.Version; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RegionName; 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.NodeVersion; +import com.yahoo.vespa.hosted.controller.versions.NodeVersions; import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Map; import java.util.Objects; -import java.util.TreeMap; /** * Serializer for {@link OsVersionStatus}. @@ -39,11 +35,14 @@ public class OsVersionStatusSerializer { private static final String hostnameField = "hostname"; private static final String regionField = "region"; private static final String environmentField = "environment"; + private static final String nodeVersionsField = "nodeVersions"; private final OsVersionSerializer osVersionSerializer; + private final NodeVersionSerializer nodeVersionSerializer; - public OsVersionStatusSerializer(OsVersionSerializer osVersionSerializer) { + public OsVersionStatusSerializer(OsVersionSerializer osVersionSerializer, NodeVersionSerializer nodeVersionSerializer) { this.osVersionSerializer = Objects.requireNonNull(osVersionSerializer, "osVersionSerializer must be non-null"); + this.nodeVersionSerializer = Objects.requireNonNull(nodeVersionSerializer, "nodeVersionSerializer must be non-null"); } public Slime toSlime(OsVersionStatus status) { @@ -53,6 +52,8 @@ public class OsVersionStatusSerializer { status.versions().forEach((version, nodes) -> { Cursor object = versions.addObject(); osVersionSerializer.toSlime(version, object); + nodeVersionSerializer.nodeVersionsToSlime(nodes, object.setArray(nodeVersionsField)); + // TODO(mpolden): Stop writing this after September 2019 nodesToSlime(nodes, object.setArray(nodesField)); }); return slime; @@ -62,40 +63,33 @@ public class OsVersionStatusSerializer { return new OsVersionStatus(osVersionsFromSlime(slime.get().field(versionsField))); } - private void nodesToSlime(List nodes, Cursor array) { - nodes.forEach(node -> nodeToSlime(node, array.addObject())); + private void nodesToSlime(NodeVersions nodeVersions, Cursor array) { + nodeVersions.asMap().values().forEach(node -> nodeToSlime(node, array.addObject())); } - private void nodeToSlime(OsVersionStatus.Node node, Cursor object) { + private void nodeToSlime(NodeVersion node, Cursor object) { object.setString(hostnameField, node.hostname().value()); - object.setString(versionField, node.version().toFullString()); - object.setString(regionField, node.region().value()); - object.setString(environmentField, node.environment().value()); + object.setString(versionField, node.currentVersion().toFullString()); + object.setString(regionField, node.zone().region().value()); + object.setString(environmentField, node.zone().environment().value()); } - private Map> osVersionsFromSlime(Inspector array) { - Map> versions = new TreeMap<>(); + private ImmutableMap osVersionsFromSlime(Inspector array) { + var versions = ImmutableSortedMap.naturalOrder(); array.traverse((ArrayTraverser) (i, object) -> { OsVersion osVersion = osVersionSerializer.fromSlime(object); - List nodes = nodesFromSlime(object.field(nodesField)); - versions.put(osVersion, nodes); + versions.put(osVersion, nodesFromSlime(object, osVersion.version())); }); - return Collections.unmodifiableMap(versions); + return versions.build(); } - private List nodesFromSlime(Inspector array) { - List nodes = new ArrayList<>(); - array.traverse((ArrayTraverser) (i, object) -> nodes.add(nodeFromSlime(object))); - return Collections.unmodifiableList(nodes); - } - - private OsVersionStatus.Node nodeFromSlime(Inspector object) { - return new OsVersionStatus.Node( - HostName.from(object.field(hostnameField).asString()), - Version.fromString(object.field(versionField).asString()), - Environment.from(object.field(environmentField).asString()), - RegionName.from(object.field(regionField).asString()) - ); + // TODO(mpolden): Simplify and in-line after September 2019 + private NodeVersions nodesFromSlime(Inspector object, Version version) { + var newField = object.field(nodeVersionsField); + if (newField.valid()) { + return nodeVersionSerializer.nodeVersionsFromSlime(newField, version); + } + return nodeVersionSerializer.nodeVersionsFromSlime(object.field(nodesField), version); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java index fd02546cf58..4373c8977de 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java @@ -1,7 +1,6 @@ // Copyright 2019 Oath Inc. 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.ImmutableMap; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostName; @@ -10,7 +9,6 @@ import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; import com.yahoo.vespa.hosted.controller.versions.DeploymentStatistics; -import com.yahoo.vespa.hosted.controller.versions.NodeVersion; import com.yahoo.vespa.hosted.controller.versions.NodeVersions; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; @@ -22,7 +20,6 @@ import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.Set; /** @@ -95,7 +92,7 @@ public class VersionStatusSerializer { } private void nodeVersionsToSlime(NodeVersions nodeVersions, Cursor array) { - nodeVersionSerializer.nodeVersionsToSlime(nodeVersions.asMap().values(), array, false); + nodeVersionSerializer.nodeVersionsToSlime(nodeVersions, array); } private void deploymentStatisticsToSlime(DeploymentStatistics statistics, Cursor object) { @@ -123,19 +120,11 @@ public class VersionStatusSerializer { object.field(isControllerVersionField).asBool(), object.field(isSystemVersionField).asBool(), object.field(isReleasedField).asBool(), - nodeVersionsFromSlime(object.field(nodeVersionsField), deploymentStatistics.version()), + nodeVersionSerializer.nodeVersionsFromSlime(object.field(nodeVersionsField), deploymentStatistics.version()), VespaVersion.Confidence.valueOf(object.field(confidenceField).asString()) ); } - private NodeVersions nodeVersionsFromSlime(Inspector object, Version version) { - var nodeVersions = ImmutableMap.builder(); - for (var nodeVersion : nodeVersionSerializer.nodeVersionsFromSlime(object, Optional.of(version))) { - nodeVersions.put(nodeVersion.hostname(), nodeVersion); - } - return new NodeVersions(nodeVersions.build()); - } - private Set configServersFromSlime(Inspector array) { Set configServerHostnames = new LinkedHashSet<>(); array.traverse((ArrayTraverser) (i, entry) -> configServerHostnames.add(HostName.from(entry.asString()))); 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 450f4481c5f..c168a057bfb 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 @@ -160,17 +160,17 @@ public class OsApiHandler extends AuditLoggingRequestHandler { Set osVersions = controller.osVersions(); Cursor versions = root.setArray("versions"); - controller.osVersionStatus().versions().forEach((osVersion, nodes) -> { + controller.osVersionStatus().versions().forEach((osVersion, nodeVersions) -> { Cursor currentVersionObject = versions.addObject(); currentVersionObject.setString("version", osVersion.version().toFullString()); currentVersionObject.setBool("targetVersion", osVersions.contains(osVersion)); currentVersionObject.setString("cloud", osVersion.cloud().value()); Cursor nodesArray = currentVersionObject.setArray("nodes"); - nodes.forEach(node -> { + nodeVersions.asMap().values().forEach(nodeVersion -> { Cursor nodeObject = nodesArray.addObject(); - nodeObject.setString("hostname", node.hostname().value()); - nodeObject.setString("environment", node.environment().value()); - nodeObject.setString("region", node.region().value()); + nodeObject.setString("hostname", nodeVersion.hostname().value()); + nodeObject.setString("environment", nodeVersion.zone().environment().value()); + nodeObject.setString("region", nodeVersion.zone().region().value()); }); }); 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 a73a20198f0..d5e83d99cdd 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 @@ -4,9 +4,7 @@ package com.yahoo.vespa.hosted.controller.versions; import com.google.common.collect.ImmutableMap; import com.yahoo.component.Version; import com.yahoo.config.provision.CloudName; -import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.application.SystemApplication; @@ -14,11 +12,11 @@ import com.yahoo.vespa.hosted.controller.maintenance.OsUpgrader; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -29,25 +27,25 @@ import java.util.stream.Collectors; */ public class OsVersionStatus { - public static final OsVersionStatus empty = new OsVersionStatus(Collections.emptyMap()); + public static final OsVersionStatus empty = new OsVersionStatus(ImmutableMap.of()); - private final Map> versions; + private final Map versions; /** Public for serialization purpose only. Use {@link OsVersionStatus#compute(Controller)} for an up-to-date status */ - public OsVersionStatus(Map> versions) { + public OsVersionStatus(ImmutableMap versions) { this.versions = ImmutableMap.copyOf(Objects.requireNonNull(versions, "versions must be non-null")); } /** All known OS versions and their nodes */ - public Map> versions() { + public Map versions() { return versions; } /** Returns nodes eligible for OS upgrades that exist in given cloud */ - public List nodesIn(CloudName cloud) { + public List nodesIn(CloudName cloud) { return versions.entrySet().stream() .filter(entry -> entry.getKey().cloud().equals(cloud)) - .flatMap(entry -> entry.getValue().stream()) + .flatMap(entry -> entry.getValue().asMap().values().stream()) .collect(Collectors.toUnmodifiableList()); } @@ -61,28 +59,52 @@ 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) { - Map> versions = new HashMap<>(); - - // Always include all target versions - controller.osVersions().forEach(osVersion -> versions.put(osVersion, new ArrayList<>())); - - for (SystemApplication application : SystemApplication.all()) { - if (!application.isEligibleForOsUpgrades()) { - continue; // Avoid querying applications that are not eligible for OS upgrades - } - for (ZoneApi zone : zonesToUpgrade(controller)) { - controller.serviceRegistry().configServer().nodeRepository().list(zone.getId(), application.id()).stream() + var osVersionStatus = controller.osVersionStatus(); + var osVersions = new HashMap>(); + var now = controller.clock().instant(); + controller.osVersions().forEach(osVersion -> osVersions.put(osVersion, new ArrayList<>())); + + for (var application : SystemApplication.all()) { + if (!application.isEligibleForOsUpgrades()) continue; + for (var zone : zonesToUpgrade(controller)) { + var targetOsVersion = controller.serviceRegistry().configServer().nodeRepository() + .targetVersionsOf(zone.getId()) + .osVersion(application.nodeType()) + .orElse(Version.emptyVersion); + controller.serviceRegistry().configServer().nodeRepository() + .list(zone.getId(), application.id()).stream() .filter(node -> OsUpgrader.eligibleForUpgrade(node, application)) - .map(node -> new Node(node.hostname(), node.currentOsVersion(), zone.getEnvironment(), zone.getRegionName())) - .forEach(node -> { - var version = new OsVersion(node.version(), zone.getCloudName()); - versions.putIfAbsent(version, new ArrayList<>()); - versions.get(version).add(node); + .map(node -> new NodeVersion(node.hostname(), zone.getId(), node.currentOsVersion(), targetOsVersion, now)) + .forEach(nodeVersion -> { + var newNodeVersion = osVersionStatus.of(nodeVersion.hostname()) + .map(nv -> nv.withCurrentVersion(nodeVersion.currentVersion(), now) + .withWantedVersion(nodeVersion.wantedVersion())) + .orElse(nodeVersion); + var version = new OsVersion(newNodeVersion.currentVersion(), zone.getCloudName()); + osVersions.putIfAbsent(version, new ArrayList<>()); + osVersions.get(version).add(newNodeVersion); }); } } - return new OsVersionStatus(versions); + var newOsVersions = ImmutableMap.builder(); + for (var osVersion : osVersions.entrySet()) { + var nodeVersions = ImmutableMap.builder(); + for (var nodeVersion : osVersion.getValue()) { + nodeVersions.put(nodeVersion.hostname(), nodeVersion); + } + newOsVersions.put(osVersion.getKey(), new NodeVersions(nodeVersions.build())); + } + return new OsVersionStatus(newOsVersions.build()); + } + + /** Returns version of node identified by given host name */ + private Optional of(HostName hostname) { + return versions.values().stream() + .map(nodeVersions -> nodeVersions.asMap().get(hostname)) + .map(Optional::ofNullable) + .flatMap(Optional::stream) + .findFirst(); } private static List zonesToUpgrade(Controller controller) { @@ -92,52 +114,4 @@ public class OsVersionStatus { .collect(Collectors.toUnmodifiableList()); } - /** A node in this system and its current OS version */ - public static class Node { - - private final HostName hostname; - private final Version version; - private final Environment environment; - private final RegionName region; - - public Node(HostName hostname, Version version, Environment environment, RegionName region) { - this.hostname = Objects.requireNonNull(hostname, "hostname must be non-null"); - this.version = Objects.requireNonNull(version, "version must be non-null"); - this.environment = Objects.requireNonNull(environment, "environment must be non-null"); - this.region = Objects.requireNonNull(region, "region must be non-null"); - } - - public HostName hostname() { - return hostname; - } - - public Version version() { - return version; - } - - public Environment environment() { - return environment; - } - - public RegionName region() { - return region; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Node node = (Node) o; - return Objects.equals(hostname, node.hostname) && - Objects.equals(version, node.version) && - environment == node.environment && - Objects.equals(region, node.region); - } - - @Override - public int hashCode() { - return Objects.hash(hostname, version, environment, region); - } - } - } 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 1af5fafbb79..5e92112d465 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 @@ -12,7 +12,7 @@ import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.integration.NodeRepositoryMock; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; -import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; +import com.yahoo.vespa.hosted.controller.versions.NodeVersion; import org.junit.Before; import org.junit.Test; @@ -111,13 +111,13 @@ public class OsUpgraderTest { 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() - .allMatch(node -> node.version().equals(version1))); + .allMatch(node -> node.currentVersion().equals(version1))); } - private List nodesOn(Version version) { + private List nodesOn(Version version) { return tester.controller().osVersionStatus().versions().entrySet().stream() .filter(entry -> entry.getKey().version().equals(version)) - .flatMap(entry -> entry.getValue().stream()) + .flatMap(entry -> entry.getValue().asMap().values().stream()) .collect(Collectors.toList()); } 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 fe7f39fd66d..e51fcff33d1 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 @@ -3,18 +3,15 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.zone.UpgradePolicy; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.vespa.hosted.controller.ControllerTester; -import com.yahoo.config.provision.zone.UpgradePolicy; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import org.junit.Test; import java.time.Duration; -import java.util.List; -import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -47,10 +44,10 @@ public class OsVersionStatusUpdaterTest { tester.controller().upgradeOsIn(cloud, version1, false); statusUpdater.maintain(); - Map> osVersions = tester.controller().osVersionStatus().versions(); + var osVersions = tester.controller().osVersionStatus().versions(); assertEquals(2, osVersions.size()); - assertFalse("All nodes on unknown version", osVersions.get(new OsVersion(Version.emptyVersion, cloud)).isEmpty()); - assertTrue("No nodes on current target", osVersions.get(new OsVersion(version1, cloud)).isEmpty()); + assertFalse("All nodes on unknown version", osVersions.get(new OsVersion(Version.emptyVersion, cloud)).asMap().isEmpty()); + assertTrue("No nodes on current target", osVersions.get(new OsVersion(version1, cloud)).asMap().isEmpty()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java index 5073f651fd3..c60137e47b4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java @@ -1,18 +1,23 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.persistence; +import com.google.common.collect.ImmutableMap; import com.yahoo.component.Version; import com.yahoo.config.provision.CloudName; -import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.config.SlimeUtils; +import com.yahoo.vespa.hosted.controller.versions.NodeVersion; +import com.yahoo.vespa.hosted.controller.versions.NodeVersions; import com.yahoo.vespa.hosted.controller.versions.OsVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import org.junit.Test; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.time.Instant; import java.util.List; -import java.util.Map; -import java.util.TreeMap; import static org.junit.Assert.assertEquals; @@ -25,22 +30,41 @@ public class OsVersionStatusSerializerTest { public void test_serialization() { Version version1 = Version.fromString("7.1"); Version version2 = Version.fromString("7.2"); - Map> versions = new TreeMap<>(); + var versions = ImmutableMap.builder(); - versions.put(new OsVersion(version1, CloudName.defaultName()), List.of( - new OsVersionStatus.Node(HostName.from("node1"), version1, Environment.prod, RegionName.from("us-west")), - new OsVersionStatus.Node(HostName.from("node2"), version1, Environment.prod, RegionName.from("us-east")) - )); - versions.put(new OsVersion(version2, CloudName.defaultName()), List.of( - new OsVersionStatus.Node(HostName.from("node3"), version2, Environment.prod, RegionName.from("us-west")), - new OsVersionStatus.Node(HostName.from("node4"), version2, Environment.prod, RegionName.from("us-east")) + versions.put(new OsVersion(version1, CloudName.defaultName()), NodeVersions.EMPTY.with(List.of( + new NodeVersion(HostName.from("node1"), ZoneId.from("prod", "us-west"), version1, version2, Instant.ofEpochMilli(1)), + new NodeVersion(HostName.from("node2"), ZoneId.from("prod", "us-east"), version1, version2, Instant.ofEpochMilli(2)) + ))); + versions.put(new OsVersion(version2, CloudName.defaultName()), NodeVersions.EMPTY.with(List.of( + new NodeVersion(HostName.from("node3"), ZoneId.from("prod", "us-west"), version2, version2, Instant.ofEpochMilli(3)), + new NodeVersion(HostName.from("node4"), ZoneId.from("prod", "us-east"), version2, version2, Instant.ofEpochMilli(4)) + ))); - )); - - OsVersionStatusSerializer serializer = new OsVersionStatusSerializer(new OsVersionSerializer()); - OsVersionStatus status = new OsVersionStatus(versions); + OsVersionStatusSerializer serializer = new OsVersionStatusSerializer(new OsVersionSerializer(), new NodeVersionSerializer()); + OsVersionStatus status = new OsVersionStatus(versions.build()); OsVersionStatus serialized = serializer.fromSlime(serializer.toSlime(status)); assertEquals(status.versions(), serialized.versions()); } + @Test + public void testLegacySerialization() throws Exception { + var data = Files.readAllBytes(Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/os-version-status-legacy-format.json")); + var serializer = new OsVersionStatusSerializer(new OsVersionSerializer(), new NodeVersionSerializer()); + var versions = ImmutableMap.of( + new OsVersion(Version.fromString("7.42"), CloudName.from("yahoo")), + NodeVersions.EMPTY.with(List.of(new NodeVersion(HostName.from("node1"), ZoneId.from("prod", "us-north-1"), + Version.fromString("7.42"), Version.emptyVersion, Instant.EPOCH), + new NodeVersion(HostName.from("node2"), ZoneId.from("prod", "us-north-2"), + Version.fromString("7.42"), Version.emptyVersion, Instant.EPOCH)))); + + var deserialized = serializer.fromSlime(SlimeUtils.jsonToSlime(data)); + assertEquals(versions, deserialized.versions()); + + + var serialized = new String(SlimeUtils.toJsonBytes(serializer.toSlime(new OsVersionStatus(versions))), StandardCharsets.UTF_8); + assertEquals("{\"versions\":[{\"version\":\"7.42.0\",\"cloud\":\"yahoo\",\"nodeVersions\":[{\"hostname\":\"node1\",\"zone\":\"prod.us-north-1\",\"wantedVersion\":\"0.0.0\",\"changedAt\":0},{\"hostname\":\"node2\",\"zone\":\"prod.us-north-2\",\"wantedVersion\":\"0.0.0\",\"changedAt\":0}],\"nodes\":[{\"hostname\":\"node1\",\"version\":\"7.42.0\",\"region\":\"us-north-1\",\"environment\":\"prod\"},{\"hostname\":\"node2\",\"version\":\"7.42.0\",\"region\":\"us-north-2\",\"environment\":\"prod\"}]}]}", + serialized); + } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/os-version-status-legacy-format.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/os-version-status-legacy-format.json new file mode 100644 index 00000000000..5a6a864cbf8 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/os-version-status-legacy-format.json @@ -0,0 +1,22 @@ +{ + "versions": [ + { + "version": "7.42", + "cloud": "yahoo", + "nodes": [ + { + "hostname": "node1", + "version": "7.42", + "region": "us-north-1", + "environment": "prod" + }, + { + "hostname": "node2", + "version": "7.42", + "region": "us-north-2", + "environment": "test" + } + ] + } + ] +} -- cgit v1.2.3 From 374b324b2f5fdbcc99761e064bb4a182e0f6aa07 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Mon, 7 Oct 2019 13:02:27 +0200 Subject: Report metric for nodes failing OS upgrade --- .../controller/maintenance/MetricsReporter.java | 28 +++++++++-- .../controller/integration/ConfigServerMock.java | 35 ++++++++++++-- .../maintenance/MetricsReporterTest.java | 56 ++++++++++++++++++++++ 3 files changed, 113 insertions(+), 6 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java index 9253e249765..361cc43da50 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java @@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobList; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.rotation.RotationLock; +import com.yahoo.vespa.hosted.controller.versions.NodeVersions; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import java.time.Clock; @@ -20,6 +21,7 @@ import java.time.Instant; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.function.Function; import java.util.stream.Collectors; /** @@ -36,6 +38,7 @@ public class MetricsReporter extends Maintainer { public static final String DEPLOYMENT_BUILD_AGE_SECONDS = "deployment.buildAgeSeconds"; public static final String DEPLOYMENT_WARNINGS = "deployment.warnings"; public static final String NODES_FAILING_SYSTEM_UPGRADE = "deployment.nodesFailingSystemUpgrade"; + public static final String NODES_FAILING_OS_UPGRADE = "deployment.nodesFailingOsUpgrade"; public static final String REMAINING_ROTATIONS = "remaining_rotations"; public static final String NAME_SERVICE_REQUESTS_QUEUED = "dns.queuedRequests"; @@ -56,6 +59,7 @@ public class MetricsReporter extends Maintainer { reportRemainingRotations(); reportQueuedNameServiceRequests(); reportNodesFailingSystemUpgrade(); + reportNodesFailingOsUpgrade(); } private void reportRemainingRotations() { @@ -103,13 +107,31 @@ public class MetricsReporter extends Maintainer { metric.set(NODES_FAILING_SYSTEM_UPGRADE, nodesFailingSystemUpgrade(), metric.createContext(Map.of())); } + private void reportNodesFailingOsUpgrade() { + metric.set(NODES_FAILING_OS_UPGRADE, nodesFailingOsUpgrade(), metric.createContext(Map.of())); + } + private int nodesFailingSystemUpgrade() { if (!controller().versionStatus().isUpgrading()) return 0; + return nodesFailingUpgrade(controller().versionStatus().versions(), (vespaVersion) -> { + if (vespaVersion.confidence() == VespaVersion.Confidence.broken) return NodeVersions.EMPTY; + return vespaVersion.nodeVersions(); + }); + } + + private int nodesFailingOsUpgrade() { + return nodesFailingUpgrade(controller().osVersionStatus().versions().entrySet(), (kv) -> { + var osVersion = kv.getKey(); + if (osVersion.version().isEmpty()) return NodeVersions.EMPTY; + return kv.getValue(); + }); + } + + private int nodesFailingUpgrade(Collection collection, Function nodeVersionsFunction) { var nodesFailingUpgrade = 0; var acceptableInstant = clock.instant().minus(NODE_UPGRADE_TIMEOUT); - for (var vespaVersion : controller().versionStatus().versions()) { - if (vespaVersion.confidence() == VespaVersion.Confidence.broken) continue; - for (var nodeVersion : vespaVersion.nodeVersions().asMap().values()) { + for (var object : collection) { + for (var nodeVersion : nodeVersionsFunction.apply(object).asMap().values()) { if (!nodeVersion.changing()) continue; if (nodeVersion.changedAt().isBefore(acceptableInstant)) nodesFailingUpgrade++; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index 6da77a967f1..6e7a50b5f81 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -151,15 +151,44 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer /** Set version for an application in a given zone */ public void setVersion(ApplicationId application, ZoneId zone, Version version) { - setVersion(application, zone, version, -1); + setVersion(application, zone, version, -1, false); } /** Set version for nodeCount number of nodes in application in a given zone */ public void setVersion(ApplicationId application, ZoneId zone, Version version, int nodeCount) { + setVersion(application, zone, version, nodeCount, false); + } + + /** Set OS version for an application in a given zone */ + public void setOsVersion(ApplicationId application, ZoneId zone, Version version) { + setOsVersion(application, zone, version, -1); + } + + /** Set OS version for an application in a given zone */ + public void setOsVersion(ApplicationId application, ZoneId zone, Version version, int nodeCount) { + setVersion(application, zone, version, nodeCount, true); + } + + private void setVersion(ApplicationId application, ZoneId zone, Version version, int nodeCount, boolean osVersion) { int n = 0; for (Node node : nodeRepository().list(zone, application)) { - nodeRepository().putByHostname(zone, new Node(node.hostname(), node.state(), node.type(), node.owner(), - version, version)); + Node newNode; + if (osVersion) { + newNode = new Node(node.hostname(), node.state(), node.type(), node.owner(), node.currentVersion(), + node.wantedVersion(), version, version, node.serviceState(), + node.restartGeneration(), node.wantedRestartGeneration(), node.rebootGeneration(), + node.wantedRebootGeneration(), node.vcpu(), node.memoryGb(), node.diskGb(), + node.bandwidthGbps(), node.fastDisk(), node.cost(), node.canonicalFlavor(), + node.clusterId(), node.clusterType()); + } else { + newNode = new Node(node.hostname(), node.state(), node.type(), node.owner(), version, + version, node.currentOsVersion(), node.wantedOsVersion(), node.serviceState(), + node.restartGeneration(), node.wantedRestartGeneration(), node.rebootGeneration(), + node.wantedRebootGeneration(), node.vcpu(), node.memoryGb(), node.diskGb(), + node.bandwidthGbps(), node.fastDisk(), node.cost(), node.canonicalFlavor(), + node.clusterId(), node.clusterType()); + } + nodeRepository().putByHostname(zone, newNode); if (++n == nodeCount) break; } } 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 9cb40d60677..44785407874 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.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.zone.UpgradePolicy; import com.yahoo.config.provision.zone.ZoneId; @@ -262,6 +263,57 @@ public class MetricsReporterTest { } } + @Test + public void test_nodes_failing_os_upgrade() { + var tester = new DeploymentTester(); + var reporter = createReporter(tester.controller()); + var zone = ZoneApiMock.fromId("prod.eu-west-1"); + var cloud = CloudName.defaultName(); + tester.controllerTester().zoneRegistry().setOsUpgradePolicy(cloud, UpgradePolicy.create().upgrade(zone)); + var osUpgrader = new OsUpgrader(tester.controller(), Duration.ofDays(1), + new JobControl(tester.controllerTester().curator()), CloudName.defaultName());; + var statusUpdater = new OsVersionStatusUpdater(tester.controller(), Duration.ofDays(1), + new JobControl(tester.controller().curator())); + tester.configServer().bootstrap(List.of(zone.getId()), SystemApplication.tenantHost); + + // All nodes upgrade to initial OS version + var version0 = Version.fromString("8.0"); + tester.controller().upgradeOsIn(cloud, version0, false); + osUpgrader.maintain(); + tester.configServer().setOsVersion(SystemApplication.tenantHost.id(), zone.getId(), version0); + statusUpdater.maintain(); + reporter.maintain(); + assertEquals(0, getNodesFailingOsUpgrade()); + + for (var version : List.of(Version.fromString("8.1"), Version.fromString("8.2"))) { + // System starts upgrading to next OS version + tester.controller().upgradeOsIn(cloud, version, false); + osUpgrader.maintain(); + statusUpdater.maintain(); + reporter.maintain(); + assertEquals(0, getNodesFailingOsUpgrade()); + + // 30 minutes pass and nothing happens + tester.clock().advance(Duration.ofMinutes(30)); + statusUpdater.maintain(); + reporter.maintain(); + assertEquals(0, getNodesFailingOsUpgrade()); + + // 1/3 nodes upgrade within timeout + tester.configServer().setOsVersion(SystemApplication.tenantHost.id(), zone.getId(), version, 1); + tester.clock().advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1))); + statusUpdater.maintain(); + reporter.maintain(); + assertEquals(2, getNodesFailingOsUpgrade()); + + // 3/3 nodes upgrade + tester.configServer().setOsVersion(SystemApplication.tenantHost.id(), zone.getId(), version); + statusUpdater.maintain(); + reporter.maintain(); + assertEquals(0, getNodesFailingOsUpgrade()); + } + } + private Duration getAverageDeploymentDuration(ApplicationId id) { return Duration.ofSeconds(getMetric(MetricsReporter.DEPLOYMENT_AVERAGE_DURATION, id).longValue()); } @@ -278,6 +330,10 @@ public class MetricsReporterTest { return metrics.getMetric(MetricsReporter.NODES_FAILING_SYSTEM_UPGRADE).intValue(); } + private int getNodesFailingOsUpgrade() { + return metrics.getMetric(MetricsReporter.NODES_FAILING_OS_UPGRADE).intValue(); + } + private Number getMetric(String name, ApplicationId id) { return metrics.getMetric((dimensions) -> id.tenant().value().equals(dimensions.get("tenant")) && appDimension(id).equals(dimensions.get("app")), -- cgit v1.2.3 From 0397a6555b392a69a5b03639e25eabe036146796 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Mon, 7 Oct 2019 12:47:45 +0200 Subject: Remove unused method --- .../hosted/controller/persistence/VersionStatusSerializer.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java index 4373c8977de..366e2c9af4b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.HostName; import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; @@ -17,10 +16,8 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; -import java.util.Set; /** * Serializer for {@link VersionStatus}. @@ -125,12 +122,6 @@ public class VersionStatusSerializer { ); } - private Set configServersFromSlime(Inspector array) { - Set configServerHostnames = new LinkedHashSet<>(); - array.traverse((ArrayTraverser) (i, entry) -> configServerHostnames.add(HostName.from(entry.asString()))); - return Collections.unmodifiableSet(configServerHostnames); - } - private DeploymentStatistics deploymentStatisticsFromSlime(Inspector object) { return new DeploymentStatistics(Version.fromString(object.field(versionField).asString()), applicationsFromSlime(object.field(failingField)), -- cgit v1.2.3 From e05f10c451a6f28d132b57aac624b81ff2889520 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Mon, 7 Oct 2019 14:34:43 +0200 Subject: Update equals/hashCode --- .../java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java | 5 +++-- .../hosted/controller/persistence/OsVersionStatusSerializerTest.java | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java index 5e463e04322..8d0232afa58 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/NodeVersion.java @@ -75,7 +75,7 @@ public class NodeVersion { @Override public String toString() { - return hostname + ": " + currentVersion + " -> " + wantedVersion + " [changedAt=" + changedAt + "]"; + return hostname + ": " + currentVersion + " -> " + wantedVersion + " [zone=" + zone + ", changedAt=" + changedAt + "]"; } @Override @@ -84,6 +84,7 @@ public class NodeVersion { if (o == null || getClass() != o.getClass()) return false; NodeVersion that = (NodeVersion) o; return hostname.equals(that.hostname) && + zone.equals(that.zone) && currentVersion.equals(that.currentVersion) && wantedVersion.equals(that.wantedVersion) && changedAt.equals(that.changedAt); @@ -91,7 +92,7 @@ public class NodeVersion { @Override public int hashCode() { - return Objects.hash(hostname, currentVersion, wantedVersion, changedAt); + return Objects.hash(hostname, zone, currentVersion, wantedVersion, changedAt); } public static NodeVersion empty(HostName hostname) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java index c60137e47b4..ba771d70d26 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OsVersionStatusSerializerTest.java @@ -55,7 +55,7 @@ public class OsVersionStatusSerializerTest { new OsVersion(Version.fromString("7.42"), CloudName.from("yahoo")), NodeVersions.EMPTY.with(List.of(new NodeVersion(HostName.from("node1"), ZoneId.from("prod", "us-north-1"), Version.fromString("7.42"), Version.emptyVersion, Instant.EPOCH), - new NodeVersion(HostName.from("node2"), ZoneId.from("prod", "us-north-2"), + new NodeVersion(HostName.from("node2"), ZoneId.from("test", "us-north-2"), Version.fromString("7.42"), Version.emptyVersion, Instant.EPOCH)))); var deserialized = serializer.fromSlime(SlimeUtils.jsonToSlime(data)); @@ -63,7 +63,7 @@ public class OsVersionStatusSerializerTest { var serialized = new String(SlimeUtils.toJsonBytes(serializer.toSlime(new OsVersionStatus(versions))), StandardCharsets.UTF_8); - assertEquals("{\"versions\":[{\"version\":\"7.42.0\",\"cloud\":\"yahoo\",\"nodeVersions\":[{\"hostname\":\"node1\",\"zone\":\"prod.us-north-1\",\"wantedVersion\":\"0.0.0\",\"changedAt\":0},{\"hostname\":\"node2\",\"zone\":\"prod.us-north-2\",\"wantedVersion\":\"0.0.0\",\"changedAt\":0}],\"nodes\":[{\"hostname\":\"node1\",\"version\":\"7.42.0\",\"region\":\"us-north-1\",\"environment\":\"prod\"},{\"hostname\":\"node2\",\"version\":\"7.42.0\",\"region\":\"us-north-2\",\"environment\":\"prod\"}]}]}", + assertEquals("{\"versions\":[{\"version\":\"7.42.0\",\"cloud\":\"yahoo\",\"nodeVersions\":[{\"hostname\":\"node1\",\"zone\":\"prod.us-north-1\",\"wantedVersion\":\"0.0.0\",\"changedAt\":0},{\"hostname\":\"node2\",\"zone\":\"test.us-north-2\",\"wantedVersion\":\"0.0.0\",\"changedAt\":0}],\"nodes\":[{\"hostname\":\"node1\",\"version\":\"7.42.0\",\"region\":\"us-north-1\",\"environment\":\"prod\"},{\"hostname\":\"node2\",\"version\":\"7.42.0\",\"region\":\"us-north-2\",\"environment\":\"test\"}]}]}", serialized); } -- cgit v1.2.3