frankgh commented on code in PR #225: URL: https://github.com/apache/cassandra-sidecar/pull/225#discussion_r2140886201
########## client/src/testFixtures/java/org/apache/cassandra/sidecar/client/SidecarClientTest.java: ########## Review Comment: can we add some unit tests for the client here? ########## client/src/main/java/org/apache/cassandra/sidecar/client/RequestContext.java: ########## @@ -563,6 +567,30 @@ public Builder nodeDecommissionRequest() return request(NODE_DECOMMISSION_REQUEST); } + /** + * Sets the {@code request} to be a {@link GossipUpdateRequest} for the + * given {@link NodeCommandRequestPayload.State state}, and returns a reference to this Builder enabling method chaining. + * + * @param state the desired state for gossip + * @return a reference to this Builder + */ + public Builder nodeGossipUpdateRequest(@NotNull NodeCommandRequestPayload.State state) + { + return request(new GossipUpdateRequest(state)); + } + + /** + * Sets the {@code request} to be a {@link NativeUpdateRequest} for the Review Comment: NIT ```suggestion * Sets the {@code request} to be a {@link NativeUpdateRequest} for the ``` ########## client/src/main/java/org/apache/cassandra/sidecar/client/RequestContext.java: ########## @@ -563,6 +567,30 @@ public Builder nodeDecommissionRequest() return request(NODE_DECOMMISSION_REQUEST); } + /** + * Sets the {@code request} to be a {@link GossipUpdateRequest} for the Review Comment: NIT ```suggestion * Sets the {@code request} to be a {@link GossipUpdateRequest} for the ``` ########## client-common/src/main/java/org/apache/cassandra/sidecar/common/request/GossipUpdateRequest.java: ########## @@ -0,0 +1,65 @@ +/* + * 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.cassandra.sidecar.common.request; + +import io.netty.handler.codec.http.HttpMethod; +import org.apache.cassandra.sidecar.common.ApiEndpointsV1; +import org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload; + +/** + * Gossip update request + */ +public class GossipUpdateRequest extends Request +{ + private final NodeCommandRequestPayload requestPayload; + + /** + * Constructs a gossip update request with the provided parameters + * + * @param requestPayload { "state": "start" } or { "state": "stop" } + */ + public GossipUpdateRequest(NodeCommandRequestPayload requestPayload) Review Comment: do we need this constructor? it seems to be unused ########## server-common/src/main/java/org/apache/cassandra/sidecar/common/server/StorageOperations.java: ########## @@ -129,4 +129,12 @@ default void outOfRangeDataCleanup(@NotNull String keyspace, @NotNull String tab * @return the name of the cluster */ String clusterName(); + + void stopNativeTransport(); Review Comment: can we add javadocs here? ########## client-common/src/main/java/org/apache/cassandra/sidecar/common/request/data/NodeCommandRequestPayload.java: ########## @@ -0,0 +1,96 @@ +/* + * 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.cassandra.sidecar.common.request.data; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.cassandra.sidecar.common.utils.Preconditions; + +/** + * Request payload for start/stop operations (gossip, native transport, etc.). + * + * <p>Valid JSON:</p> + * <pre> + * { "state": "start" } + * { "state": "stop" } + * </pre> + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public class NodeCommandRequestPayload +{ + /** + * Node Command State + */ + public enum State + { + START, STOP; + + @JsonCreator + public static State fromString(String s) + { + if (s == null) return null; + switch (s.trim().toLowerCase()) + { + case "start": + return START; + case "stop": + return STOP; + default: + throw new IllegalArgumentException("Unknown state: " + s); + } + } + + @JsonProperty + public String toValue() + { + return name().toLowerCase(); + } + } + + private final State state; + + /** + * @param state the desired operation, must be "start" or "stop" + */ + @JsonCreator + public NodeCommandRequestPayload( + @JsonProperty(value = "state", required = true) String state + ) + { + Preconditions.checkArgument(state != null && !state.isEmpty(), + "state must be provided and non-empty"); + this.state = State.fromString(state); + } + + /** + * @return the parsed enum (START or STOP) + */ + @JsonProperty("state") + public State getState() Review Comment: minor suggestion to drop `get` in line with Cassandra code style recommendations. ```suggestion public State state() ``` ########## server/src/main/java/org/apache/cassandra/sidecar/handlers/GossipUpdateHandler.java: ########## @@ -0,0 +1,98 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.Collections; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Inject; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.net.SocketAddress; +import io.vertx.ext.auth.authorization.Authorization; +import io.vertx.ext.web.RoutingContext; +import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions; +import org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.concurrent.ExecutorPools; +import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher; +import org.jetbrains.annotations.NotNull; + +/** + * Handles {@code PUT /api/v1/cassandra/gossip} requests to start or stop Cassandra gossip. + * + * <p>Expects a JSON payload: + * { "state": "start" } or { "state": "stop" } + * and will asynchronously invoke the corresponding JMX operation.</p> + */ +public class GossipUpdateHandler extends NodeCommandHandler implements AccessProtected +{ + private final ExecutorPools executorPools; + private static final Logger LOGGER = LoggerFactory.getLogger(GossipUpdateHandler.class); + + + @Inject + public GossipUpdateHandler(InstanceMetadataFetcher metadataFetcher, + ExecutorPools executorPools) + { + super(metadataFetcher, executorPools, null); + this.executorPools = executorPools; + } + + @Override + public Set<Authorization> requiredAuthorizations() + { + return Collections.singleton(BasicPermissions.WRITE_GOSSIP.toAuthorization()); + } + + @Override + protected void handleInternal(RoutingContext context, + HttpServerRequest httpRequest, + @NotNull String host, + SocketAddress remoteAddress, + NodeCommandRequestPayload request) + { + StorageOperations storageOps = metadataFetcher.delegate(host).storageOperations(); + + executorPools.service() + .runBlocking(() -> { + switch (request.getState()) + { + case START: + storageOps.startGossiping(); + break; + case STOP: + storageOps.stopGossiping(); + break; + default: + throw new IllegalStateException("Unknown state: " + request.getState()); + } + }) + .onSuccess(ignored -> { + context.response().setStatusCode(HttpResponseStatus.ACCEPTED.code()).end(); Review Comment: we always respond with a JSON payload. Maybe status: OK , or status: Accepted are possible responses? ########## server/src/main/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandler.java: ########## @@ -0,0 +1,99 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.Collections; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Inject; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.net.SocketAddress; +import io.vertx.ext.auth.authorization.Authorization; +import io.vertx.ext.web.RoutingContext; +import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions; +import org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.concurrent.ExecutorPools; +import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher; +import org.jetbrains.annotations.NotNull; + + +/** + * Handler for starting or stopping native transport on a Cassandra node. + * <p> + * Expects a JSON body: + * { "state": "start" } or { "state": "stop" } + * </p> + */ +public class NativeUpdateHandler extends NodeCommandHandler implements AccessProtected +{ + private final ExecutorPools executorPools; + private static final Logger LOGGER = LoggerFactory.getLogger(NativeUpdateHandler.class); Review Comment: we can omit executorPools since it's already available in the super. LOGGER is unused so I suggest we remove it ########## server/src/main/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandler.java: ########## @@ -0,0 +1,99 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.Collections; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Inject; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.net.SocketAddress; +import io.vertx.ext.auth.authorization.Authorization; +import io.vertx.ext.web.RoutingContext; +import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions; +import org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.concurrent.ExecutorPools; +import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher; +import org.jetbrains.annotations.NotNull; + + +/** + * Handler for starting or stopping native transport on a Cassandra node. + * <p> + * Expects a JSON body: + * { "state": "start" } or { "state": "stop" } + * </p> + */ +public class NativeUpdateHandler extends NodeCommandHandler implements AccessProtected Review Comment: can we annotate with singleton? ########## server/src/main/java/org/apache/cassandra/sidecar/modules/multibindings/VertxRouteMapKeys.java: ########## @@ -253,4 +253,14 @@ interface UpdateServiceConfigurationRouteKey extends RouteClassKey HttpMethod HTTP_METHOD = HttpMethod.PUT; String ROUTE_URI = ApiEndpointsV1.SERVICE_CONFIG_ROUTE; } + interface UpdateNodeGossipRouteKey extends RouteClassKey + { + HttpMethod HTTP_METHOD = HttpMethod.PUT; + String ROUTE_URI = ApiEndpointsV1.GOSSIP_ROUTE; + } + interface UpdateNodeNativeRouteKey extends RouteClassKey Review Comment: NIT ```suggestion interface UpdateNodeNativeStateRouteKey extends RouteClassKey ``` ########## server/src/main/java/org/apache/cassandra/sidecar/modules/multibindings/VertxRouteMapKeys.java: ########## @@ -253,4 +253,14 @@ interface UpdateServiceConfigurationRouteKey extends RouteClassKey HttpMethod HTTP_METHOD = HttpMethod.PUT; String ROUTE_URI = ApiEndpointsV1.SERVICE_CONFIG_ROUTE; } + interface UpdateNodeGossipRouteKey extends RouteClassKey Review Comment: NIT ```suggestion interface UpdateNodeGossipStateRouteKey extends RouteClassKey ``` ########## server/src/test/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandlerTest.java: ########## @@ -0,0 +1,153 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.Collections; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Module; +import com.google.inject.Provides; +import com.google.inject.Singleton; +import com.google.inject.util.Modules; +import io.vertx.core.Vertx; +import io.vertx.ext.web.client.WebClient; +import io.vertx.ext.web.client.predicate.ResponsePredicate; +import io.vertx.junit5.VertxExtension; +import io.vertx.junit5.VertxTestContext; +import org.apache.cassandra.sidecar.TestModule; +import org.apache.cassandra.sidecar.cluster.CassandraAdapterDelegate; +import org.apache.cassandra.sidecar.cluster.InstancesMetadata; +import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.modules.SidecarModules; +import org.apache.cassandra.sidecar.server.Server; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Test for {@link NativeUpdateHandler} + */ +@ExtendWith(VertxExtension.class) +public class NativeUpdateHandlerTest +{ + static final Logger LOGGER = LoggerFactory.getLogger(GossipUpdateHandlerTest.class); + Vertx vertx; + Server server; + StorageOperations mockStorageOperations = mock(StorageOperations.class); + + @BeforeEach + void before() throws InterruptedException + { + Injector injector; + Module testOverride = Modules.override(new TestModule()).with(new NativeUpdateHandlerTest.NativeUpdateHandlerTestModule()); + injector = Guice.createInjector(Modules.override(SidecarModules.all()).with(testOverride)); + vertx = injector.getInstance(Vertx.class); + server = injector.getInstance(Server.class); + VertxTestContext context = new VertxTestContext(); + server.start().onSuccess(s -> context.completeNow()).onFailure(context::failNow); + context.awaitCompletion(5, TimeUnit.SECONDS); + } + + @AfterEach + void after() throws InterruptedException + { + CountDownLatch closeLatch = new CountDownLatch(1); + server.close().onSuccess(res -> closeLatch.countDown()); + if (closeLatch.await(60, TimeUnit.SECONDS)) LOGGER.info("Close event received before timeout."); + else LOGGER.error("Close event timed out."); + } + + @Test + void testStartNative(VertxTestContext ctx) + { + WebClient client = WebClient.create(vertx); + String payload = "{\"state\":\"start\"}"; + client.put(server.actualPort(), "127.0.0.1", "/api/v1/cassandra/native").expect(ResponsePredicate.SC_ACCEPTED).sendBuffer(io.vertx.core.buffer.Buffer.buffer(payload), ctx.succeeding(resp -> { + verify(mockStorageOperations, times(1)).startNativeTransport(); + ctx.completeNow(); + })); + } + + @Test + void testStopNative(VertxTestContext ctx) + { + WebClient client = WebClient.create(vertx); + String payload = "{\"state\":\"stop\"}"; + client.put(server.actualPort(), "127.0.0.1", "/api/v1/cassandra/native").expect(ResponsePredicate.SC_ACCEPTED).sendBuffer(io.vertx.core.buffer.Buffer.buffer(payload), ctx.succeeding(resp -> { + verify(mockStorageOperations, times(1)).stopNativeTransport(); + ctx.completeNow(); + })); + } + + @Test + void testInvalidState(VertxTestContext ctx) + { + WebClient client = WebClient.create(vertx); + String payload = "{\"state\":\"foo\"}"; + client.put(server.actualPort(), "127.0.0.1", "/api/v1/cassandra/native").expect(ResponsePredicate.SC_BAD_REQUEST).sendBuffer(io.vertx.core.buffer.Buffer.buffer(payload), ctx.succeeding(resp -> { + ctx.completeNow(); + })); + } + + + /** + * Test guice module for Node Decommission handler tests + */ + class NativeUpdateHandlerTestModule extends AbstractModule + { + @Provides + @Singleton + public InstancesMetadata instanceMetadata() + { + final int instanceId = 100; + final String host = "127.0.0.1"; + final InstanceMetadata instanceMetadata = mock(InstanceMetadata.class); Review Comment: NIT, omit final modifier for local fields ```suggestion int instanceId = 100; String host = "127.0.0.1"; InstanceMetadata instanceMetadata = mock(InstanceMetadata.class); ``` ########## client-common/src/main/java/org/apache/cassandra/sidecar/common/request/data/NodeCommandRequestPayload.java: ########## @@ -0,0 +1,96 @@ +/* + * 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.cassandra.sidecar.common.request.data; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.cassandra.sidecar.common.utils.Preconditions; + +/** + * Request payload for start/stop operations (gossip, native transport, etc.). + * + * <p>Valid JSON:</p> + * <pre> + * { "state": "start" } + * { "state": "stop" } + * </pre> + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public class NodeCommandRequestPayload +{ + /** + * Node Command State + */ + public enum State + { + START, STOP; + + @JsonCreator + public static State fromString(String s) + { + if (s == null) return null; Review Comment: should we throw here instead? ########## server/src/main/java/org/apache/cassandra/sidecar/handlers/GossipUpdateHandler.java: ########## @@ -0,0 +1,98 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.Collections; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Inject; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.net.SocketAddress; +import io.vertx.ext.auth.authorization.Authorization; +import io.vertx.ext.web.RoutingContext; +import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions; +import org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.concurrent.ExecutorPools; +import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher; +import org.jetbrains.annotations.NotNull; + +/** + * Handles {@code PUT /api/v1/cassandra/gossip} requests to start or stop Cassandra gossip. + * + * <p>Expects a JSON payload: + * { "state": "start" } or { "state": "stop" } + * and will asynchronously invoke the corresponding JMX operation.</p> + */ +public class GossipUpdateHandler extends NodeCommandHandler implements AccessProtected +{ + private final ExecutorPools executorPools; + private static final Logger LOGGER = LoggerFactory.getLogger(GossipUpdateHandler.class); + + + @Inject + public GossipUpdateHandler(InstanceMetadataFetcher metadataFetcher, + ExecutorPools executorPools) + { + super(metadataFetcher, executorPools, null); + this.executorPools = executorPools; Review Comment: this can be omitted ########## client/src/main/java/org/apache/cassandra/sidecar/client/RequestContext.java: ########## Review Comment: We also need to expose the new methods via the client (`org.apache.cassandra.sidecar.client.SidecarClient`) ########## server/src/main/java/org/apache/cassandra/sidecar/handlers/GossipUpdateHandler.java: ########## @@ -0,0 +1,98 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.Collections; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Inject; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.net.SocketAddress; +import io.vertx.ext.auth.authorization.Authorization; +import io.vertx.ext.web.RoutingContext; +import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions; +import org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.concurrent.ExecutorPools; +import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher; +import org.jetbrains.annotations.NotNull; + +/** + * Handles {@code PUT /api/v1/cassandra/gossip} requests to start or stop Cassandra gossip. + * + * <p>Expects a JSON payload: + * { "state": "start" } or { "state": "stop" } + * and will asynchronously invoke the corresponding JMX operation.</p> + */ +public class GossipUpdateHandler extends NodeCommandHandler implements AccessProtected +{ + private final ExecutorPools executorPools; + private static final Logger LOGGER = LoggerFactory.getLogger(GossipUpdateHandler.class); Review Comment: executorPools is already available on the super class, so can be omitted. LOGGER is unused so it should be removed unless it is needed ########## server/src/main/java/org/apache/cassandra/sidecar/handlers/GossipUpdateHandler.java: ########## @@ -0,0 +1,98 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.Collections; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Inject; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.net.SocketAddress; +import io.vertx.ext.auth.authorization.Authorization; +import io.vertx.ext.web.RoutingContext; +import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions; +import org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.concurrent.ExecutorPools; +import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher; +import org.jetbrains.annotations.NotNull; + +/** + * Handles {@code PUT /api/v1/cassandra/gossip} requests to start or stop Cassandra gossip. + * + * <p>Expects a JSON payload: + * { "state": "start" } or { "state": "stop" } + * and will asynchronously invoke the corresponding JMX operation.</p> + */ +public class GossipUpdateHandler extends NodeCommandHandler implements AccessProtected Review Comment: we should probably annotate as singleton ########## server/src/main/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandler.java: ########## @@ -0,0 +1,99 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.Collections; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Inject; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.net.SocketAddress; +import io.vertx.ext.auth.authorization.Authorization; +import io.vertx.ext.web.RoutingContext; +import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions; +import org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.concurrent.ExecutorPools; +import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher; +import org.jetbrains.annotations.NotNull; + + +/** + * Handler for starting or stopping native transport on a Cassandra node. + * <p> + * Expects a JSON body: + * { "state": "start" } or { "state": "stop" } + * </p> + */ +public class NativeUpdateHandler extends NodeCommandHandler implements AccessProtected +{ + private final ExecutorPools executorPools; + private static final Logger LOGGER = LoggerFactory.getLogger(NativeUpdateHandler.class); + + + @Inject + public NativeUpdateHandler(InstanceMetadataFetcher metadataFetcher, + ExecutorPools executorPools) + { + super(metadataFetcher, executorPools, null); + this.executorPools = executorPools; + } + + @Override + public Set<Authorization> requiredAuthorizations() + { + return Collections.singleton(BasicPermissions.WRITE_NATIVE.toAuthorization()); + } + + @Override + protected void handleInternal(RoutingContext context, + HttpServerRequest httpRequest, + @NotNull String host, + SocketAddress remoteAddress, + NodeCommandRequestPayload request) + { + StorageOperations storageOps = metadataFetcher.delegate(host).storageOperations(); + + executorPools.service() + .runBlocking(() -> { + switch (request.getState()) + { + case START: + storageOps.startNativeTransport(); + break; + case STOP: + storageOps.stopNativeTransport(); + break; + default: + throw new IllegalStateException("Unknown state: " + request.getState()); + } + }) + .onSuccess(ignored -> { + context.response().setStatusCode(HttpResponseStatus.ACCEPTED.code()).end(); Review Comment: let's respond with a json payload, status: OK is acceptable, or something along those lines ########## server/src/main/java/org/apache/cassandra/sidecar/modules/CassandraOperationsModule.java: ########## @@ -173,4 +175,26 @@ VertxRoute tableStatsRoute(RouteBuilder.Factory factory, .handler(validateTableExistenceHandler) .handler(tableStatsHandler).build(); } + + @ProvidesIntoMap + @KeyClassMapKey(VertxRouteMapKeys.UpdateNodeGossipRouteKey.class) + VertxRoute cassandraGossipRoute(RouteBuilder.Factory factory, Review Comment: NIT ```suggestion VertxRoute cassandraChangeGossipStateRoute(RouteBuilder.Factory factory, ``` ########## server/src/main/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandler.java: ########## @@ -0,0 +1,99 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.Collections; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Inject; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.net.SocketAddress; +import io.vertx.ext.auth.authorization.Authorization; +import io.vertx.ext.web.RoutingContext; +import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions; +import org.apache.cassandra.sidecar.common.request.data.NodeCommandRequestPayload; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.concurrent.ExecutorPools; +import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher; +import org.jetbrains.annotations.NotNull; + + +/** + * Handler for starting or stopping native transport on a Cassandra node. + * <p> + * Expects a JSON body: + * { "state": "start" } or { "state": "stop" } + * </p> + */ +public class NativeUpdateHandler extends NodeCommandHandler implements AccessProtected +{ + private final ExecutorPools executorPools; + private static final Logger LOGGER = LoggerFactory.getLogger(NativeUpdateHandler.class); + + + @Inject + public NativeUpdateHandler(InstanceMetadataFetcher metadataFetcher, + ExecutorPools executorPools) + { + super(metadataFetcher, executorPools, null); + this.executorPools = executorPools; Review Comment: should be omitted ########## server/src/main/java/org/apache/cassandra/sidecar/modules/CassandraOperationsModule.java: ########## @@ -173,4 +175,26 @@ VertxRoute tableStatsRoute(RouteBuilder.Factory factory, .handler(validateTableExistenceHandler) .handler(tableStatsHandler).build(); } + + @ProvidesIntoMap + @KeyClassMapKey(VertxRouteMapKeys.UpdateNodeGossipRouteKey.class) + VertxRoute cassandraGossipRoute(RouteBuilder.Factory factory, + GossipUpdateHandler nodeGossipHandler) + { + return factory.builderForRoute() + .setBodyHandler(true) + .handler(nodeGossipHandler) + .build(); + } + + @ProvidesIntoMap + @KeyClassMapKey(VertxRouteMapKeys.UpdateNodeNativeRouteKey.class) + VertxRoute cassandraNativeRoute(RouteBuilder.Factory factory, Review Comment: NIT ```suggestion VertxRoute cassandraChangeNativeStateRoute(RouteBuilder.Factory factory, ``` ########## server/src/test/java/org/apache/cassandra/sidecar/handlers/NativeUpdateHandlerTest.java: ########## @@ -0,0 +1,153 @@ +/* + * 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.cassandra.sidecar.handlers; + +import java.util.Collections; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Module; +import com.google.inject.Provides; +import com.google.inject.Singleton; +import com.google.inject.util.Modules; +import io.vertx.core.Vertx; +import io.vertx.ext.web.client.WebClient; +import io.vertx.ext.web.client.predicate.ResponsePredicate; +import io.vertx.junit5.VertxExtension; +import io.vertx.junit5.VertxTestContext; +import org.apache.cassandra.sidecar.TestModule; +import org.apache.cassandra.sidecar.cluster.CassandraAdapterDelegate; +import org.apache.cassandra.sidecar.cluster.InstancesMetadata; +import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.modules.SidecarModules; +import org.apache.cassandra.sidecar.server.Server; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Test for {@link NativeUpdateHandler} + */ +@ExtendWith(VertxExtension.class) +public class NativeUpdateHandlerTest +{ + static final Logger LOGGER = LoggerFactory.getLogger(GossipUpdateHandlerTest.class); Review Comment: Do we want this to be the `NativeUpdateHandlerTest` class instead? ```suggestion static final Logger LOGGER = LoggerFactory.getLogger(NativeUpdateHandlerTest.class); ``` -- 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: pr-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For additional commands, e-mail: pr-h...@cassandra.apache.org