Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

IGNITE-23054 Improve cluster status REST endpoint #4614

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

IGNITE-23054 Improve cluster status REST endpoint #4614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e40a9e8
IGNITE-23054 Improve cluster status REST endpoint
Pochatkin Oct 22, 2024
312daa8
Fix timeout
Pochatkin Oct 22, 2024
7fd3564
Fix tests
Pochatkin Oct 22, 2024
909cc35
Fix comments and test
Pochatkin Nov 1, 2024
124a6f7
Fix comments
Pochatkin Nov 20, 2024
35eaa84
Merge branch 'main' into IGNITE-23054
Pochatkin Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 52 additions & 11 deletions ...he/ignite/internal/cli/commands/cluster/status/ItClusterStatusCommandInitializedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,37 +17,78 @@

package org.apache.ignite.internal.cli.commands.cluster.status;

import static java.util.function.Function.identity;
import static java.util.stream.Collectors.joining;
import static org.junit.jupiter.api.Assertions.assertAll;

import java.util.Arrays;
import org.apache.ignite.Ignite;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import org.apache.ignite.internal.cli.CliIntegrationTest;
import org.jetbrains.annotations.Nullable;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

/**
* Tests for {@link ClusterStatusCommand} for the cluster that is initialized.
*/
class ItClusterStatusCommandInitializedTest extends CliIntegrationTest {
private Function<int[], String> mapper;

@Override
protected @Nullable int[] metastoreNodes() {
return new int[] { 0 };
}

@Override
protected @Nullable int[] cmgNodes() {
return new int[] { 1 };
}

@Test
@DisplayName("Should print status when valid cluster url is given but cluster is initialized")
void printStatus() {
String cmgNodes = Arrays.stream(cmgMetastoreNodes())
.mapToObj(CLUSTER::node)
.map(Ignite::name)
void printStatus() throws InterruptedException {
String node0Url = NODE_URL;
String node1Url = "http://localhost:" + CLUSTER.httpPort(1);

Map<Integer, String> nodeNames = IntStream.range(0, initialNodes())
.boxed()
.collect(Collectors.toMap(identity(), i -> CLUSTER.node(i).name()));

mapper = nodes -> Arrays.stream(nodes)
.mapToObj(nodeNames::get)
.collect(joining(", ", "[", "]"));

execute("cluster", "status", "--url", NODE_URL);
CLUSTER.stopNode(0);
execute("cluster", "status", "--url", node1Url);
assertOutput("cluster", 2, "Metastore majority lost", cmgNodes(), metastoreNodes());

CLUSTER.startNode(0);
execute("cluster", "status", "--url", node1Url);
assertOutput("cluster", 3, "active", cmgNodes(), metastoreNodes());

CLUSTER.stopNode(1);
execute("cluster", "status", "--url", node0Url);
assertOutput("N/A", 2, "CMG majority lost", new int[0], new int[0]);
}

private void assertOutput(
String name,
int nodesCount,
String statusStr,
int[] cmgNodes,
int[] metastoreNodes
) {
assertAll(
this::assertExitCodeIsZero,
this::assertErrOutputIsEmpty,
() -> assertOutputContains("name: cluster"),
() -> assertOutputContains("nodes: 3"),
() -> assertOutputContains("status: active"),
() -> assertOutputContains("cmgNodes: " + cmgNodes),
() -> assertOutputContains("msNodes: " + cmgNodes)
() -> assertOutputContains("name: " + name),
() -> assertOutputContains("nodes: " + nodesCount),
() -> assertOutputContains("status: " + statusStr),
() -> assertOutputContains("cmgNodes: " + mapper.apply(cmgNodes)),
() -> assertOutputContains("msNodes: " + mapper.apply(metastoreNodes))
);
}
}
6 changes: 3 additions & 3 deletions ...i/src/integrationTest/java/org/apache/ignite/internal/rest/ItGeneratedRestClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
import org.apache.ignite.rest.client.api.TopologyApi;
import org.apache.ignite.rest.client.invoker.ApiClient;
import org.apache.ignite.rest.client.invoker.ApiException;
import org.apache.ignite.rest.client.model.ClusterState;
import org.apache.ignite.rest.client.model.ClusterStateDto;
import org.apache.ignite.rest.client.model.DeployMode;
import org.apache.ignite.rest.client.model.InitCommand;
import org.apache.ignite.rest.client.model.NodeState;
Expand Down Expand Up @@ -244,11 +244,11 @@ void initCluster() {
@Test
void clusterState() {
assertDoesNotThrow(() -> {
ClusterState clusterState = clusterManagementApi.clusterState();
ClusterStateDto clusterState = clusterManagementApi.clusterState();

assertThat(clusterState, is(notNullValue()));
assertThat(clusterState.getClusterTag().getClusterName(), is(equalTo("cluster")));
assertThat(clusterState.getCmgNodes(), contains(firstNodeName));
assertThat(clusterState.getCmgStatus().getAliveNodes(), contains(firstNodeName));
});
}

Expand Down
81 changes: 50 additions & 31 deletions ...li/call/cluster/status/ClusterStatus.java → ...ll/cluster/status/ClusterStateOutput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@

package org.apache.ignite.internal.cli.call.cluster.status;

import java.util.List;
import org.apache.ignite.rest.client.model.ClusterStateDtoCmgStatus;
import org.apache.ignite.rest.client.model.ClusterStateDtoMetastoreStatus;

/**
* Class that represents the cluster status.
*/
public class ClusterStatus {
public class ClusterStateOutput {

private final int nodeCount;

Expand All @@ -34,19 +35,26 @@ public class ClusterStatus {

private final String nodeUrl;

private final List<String> cmgNodes;
private final ClusterStateDtoCmgStatus cmgStatus;

private final List<String> metadataStorageNodes;
private final ClusterStateDtoMetastoreStatus metastoreStatus;

private ClusterStatus(int nodeCount, boolean initialized, String name,
boolean connected, String nodeUrl, List<String> cmgNodes, List<String> metadataStorageNodes) {
private ClusterStateOutput(
int nodeCount,
boolean initialized,
String name,
boolean connected,
String nodeUrl,
ClusterStateDtoMetastoreStatus metastoreStatus,
ClusterStateDtoCmgStatus cmgStatus
) {
this.nodeCount = nodeCount;
this.initialized = initialized;
this.name = name;
this.connected = connected;
this.nodeUrl = nodeUrl;
this.cmgNodes = cmgNodes;
this.metadataStorageNodes = metadataStorageNodes;
this.cmgStatus = cmgStatus;
this.metastoreStatus = metastoreStatus;
}

public String nodeCount() {
Expand All @@ -69,25 +77,25 @@ public String getNodeUrl() {
return nodeUrl;
}

public List<String> getCmgNodes() {
return cmgNodes;
public ClusterStateDtoCmgStatus getCmgStatus() {
return cmgStatus;
}

public List<String> getMsNodes() {
return metadataStorageNodes;
public ClusterStateDtoMetastoreStatus metastoreStatus() {
return metastoreStatus;
}

/**
* Builder for {@link ClusterStatus}.
* Builder for {@link ClusterStateOutput}.
*/
public static ClusterStatusBuilder builder() {
return new ClusterStatusBuilder();
public static ClusterStateBuilder builder() {
return new ClusterStateBuilder();
}

/**
* Builder for {@link ClusterStatus}.
* Builder for {@link ClusterStateOutput}.
*/
public static class ClusterStatusBuilder {
public static class ClusterStateBuilder {
private int nodeCount;

private boolean initialized;
Expand All @@ -98,51 +106,62 @@ public static class ClusterStatusBuilder {

private String connectedNodeUrl;

private List<String> cmgNodes;
private ClusterStateDtoMetastoreStatus metastoreStatus;

private List<String> metadataStorageNodes;
private ClusterStateDtoCmgStatus cmgStatus;

private ClusterStatusBuilder() {
private ClusterStateBuilder() {

}

public ClusterStatusBuilder nodeCount(int nodeCount) {
public ClusterStateBuilder nodeCount(int nodeCount) {
this.nodeCount = nodeCount;
return this;
}

public ClusterStatusBuilder initialized(boolean initialized) {
public ClusterStateBuilder initialized(boolean initialized) {
this.initialized = initialized;
return this;
}

public ClusterStatusBuilder name(String name) {
public ClusterStateBuilder name(String name) {
this.name = name;
return this;
}

public ClusterStatusBuilder connected(boolean connected) {
public ClusterStateBuilder connected(boolean connected) {
this.connected = connected;
return this;
}

public ClusterStatusBuilder connectedNodeUrl(String connectedNodeUrl) {
public ClusterStateBuilder connectedNodeUrl(String connectedNodeUrl) {
this.connectedNodeUrl = connectedNodeUrl;
return this;
}

public ClusterStatusBuilder cmgNodes(List<String> cmgNodes) {
this.cmgNodes = cmgNodes;
public ClusterStateBuilder metastoreStatus(ClusterStateDtoMetastoreStatus status) {
this.metastoreStatus = status;
return this;
}

public ClusterStatusBuilder metadataStorageNodes(List<String> metadataStorageNodes) {
this.metadataStorageNodes = metadataStorageNodes;
public ClusterStateBuilder cmgStatus(ClusterStateDtoCmgStatus status) {
this.cmgStatus = status;
return this;
}

public ClusterStatus build() {
return new ClusterStatus(nodeCount, initialized, name, connected, connectedNodeUrl, cmgNodes, metadataStorageNodes);
/**
* Returns new cluster state instance.
*/
public ClusterStateOutput build() {
return new ClusterStateOutput(
nodeCount,
initialized,
name,
connected,
connectedNodeUrl,
metastoreStatus,
cmgStatus
);
}
}
}
36 changes: 23 additions & 13 deletions ...i/src/main/java/org/apache/ignite/internal/cli/call/cluster/status/ClusterStatusCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import jakarta.inject.Singleton;
import java.util.List;
import org.apache.ignite.internal.cli.call.cluster.status.ClusterStatus.ClusterStatusBuilder;
import org.apache.ignite.internal.cli.call.cluster.status.ClusterStateOutput.ClusterStateBuilder;
import org.apache.ignite.internal.cli.call.cluster.topology.PhysicalTopologyCall;
import org.apache.ignite.internal.cli.core.call.Call;
import org.apache.ignite.internal.cli.core.call.CallOutput;
Expand All @@ -30,13 +30,21 @@
import org.apache.ignite.rest.client.api.ClusterManagementApi;
import org.apache.ignite.rest.client.invoker.ApiException;
import org.apache.ignite.rest.client.model.ClusterNode;
import org.apache.ignite.rest.client.model.ClusterState;
import org.apache.ignite.rest.client.model.ClusterStateDto;

/**
* Call to get cluster status.
*/
@Singleton
public class ClusterStatusCall implements Call<UrlCallInput, ClusterStatus> {
public class ClusterStatusCall implements Call<UrlCallInput, ClusterStateOutput> {
/**
* We need to overlap timeout from raft client.
* We can't determine timeout value because it's configurable for each node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can read the node configuration here and just add a couple of seconds to the configured timeout?

* And moreover retry counter can be increased.
* See {@link org.apache.ignite.internal.rest.cluster.ClusterManagementController}
* {@link org.apache.ignite.internal.raft.configuration.RaftConfigurationSchema}.
*/
private static final int READ_TIMEOUT = 20_000;

private final PhysicalTopologyCall physicalTopologyCall;

Expand All @@ -48,28 +56,28 @@ public ClusterStatusCall(PhysicalTopologyCall physicalTopologyCall, ApiClientFac
}

@Override
public CallOutput<ClusterStatus> execute(UrlCallInput input) {
ClusterStatusBuilder clusterStatusBuilder = ClusterStatus.builder();
public CallOutput<ClusterStateOutput> execute(UrlCallInput input) {
ClusterStateBuilder clusterStateBuilder = ClusterStateOutput.builder();
String clusterUrl = input.getUrl();
try {
ClusterState clusterState = fetchClusterState(clusterUrl);
clusterStatusBuilder
ClusterStateDto clusterState = fetchClusterState(clusterUrl);
clusterStateBuilder
.nodeCount(fetchNumberOfAllNodes(input))
.initialized(true)
.name(clusterState.getClusterTag().getClusterName())
.metadataStorageNodes(clusterState.getMsNodes())
.cmgNodes(clusterState.getCmgNodes());
.metastoreStatus(clusterState.getMetastoreStatus())
.cmgStatus(clusterState.getCmgStatus());
} catch (ApiException e) {
if (e.getCode() == 409) { // CONFLICT means the cluster is not initialized yet
clusterStatusBuilder.initialized(false).nodeCount(fetchNumberOfAllNodes(input));
clusterStateBuilder.initialized(false).nodeCount(fetchNumberOfAllNodes(input));
} else {
return DefaultCallOutput.failure(new IgniteCliApiException(e, clusterUrl));
}
} catch (IllegalArgumentException e) {
return DefaultCallOutput.failure(new IgniteCliApiException(e, clusterUrl));
}

return DefaultCallOutput.success(clusterStatusBuilder.build());
return DefaultCallOutput.success(clusterStateBuilder.build());
}

private int fetchNumberOfAllNodes(UrlCallInput input) {
Expand All @@ -80,7 +88,9 @@ private int fetchNumberOfAllNodes(UrlCallInput input) {
return body.size();
}

private ClusterState fetchClusterState(String url) throws ApiException {
return new ClusterManagementApi(clientFactory.getClient(url)).clusterState();
private ClusterStateDto fetchClusterState(String url) throws ApiException {
return new ClusterManagementApi(clientFactory.getClient(url)
.setReadTimeout(READ_TIMEOUT))
.clusterState();
}
}
20 changes: 9 additions & 11 deletions .../main/java/org/apache/ignite/internal/cli/call/recovery/cluster/MigrateToClusterCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import org.apache.ignite.rest.client.api.ClusterManagementApi;
import org.apache.ignite.rest.client.api.RecoveryApi;
import org.apache.ignite.rest.client.invoker.ApiException;
import org.apache.ignite.rest.client.model.ClusterState;
import org.apache.ignite.rest.client.model.ClusterStateDto;
import org.apache.ignite.rest.client.model.MigrateRequest;

/** Call to migrate nodes from old to new cluster. */
Expand All @@ -43,24 +43,22 @@ public DefaultCallOutput<String> execute(MigrateToClusterCallInput input) {
ClusterManagementApi newClusterManagementClient = new ClusterManagementApi(clientFactory.getClient(input.newClusterUrl()));
RecoveryApi oldRecoveryClient = new RecoveryApi(clientFactory.getClient(input.oldClusterUrl()));

ClusterState newClusterState;
ClusterStateDto newClusterState;
try {
newClusterState = newClusterManagementClient.clusterState();
} catch (ApiException e) {
return DefaultCallOutput.failure(new IgniteCliApiException(e, input.newClusterUrl()));
}

MigrateRequest command = new MigrateRequest();

command.setCmgNodes(newClusterState.getCmgNodes());
command.setMetaStorageNodes(newClusterState.getMsNodes());
command.setVersion(newClusterState.getIgniteVersion());
command.setClusterId(newClusterState.getClusterTag().getClusterId());
command.setClusterName(newClusterState.getClusterTag().getClusterName());
command.setFormerClusterIds(newClusterState.getFormerClusterIds());
MigrateRequest migrateRequest = new MigrateRequest().cmgNodes(newClusterState.getCmgStatus().getAliveNodes())
.metaStorageNodes(newClusterState.getMetastoreStatus().getAliveNodes())
.version(newClusterState.getIgniteVersion())
.clusterId(newClusterState.getClusterTag().getClusterId())
.clusterName(newClusterState.getClusterTag().getClusterName())
.formerClusterIds(newClusterState.getFormerClusterIds());

try {
oldRecoveryClient.migrate(command);
oldRecoveryClient.migrate(migrateRequest);
} catch (ApiException e) {
if (e.getCause() instanceof IOException) {
return DefaultCallOutput.success("Node has gone, this most probably means that migration is initiated and "
Expand Down
Loading