sashapolo commented on code in PR #816: URL: https://github.com/apache/ignite-3/pull/816#discussion_r885884267
########## modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.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.internal.cluster.management.rest; + +import io.micronaut.http.MediaType; +import io.micronaut.http.annotation.Body; +import io.micronaut.http.annotation.Consumes; +import io.micronaut.http.annotation.Controller; +import io.micronaut.http.annotation.Post; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.tags.Tag; +import java.util.concurrent.ExecutionException; +import org.apache.ignite.internal.cluster.management.ClusterInitializer; +import org.apache.ignite.internal.cluster.management.rest.exception.InvalidNodesInitClusterException; +import org.apache.ignite.lang.IgniteException; +import org.apache.ignite.lang.IgniteInternalException; +import org.apache.ignite.lang.IgniteLogger; + +/** + * Cluster management controller. + */ +@Controller("/management/v1/cluster/init") +@ApiResponse(responseCode = "400", description = "Incorrect body") +@ApiResponse(responseCode = "500", description = "Internal error") +@Tag(name = "clusterManagement") +public class ClusterManagementController { + private static final IgniteLogger log = IgniteLogger.forClass(ClusterManagementController.class); + + private final ClusterInitializer clusterInitializer; + + /** + * Cluster management controller constructor. + * + * @param clusterInitializer cluster initializer. + */ + public ClusterManagementController(ClusterInitializer clusterInitializer) { + this.clusterInitializer = clusterInitializer; + } + + /** + * Initializes cluster. + */ + @Post + @Operation(operationId = "init") + @ApiResponse(responseCode = "200", description = "Cluster initialized") + @ApiResponse(responseCode = "409", description = "Cluster already initialized") + @Consumes(MediaType.APPLICATION_JSON) + public void init(@Body InitCommand initCommand) throws ExecutionException, InterruptedException { Review Comment: Same here: I think it should be possible to return a CompletableFuture directly ########## modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/rest/ItClusterManagementControllerTest.java: ########## @@ -0,0 +1,176 @@ +/* + * 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.internal.cluster.management.rest; + +import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import io.micronaut.context.annotation.Bean; +import io.micronaut.context.annotation.Factory; +import io.micronaut.context.annotation.Replaces; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.client.HttpClient; +import io.micronaut.http.client.annotation.Client; +import io.micronaut.http.client.exceptions.HttpClientResponseException; +import io.micronaut.runtime.server.EmbeddedServer; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import jakarta.inject.Inject; +import java.io.IOException; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import org.apache.ignite.internal.cluster.management.ClusterInitializer; +import org.apache.ignite.internal.cluster.management.MockNode; +import org.apache.ignite.internal.rest.api.ErrorResult; +import org.apache.ignite.internal.testframework.WorkDirectory; +import org.apache.ignite.internal.testframework.WorkDirectoryExtension; +import org.apache.ignite.network.ClusterService; +import org.apache.ignite.network.NetworkAddress; +import org.apache.ignite.network.StaticNodeFinder; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Cluster management REST test. + */ +@MicronautTest +@ExtendWith(WorkDirectoryExtension.class) +public class ItClusterManagementControllerTest { + + private static final int PORT_BASE = 10000; + + private static final List<MockNode> cluster = new ArrayList<>(); Review Comment: please separate the fields with line breaks ########## modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.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.internal.cluster.management.rest; + +import io.micronaut.http.MediaType; +import io.micronaut.http.annotation.Body; +import io.micronaut.http.annotation.Consumes; +import io.micronaut.http.annotation.Controller; +import io.micronaut.http.annotation.Post; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.tags.Tag; +import java.util.concurrent.ExecutionException; +import org.apache.ignite.internal.cluster.management.ClusterInitializer; +import org.apache.ignite.internal.cluster.management.rest.exception.InvalidNodesInitClusterException; +import org.apache.ignite.lang.IgniteException; +import org.apache.ignite.lang.IgniteInternalException; +import org.apache.ignite.lang.IgniteLogger; + +/** + * Cluster management controller. + */ +@Controller("/management/v1/cluster/init") +@ApiResponse(responseCode = "400", description = "Incorrect body") +@ApiResponse(responseCode = "500", description = "Internal error") +@Tag(name = "clusterManagement") +public class ClusterManagementController { + private static final IgniteLogger log = IgniteLogger.forClass(ClusterManagementController.class); + + private final ClusterInitializer clusterInitializer; + + /** + * Cluster management controller constructor. + * + * @param clusterInitializer cluster initializer. + */ + public ClusterManagementController(ClusterInitializer clusterInitializer) { + this.clusterInitializer = clusterInitializer; + } + + /** + * Initializes cluster. + */ + @Post + @Operation(operationId = "init") + @ApiResponse(responseCode = "200", description = "Cluster initialized") + @ApiResponse(responseCode = "409", description = "Cluster already initialized") + @Consumes(MediaType.APPLICATION_JSON) + public void init(@Body InitCommand initCommand) throws ExecutionException, InterruptedException { + if (log.isInfoEnabled()) { + log.info("Received init command:\n\tMeta Storage nodes: {}\n\tCMG nodes: {}", initCommand.metaStorageNodes(), + initCommand.cmgNodes()); + } + + try { + clusterInitializer.initCluster(initCommand.metaStorageNodes(), initCommand.cmgNodes(), initCommand.clusterName()).get(); + } catch (ExecutionException ex) { + Throwable cause = ex.getCause(); + handleThrowable(cause); + } + } + + private void handleThrowable(Throwable cause) { + if (cause instanceof IgniteInternalException) { + throw (IgniteInternalException) cause; + } else if (cause instanceof IllegalArgumentException) { + throw new InvalidNodesInitClusterException(cause); Review Comment: I'm not sure that it is safe to say that `IllegalArgumentException` is always related to "invalid nodes" ########## modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java: ########## @@ -0,0 +1,44 @@ +/* + * 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.internal.cluster.management.rest.exception.handler; + +import io.micronaut.context.annotation.Requires; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.HttpResponseFactory; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.annotation.Produces; +import io.micronaut.http.server.exceptions.ExceptionHandler; +import jakarta.inject.Singleton; +import org.apache.ignite.internal.rest.api.ErrorResult; +import org.apache.ignite.lang.IgniteInternalException; + +/** + * Handles {@link IgniteInternalException} and represents it as a rest response. + */ +@Produces +@Singleton +@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class}) +public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> { + + @Override + public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) { + final ErrorResult errorResult = new ErrorResult("ALREADY_INITIALIZED", exception.getMessage()); Review Comment: Again, I don't think that all `IgniteInternalException`s should be described as `ALREADY_INITIALIZED` ########## modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java: ########## @@ -17,151 +17,147 @@ package org.apache.ignite.internal.rest; -import io.netty.bootstrap.ServerBootstrap; -import io.netty.channel.Channel; -import io.netty.channel.ChannelFuture; -import io.netty.handler.logging.LogLevel; -import io.netty.handler.logging.LoggingHandler; +import io.micronaut.context.ApplicationContext; +import io.micronaut.http.server.exceptions.ServerStartupException; +import io.micronaut.openapi.annotation.OpenAPIInclude; +import io.micronaut.runtime.Micronaut; +import io.micronaut.runtime.exceptions.ApplicationStartupException; +import io.swagger.v3.oas.annotations.OpenAPIDefinition; +import io.swagger.v3.oas.annotations.info.Contact; +import io.swagger.v3.oas.annotations.info.Info; +import io.swagger.v3.oas.annotations.info.License; import java.net.BindException; -import java.net.InetSocketAddress; -import java.util.function.Consumer; +import java.util.List; +import java.util.Map; import org.apache.ignite.configuration.schemas.rest.RestConfiguration; import org.apache.ignite.configuration.schemas.rest.RestView; -import org.apache.ignite.internal.configuration.ConfigurationManager; -import org.apache.ignite.internal.configuration.ConfigurationRegistry; +import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController; import org.apache.ignite.internal.manager.IgniteComponent; -import org.apache.ignite.internal.rest.api.RestHandlersRegister; -import org.apache.ignite.internal.rest.api.Routes; -import org.apache.ignite.internal.rest.netty.RestApiInitializer; -import org.apache.ignite.internal.rest.routes.Router; -import org.apache.ignite.internal.rest.routes.SimpleRouter; -import org.apache.ignite.lang.IgniteException; +import org.apache.ignite.internal.rest.api.RestFactory; +import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController; +import org.apache.ignite.internal.rest.configuration.NodeConfigurationController; import org.apache.ignite.lang.IgniteInternalException; import org.apache.ignite.lang.IgniteLogger; -import org.apache.ignite.network.NettyBootstrapFactory; /** - * Rest component is responsible for starting REST endpoints. + * Rest module is responsible for starting a REST endpoints for accessing and managing configuration. * - * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in + * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in * resources for the example. */ -public class RestComponent implements RestHandlersRegister, IgniteComponent { +@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org";), contact = @Contact(email = "[email protected]"))) +@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class}) +public class RestComponent implements IgniteComponent { + /** Default port. */ + public static final int DFLT_PORT = 10300; + /** Ignite logger. */ private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class); - /** Node configuration register. */ - private final ConfigurationRegistry nodeCfgRegistry; - - /** Netty bootstrap factory. */ - private final NettyBootstrapFactory bootstrapFactory; - - private final SimpleRouter router = new SimpleRouter(); - - /** Netty channel. */ - private volatile Channel channel; + /** Factories that produce beans needed for REST controllers. */ + private final List<RestFactory> restFactories; + private final RestConfiguration restConfiguration; + /** Server host. */ + private final String host = "localhost"; + /** Micronaut application context. */ + private ApplicationContext context; + /** Server port. */ + private int port; /** * Creates a new instance of REST module. - * - * @param nodeCfgMgr Node configuration manager. - * @param bootstrapFactory Netty bootstrap factory. */ - public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) { - nodeCfgRegistry = nodeCfgMgr.configurationRegistry(); - - this.bootstrapFactory = bootstrapFactory; - } - - /** {@inheritDoc} */ - @Override - public void registerHandlers(Consumer<Routes> registerAction) { - registerAction.accept(router); + public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) { + this.restFactories = restFactories; + this.restConfiguration = restConfiguration; } /** {@inheritDoc} */ @Override public void start() { - if (channel != null) { - throw new IgniteException("RestModule is already started."); - } - - channel = startRestEndpoint(router).channel(); - } - - /** - * Start endpoint. - * - * @param router Dispatcher of http requests. - * @return Future which will be notified when this channel is closed. - * @throws RuntimeException if this module cannot be bound to a port. - */ - private ChannelFuture startRestEndpoint(Router router) { - RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value(); + RestView restConfigurationView = restConfiguration.value(); int desiredPort = restConfigurationView.port(); int portRange = restConfigurationView.portRange(); - int port = 0; - Channel ch = null; - - ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap() - .handler(new LoggingHandler(LogLevel.INFO)) - .childHandler(new RestApiInitializer(router)); - for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) { - ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly(); - - if (bindRes.isSuccess()) { - ch = bindRes.channel(); + try { + context = buildMicronautContext(portCandidate).start(); port = portCandidate; + log.info("REST protocol started successfully"); break; Review Comment: you can replace this with `return` and remove the `context == null` check below ########## modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java: ########## @@ -17,151 +17,147 @@ package org.apache.ignite.internal.rest; -import io.netty.bootstrap.ServerBootstrap; -import io.netty.channel.Channel; -import io.netty.channel.ChannelFuture; -import io.netty.handler.logging.LogLevel; -import io.netty.handler.logging.LoggingHandler; +import io.micronaut.context.ApplicationContext; +import io.micronaut.http.server.exceptions.ServerStartupException; +import io.micronaut.openapi.annotation.OpenAPIInclude; +import io.micronaut.runtime.Micronaut; +import io.micronaut.runtime.exceptions.ApplicationStartupException; +import io.swagger.v3.oas.annotations.OpenAPIDefinition; +import io.swagger.v3.oas.annotations.info.Contact; +import io.swagger.v3.oas.annotations.info.Info; +import io.swagger.v3.oas.annotations.info.License; import java.net.BindException; -import java.net.InetSocketAddress; -import java.util.function.Consumer; +import java.util.List; +import java.util.Map; import org.apache.ignite.configuration.schemas.rest.RestConfiguration; import org.apache.ignite.configuration.schemas.rest.RestView; -import org.apache.ignite.internal.configuration.ConfigurationManager; -import org.apache.ignite.internal.configuration.ConfigurationRegistry; +import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController; import org.apache.ignite.internal.manager.IgniteComponent; -import org.apache.ignite.internal.rest.api.RestHandlersRegister; -import org.apache.ignite.internal.rest.api.Routes; -import org.apache.ignite.internal.rest.netty.RestApiInitializer; -import org.apache.ignite.internal.rest.routes.Router; -import org.apache.ignite.internal.rest.routes.SimpleRouter; -import org.apache.ignite.lang.IgniteException; +import org.apache.ignite.internal.rest.api.RestFactory; +import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController; +import org.apache.ignite.internal.rest.configuration.NodeConfigurationController; import org.apache.ignite.lang.IgniteInternalException; import org.apache.ignite.lang.IgniteLogger; -import org.apache.ignite.network.NettyBootstrapFactory; /** - * Rest component is responsible for starting REST endpoints. + * Rest module is responsible for starting a REST endpoints for accessing and managing configuration. * - * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in + * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in * resources for the example. */ -public class RestComponent implements RestHandlersRegister, IgniteComponent { +@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org";), contact = @Contact(email = "[email protected]"))) +@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class}) +public class RestComponent implements IgniteComponent { + /** Default port. */ + public static final int DFLT_PORT = 10300; + /** Ignite logger. */ private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class); - /** Node configuration register. */ - private final ConfigurationRegistry nodeCfgRegistry; - - /** Netty bootstrap factory. */ - private final NettyBootstrapFactory bootstrapFactory; - - private final SimpleRouter router = new SimpleRouter(); - - /** Netty channel. */ - private volatile Channel channel; + /** Factories that produce beans needed for REST controllers. */ + private final List<RestFactory> restFactories; + private final RestConfiguration restConfiguration; + /** Server host. */ + private final String host = "localhost"; Review Comment: Why is this not a static constant? ########## modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/rest/ItClusterManagementControllerTest.java: ########## @@ -0,0 +1,176 @@ +/* + * 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.internal.cluster.management.rest; + +import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import io.micronaut.context.annotation.Bean; +import io.micronaut.context.annotation.Factory; +import io.micronaut.context.annotation.Replaces; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.client.HttpClient; +import io.micronaut.http.client.annotation.Client; +import io.micronaut.http.client.exceptions.HttpClientResponseException; +import io.micronaut.runtime.server.EmbeddedServer; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import jakarta.inject.Inject; +import java.io.IOException; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import org.apache.ignite.internal.cluster.management.ClusterInitializer; +import org.apache.ignite.internal.cluster.management.MockNode; +import org.apache.ignite.internal.rest.api.ErrorResult; +import org.apache.ignite.internal.testframework.WorkDirectory; +import org.apache.ignite.internal.testframework.WorkDirectoryExtension; +import org.apache.ignite.network.ClusterService; +import org.apache.ignite.network.NetworkAddress; +import org.apache.ignite.network.StaticNodeFinder; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Cluster management REST test. + */ +@MicronautTest +@ExtendWith(WorkDirectoryExtension.class) +public class ItClusterManagementControllerTest { + + private static final int PORT_BASE = 10000; + + private static final List<MockNode> cluster = new ArrayList<>(); + static ClusterService clusterService; + @WorkDirectory + private static Path workDir; + @Inject + EmbeddedServer server; Review Comment: Is it possible to inject into private fields? If yes, please make them private ########## modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/ClusterManagementController.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.internal.cluster.management.rest; + +import io.micronaut.http.MediaType; +import io.micronaut.http.annotation.Body; +import io.micronaut.http.annotation.Consumes; +import io.micronaut.http.annotation.Controller; +import io.micronaut.http.annotation.Post; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.tags.Tag; +import java.util.concurrent.ExecutionException; +import org.apache.ignite.internal.cluster.management.ClusterInitializer; +import org.apache.ignite.internal.cluster.management.rest.exception.InvalidNodesInitClusterException; +import org.apache.ignite.lang.IgniteException; +import org.apache.ignite.lang.IgniteInternalException; +import org.apache.ignite.lang.IgniteLogger; + +/** + * Cluster management controller. + */ +@Controller("/management/v1/cluster/init") +@ApiResponse(responseCode = "400", description = "Incorrect body") +@ApiResponse(responseCode = "500", description = "Internal error") +@Tag(name = "clusterManagement") +public class ClusterManagementController { + private static final IgniteLogger log = IgniteLogger.forClass(ClusterManagementController.class); + + private final ClusterInitializer clusterInitializer; + + /** + * Cluster management controller constructor. + * + * @param clusterInitializer cluster initializer. + */ + public ClusterManagementController(ClusterInitializer clusterInitializer) { + this.clusterInitializer = clusterInitializer; + } + + /** + * Initializes cluster. + */ + @Post + @Operation(operationId = "init") + @ApiResponse(responseCode = "200", description = "Cluster initialized") + @ApiResponse(responseCode = "409", description = "Cluster already initialized") + @Consumes(MediaType.APPLICATION_JSON) + public void init(@Body InitCommand initCommand) throws ExecutionException, InterruptedException { + if (log.isInfoEnabled()) { + log.info("Received init command:\n\tMeta Storage nodes: {}\n\tCMG nodes: {}", initCommand.metaStorageNodes(), + initCommand.cmgNodes()); + } + + try { + clusterInitializer.initCluster(initCommand.metaStorageNodes(), initCommand.cmgNodes(), initCommand.clusterName()).get(); Review Comment: How does Micronaut actually dispatch user requests on top of Netty? Do we still serve them in a single thread? Is it ok to use long running operations in controller methods? ########## modules/cli/src/integrationTest/resources/hardcoded-ports-config.json: ########## @@ -8,6 +8,6 @@ }, "rest": { "port": <REST_PORT>, - "portRange": 0 + "portRange": 5 Review Comment: Why is this change needed? ########## modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java: ########## @@ -0,0 +1,44 @@ +/* + * 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.internal.cluster.management.rest.exception.handler; + +import io.micronaut.context.annotation.Requires; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.HttpResponseFactory; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.annotation.Produces; +import io.micronaut.http.server.exceptions.ExceptionHandler; +import jakarta.inject.Singleton; +import org.apache.ignite.internal.rest.api.ErrorResult; +import org.apache.ignite.lang.IgniteInternalException; + +/** + * Handles {@link IgniteInternalException} and represents it as a rest response. + */ +@Produces +@Singleton +@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class}) +public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> { + + @Override + public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) { + final ErrorResult errorResult = new ErrorResult("ALREADY_INITIALIZED", exception.getMessage()); Review Comment: ```suggestion ErrorResult errorResult = new ErrorResult("ALREADY_INITIALIZED", exception.getMessage()); ``` ########## modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java: ########## @@ -0,0 +1,44 @@ +/* + * 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.internal.cluster.management.rest.exception.handler; + +import io.micronaut.context.annotation.Requires; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.HttpResponseFactory; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.annotation.Produces; +import io.micronaut.http.server.exceptions.ExceptionHandler; +import jakarta.inject.Singleton; +import org.apache.ignite.internal.rest.api.ErrorResult; +import org.apache.ignite.lang.IgniteInternalException; + +/** + * Handles {@link IgniteInternalException} and represents it as a rest response. + */ +@Produces +@Singleton +@Requires(classes = {IgniteInternalException.class, ExceptionHandler.class}) +public class IgniteInternalExceptionHandler implements ExceptionHandler<IgniteInternalException, HttpResponse<ErrorResult>> { + + @Override + public HttpResponse<ErrorResult> handle(HttpRequest request, IgniteInternalException exception) { + final ErrorResult errorResult = new ErrorResult("ALREADY_INITIALIZED", exception.getMessage()); + return HttpResponseFactory.INSTANCE.status(HttpStatus.CONFLICT).body(errorResult); Review Comment: Why `CONFLICT`? ########## modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ConfigurationControllerBaseTest.java: ########## @@ -0,0 +1,147 @@ +/* + * 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.internal.rest.configuration; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.micronaut.context.ApplicationContext; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.MediaType; +import io.micronaut.http.client.HttpClient; +import io.micronaut.http.client.exceptions.HttpClientResponseException; +import io.micronaut.runtime.server.EmbeddedServer; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import jakarta.inject.Inject; +import org.apache.ignite.internal.configuration.ConfigurationRegistry; +import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation; +import org.apache.ignite.internal.configuration.rest.presentation.TestRootConfiguration; +import org.apache.ignite.internal.rest.api.ErrorResult; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * The base test for configuration controllers. + */ +@MicronautTest +public abstract class ConfigurationControllerBaseTest { + @Inject + EmbeddedServer server; + + @Inject + ConfigurationPresentation<String> cfgPresentation; + + @Inject + ConfigurationRegistry configurationRegistry; + + @Inject + ApplicationContext context; + + abstract HttpClient client(); + + @BeforeEach + void beforeEach() throws Exception { + var cfg = configurationRegistry.getConfiguration(TestRootConfiguration.KEY); + cfg.change(c -> c.changeFoo("foo").changeSubCfg(subCfg -> subCfg.changeBar("bar"))).get(1, SECONDS); + } + + @Test + void testGetConfig() { + var response = client().toBlocking().exchange("", String.class); + + assertEquals(HttpStatus.OK, response.status()); + assertEquals(cfgPresentation.represent(), response.body()); + } + + @Test + void testGetConfigByPath() { + var response = client().toBlocking().exchange("/root.subCfg", String.class); + + assertEquals(HttpStatus.OK, response.status()); + assertEquals(cfgPresentation.representByPath("root.subCfg"), response.body()); + } + + @Test + void testUpdateConfig() { + String givenChangedConfig = "{root:{foo:foo,subCfg:{bar:changed}}}"; + + var response = client().toBlocking().exchange( + HttpRequest.PATCH("", givenChangedConfig).contentType(MediaType.TEXT_PLAIN) + ); + assertEquals(response.status(), HttpStatus.OK); + + String changedConfigValue = client().toBlocking().exchange("/root.subCfg.bar", String.class).body(); + assertEquals("\"changed\"", changedConfigValue); + } + + @Test + void testUnrecognizedConfigPath() { + var thrown = assertThrows( + HttpClientResponseException.class, + () -> client().toBlocking().exchange("/no-such-root.some-value") + ); + + assertEquals(HttpStatus.BAD_REQUEST, thrown.getResponse().status()); + + var errorResult = getErrorResult(thrown); + assertEquals("CONFIG_PATH_UNRECOGNIZED", errorResult.type()); + assertTrue(errorResult.message().contains("no-such-root")); Review Comment: I think it's better to validate whole error messages ########## modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/NodeConfigurationController.java: ########## @@ -0,0 +1,95 @@ +/* + * 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.internal.rest.configuration; + +import io.micronaut.http.MediaType; +import io.micronaut.http.annotation.Body; +import io.micronaut.http.annotation.Consumes; +import io.micronaut.http.annotation.Controller; +import io.micronaut.http.annotation.Get; +import io.micronaut.http.annotation.Patch; +import io.micronaut.http.annotation.PathVariable; +import io.micronaut.http.annotation.Produces; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.tags.Tag; +import jakarta.inject.Named; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation; + +/** + * Node configuration controller. + */ +@Controller("/management/v1/configuration/node") +@ApiResponse(responseCode = "400", description = "Incorrect configuration") +@ApiResponse(responseCode = "500", description = "Internal error") +@Tag(name = "nodeConfiguration") +public class NodeConfigurationController extends AbstractConfigurationController { + + public NodeConfigurationController(@Named("nodeCfgPresentation") ConfigurationPresentation<String> nodeCfgPresentation) { + super(nodeCfgPresentation); + } + + /** + * Returns node configuration in HOCON format. This is represented as a plain text. + * + * @return the whole node configuration in HOCON format. + */ + @Operation(operationId = "getNodeConfiguration") + @ApiResponse(responseCode = "200", + content = @Content(mediaType = MediaType.TEXT_PLAIN, + schema = @Schema(type = "string")), + description = "Whole node configuration") + @Produces(MediaType.TEXT_PLAIN) + @Get + public String getConfiguration() { Review Comment: Same here: missing `@Override` ########## modules/configuration/src/main/java/org/apache/ignite/internal/rest/configuration/ClusterConfigurationController.java: ########## @@ -0,0 +1,95 @@ +/* + * 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.internal.rest.configuration; + +import io.micronaut.http.MediaType; +import io.micronaut.http.annotation.Body; +import io.micronaut.http.annotation.Consumes; +import io.micronaut.http.annotation.Controller; +import io.micronaut.http.annotation.Get; +import io.micronaut.http.annotation.Patch; +import io.micronaut.http.annotation.PathVariable; +import io.micronaut.http.annotation.Produces; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.tags.Tag; +import jakarta.inject.Named; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation; + +/** + * Cluster configuration controller. + */ +@Controller("/management/v1/configuration/cluster/") +@ApiResponse(responseCode = "400", description = "Incorrect configuration") +@ApiResponse(responseCode = "500", description = "Internal error") +@Tag(name = "clusterConfiguration") +public class ClusterConfigurationController extends AbstractConfigurationController { + + public ClusterConfigurationController(@Named("clusterCfgPresentation") ConfigurationPresentation<String> clusterCfgPresentation) { + super(clusterCfgPresentation); + } + + /** + * Returns cluster configuration in HOCON format. This is represented as a plain text. + * + * @return the whole cluster configuration in HOCON format. + */ + @Operation(operationId = "getClusterConfiguration") + @ApiResponse( + responseCode = "200", + content = @Content(mediaType = MediaType.TEXT_PLAIN, schema = @Schema(type = "string")), + description = "Get cluster configuration") + @Produces(MediaType.TEXT_PLAIN) + @Get + public String getConfiguration() { Review Comment: missing `@Override` here and on other methods ########## modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ConfigurationControllerBaseTest.java: ########## @@ -0,0 +1,147 @@ +/* + * 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.internal.rest.configuration; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.micronaut.context.ApplicationContext; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.MediaType; +import io.micronaut.http.client.HttpClient; +import io.micronaut.http.client.exceptions.HttpClientResponseException; +import io.micronaut.runtime.server.EmbeddedServer; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import jakarta.inject.Inject; +import org.apache.ignite.internal.configuration.ConfigurationRegistry; +import org.apache.ignite.internal.configuration.rest.presentation.ConfigurationPresentation; +import org.apache.ignite.internal.configuration.rest.presentation.TestRootConfiguration; +import org.apache.ignite.internal.rest.api.ErrorResult; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * The base test for configuration controllers. + */ +@MicronautTest +public abstract class ConfigurationControllerBaseTest { + @Inject + EmbeddedServer server; Review Comment: Same question: can these fields be `private`? ########## modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java: ########## @@ -17,151 +17,147 @@ package org.apache.ignite.internal.rest; -import io.netty.bootstrap.ServerBootstrap; -import io.netty.channel.Channel; -import io.netty.channel.ChannelFuture; -import io.netty.handler.logging.LogLevel; -import io.netty.handler.logging.LoggingHandler; +import io.micronaut.context.ApplicationContext; +import io.micronaut.http.server.exceptions.ServerStartupException; +import io.micronaut.openapi.annotation.OpenAPIInclude; +import io.micronaut.runtime.Micronaut; +import io.micronaut.runtime.exceptions.ApplicationStartupException; +import io.swagger.v3.oas.annotations.OpenAPIDefinition; +import io.swagger.v3.oas.annotations.info.Contact; +import io.swagger.v3.oas.annotations.info.Info; +import io.swagger.v3.oas.annotations.info.License; import java.net.BindException; -import java.net.InetSocketAddress; -import java.util.function.Consumer; +import java.util.List; +import java.util.Map; import org.apache.ignite.configuration.schemas.rest.RestConfiguration; import org.apache.ignite.configuration.schemas.rest.RestView; -import org.apache.ignite.internal.configuration.ConfigurationManager; -import org.apache.ignite.internal.configuration.ConfigurationRegistry; +import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController; import org.apache.ignite.internal.manager.IgniteComponent; -import org.apache.ignite.internal.rest.api.RestHandlersRegister; -import org.apache.ignite.internal.rest.api.Routes; -import org.apache.ignite.internal.rest.netty.RestApiInitializer; -import org.apache.ignite.internal.rest.routes.Router; -import org.apache.ignite.internal.rest.routes.SimpleRouter; -import org.apache.ignite.lang.IgniteException; +import org.apache.ignite.internal.rest.api.RestFactory; +import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController; +import org.apache.ignite.internal.rest.configuration.NodeConfigurationController; import org.apache.ignite.lang.IgniteInternalException; import org.apache.ignite.lang.IgniteLogger; -import org.apache.ignite.network.NettyBootstrapFactory; /** - * Rest component is responsible for starting REST endpoints. + * Rest module is responsible for starting a REST endpoints for accessing and managing configuration. * - * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in + * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in * resources for the example. */ -public class RestComponent implements RestHandlersRegister, IgniteComponent { +@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org";), contact = @Contact(email = "[email protected]"))) +@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class}) +public class RestComponent implements IgniteComponent { + /** Default port. */ + public static final int DFLT_PORT = 10300; + /** Ignite logger. */ private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class); - /** Node configuration register. */ - private final ConfigurationRegistry nodeCfgRegistry; - - /** Netty bootstrap factory. */ - private final NettyBootstrapFactory bootstrapFactory; - - private final SimpleRouter router = new SimpleRouter(); - - /** Netty channel. */ - private volatile Channel channel; + /** Factories that produce beans needed for REST controllers. */ + private final List<RestFactory> restFactories; + private final RestConfiguration restConfiguration; + /** Server host. */ + private final String host = "localhost"; + /** Micronaut application context. */ + private ApplicationContext context; Review Comment: I would suggest to mark `context` and `port` as `volatile` ########## modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java: ########## @@ -17,151 +17,147 @@ package org.apache.ignite.internal.rest; -import io.netty.bootstrap.ServerBootstrap; -import io.netty.channel.Channel; -import io.netty.channel.ChannelFuture; -import io.netty.handler.logging.LogLevel; -import io.netty.handler.logging.LoggingHandler; +import io.micronaut.context.ApplicationContext; +import io.micronaut.http.server.exceptions.ServerStartupException; +import io.micronaut.openapi.annotation.OpenAPIInclude; +import io.micronaut.runtime.Micronaut; +import io.micronaut.runtime.exceptions.ApplicationStartupException; +import io.swagger.v3.oas.annotations.OpenAPIDefinition; +import io.swagger.v3.oas.annotations.info.Contact; +import io.swagger.v3.oas.annotations.info.Info; +import io.swagger.v3.oas.annotations.info.License; import java.net.BindException; -import java.net.InetSocketAddress; -import java.util.function.Consumer; +import java.util.List; +import java.util.Map; import org.apache.ignite.configuration.schemas.rest.RestConfiguration; import org.apache.ignite.configuration.schemas.rest.RestView; -import org.apache.ignite.internal.configuration.ConfigurationManager; -import org.apache.ignite.internal.configuration.ConfigurationRegistry; +import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController; import org.apache.ignite.internal.manager.IgniteComponent; -import org.apache.ignite.internal.rest.api.RestHandlersRegister; -import org.apache.ignite.internal.rest.api.Routes; -import org.apache.ignite.internal.rest.netty.RestApiInitializer; -import org.apache.ignite.internal.rest.routes.Router; -import org.apache.ignite.internal.rest.routes.SimpleRouter; -import org.apache.ignite.lang.IgniteException; +import org.apache.ignite.internal.rest.api.RestFactory; +import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController; +import org.apache.ignite.internal.rest.configuration.NodeConfigurationController; import org.apache.ignite.lang.IgniteInternalException; import org.apache.ignite.lang.IgniteLogger; -import org.apache.ignite.network.NettyBootstrapFactory; /** - * Rest component is responsible for starting REST endpoints. + * Rest module is responsible for starting a REST endpoints for accessing and managing configuration. Review Comment: ```suggestion * Rest module is responsible for starting REST endpoints for accessing and managing configuration. ``` ########## modules/rest/pom.xml: ########## @@ -151,7 +128,21 @@ <artifactId>ignite-configuration-annotation-processor</artifactId> <version>${project.version}</version> </path> + <path> + <groupId>io.micronaut</groupId> + <artifactId>micronaut-inject-java</artifactId> + <version>${micronaut.version}</version> + </path> + <path> + <groupId>io.micronaut.openapi</groupId> + <artifactId>micronaut-openapi</artifactId> + <version>${maven.micronaut.openapi.plugin.version}</version> + </path> </annotationProcessorPaths> + <fork>true</fork> Review Comment: what is this parameter for? ########## modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java: ########## @@ -17,151 +17,147 @@ package org.apache.ignite.internal.rest; -import io.netty.bootstrap.ServerBootstrap; -import io.netty.channel.Channel; -import io.netty.channel.ChannelFuture; -import io.netty.handler.logging.LogLevel; -import io.netty.handler.logging.LoggingHandler; +import io.micronaut.context.ApplicationContext; +import io.micronaut.http.server.exceptions.ServerStartupException; +import io.micronaut.openapi.annotation.OpenAPIInclude; +import io.micronaut.runtime.Micronaut; +import io.micronaut.runtime.exceptions.ApplicationStartupException; +import io.swagger.v3.oas.annotations.OpenAPIDefinition; +import io.swagger.v3.oas.annotations.info.Contact; +import io.swagger.v3.oas.annotations.info.Info; +import io.swagger.v3.oas.annotations.info.License; import java.net.BindException; -import java.net.InetSocketAddress; -import java.util.function.Consumer; +import java.util.List; +import java.util.Map; import org.apache.ignite.configuration.schemas.rest.RestConfiguration; import org.apache.ignite.configuration.schemas.rest.RestView; -import org.apache.ignite.internal.configuration.ConfigurationManager; -import org.apache.ignite.internal.configuration.ConfigurationRegistry; +import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController; import org.apache.ignite.internal.manager.IgniteComponent; -import org.apache.ignite.internal.rest.api.RestHandlersRegister; -import org.apache.ignite.internal.rest.api.Routes; -import org.apache.ignite.internal.rest.netty.RestApiInitializer; -import org.apache.ignite.internal.rest.routes.Router; -import org.apache.ignite.internal.rest.routes.SimpleRouter; -import org.apache.ignite.lang.IgniteException; +import org.apache.ignite.internal.rest.api.RestFactory; +import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController; +import org.apache.ignite.internal.rest.configuration.NodeConfigurationController; import org.apache.ignite.lang.IgniteInternalException; import org.apache.ignite.lang.IgniteLogger; -import org.apache.ignite.network.NettyBootstrapFactory; /** - * Rest component is responsible for starting REST endpoints. + * Rest module is responsible for starting a REST endpoints for accessing and managing configuration. * - * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in + * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in * resources for the example. */ -public class RestComponent implements RestHandlersRegister, IgniteComponent { +@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org";), contact = @Contact(email = "[email protected]"))) +@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class}) +public class RestComponent implements IgniteComponent { + /** Default port. */ + public static final int DFLT_PORT = 10300; + /** Ignite logger. */ private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class); - /** Node configuration register. */ - private final ConfigurationRegistry nodeCfgRegistry; - - /** Netty bootstrap factory. */ - private final NettyBootstrapFactory bootstrapFactory; - - private final SimpleRouter router = new SimpleRouter(); - - /** Netty channel. */ - private volatile Channel channel; + /** Factories that produce beans needed for REST controllers. */ + private final List<RestFactory> restFactories; + private final RestConfiguration restConfiguration; + /** Server host. */ + private final String host = "localhost"; + /** Micronaut application context. */ + private ApplicationContext context; Review Comment: Actually, `port` can be left as-is if you carefully guard it with `context` accesses ########## modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/rest/ItClusterManagementControllerTest.java: ########## @@ -0,0 +1,176 @@ +/* + * 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.internal.cluster.management.rest; + +import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import io.micronaut.context.annotation.Bean; +import io.micronaut.context.annotation.Factory; +import io.micronaut.context.annotation.Replaces; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.client.HttpClient; +import io.micronaut.http.client.annotation.Client; +import io.micronaut.http.client.exceptions.HttpClientResponseException; +import io.micronaut.runtime.server.EmbeddedServer; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import jakarta.inject.Inject; +import java.io.IOException; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import org.apache.ignite.internal.cluster.management.ClusterInitializer; +import org.apache.ignite.internal.cluster.management.MockNode; +import org.apache.ignite.internal.rest.api.ErrorResult; +import org.apache.ignite.internal.testframework.WorkDirectory; +import org.apache.ignite.internal.testframework.WorkDirectoryExtension; +import org.apache.ignite.network.ClusterService; +import org.apache.ignite.network.NetworkAddress; +import org.apache.ignite.network.StaticNodeFinder; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Cluster management REST test. + */ +@MicronautTest +@ExtendWith(WorkDirectoryExtension.class) +public class ItClusterManagementControllerTest { + + private static final int PORT_BASE = 10000; + + private static final List<MockNode> cluster = new ArrayList<>(); + static ClusterService clusterService; + @WorkDirectory + private static Path workDir; + @Inject + EmbeddedServer server; + @Inject + @Client("/management/v1/cluster/init/") + HttpClient client; + @Inject + ClusterInitializer clusterInitializer; Review Comment: This field is not used ########## modules/rest/src/main/java/org/apache/ignite/internal/rest/RestComponent.java: ########## @@ -17,151 +17,147 @@ package org.apache.ignite.internal.rest; -import io.netty.bootstrap.ServerBootstrap; -import io.netty.channel.Channel; -import io.netty.channel.ChannelFuture; -import io.netty.handler.logging.LogLevel; -import io.netty.handler.logging.LoggingHandler; +import io.micronaut.context.ApplicationContext; +import io.micronaut.http.server.exceptions.ServerStartupException; +import io.micronaut.openapi.annotation.OpenAPIInclude; +import io.micronaut.runtime.Micronaut; +import io.micronaut.runtime.exceptions.ApplicationStartupException; +import io.swagger.v3.oas.annotations.OpenAPIDefinition; +import io.swagger.v3.oas.annotations.info.Contact; +import io.swagger.v3.oas.annotations.info.Info; +import io.swagger.v3.oas.annotations.info.License; import java.net.BindException; -import java.net.InetSocketAddress; -import java.util.function.Consumer; +import java.util.List; +import java.util.Map; import org.apache.ignite.configuration.schemas.rest.RestConfiguration; import org.apache.ignite.configuration.schemas.rest.RestView; -import org.apache.ignite.internal.configuration.ConfigurationManager; -import org.apache.ignite.internal.configuration.ConfigurationRegistry; +import org.apache.ignite.internal.cluster.management.rest.ClusterManagementController; import org.apache.ignite.internal.manager.IgniteComponent; -import org.apache.ignite.internal.rest.api.RestHandlersRegister; -import org.apache.ignite.internal.rest.api.Routes; -import org.apache.ignite.internal.rest.netty.RestApiInitializer; -import org.apache.ignite.internal.rest.routes.Router; -import org.apache.ignite.internal.rest.routes.SimpleRouter; -import org.apache.ignite.lang.IgniteException; +import org.apache.ignite.internal.rest.api.RestFactory; +import org.apache.ignite.internal.rest.configuration.ClusterConfigurationController; +import org.apache.ignite.internal.rest.configuration.NodeConfigurationController; import org.apache.ignite.lang.IgniteInternalException; import org.apache.ignite.lang.IgniteLogger; -import org.apache.ignite.network.NettyBootstrapFactory; /** - * Rest component is responsible for starting REST endpoints. + * Rest module is responsible for starting a REST endpoints for accessing and managing configuration. * - * <p>It is started on port 10300 by default, but it is possible to change this in configuration itself. Refer to default config file in + * <p>It is started on port 10300 by default but it is possible to change this in configuration itself. Refer to default config file in * resources for the example. */ -public class RestComponent implements RestHandlersRegister, IgniteComponent { +@OpenAPIDefinition(info = @Info(title = "Ignite REST module", version = "3.0.0-alpha", license = @License(name = "Apache 2.0", url = "https://ignite.apache.org";), contact = @Contact(email = "[email protected]"))) +@OpenAPIInclude(classes = {ClusterConfigurationController.class, NodeConfigurationController.class, ClusterManagementController.class}) +public class RestComponent implements IgniteComponent { + /** Default port. */ + public static final int DFLT_PORT = 10300; + /** Ignite logger. */ private final IgniteLogger log = IgniteLogger.forClass(RestComponent.class); - /** Node configuration register. */ - private final ConfigurationRegistry nodeCfgRegistry; - - /** Netty bootstrap factory. */ - private final NettyBootstrapFactory bootstrapFactory; - - private final SimpleRouter router = new SimpleRouter(); - - /** Netty channel. */ - private volatile Channel channel; + /** Factories that produce beans needed for REST controllers. */ + private final List<RestFactory> restFactories; + private final RestConfiguration restConfiguration; + /** Server host. */ + private final String host = "localhost"; + /** Micronaut application context. */ + private ApplicationContext context; + /** Server port. */ + private int port; /** * Creates a new instance of REST module. - * - * @param nodeCfgMgr Node configuration manager. - * @param bootstrapFactory Netty bootstrap factory. */ - public RestComponent(ConfigurationManager nodeCfgMgr, NettyBootstrapFactory bootstrapFactory) { - nodeCfgRegistry = nodeCfgMgr.configurationRegistry(); - - this.bootstrapFactory = bootstrapFactory; - } - - /** {@inheritDoc} */ - @Override - public void registerHandlers(Consumer<Routes> registerAction) { - registerAction.accept(router); + public RestComponent(List<RestFactory> restFactories, RestConfiguration restConfiguration) { + this.restFactories = restFactories; + this.restConfiguration = restConfiguration; } /** {@inheritDoc} */ @Override public void start() { - if (channel != null) { - throw new IgniteException("RestModule is already started."); - } - - channel = startRestEndpoint(router).channel(); - } - - /** - * Start endpoint. - * - * @param router Dispatcher of http requests. - * @return Future which will be notified when this channel is closed. - * @throws RuntimeException if this module cannot be bound to a port. - */ - private ChannelFuture startRestEndpoint(Router router) { - RestView restConfigurationView = nodeCfgRegistry.getConfiguration(RestConfiguration.KEY).value(); + RestView restConfigurationView = restConfiguration.value(); int desiredPort = restConfigurationView.port(); int portRange = restConfigurationView.portRange(); - int port = 0; - Channel ch = null; - - ServerBootstrap bootstrap = bootstrapFactory.createServerBootstrap() - .handler(new LoggingHandler(LogLevel.INFO)) - .childHandler(new RestApiInitializer(router)); - for (int portCandidate = desiredPort; portCandidate <= desiredPort + portRange; portCandidate++) { - ChannelFuture bindRes = bootstrap.bind(portCandidate).awaitUninterruptibly(); - - if (bindRes.isSuccess()) { - ch = bindRes.channel(); + try { + context = buildMicronautContext(portCandidate).start(); port = portCandidate; + log.info("REST protocol started successfully"); break; - } else if (!(bindRes.cause() instanceof BindException)) { - throw new RuntimeException(bindRes.cause()); + } catch (ApplicationStartupException e) { + log.error("Got exception " + e.getCause() + " during node start on port " + portCandidate + " , trying again"); Review Comment: That's a bit strange. Are we sure that if we get this exception then the only reason for it is that the target port is busy? -- 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]
