valepakh commented on code in PR #1991:
URL: https://github.com/apache/ignite-3/pull/1991#discussion_r1182328298
##########
modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/cluster/init/ClusterInitOptions.java:
##########
@@ -55,22 +68,64 @@ public class ClusterInitOptions {
@Option(names = {CLUSTER_NAME_OPTION, CLUSTER_NAME_OPTION_SHORT}, required
= true, description = CLUSTER_NAME_OPTION_DESC)
private String clusterName;
- @Mixin
- private AuthenticationOptions authenticationOptions;
+ @ArgGroup
+ private ClusterConfigOptions clusterConfigOptions;
+ private static class ClusterConfigOptions {
+ @Option(names = {CLUSTER_CONFIG_OPTION, CLUSTER_CONFIG_OPTION_SHORT},
description = CLUSTER_CONFIG_OPTION_DESC)
+ private String config;
+
+ @Option(names = {CLUSTER_CONFIG_FILE_OPTION,
CLUSTER_CONFIG_FILE_OPTION_SHORT}, description =
CLUSTER_CONFIG_FILE_OPTION_DESC)
+ private File file;
+ }
+
+ /**
+ * Consistent IDs of the nodes that will host the Meta Storage.
+ *
+ * @return Meta storage node ids.
+ */
public List<String> metaStorageNodes() {
return metaStorageNodes;
}
+ /**
+ * Consistent IDs of the nodes that will host the Cluster Management
Group; if empty,
+ * {@code metaStorageNodeIds} will be used to host the CMG as well.
+ *
+ * @return Cluster management node ids.
+ */
public List<String> cmgNodes() {
return cmgNodes;
}
+ /**
+ * Human-readable name of the cluster.
+ *
+ * @return Cluster name.
+ */
public String clusterName() {
return clusterName;
}
- public AuthenticationOptions authenticationOptions() {
- return authenticationOptions;
+ /**
+ * Cluster configuration.
+ *
+ * @return Cluster configuration.
+ */
+ @Nullable
+ public String clusterConfiguration() {
+ if (clusterConfigOptions == null) {
+ return null;
+ } else if (clusterConfigOptions.config != null) {
+ return clusterConfigOptions.config;
+ } else if (clusterConfigOptions.file != null) {
+ try (Stream<String> lines =
Files.lines(clusterConfigOptions.file.toPath())) {
+ return
lines.collect(Collectors.joining(System.lineSeparator()));
Review Comment:
Can't we just read the file to the String?
##########
modules/api/src/main/java/org/apache/ignite/InitParametersBuilder.java:
##########
@@ -93,19 +97,29 @@ public InitParametersBuilder clusterName(String
clusterName) {
}
/**
- * Sets authentication configuration, that will be applied after
initialization. If not set, the authentication will be disabled.
+ * Sets cluster configuration, that will be applied after initialization.
*
- * @param authenticationConfig Authentication configuration.
+ * @param clusterConfiguration Cluster configuration.
* @return {@code this} for chaining.
*/
- public InitParametersBuilder authenticationConfig(AuthenticationConfig
authenticationConfig) {
- if (authenticationConfig == null) {
- throw new IllegalArgumentException("Authentication config cannot
be null.");
- }
- this.authenticationConfig = authenticationConfig;
+ public InitParametersBuilder clusterConfiguration(String
clusterConfiguration) {
+ this.clusterConfiguration = clusterConfiguration;
return this;
}
+ /**
+ * Sets cluster configuration, that will be applied after initialization.
+ *
+ * @param clusterConfiguration Cluster configuration.
+ * @return {@code this} for chaining.
+ */
+ public InitParametersBuilder clusterConfiguration(File
clusterConfiguration) throws IOException {
+ try (Stream<String> lines =
Files.lines(clusterConfiguration.toPath())) {
+ this.clusterConfiguration =
lines.collect(Collectors.joining(System.lineSeparator()));
+ return this;
+ }
Review Comment:
Do we really need this builder method? I feel like the CLI should be
responsible for reading the file.
##########
modules/api/src/main/java/org/apache/ignite/InitParameters.java:
##########
@@ -47,27 +47,26 @@ public class InitParameters {
* @param metaStorageNodeNames Names of nodes that will host the Meta
Storage.
* @param cmgNodeNames Names of nodes that will host the CMG.
* @param clusterName Human-readable name of the cluster.
- * @param authenticationConfig authentication configuration.
+ * @param clusterConfiguration cluster configuration.
Review Comment:
```suggestion
* @param clusterConfiguration Cluster configuration.
```
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/configuration/ItDistributedConfigurationPropertiesTest.java:
##########
@@ -143,7 +143,6 @@ private static class Node {
var logicalTopology = new LogicalTopologyImpl(clusterStateStorage);
distributedConfigurationUpdater = new
DistributedConfigurationUpdater();
-
distributedConfigurationUpdater.setClusterRestConfiguration(securityConfiguration);
Review Comment:
`securityConfiguration` is not used anymore in this and other tests.
##########
modules/cli/src/test/resources/cluster-configuration-with-enabled-auth.json:
##########
@@ -0,0 +1,15 @@
+{
Review Comment:
This file duplicates the file in integrationTest, maybe move it to
testFixtures?
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterInitializer.java:
##########
@@ -83,14 +81,14 @@ public CompletableFuture<Void> initCluster(
* @param cmgNodeNames Names of nodes that will host the Cluster
Management Group. Can be empty, in which case {@code
* metaStorageNodeNames} will be used instead.
* @param clusterName Human-readable name of the cluster.
- * @param authenticationConfig REST authentication configuration.
+ * @param clusterConfiguration cluster configuration.
Review Comment:
```suggestion
* @param clusterConfiguration Cluster configuration.
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
