From b943b03af2230a9c587ab1bd4bcd76d719bccd9d Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Wed, 6 Jan 2021 10:30:28 +0100 Subject: Revert "Make clients use orchestratorStatus instead of allowedToBeDown" --- .../api/integration/configserver/Node.java | 4 +-- .../integration/configserver/NodeRepository.java | 16 ++++-------- .../noderepository/NodeRepositoryNode.java | 18 ++++--------- .../noderepository/OrchestratorStatus.java | 6 ----- .../configserver/noderepository/NodeSpec.java | 30 +++++++++++----------- .../noderepository/OrchestratorStatus.java | 23 ----------------- .../noderepository/RealNodeRepository.java | 2 +- .../bindings/NodeRepositoryNode.java | 6 ++--- .../maintenance/coredump/CoredumpHandler.java | 3 ++- .../hosted/node/admin/nodeagent/NodeAgentImpl.java | 2 +- .../node/admin/nodeagent/NodeAgentImplTest.java | 9 +++---- .../hosted/provision/restapi/NodesResponse.java | 2 +- 12 files changed, 38 insertions(+), 83 deletions(-) delete mode 100644 controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/OrchestratorStatus.java delete mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/OrchestratorStatus.java diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java index b9a81ba8a02..04f1448bec1 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/Node.java @@ -263,9 +263,7 @@ public class Node { public enum ServiceState { expectedUp, allowedDown, - permanentlyDown, - unorchestrated, - unknown + unorchestrated } /** Known cluster types. */ 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 76055e4ddf2..7bbf0fb0578 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 @@ -13,7 +13,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeList import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeMembership; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryNode; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeState; -import com.yahoo.vespa.hosted.controller.api.integration.noderepository.OrchestratorStatus; import java.time.Duration; import java.time.Instant; @@ -119,7 +118,7 @@ public interface NodeRepository { versionFrom(node.getWantedOsVersion()), Optional.ofNullable(node.getCurrentFirmwareCheck()).map(Instant::ofEpochMilli), Optional.ofNullable(node.getWantedFirmwareCheck()).map(Instant::ofEpochMilli), - toServiceState(node.getOrchestratorStatus()), + fromBoolean(node.getAllowedToBeDown()), Optional.ofNullable(node.suspendedSinceMillis()).map(Instant::ofEpochMilli), toInt(node.getCurrentRestartGeneration()), toInt(node.getRestartGeneration()), @@ -205,15 +204,10 @@ public interface NodeRepository { } } - private static Node.ServiceState toServiceState(OrchestratorStatus orchestratorStatus) { - switch (orchestratorStatus) { - case ALLOWED_TO_BE_DOWN: return Node.ServiceState.allowedDown; - case PERMANENTLY_DOWN: return Node.ServiceState.permanentlyDown; - case NO_REMARKS: return Node.ServiceState.expectedUp; - case OTHER: return Node.ServiceState.unknown; - } - - return Node.ServiceState.unknown; + private static Node.ServiceState fromBoolean(Boolean allowedDown) { + return (allowedDown == null) + ? Node.ServiceState.unorchestrated + : allowedDown ? Node.ServiceState.allowedDown : Node.ServiceState.expectedUp; } private static double toDouble(Double d) { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java index a63058572f9..0533d30b584 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java @@ -11,7 +11,6 @@ import com.fasterxml.jackson.databind.JsonNode; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Stream; @JsonIgnoreProperties(ignoreUnknown = true) @JsonInclude(JsonInclude.Include.NON_NULL) @@ -83,8 +82,8 @@ public class NodeRepositoryNode { private Integer cost; @JsonProperty("history") private List history; - @JsonProperty("orchestratorStatus") - private String orchestratorStatus; + @JsonProperty("allowedToBeDown") + private Boolean allowedToBeDown; @JsonProperty("suspendedSinceMillis") private Long suspendedSinceMillis; @JsonProperty("reports") @@ -328,15 +327,8 @@ public class NodeRepositoryNode { this.history = history; } - public OrchestratorStatus getOrchestratorStatus() { - if (orchestratorStatus == null) { - return OrchestratorStatus.NO_REMARKS; - } - - return Stream.of(OrchestratorStatus.values()) - .filter(status -> status.name().equalsIgnoreCase(orchestratorStatus)) - .findAny() - .orElse(OrchestratorStatus.OTHER); + public Boolean getAllowedToBeDown() { + return allowedToBeDown; } public Long suspendedSinceMillis() { @@ -449,7 +441,7 @@ public class NodeRepositoryNode { ", wantToDeprovision=" + wantToDeprovision + ", cost=" + cost + ", history=" + history + - ", orchestratorStatus=" + orchestratorStatus + + ", allowedToBeDown=" + allowedToBeDown + ", reports=" + reports + ", modelName=" + modelName + ", reservedTo=" + reservedTo + diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/OrchestratorStatus.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/OrchestratorStatus.java deleted file mode 100644 index 51942b56b3d..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/OrchestratorStatus.java +++ /dev/null @@ -1,6 +0,0 @@ -// 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.api.integration.noderepository; - -public enum OrchestratorStatus { - NO_REMARKS, ALLOWED_TO_BE_DOWN, PERMANENTLY_DOWN, OTHER -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java index 9d12efbab1b..f7f231d5e0c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java @@ -48,7 +48,7 @@ public class NodeSpec { private final Optional modelName; - private final OrchestratorStatus orchestratorStatus; + private final Optional allowedToBeDown; private final Optional owner; private final Optional membership; @@ -71,7 +71,7 @@ public class NodeSpec { Optional currentVespaVersion, Optional wantedOsVersion, Optional currentOsVersion, - OrchestratorStatus orchestratorStatus, + Optional allowedToBeDown, Optional owner, Optional membership, Optional wantedRestartGeneration, @@ -106,7 +106,7 @@ public class NodeSpec { this.currentVespaVersion = Objects.requireNonNull(currentVespaVersion); this.wantedOsVersion = Objects.requireNonNull(wantedOsVersion); this.currentOsVersion = Objects.requireNonNull(currentOsVersion); - this.orchestratorStatus = Objects.requireNonNull(orchestratorStatus); + this.allowedToBeDown = Objects.requireNonNull(allowedToBeDown); this.owner = Objects.requireNonNull(owner); this.membership = Objects.requireNonNull(membership); this.wantedRestartGeneration = wantedRestartGeneration; @@ -190,8 +190,8 @@ public class NodeSpec { return modelName; } - public OrchestratorStatus orchestratorStatus() { - return orchestratorStatus; + public Optional allowedToBeDown() { + return allowedToBeDown; } public Optional owner() { @@ -261,7 +261,7 @@ public class NodeSpec { Objects.equals(currentVespaVersion, that.currentVespaVersion) && Objects.equals(wantedOsVersion, that.wantedOsVersion) && Objects.equals(currentOsVersion, that.currentOsVersion) && - Objects.equals(orchestratorStatus, that.orchestratorStatus) && + Objects.equals(allowedToBeDown, that.allowedToBeDown) && Objects.equals(owner, that.owner) && Objects.equals(membership, that.membership) && Objects.equals(wantedRestartGeneration, that.wantedRestartGeneration) && @@ -290,7 +290,7 @@ public class NodeSpec { currentVespaVersion, wantedOsVersion, currentOsVersion, - orchestratorStatus, + allowedToBeDown, owner, membership, wantedRestartGeneration, @@ -319,7 +319,7 @@ public class NodeSpec { + " currentVespaVersion=" + currentVespaVersion + " wantedOsVersion=" + wantedOsVersion + " currentOsVersion=" + currentOsVersion - + " allowedToBeDown=" + orchestratorStatus + + " allowedToBeDown=" + allowedToBeDown + " owner=" + owner + " membership=" + membership + " wantedRestartGeneration=" + wantedRestartGeneration @@ -347,7 +347,7 @@ public class NodeSpec { private Optional currentVespaVersion = Optional.empty(); private Optional wantedOsVersion = Optional.empty(); private Optional currentOsVersion = Optional.empty(); - private OrchestratorStatus orchestratorStatus = OrchestratorStatus.NO_REMARKS; + private Optional allowedToBeDown = Optional.empty(); private Optional owner = Optional.empty(); private Optional membership = Optional.empty(); private Optional wantedRestartGeneration = Optional.empty(); @@ -375,7 +375,6 @@ public class NodeSpec { additionalIpAddresses(node.additionalIpAddresses); wantedRebootGeneration(node.wantedRebootGeneration); currentRebootGeneration(node.currentRebootGeneration); - orchestratorStatus(node.orchestratorStatus); reports(new NodeReports(node.reports)); node.wantedDockerImage.ifPresent(this::wantedDockerImage); node.currentDockerImage.ifPresent(this::currentDockerImage); @@ -383,6 +382,7 @@ public class NodeSpec { node.currentVespaVersion.ifPresent(this::currentVespaVersion); node.wantedOsVersion.ifPresent(this::wantedOsVersion); node.currentOsVersion.ifPresent(this::currentOsVersion); + node.allowedToBeDown.ifPresent(this::allowedToBeDown); node.owner.ifPresent(this::owner); node.membership.ifPresent(this::membership); node.wantedRestartGeneration.ifPresent(this::wantedRestartGeneration); @@ -442,8 +442,8 @@ public class NodeSpec { return this; } - public Builder orchestratorStatus(OrchestratorStatus orchestratorStatus) { - this.orchestratorStatus = orchestratorStatus; + public Builder allowedToBeDown(boolean allowedToBeDown) { + this.allowedToBeDown = Optional.of(allowedToBeDown); return this; } @@ -592,8 +592,8 @@ public class NodeSpec { return currentOsVersion; } - public OrchestratorStatus orchestratorStatus() { - return orchestratorStatus; + public Optional allowedToBeDown() { + return allowedToBeDown; } public Optional owner() { @@ -642,7 +642,7 @@ public class NodeSpec { public NodeSpec build() { return new NodeSpec(hostname, wantedDockerImage, currentDockerImage, state, type, flavor, - wantedVespaVersion, currentVespaVersion, wantedOsVersion, currentOsVersion, orchestratorStatus, + wantedVespaVersion, currentVespaVersion, wantedOsVersion, currentOsVersion, allowedToBeDown, owner, membership, wantedRestartGeneration, currentRestartGeneration, wantedRebootGeneration, currentRebootGeneration, diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/OrchestratorStatus.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/OrchestratorStatus.java deleted file mode 100644 index 689882d61c5..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/OrchestratorStatus.java +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; - -import java.util.stream.Stream; - -public enum OrchestratorStatus { - NO_REMARKS, ALLOWED_TO_BE_DOWN, PERMANENTLY_DOWN, UNKNOWN; - - public static OrchestratorStatus fromString(String statusString) { - return Stream.of(values()) - .filter(status -> status.asString().equals(statusString)) - .findFirst() - .orElse(UNKNOWN); - } - - public String asString() { - return name(); - } - - public boolean isSuspended() { - return this != NO_REMARKS; - } -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index 9ad4d3c565d..2e47410f1b5 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -161,7 +161,7 @@ public class RealNodeRepository implements NodeRepository { Optional.ofNullable(node.vespaVersion).map(Version::fromString), Optional.ofNullable(node.wantedOsVersion).map(Version::fromString), Optional.ofNullable(node.currentOsVersion).map(Version::fromString), - Optional.ofNullable(node.orchestratorStatus).map(OrchestratorStatus::fromString).orElse(OrchestratorStatus.NO_REMARKS), + Optional.ofNullable(node.allowedToBeDown), Optional.ofNullable(node.owner).map(o -> ApplicationId.from(o.tenant, o.application, o.instance)), membership, Optional.ofNullable(node.restartGeneration), diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java index ff9c502a60f..932c1384e7e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java @@ -74,8 +74,8 @@ public class NodeRepositoryNode { public Boolean wantToRetire; @JsonProperty("wantToDeprovision") public Boolean wantToDeprovision; - @JsonProperty("orchestratorStatus") - public String orchestratorStatus; + @JsonProperty("allowedToBeDown") + public Boolean allowedToBeDown; @JsonProperty("reports") public Map reports = null; @@ -112,7 +112,7 @@ public class NodeRepositoryNode { ", parentHostname='" + parentHostname + '\'' + ", wantToRetire=" + wantToRetire + ", wantToDeprovision=" + wantToDeprovision + - ", orchestratorStatus=" + orchestratorStatus + + ", allowedToBeDown=" + allowedToBeDown + ", reports=" + reports + '}'; } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java index f1a0ecdb1a3..5d0c7c19b5d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java @@ -244,7 +244,8 @@ public class CoredumpHandler { ); node.parentHostname().ifPresent(parent -> dimensionsBuilder.add("parentHostname", parent)); - dimensionsBuilder.add("orchestratorState", node.orchestratorStatus().asString()); + node.allowedToBeDown().ifPresent(allowed -> + dimensionsBuilder.add("orchestratorState", allowed ? "ALLOWED_TO_BE_DOWN" : "NO_REMARKS")); node.currentVespaVersion().ifPresent(vespaVersion -> dimensionsBuilder.add("vespaVersion", vespaVersion.toFullString())); return dimensionsBuilder.build(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 9fa21e5a676..ae118e6d541 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -493,7 +493,7 @@ public class NodeAgentImpl implements NodeAgent { // - Slobrok and internal orchestrator state is used to determine whether // to allow upgrade (suspend). updateNodeRepoWithCurrentAttributes(context); - if (suspendedInOrchestrator || node.orchestratorStatus().isSuspended()) { + if (suspendedInOrchestrator || node.allowedToBeDown().orElse(false)) { context.log(logger, "Call resume against Orchestrator"); orchestrator.resume(context.hostname().value()); suspendedInOrchestrator = false; diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index eea775f9a63..fd587da5d71 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -18,7 +18,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttribu import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState; -import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.OrchestratorStatus; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorException; import com.yahoo.vespa.hosted.node.admin.docker.ContainerOperations; @@ -74,7 +73,7 @@ public class NodeAgentImplTest { final NodeSpec node = nodeBuilder(NodeState.active) .wantedDockerImage(dockerImage).currentDockerImage(dockerImage) .wantedVespaVersion(vespaVersion).currentVespaVersion(vespaVersion) - .orchestratorStatus(OrchestratorStatus.NO_REMARKS) + .allowedToBeDown(false) .build(); NodeAgentContext context = createContext(node); @@ -204,7 +203,7 @@ public class NodeAgentImplTest { NodeSpec.Builder specBuilder = nodeBuilder(NodeState.active) .wantedDockerImage(dockerImage).currentDockerImage(dockerImage) .wantedVespaVersion(vespaVersion).currentVespaVersion(vespaVersion) - .orchestratorStatus(OrchestratorStatus.NO_REMARKS); + .allowedToBeDown(false); NodeAgentContext firstContext = createContext(specBuilder.build()); NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true); @@ -502,7 +501,7 @@ public class NodeAgentImplTest { .wantedDockerImage(dockerImage).currentDockerImage(dockerImage) .currentVespaVersion(vespaVersion) .wantedRestartGeneration(1).currentRestartGeneration(1) - .orchestratorStatus(OrchestratorStatus.ALLOWED_TO_BE_DOWN) + .allowedToBeDown(true) .build(); NodeAgentContext context = createContext(node); @@ -573,7 +572,7 @@ public class NodeAgentImplTest { .type(NodeType.config) .wantedDockerImage(dockerImage) .wantedVespaVersion(vespaVersion) - .orchestratorStatus(OrchestratorStatus.ALLOWED_TO_BE_DOWN) + .allowedToBeDown(true) .build(); NodeAgentContext context = createContext(node); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java index e12952062b8..3a9246379cf 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesResponse.java @@ -166,7 +166,7 @@ class NodesResponse extends HttpResponse { orchestrator.apply(new HostName(node.hostname())) .ifPresent(info -> { object.setBool("allowedToBeDown", info.status().isSuspended()); - // TODO: Remove allowedToBeDown as a special-case of orchestratorStatus + // TODO: Remove allowedToBeDown as a special-case of orchestratorHostStatus if (info.status() != HostStatus.NO_REMARKS) { object.setString("orchestratorStatus", info.status().asString()); } -- cgit v1.2.3