diff options
author | jonmv <venstad@gmail.com> | 2022-04-29 11:02:46 +0200 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2022-04-29 11:02:46 +0200 |
commit | ca053bdad4a42903f760579e7f0038b8fcac602f (patch) | |
tree | bce6560da6cc47814437b3eb58b4bfeb912e2fab /orchestrator | |
parent | 130958eea280ecfa70f86dd8b3a7fc6ab1b0f85c (diff) |
Differentiate between trying and requiring node state change
Diffstat (limited to 'orchestrator')
6 files changed, 42 insertions, 24 deletions
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java index db18a71c805..5a9d75a9d41 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java @@ -425,7 +425,7 @@ public class OrchestratorImpl implements Orchestrator { ClusterControllerClient client = clusterControllerClientFactory.createClient(clusterControllers, cluster.clusterId().s()); for (ServiceInstance service : cluster.serviceInstances()) { try { - if ( ! client.setNodeState(context, service.hostName(), VespaModelUtil.getStorageNodeIndex(service.configId()), MAINTENANCE)) + if ( ! client.trySetNodeState(context, service.hostName(), VespaModelUtil.getStorageNodeIndex(service.configId()), MAINTENANCE)) return false; } catch (Exception e) { diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java index 2c31b475b21..98ca9f805b4 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java @@ -18,8 +18,16 @@ public interface ClusterControllerClient { * @return false is this was a probe operation, and permission would be denied. * @throws HostStateChangeDeniedException if operation fails, or is otherwise disallowed. */ - boolean setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, - ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException; + boolean trySetNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, + ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException; + + /** + * Requests that a cluster controller sets the requested node to the requested state. + * + * @throws HostStateChangeDeniedException if operation fails, or is disallowed. + */ + void setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, + ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException; /** * Requests that a cluster controller sets all nodes in the cluster to the requested state. diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java index b4966e71bfa..7c7dca3b03e 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java @@ -52,8 +52,8 @@ public class ClusterControllerClientImpl implements ClusterControllerClient { this.client = client; } - @Override - public boolean setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, ClusterControllerNodeState wantedState) { + private boolean setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, + ClusterControllerNodeState wantedState, boolean throwOnFailure) { try { ClusterControllerClientTimeouts timeouts = context.getClusterControllerTimeouts(); Inspector response = client.send(strategy(hosts), Method.POST) @@ -64,13 +64,12 @@ public class ClusterControllerClientImpl implements ClusterControllerClient { .throwing(retryOnRedirect) .read(SlimeUtils::jsonToSlime).get(); if ( ! response.field("wasModified").asBool()) { - if (context.isProbe()) - return false; - - throw new HostStateChangeDeniedException(host, - HostedVespaPolicy.SET_NODE_STATE_CONSTRAINT, - "Failed to set state to " + wantedState + - " in cluster controller: " + response.field("reason").asString()); + if (throwOnFailure) + throw new HostStateChangeDeniedException(host, + HostedVespaPolicy.SET_NODE_STATE_CONSTRAINT, + "Failed to set state to " + wantedState + + " in cluster controller: " + response.field("reason").asString()); + return false; } return true; } @@ -100,6 +99,16 @@ public class ClusterControllerClientImpl implements ClusterControllerClient { } @Override + public boolean trySetNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException { + return setNodeState(context, host, storageNodeIndex, wantedState, false); + } + + @Override + public void setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException { + setNodeState(context, host, storageNodeIndex, wantedState, true); + } + + @Override public void setApplicationState(OrchestratorContext context, ApplicationInstanceId applicationId, ClusterControllerNodeState wantedState) throws ApplicationStateChangeDeniedException { try { diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java index 6e8e1eb0ccb..f5b7771d5f4 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java @@ -459,16 +459,16 @@ public class OrchestratorImplTest { applicationApiFactory, flagSource); - when(fooClient.setNodeState(any(), any(), eq(1), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); - when(fooClient.setNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); - when(barClient.setNodeState(any(), any(), eq(0), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); - when(barClient.setNodeState(any(), any(), eq(3), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); + when(fooClient.trySetNodeState(any(), any(), eq(1), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); + when(fooClient.trySetNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); + when(barClient.trySetNodeState(any(), any(), eq(0), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); + when(barClient.trySetNodeState(any(), any(), eq(3), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); assertTrue(orchestrator.isQuiescent(id)); - when(fooClient.setNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(false); + when(fooClient.trySetNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(false); assertFalse(orchestrator.isQuiescent(id)); - when(fooClient.setNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenThrow(new RuntimeException()); + when(fooClient.trySetNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenThrow(new RuntimeException()); assertFalse(orchestrator.isQuiescent(id)); } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java index 27928618eeb..597487ccfc5 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java @@ -53,13 +53,14 @@ public class ClusterControllerClientFactoryMock implements ClusterControllerClie @Override public ClusterControllerClient createClient(List<HostName> clusterControllers, String clusterName) { return new ClusterControllerClient() { - @Override public boolean setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, - ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException { + @Override public boolean trySetNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, ClusterControllerNodeState wantedState) { nodes.put(clusterName + "/" + storageNodeIndex, wantedState); return true; } - @Override public void setApplicationState(OrchestratorContext context, ApplicationInstanceId applicationId, - ClusterControllerNodeState wantedState) throws ApplicationStateChangeDeniedException { + @Override public void setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, ClusterControllerNodeState wantedState) { + trySetNodeState(context, host, storageNodeIndex, wantedState); + } + @Override public void setApplicationState(OrchestratorContext context, ApplicationInstanceId applicationId, ClusterControllerNodeState wantedState) { nodes.replaceAll((key, state) -> key.startsWith(clusterName + "/") ? wantedState : state); } }; diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java index 7746eff457e..2570a8035bb 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java @@ -66,7 +66,7 @@ public class ClusterControllerClientImplTest { return "{ \"wasModified\": true }"; }, 200); - assertTrue(client.setNodeState(context, host, 2, DOWN)); + client.setNodeState(context, host, 2, DOWN); clock.advance(Duration.ofSeconds(9)); wire.expect((url, body) -> { @@ -93,7 +93,7 @@ public class ClusterControllerClientImplTest { return "{ \"wasModified\": false, \"reason\": \"no reason\" }"; }, 200); - assertFalse(client.setNodeState(OrchestratorContext.createContextForBatchProbe(clock), host, 2, MAINTENANCE)); + assertFalse(client.trySetNodeState(OrchestratorContext.createContextForBatchProbe(clock), host, 2, MAINTENANCE)); } @Test |