You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/02/09 10:48:25 UTC
[GitHub] [ignite-3] rpuch opened a new pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
rpuch opened a new pull request #647:
URL: https://github.com/apache/ignite-3/pull/647
https://issues.apache.org/jira/browse/IGNITE-14871
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803386862
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -581,17 +619,200 @@ void setJson() throws IOException, InterruptedException {
+ "local.baseline.autoAdjust.enabled=true --type node"
).split(" "));
- Assertions.assertEquals(0, exitCode);
- verify(httpClient).send(
- argThat(r -> "http://localhost:8081/management/v1/configuration/node/".equals(r.uri().toString())
- && "PATCH".equals(r.method())
- && r.bodyPublisher().get().contentLength() == expSentContent.getBytes(UTF_8).length
- && "application/json".equals(r.headers().firstValue("Content-Type").get())),
- any());
- assertEquals("Configuration was updated successfully.\n\n"
+ assertThatExitCodeMeansSuccess(exitCode);
+
+ verify(httpClient).send(requestCaptor.capture(), any());
+ HttpRequest capturedRequest = requestCaptor.getValue();
+
+ assertThat(capturedRequest.uri().toString(), is("http://localhost:8081/management/v1/configuration/node/"));
+ assertThat(capturedRequest.method(), is("PATCH"));
+ assertThat(requestBodyBytes(capturedRequest), is(expSentContent.getBytes(UTF_8)));
+ assertThat(capturedRequest.headers().firstValue("Content-Type"), isPresentAnd(is("application/json")));
+
+ assertOutputEqual("Configuration was updated successfully.\n\n"
+ "Use the " + cmd.getColorScheme().commandText("ignite config get")
- + " command to view the updated configuration.\n", out.toString());
+ + " command to view the updated configuration.\n", out.toString(UTF_8));
+ assertThatStderrIsEmpty();
+ }
+ }
+
+ /**
+ * Tests "cluster" command.
+ */
+ @Nested
+ @DisplayName("cluster")
+ class Cluster {
Review comment:
a ticket seems fine
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -581,17 +619,200 @@ void setJson() throws IOException, InterruptedException {
+ "local.baseline.autoAdjust.enabled=true --type node"
).split(" "));
- Assertions.assertEquals(0, exitCode);
- verify(httpClient).send(
- argThat(r -> "http://localhost:8081/management/v1/configuration/node/".equals(r.uri().toString())
- && "PATCH".equals(r.method())
- && r.bodyPublisher().get().contentLength() == expSentContent.getBytes(UTF_8).length
- && "application/json".equals(r.headers().firstValue("Content-Type").get())),
- any());
- assertEquals("Configuration was updated successfully.\n\n"
+ assertThatExitCodeMeansSuccess(exitCode);
+
+ verify(httpClient).send(requestCaptor.capture(), any());
+ HttpRequest capturedRequest = requestCaptor.getValue();
+
+ assertThat(capturedRequest.uri().toString(), is("http://localhost:8081/management/v1/configuration/node/"));
+ assertThat(capturedRequest.method(), is("PATCH"));
+ assertThat(requestBodyBytes(capturedRequest), is(expSentContent.getBytes(UTF_8)));
+ assertThat(capturedRequest.headers().firstValue("Content-Type"), isPresentAnd(is("application/json")));
+
+ assertOutputEqual("Configuration was updated successfully.\n\n"
+ "Use the " + cmd.getColorScheme().commandText("ignite config get")
- + " command to view the updated configuration.\n", out.toString());
+ + " command to view the updated configuration.\n", out.toString(UTF_8));
+ assertThatStderrIsEmpty();
+ }
+ }
+
+ /**
+ * Tests "cluster" command.
+ */
+ @Nested
+ @DisplayName("cluster")
+ class Cluster {
Review comment:
ok!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803377250
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/ClusterApiClient.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.apache.ignite.cli.IgniteCliException;
+
+/**
+ * Client for Ignite Cluster management API.
+ */
+@Singleton
+public class ClusterApiClient {
+ private static final String CLUSTER_INIT_URL = "/management/v1/cluster/init/";
+
+ private final HttpClient httpClient;
+
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ @Inject
+ public ClusterApiClient(HttpClient httpClient) {
+ this.httpClient = httpClient;
+ }
+
+ /**
+ * Sends 'cluster init' command to the specified node.
+ *
+ * @param nodeEndpoint host:port on which REST API is listening
+ * @param metastorageNodeIds consistent IDs of the nodes that will form metastorage RAFT group
+ * @param cmgNodeIds consistent IDs of the nodes that will form Cluster Management RAFT Group; if empty,
+ * then nodes specified in metastorageNodeIds will form Cluster Management RAFT Group as well
+ * @param out {@link PrintWriter} to which to report about the command outcome
+ */
+ public void init(String nodeEndpoint, List<String> metastorageNodeIds, List<String> cmgNodeIds, PrintWriter out) {
Review comment:
Here is how it looks now:
```
java.lang.IllegalArgumentException: Illegal character in authority at index 7: http://some shit/management/v1/cluster/init/
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803364683
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/spec/ClusterCommandSpec.java
##########
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.spec;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.inject.Inject;
+import org.apache.ignite.cli.builtins.cluster.ClusterApiClient;
+import picocli.CommandLine;
+import picocli.CommandLine.Option;
+
+/**
+ * Commands for managing Ignite cluster as a whole.
+ */
+@CommandLine.Command(
+ name = "cluster",
+ description = "Manages an Ignite cluster as a whole.",
+ subcommands = {
+ ClusterCommandSpec.InitClusterCommandSpec.class,
+ }
+)
+public class ClusterCommandSpec extends CategorySpec {
+ /**
+ * Initializes an Ignite cluster.
+ */
+ @CommandLine.Command(name = "init", description = "Initializes an Ignite cluster.")
+ public static class InitClusterCommandSpec extends CommandSpec {
+
+ @Inject
+ private ClusterApiClient clusterApiClient;
+
+ /**
+ * Address of the REST endpoint of the receiving node in host:port format.
+ */
+ @Option(names = "--node-endpoint", required = true,
+ description = "Address of the REST endpoint of the receiving node in host:port format")
+ private String nodeEndpoint;
Review comment:
`NodeEndpointOptions` allows the endpoint to be omitted. In the case of the init command, it seems super-important to make sure that the user knows exactly what endpoint they use, so I made it mandatory, hence `NodeEndpointOptions` cannot be used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803390603
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/ClusterApiClient.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.apache.ignite.cli.IgniteCliException;
+
+/**
+ * Client for Ignite Cluster management API.
+ */
+@Singleton
+public class ClusterApiClient {
+ private static final String CLUSTER_INIT_URL = "/management/v1/cluster/init/";
+
+ private final HttpClient httpClient;
+
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ @Inject
+ public ClusterApiClient(HttpClient httpClient) {
+ this.httpClient = httpClient;
+ }
+
+ /**
+ * Sends 'cluster init' command to the specified node.
+ *
+ * @param nodeEndpoint host:port on which REST API is listening
+ * @param metastorageNodeIds consistent IDs of the nodes that will form metastorage RAFT group
+ * @param cmgNodeIds consistent IDs of the nodes that will form Cluster Management RAFT Group; if empty,
+ * then nodes specified in metastorageNodeIds will form Cluster Management RAFT Group as well
+ * @param out {@link PrintWriter} to which to report about the command outcome
+ */
+ public void init(String nodeEndpoint, List<String> metastorageNodeIds, List<String> cmgNodeIds, PrintWriter out) {
Review comment:
How do other input errors look like? If they look similarly, then I'm fine with the current approach
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803371335
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/ClusterApiClient.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.apache.ignite.cli.IgniteCliException;
+
+/**
+ * Client for Ignite Cluster management API.
+ */
+@Singleton
+public class ClusterApiClient {
+ private static final String CLUSTER_INIT_URL = "/management/v1/cluster/init/";
+
+ private final HttpClient httpClient;
+
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ @Inject
+ public ClusterApiClient(HttpClient httpClient) {
+ this.httpClient = httpClient;
+ }
+
+ /**
+ * Sends 'cluster init' command to the specified node.
+ *
+ * @param nodeEndpoint host:port on which REST API is listening
+ * @param metastorageNodeIds consistent IDs of the nodes that will form metastorage RAFT group
+ * @param cmgNodeIds consistent IDs of the nodes that will form Cluster Management RAFT Group; if empty,
+ * then nodes specified in metastorageNodeIds will form Cluster Management RAFT Group as well
Review comment:
Applied your suggestion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803369085
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -581,17 +619,200 @@ void setJson() throws IOException, InterruptedException {
+ "local.baseline.autoAdjust.enabled=true --type node"
).split(" "));
- Assertions.assertEquals(0, exitCode);
- verify(httpClient).send(
- argThat(r -> "http://localhost:8081/management/v1/configuration/node/".equals(r.uri().toString())
- && "PATCH".equals(r.method())
- && r.bodyPublisher().get().contentLength() == expSentContent.getBytes(UTF_8).length
- && "application/json".equals(r.headers().firstValue("Content-Type").get())),
- any());
- assertEquals("Configuration was updated successfully.\n\n"
+ assertThatExitCodeMeansSuccess(exitCode);
+
+ verify(httpClient).send(requestCaptor.capture(), any());
+ HttpRequest capturedRequest = requestCaptor.getValue();
+
+ assertThat(capturedRequest.uri().toString(), is("http://localhost:8081/management/v1/configuration/node/"));
+ assertThat(capturedRequest.method(), is("PATCH"));
+ assertThat(requestBodyBytes(capturedRequest), is(expSentContent.getBytes(UTF_8)));
+ assertThat(capturedRequest.headers().firstValue("Content-Type"), isPresentAnd(is("application/json")));
+
+ assertOutputEqual("Configuration was updated successfully.\n\n"
+ "Use the " + cmd.getColorScheme().commandText("ignite config get")
- + " command to view the updated configuration.\n", out.toString());
+ + " command to view the updated configuration.\n", out.toString(UTF_8));
+ assertThatStderrIsEmpty();
+ }
+ }
+
+ /**
+ * Tests "cluster" command.
+ */
+ @Nested
+ @DisplayName("cluster")
+ class Cluster {
Review comment:
Yes, it follows the pattern given by its neighbors. I'm not sure whether it is a good idea to change it in this task (see the previous comment, maybe a separate JIRA ticket is an option).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803367968
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -581,17 +619,200 @@ void setJson() throws IOException, InterruptedException {
+ "local.baseline.autoAdjust.enabled=true --type node"
).split(" "));
- Assertions.assertEquals(0, exitCode);
- verify(httpClient).send(
- argThat(r -> "http://localhost:8081/management/v1/configuration/node/".equals(r.uri().toString())
- && "PATCH".equals(r.method())
- && r.bodyPublisher().get().contentLength() == expSentContent.getBytes(UTF_8).length
- && "application/json".equals(r.headers().firstValue("Content-Type").get())),
- any());
- assertEquals("Configuration was updated successfully.\n\n"
+ assertThatExitCodeMeansSuccess(exitCode);
+
+ verify(httpClient).send(requestCaptor.capture(), any());
+ HttpRequest capturedRequest = requestCaptor.getValue();
+
+ assertThat(capturedRequest.uri().toString(), is("http://localhost:8081/management/v1/configuration/node/"));
+ assertThat(capturedRequest.method(), is("PATCH"));
+ assertThat(requestBodyBytes(capturedRequest), is(expSentContent.getBytes(UTF_8)));
+ assertThat(capturedRequest.headers().firstValue("Content-Type"), isPresentAnd(is("application/json")));
+
+ assertOutputEqual("Configuration was updated successfully.\n\n"
+ "Use the " + cmd.getColorScheme().commandText("ignite config get")
- + " command to view the updated configuration.\n", out.toString());
+ + " command to view the updated configuration.\n", out.toString(UTF_8));
+ assertThatStderrIsEmpty();
+ }
+ }
+
+ /**
+ * Tests "cluster" command.
+ */
+ @Nested
+ @DisplayName("cluster")
+ class Cluster {
Review comment:
It uses some infrastructure defined in the containing class, that would probably require to transform all the internal test classes defined here for consistency. Probably a separaate JIRA issue for refactoring tests seems a good option, WDYT?
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -581,17 +619,200 @@ void setJson() throws IOException, InterruptedException {
+ "local.baseline.autoAdjust.enabled=true --type node"
).split(" "));
- Assertions.assertEquals(0, exitCode);
- verify(httpClient).send(
- argThat(r -> "http://localhost:8081/management/v1/configuration/node/".equals(r.uri().toString())
- && "PATCH".equals(r.method())
- && r.bodyPublisher().get().contentLength() == expSentContent.getBytes(UTF_8).length
- && "application/json".equals(r.headers().firstValue("Content-Type").get())),
- any());
- assertEquals("Configuration was updated successfully.\n\n"
+ assertThatExitCodeMeansSuccess(exitCode);
+
+ verify(httpClient).send(requestCaptor.capture(), any());
+ HttpRequest capturedRequest = requestCaptor.getValue();
+
+ assertThat(capturedRequest.uri().toString(), is("http://localhost:8081/management/v1/configuration/node/"));
+ assertThat(capturedRequest.method(), is("PATCH"));
+ assertThat(requestBodyBytes(capturedRequest), is(expSentContent.getBytes(UTF_8)));
+ assertThat(capturedRequest.headers().firstValue("Content-Type"), isPresentAnd(is("application/json")));
+
+ assertOutputEqual("Configuration was updated successfully.\n\n"
+ "Use the " + cmd.getColorScheme().commandText("ignite config get")
- + " command to view the updated configuration.\n", out.toString());
+ + " command to view the updated configuration.\n", out.toString(UTF_8));
+ assertThatStderrIsEmpty();
+ }
+ }
+
+ /**
+ * Tests "cluster" command.
+ */
+ @Nested
+ @DisplayName("cluster")
+ class Cluster {
Review comment:
It uses some infrastructure defined in the containing class, that would probably require to transform all the internal test classes defined here for consistency. Probably a separaate JIRA issue for refactoring tests seems to be a good option, WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
sashapolo commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r802693161
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/ClusterApiClient.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.apache.ignite.cli.IgniteCliException;
+
+/**
+ * Client for Ignite Cluster management API.
+ */
+@Singleton
+public class ClusterApiClient {
+ private static final String CLUSTER_INIT_URL = "/management/v1/cluster/init/";
+
+ private final HttpClient httpClient;
+
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ @Inject
+ public ClusterApiClient(HttpClient httpClient) {
+ this.httpClient = httpClient;
+ }
+
+ /**
+ * Sends 'cluster init' command to the specified node.
+ *
+ * @param nodeEndpoint host:port on which REST API is listening
+ * @param metastorageNodeIds consistent IDs of the nodes that will form metastorage RAFT group
Review comment:
```suggestion
* @param metastorageNodeIds consistent IDs of the nodes that will host the Meta Storage Raft group
```
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -603,10 +824,10 @@ void setJson() throws IOException, InterruptedException {
* @param actual Actual result.
* @see Object#equals(Object)
*/
- private static void assertEquals(String exp, String actual) {
- Assertions.assertEquals(
- exp.lines().collect(Collectors.toList()),
- actual.lines().collect(Collectors.toList())
+ private static void assertOutputEqual(String exp, String actual) {
+ assertEquals(
+ exp.lines().collect(toList()),
Review comment:
why do you need to call `lines` here?
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/ClusterApiClient.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.apache.ignite.cli.IgniteCliException;
+
+/**
+ * Client for Ignite Cluster management API.
+ */
+@Singleton
+public class ClusterApiClient {
+ private static final String CLUSTER_INIT_URL = "/management/v1/cluster/init/";
+
+ private final HttpClient httpClient;
+
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ @Inject
+ public ClusterApiClient(HttpClient httpClient) {
+ this.httpClient = httpClient;
+ }
+
+ /**
+ * Sends 'cluster init' command to the specified node.
+ *
+ * @param nodeEndpoint host:port on which REST API is listening
+ * @param metastorageNodeIds consistent IDs of the nodes that will form metastorage RAFT group
+ * @param cmgNodeIds consistent IDs of the nodes that will form Cluster Management RAFT Group; if empty,
+ * then nodes specified in metastorageNodeIds will form Cluster Management RAFT Group as well
Review comment:
I would suggest to rephrase this part:
```
If omitted, {@code metastorageNodeIds} will be used to host the CMG as well.
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/InitClusterRequest.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.List;
+
+/**
+ * A request to the Cluster Init REST API.
+ */
+public class InitClusterRequest {
+ @SuppressWarnings("unused")
Review comment:
I think you don't need to suppress anything here, you can set up IDEA to ignore fields annotated with `@JsonProperty`
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -581,17 +619,200 @@ void setJson() throws IOException, InterruptedException {
+ "local.baseline.autoAdjust.enabled=true --type node"
).split(" "));
- Assertions.assertEquals(0, exitCode);
- verify(httpClient).send(
- argThat(r -> "http://localhost:8081/management/v1/configuration/node/".equals(r.uri().toString())
- && "PATCH".equals(r.method())
- && r.bodyPublisher().get().contentLength() == expSentContent.getBytes(UTF_8).length
- && "application/json".equals(r.headers().firstValue("Content-Type").get())),
- any());
- assertEquals("Configuration was updated successfully.\n\n"
+ assertThatExitCodeMeansSuccess(exitCode);
+
+ verify(httpClient).send(requestCaptor.capture(), any());
+ HttpRequest capturedRequest = requestCaptor.getValue();
+
+ assertThat(capturedRequest.uri().toString(), is("http://localhost:8081/management/v1/configuration/node/"));
+ assertThat(capturedRequest.method(), is("PATCH"));
+ assertThat(requestBodyBytes(capturedRequest), is(expSentContent.getBytes(UTF_8)));
+ assertThat(capturedRequest.headers().firstValue("Content-Type"), isPresentAnd(is("application/json")));
+
+ assertOutputEqual("Configuration was updated successfully.\n\n"
+ "Use the " + cmd.getColorScheme().commandText("ignite config get")
- + " command to view the updated configuration.\n", out.toString());
+ + " command to view the updated configuration.\n", out.toString(UTF_8));
+ assertThatStderrIsEmpty();
+ }
+ }
+
+ /**
+ * Tests "cluster" command.
+ */
+ @Nested
+ @DisplayName("cluster")
+ class Cluster {
Review comment:
Is it possible to extract this class to an upper level?
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/spec/NodeEndpointOptions.java
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.spec;
+
+import org.apache.ignite.cli.IgniteCliException;
+import picocli.CommandLine;
+
+/**
+ * Prepared picocli mixin for generic node hostname option.
+ */
+class NodeEndpointOptions {
+ /**
+ * Custom node REST endpoint address.
+ */
+ @CommandLine.Option(
+ names = "--node-endpoint",
+ description = "Ignite server node's REST API address and port number",
+ paramLabel = "host:port"
+ )
+ private String endpoint;
+
+ /**
+ * Returns REST endpoint port.
+ *
+ * @return REST endpoint port.
+ */
+ int port() {
+ if (endpoint == null) {
+ return 10300;
+ }
+
+ var hostPort = parse();
+
+ try {
+ return Integer.parseInt(hostPort[1]);
+ } catch (NumberFormatException ex) {
+ throw new IgniteCliException("Can't parse port from " + hostPort[1] + " value");
+ }
+ }
+
+ /**
+ * Returns REST endpoint host.
+ *
+ * @return REST endpoint host.
+ */
+ String host() {
+ return endpoint != null ? parse()[0] : "localhost";
+ }
+
+ /**
+ * Parses REST endpoint host and port from string.
+ *
+ * @return 2-elements array [host, port].
+ */
+ private String[] parse() {
+ var hostPort = endpoint.split(":");
+
+ if (hostPort.length != 2) {
+ throw new IgniteCliException("Incorrect host:port pair provided "
+ + "(example of valid value 'localhost:10300')");
Review comment:
I would suggest to also add the provided value to the message
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/ClusterApiClient.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.apache.ignite.cli.IgniteCliException;
+
+/**
+ * Client for Ignite Cluster management API.
+ */
+@Singleton
+public class ClusterApiClient {
+ private static final String CLUSTER_INIT_URL = "/management/v1/cluster/init/";
+
+ private final HttpClient httpClient;
+
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ @Inject
+ public ClusterApiClient(HttpClient httpClient) {
+ this.httpClient = httpClient;
+ }
+
+ /**
+ * Sends 'cluster init' command to the specified node.
+ *
+ * @param nodeEndpoint host:port on which REST API is listening
+ * @param metastorageNodeIds consistent IDs of the nodes that will form metastorage RAFT group
+ * @param cmgNodeIds consistent IDs of the nodes that will form Cluster Management RAFT Group; if empty,
Review comment:
```suggestion
* @param cmgNodeIds consistent IDs of the nodes that will host the Cluster Management Raft Group; if empty,
```
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/ClusterApiClient.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.apache.ignite.cli.IgniteCliException;
+
+/**
+ * Client for Ignite Cluster management API.
+ */
+@Singleton
+public class ClusterApiClient {
+ private static final String CLUSTER_INIT_URL = "/management/v1/cluster/init/";
+
+ private final HttpClient httpClient;
+
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ @Inject
+ public ClusterApiClient(HttpClient httpClient) {
+ this.httpClient = httpClient;
+ }
+
+ /**
+ * Sends 'cluster init' command to the specified node.
+ *
+ * @param nodeEndpoint host:port on which REST API is listening
+ * @param metastorageNodeIds consistent IDs of the nodes that will form metastorage RAFT group
+ * @param cmgNodeIds consistent IDs of the nodes that will form Cluster Management RAFT Group; if empty,
+ * then nodes specified in metastorageNodeIds will form Cluster Management RAFT Group as well
+ * @param out {@link PrintWriter} to which to report about the command outcome
+ */
+ public void init(String nodeEndpoint, List<String> metastorageNodeIds, List<String> cmgNodeIds, PrintWriter out) {
+ InitClusterRequest requestPayload = new InitClusterRequest(metastorageNodeIds, cmgNodeIds);
+ String requestJson = toJson(requestPayload);
+
+ var httpRequest = HttpRequest
+ .newBuilder()
+ .uri(URI.create("http://" + nodeEndpoint + CLUSTER_INIT_URL))
+ .method("POST", BodyPublishers.ofString(requestJson))
+ .header("Content-Type", "application/json")
+ .build();
+
+ HttpResponse<String> httpResponse;
+ try {
+ httpResponse = httpClient.send(httpRequest, BodyHandlers.ofString());
+ } catch (IOException | InterruptedException e) {
+ throw new IgniteCliException("Connection issues while trying to send http request", e);
+ }
+
+ if (httpResponse.statusCode() == HttpURLConnection.HTTP_OK) {
+ out.println("Cluster was initialized successfully.");
+ } else {
+ throw error("Failed to initialize cluster", httpResponse);
+ }
+ }
+
+ private String toJson(InitClusterRequest requestPayload) {
+ try {
+ return objectMapper.writeValueAsString(requestPayload);
+ } catch (JsonProcessingException e) {
+ throw new IllegalStateException("Cannot serialize JSON", e);
+ }
+ }
+
+ private IgniteCliException error(String message, HttpResponse<String> httpResponse) {
+ String errorPayload;
+ try {
+ errorPayload = prettifyJson(httpResponse);
+ } catch (JsonProcessingException e) {
+ // not a valid JSON probably
+ errorPayload = httpResponse.body();
+ }
+
+ return new IgniteCliException(message + "\n\n" + errorPayload);
Review comment:
Does "\n\n" work on Windows?
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/ClusterApiClient.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.apache.ignite.cli.IgniteCliException;
+
+/**
+ * Client for Ignite Cluster management API.
+ */
+@Singleton
+public class ClusterApiClient {
+ private static final String CLUSTER_INIT_URL = "/management/v1/cluster/init/";
+
+ private final HttpClient httpClient;
+
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ @Inject
+ public ClusterApiClient(HttpClient httpClient) {
+ this.httpClient = httpClient;
+ }
+
+ /**
+ * Sends 'cluster init' command to the specified node.
+ *
+ * @param nodeEndpoint host:port on which REST API is listening
+ * @param metastorageNodeIds consistent IDs of the nodes that will form metastorage RAFT group
+ * @param cmgNodeIds consistent IDs of the nodes that will form Cluster Management RAFT Group; if empty,
+ * then nodes specified in metastorageNodeIds will form Cluster Management RAFT Group as well
+ * @param out {@link PrintWriter} to which to report about the command outcome
+ */
+ public void init(String nodeEndpoint, List<String> metastorageNodeIds, List<String> cmgNodeIds, PrintWriter out) {
Review comment:
Do you validate the `nodeEndpoint` format anywhere?
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/InitClusterRequest.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.List;
+
+/**
+ * A request to the Cluster Init REST API.
+ */
+public class InitClusterRequest {
+ @SuppressWarnings("unused")
+ @JsonProperty
+ private final List<String> metastorageNodes;
+ @SuppressWarnings("unused")
Review comment:
```suggestion
@SuppressWarnings("unused")
```
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/spec/ClusterCommandSpec.java
##########
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.spec;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.inject.Inject;
+import org.apache.ignite.cli.builtins.cluster.ClusterApiClient;
+import picocli.CommandLine;
+import picocli.CommandLine.Option;
+
+/**
+ * Commands for managing Ignite cluster as a whole.
+ */
+@CommandLine.Command(
+ name = "cluster",
+ description = "Manages an Ignite cluster as a whole.",
+ subcommands = {
+ ClusterCommandSpec.InitClusterCommandSpec.class,
+ }
+)
+public class ClusterCommandSpec extends CategorySpec {
+ /**
+ * Initializes an Ignite cluster.
+ */
+ @CommandLine.Command(name = "init", description = "Initializes an Ignite cluster.")
+ public static class InitClusterCommandSpec extends CommandSpec {
+
+ @Inject
+ private ClusterApiClient clusterApiClient;
+
+ /**
+ * Address of the REST endpoint of the receiving node in host:port format.
+ */
+ @Option(names = "--node-endpoint", required = true,
+ description = "Address of the REST endpoint of the receiving node in host:port format")
+ private String nodeEndpoint;
+
+ /**
+ * List of names of the nodes (each represented by a separate command line argument) that will host the Metastorage Raft group.
+ * If the "--cmg-nodes" parameter is omitted, the same nodes will also host the Cluster Management Raft group.
+ */
+ @Option(names = "--meta-storage-node", required = true, description = {
+ "Name of the node (repeat like '--meta-storage-node node1 --meta-storage-node node2' to specify more than one node)",
+ "that will host the Metastorage Raft group.",
Review comment:
```suggestion
"that will host the Meta Storage Raft group.",
```
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/spec/ClusterCommandSpec.java
##########
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.spec;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.inject.Inject;
+import org.apache.ignite.cli.builtins.cluster.ClusterApiClient;
+import picocli.CommandLine;
+import picocli.CommandLine.Option;
+
+/**
+ * Commands for managing Ignite cluster as a whole.
+ */
+@CommandLine.Command(
+ name = "cluster",
+ description = "Manages an Ignite cluster as a whole.",
+ subcommands = {
+ ClusterCommandSpec.InitClusterCommandSpec.class,
+ }
+)
+public class ClusterCommandSpec extends CategorySpec {
+ /**
+ * Initializes an Ignite cluster.
+ */
+ @CommandLine.Command(name = "init", description = "Initializes an Ignite cluster.")
+ public static class InitClusterCommandSpec extends CommandSpec {
+
+ @Inject
+ private ClusterApiClient clusterApiClient;
+
+ /**
+ * Address of the REST endpoint of the receiving node in host:port format.
+ */
+ @Option(names = "--node-endpoint", required = true,
+ description = "Address of the REST endpoint of the receiving node in host:port format")
+ private String nodeEndpoint;
Review comment:
Should you use `NodeEndpointOptions` here?
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -581,17 +619,200 @@ void setJson() throws IOException, InterruptedException {
+ "local.baseline.autoAdjust.enabled=true --type node"
).split(" "));
- Assertions.assertEquals(0, exitCode);
- verify(httpClient).send(
- argThat(r -> "http://localhost:8081/management/v1/configuration/node/".equals(r.uri().toString())
- && "PATCH".equals(r.method())
- && r.bodyPublisher().get().contentLength() == expSentContent.getBytes(UTF_8).length
- && "application/json".equals(r.headers().firstValue("Content-Type").get())),
- any());
- assertEquals("Configuration was updated successfully.\n\n"
+ assertThatExitCodeMeansSuccess(exitCode);
+
+ verify(httpClient).send(requestCaptor.capture(), any());
+ HttpRequest capturedRequest = requestCaptor.getValue();
+
+ assertThat(capturedRequest.uri().toString(), is("http://localhost:8081/management/v1/configuration/node/"));
+ assertThat(capturedRequest.method(), is("PATCH"));
+ assertThat(requestBodyBytes(capturedRequest), is(expSentContent.getBytes(UTF_8)));
+ assertThat(capturedRequest.headers().firstValue("Content-Type"), isPresentAnd(is("application/json")));
+
+ assertOutputEqual("Configuration was updated successfully.\n\n"
+ "Use the " + cmd.getColorScheme().commandText("ignite config get")
- + " command to view the updated configuration.\n", out.toString());
+ + " command to view the updated configuration.\n", out.toString(UTF_8));
+ assertThatStderrIsEmpty();
+ }
+ }
+
+ /**
+ * Tests "cluster" command.
+ */
+ @Nested
+ @DisplayName("cluster")
+ class Cluster {
Review comment:
IDEA complains that this class is not called something like `ClusterTest`
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -581,17 +619,200 @@ void setJson() throws IOException, InterruptedException {
+ "local.baseline.autoAdjust.enabled=true --type node"
).split(" "));
- Assertions.assertEquals(0, exitCode);
- verify(httpClient).send(
- argThat(r -> "http://localhost:8081/management/v1/configuration/node/".equals(r.uri().toString())
- && "PATCH".equals(r.method())
- && r.bodyPublisher().get().contentLength() == expSentContent.getBytes(UTF_8).length
- && "application/json".equals(r.headers().firstValue("Content-Type").get())),
- any());
- assertEquals("Configuration was updated successfully.\n\n"
+ assertThatExitCodeMeansSuccess(exitCode);
+
+ verify(httpClient).send(requestCaptor.capture(), any());
+ HttpRequest capturedRequest = requestCaptor.getValue();
+
+ assertThat(capturedRequest.uri().toString(), is("http://localhost:8081/management/v1/configuration/node/"));
+ assertThat(capturedRequest.method(), is("PATCH"));
+ assertThat(requestBodyBytes(capturedRequest), is(expSentContent.getBytes(UTF_8)));
+ assertThat(capturedRequest.headers().firstValue("Content-Type"), isPresentAnd(is("application/json")));
+
+ assertOutputEqual("Configuration was updated successfully.\n\n"
+ "Use the " + cmd.getColorScheme().commandText("ignite config get")
- + " command to view the updated configuration.\n", out.toString());
+ + " command to view the updated configuration.\n", out.toString(UTF_8));
+ assertThatStderrIsEmpty();
+ }
+ }
+
+ /**
+ * Tests "cluster" command.
+ */
+ @Nested
+ @DisplayName("cluster")
+ class Cluster {
+ @Mock
+ private HttpClient httpClient;
+
+ @Mock
+ private HttpResponse<String> response;
+
+ @BeforeEach
+ void setUp() {
+ ctx.registerSingleton(httpClient);
+ }
+
+ @Test
+ @DisplayName("init --node-endpoint=127.0.0.1:17300 --meta-storage-node node1ConsistentId --meta-storage-node node2ConsistentId "
+ + "--cmg-node node2ConsistentId --cmg-node node3ConsistentId")
+ void initSuccess() throws Exception {
+ when(response.statusCode()).thenReturn(HttpURLConnection.HTTP_OK);
+ when(httpClient.<String>send(any(), any())).thenReturn(response);
+
+ var expSentContent = "{\"metastorageNodes\":[\"node1ConsistentId\",\"node2ConsistentId\"],"
+ + "\"cmgNodes\":[\"node2ConsistentId\",\"node3ConsistentId\"]}";
+
+ var cmd = cmd(ctx);
Review comment:
This usage of `var` is illegal by our convention
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803370870
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -581,17 +619,200 @@ void setJson() throws IOException, InterruptedException {
+ "local.baseline.autoAdjust.enabled=true --type node"
).split(" "));
- Assertions.assertEquals(0, exitCode);
- verify(httpClient).send(
- argThat(r -> "http://localhost:8081/management/v1/configuration/node/".equals(r.uri().toString())
- && "PATCH".equals(r.method())
- && r.bodyPublisher().get().contentLength() == expSentContent.getBytes(UTF_8).length
- && "application/json".equals(r.headers().firstValue("Content-Type").get())),
- any());
- assertEquals("Configuration was updated successfully.\n\n"
+ assertThatExitCodeMeansSuccess(exitCode);
+
+ verify(httpClient).send(requestCaptor.capture(), any());
+ HttpRequest capturedRequest = requestCaptor.getValue();
+
+ assertThat(capturedRequest.uri().toString(), is("http://localhost:8081/management/v1/configuration/node/"));
+ assertThat(capturedRequest.method(), is("PATCH"));
+ assertThat(requestBodyBytes(capturedRequest), is(expSentContent.getBytes(UTF_8)));
+ assertThat(capturedRequest.headers().firstValue("Content-Type"), isPresentAnd(is("application/json")));
+
+ assertOutputEqual("Configuration was updated successfully.\n\n"
+ "Use the " + cmd.getColorScheme().commandText("ignite config get")
- + " command to view the updated configuration.\n", out.toString());
+ + " command to view the updated configuration.\n", out.toString(UTF_8));
+ assertThatStderrIsEmpty();
+ }
+ }
+
+ /**
+ * Tests "cluster" command.
+ */
+ @Nested
+ @DisplayName("cluster")
+ class Cluster {
+ @Mock
+ private HttpClient httpClient;
+
+ @Mock
+ private HttpResponse<String> response;
+
+ @BeforeEach
+ void setUp() {
+ ctx.registerSingleton(httpClient);
+ }
+
+ @Test
+ @DisplayName("init --node-endpoint=127.0.0.1:17300 --meta-storage-node node1ConsistentId --meta-storage-node node2ConsistentId "
+ + "--cmg-node node2ConsistentId --cmg-node node3ConsistentId")
+ void initSuccess() throws Exception {
+ when(response.statusCode()).thenReturn(HttpURLConnection.HTTP_OK);
+ when(httpClient.<String>send(any(), any())).thenReturn(response);
+
+ var expSentContent = "{\"metastorageNodes\":[\"node1ConsistentId\",\"node2ConsistentId\"],"
+ + "\"cmgNodes\":[\"node2ConsistentId\",\"node3ConsistentId\"]}";
+
+ var cmd = cmd(ctx);
Review comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r802757506
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/InitClusterRequest.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.List;
+
+/**
+ * A request to the Cluster Init REST API.
+ */
+public class InitClusterRequest {
+ @SuppressWarnings("unused")
Review comment:
I thought similarly, but PMD has its own opinion :) It just fails the build at TC if it sees an unused field.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r802757506
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/InitClusterRequest.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.List;
+
+/**
+ * A request to the Cluster Init REST API.
+ */
+public class InitClusterRequest {
+ @SuppressWarnings("unused")
Review comment:
I though similarly, but PMD has its own opinion :) It just fails the build at TC if it sees an unused field.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r802756703
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -603,10 +824,10 @@ void setJson() throws IOException, InterruptedException {
* @param actual Actual result.
* @see Object#equals(Object)
*/
- private static void assertEquals(String exp, String actual) {
- Assertions.assertEquals(
- exp.lines().collect(Collectors.toList()),
- actual.lines().collect(Collectors.toList())
+ private static void assertOutputEqual(String exp, String actual) {
+ assertEquals(
+ exp.lines().collect(toList()),
Review comment:
It worked like this before me, so I just did not want to change the logic without a good reason. Maybe the guys wanted to remove newlines.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ibessonov merged pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
ibessonov merged pull request #647:
URL: https://github.com/apache/ignite-3/pull/647
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803371012
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/spec/NodeEndpointOptions.java
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.spec;
+
+import org.apache.ignite.cli.IgniteCliException;
+import picocli.CommandLine;
+
+/**
+ * Prepared picocli mixin for generic node hostname option.
+ */
+class NodeEndpointOptions {
+ /**
+ * Custom node REST endpoint address.
+ */
+ @CommandLine.Option(
+ names = "--node-endpoint",
+ description = "Ignite server node's REST API address and port number",
+ paramLabel = "host:port"
+ )
+ private String endpoint;
+
+ /**
+ * Returns REST endpoint port.
+ *
+ * @return REST endpoint port.
+ */
+ int port() {
+ if (endpoint == null) {
+ return 10300;
+ }
+
+ var hostPort = parse();
+
+ try {
+ return Integer.parseInt(hostPort[1]);
+ } catch (NumberFormatException ex) {
+ throw new IgniteCliException("Can't parse port from " + hostPort[1] + " value");
+ }
+ }
+
+ /**
+ * Returns REST endpoint host.
+ *
+ * @return REST endpoint host.
+ */
+ String host() {
+ return endpoint != null ? parse()[0] : "localhost";
+ }
+
+ /**
+ * Parses REST endpoint host and port from string.
+ *
+ * @return 2-elements array [host, port].
+ */
+ private String[] parse() {
+ var hostPort = endpoint.split(":");
+
+ if (hostPort.length != 2) {
+ throw new IgniteCliException("Incorrect host:port pair provided "
+ + "(example of valid value 'localhost:10300')");
Review comment:
Added it to the exception message
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803391236
##########
File path: modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -581,17 +619,200 @@ void setJson() throws IOException, InterruptedException {
+ "local.baseline.autoAdjust.enabled=true --type node"
).split(" "));
- Assertions.assertEquals(0, exitCode);
- verify(httpClient).send(
- argThat(r -> "http://localhost:8081/management/v1/configuration/node/".equals(r.uri().toString())
- && "PATCH".equals(r.method())
- && r.bodyPublisher().get().contentLength() == expSentContent.getBytes(UTF_8).length
- && "application/json".equals(r.headers().firstValue("Content-Type").get())),
- any());
- assertEquals("Configuration was updated successfully.\n\n"
+ assertThatExitCodeMeansSuccess(exitCode);
+
+ verify(httpClient).send(requestCaptor.capture(), any());
+ HttpRequest capturedRequest = requestCaptor.getValue();
+
+ assertThat(capturedRequest.uri().toString(), is("http://localhost:8081/management/v1/configuration/node/"));
+ assertThat(capturedRequest.method(), is("PATCH"));
+ assertThat(requestBodyBytes(capturedRequest), is(expSentContent.getBytes(UTF_8)));
+ assertThat(capturedRequest.headers().firstValue("Content-Type"), isPresentAnd(is("application/json")));
+
+ assertOutputEqual("Configuration was updated successfully.\n\n"
+ "Use the " + cmd.getColorScheme().commandText("ignite config get")
- + " command to view the updated configuration.\n", out.toString());
+ + " command to view the updated configuration.\n", out.toString(UTF_8));
+ assertThatStderrIsEmpty();
+ }
+ }
+
+ /**
+ * Tests "cluster" command.
+ */
+ @Nested
+ @DisplayName("cluster")
+ class Cluster {
Review comment:
I filed a ticket https://issues.apache.org/jira/browse/IGNITE-16520
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803363335
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/ClusterApiClient.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.apache.ignite.cli.IgniteCliException;
+
+/**
+ * Client for Ignite Cluster management API.
+ */
+@Singleton
+public class ClusterApiClient {
+ private static final String CLUSTER_INIT_URL = "/management/v1/cluster/init/";
+
+ private final HttpClient httpClient;
+
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ @Inject
+ public ClusterApiClient(HttpClient httpClient) {
+ this.httpClient = httpClient;
+ }
+
+ /**
+ * Sends 'cluster init' command to the specified node.
+ *
+ * @param nodeEndpoint host:port on which REST API is listening
+ * @param metastorageNodeIds consistent IDs of the nodes that will form metastorage RAFT group
+ * @param cmgNodeIds consistent IDs of the nodes that will form Cluster Management RAFT Group; if empty,
+ * then nodes specified in metastorageNodeIds will form Cluster Management RAFT Group as well
+ * @param out {@link PrintWriter} to which to report about the command outcome
+ */
+ public void init(String nodeEndpoint, List<String> metastorageNodeIds, List<String> cmgNodeIds, PrintWriter out) {
+ InitClusterRequest requestPayload = new InitClusterRequest(metastorageNodeIds, cmgNodeIds);
+ String requestJson = toJson(requestPayload);
+
+ var httpRequest = HttpRequest
+ .newBuilder()
+ .uri(URI.create("http://" + nodeEndpoint + CLUSTER_INIT_URL))
+ .method("POST", BodyPublishers.ofString(requestJson))
+ .header("Content-Type", "application/json")
+ .build();
+
+ HttpResponse<String> httpResponse;
+ try {
+ httpResponse = httpClient.send(httpRequest, BodyHandlers.ofString());
+ } catch (IOException | InterruptedException e) {
+ throw new IgniteCliException("Connection issues while trying to send http request", e);
+ }
+
+ if (httpResponse.statusCode() == HttpURLConnection.HTTP_OK) {
+ out.println("Cluster was initialized successfully.");
+ } else {
+ throw error("Failed to initialize cluster", httpResponse);
+ }
+ }
+
+ private String toJson(InitClusterRequest requestPayload) {
+ try {
+ return objectMapper.writeValueAsString(requestPayload);
+ } catch (JsonProcessingException e) {
+ throw new IllegalStateException("Cannot serialize JSON", e);
+ }
+ }
+
+ private IgniteCliException error(String message, HttpResponse<String> httpResponse) {
+ String errorPayload;
+ try {
+ errorPayload = prettifyJson(httpResponse);
+ } catch (JsonProcessingException e) {
+ // not a valid JSON probably
+ errorPayload = httpResponse.body();
+ }
+
+ return new IgniteCliException(message + "\n\n" + errorPayload);
Review comment:
No it doesn't, good catch. Fixed this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a change in pull request #647: IGNITE-14871 Add cluster initialization commands to the Ignite CLI
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #647:
URL: https://github.com/apache/ignite-3/pull/647#discussion_r803363875
##########
File path: modules/cli/src/main/java/org/apache/ignite/cli/builtins/cluster/ClusterApiClient.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.cli.builtins.cluster;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.apache.ignite.cli.IgniteCliException;
+
+/**
+ * Client for Ignite Cluster management API.
+ */
+@Singleton
+public class ClusterApiClient {
+ private static final String CLUSTER_INIT_URL = "/management/v1/cluster/init/";
+
+ private final HttpClient httpClient;
+
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ @Inject
+ public ClusterApiClient(HttpClient httpClient) {
+ this.httpClient = httpClient;
+ }
+
+ /**
+ * Sends 'cluster init' command to the specified node.
+ *
+ * @param nodeEndpoint host:port on which REST API is listening
+ * @param metastorageNodeIds consistent IDs of the nodes that will form metastorage RAFT group
+ * @param cmgNodeIds consistent IDs of the nodes that will form Cluster Management RAFT Group; if empty,
+ * then nodes specified in metastorageNodeIds will form Cluster Management RAFT Group as well
+ * @param out {@link PrintWriter} to which to report about the command outcome
+ */
+ public void init(String nodeEndpoint, List<String> metastorageNodeIds, List<String> cmgNodeIds, PrintWriter out) {
Review comment:
No, the user will see an exception with the full URL containing any strange thing they will specify here. Do you believe we need to validatee it manually ourselves?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org