diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/AdministrativeService.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/AdministrativeService.java index 9450e32e6b..fafc680ff4 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/AdministrativeService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/AdministrativeService.java @@ -88,45 +88,60 @@ public CompletableFuture updateStatus(ServiceRequestContext ctx, return rejectStatusPatch(patch); } + // writable | replicating | result + //-------------------+---------------+----------------------------------- + // true -> true | true -> true | not_modified exception + // | true -> false | bad_request exception + //-------------------+---------------+----------------------------------- + // true -> false | true -> true | setWritable(false) + // | true -> false | setWritable(false) & stop executor. + //-------------------+---------------+----------------------------------- + // false -> true | true -> true | setWritable(true) + // | false -> true | setWritable(true) & start executor. + //-------------------+---------------+----------------------------------- + // false -> false | true -> false | Stop executor. + // | false -> true | Start executor. final boolean writable = writableNode.asBoolean(); final boolean replicating = replicatingNode.asBoolean(); if (writable && !replicating) { return HttpApiUtil.throwResponse(ctx, HttpStatus.BAD_REQUEST, "'replicating' must be 'true' if 'writable' is 'true'."); } + if (oldStatus.writable == writable && oldStatus.replicating == replicating) { + throw HttpStatusException.of(HttpStatus.NOT_MODIFIED); + } - if (oldStatus.writable) { - if (!writable) { // writable -> unwritable - executor().setWritable(false); - if (replicating) { - logger.warn("Entered read-only mode with replication enabled"); - return CompletableFuture.completedFuture(status()); - } else { - logger.warn("Entering read-only mode with replication disabled .."); - return executor().stop().handle((unused, cause) -> { - if (cause != null) { - logger.warn("Failed to stop the command executor:", cause); - } else { - logger.info("Entered read-only mode with replication disabled"); - } - return status(); - }); - } + if (oldStatus.writable != writable) { + executor().setWritable(writable); + if (writable) { + logger.warn("Left read-only mode."); + } else { + logger.warn("Entered read-only mode. replication: {}", replicating); + } + } + + if (oldStatus.replicating != replicating) { + if (replicating) { + return executor().start().handle((unused, cause) -> { + if (cause != null) { + logger.warn("Failed to start the command executor:", cause); + } else { + logger.info("Enabled replication. read-only: {}", !writable); + } + return status(); + }); } - } else if (writable) { // unwritable -> writable - logger.warn("Leaving read-only mode .."); - executor().setWritable(true); - return executor().start().handle((unused, cause) -> { + return executor().stop().handle((unused, cause) -> { if (cause != null) { - logger.warn("Failed to leave read-only mode:", cause); + logger.warn("Failed to stop the command executor:", cause); } else { - logger.info("Left read-only mode"); + logger.info("Disabled replication"); } return status(); }); } - throw HttpStatusException.of(HttpStatus.NOT_MODIFIED); + return CompletableFuture.completedFuture(status()); } private static CompletableFuture rejectStatusPatch(JsonNode patch) { diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AdministrativeServiceTest.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AdministrativeServiceTest.java index b675174902..c7e011c480 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AdministrativeServiceTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AdministrativeServiceTest.java @@ -63,7 +63,7 @@ void updateStatus_setUnwritable() { final AggregatedHttpResponse res = client.execute( RequestHeaders.of(HttpMethod.PATCH, API_V1_PATH_PREFIX + "status", HttpHeaderNames.CONTENT_TYPE, MediaType.JSON_PATCH), - "[{ \"op\": \"replace\", \"path\": \"/writable\", \"value\": false }]").aggregate().join(); + "[" + writable(false) + "]").aggregate().join(); assertThat(res.status()).isEqualTo(HttpStatus.OK); assertThatJson(res.contentUtf8()).isEqualTo( @@ -76,8 +76,7 @@ void updateStatus_setUnwritableAndNonReplicating() { final AggregatedHttpResponse res = client.execute( RequestHeaders.of(HttpMethod.PATCH, API_V1_PATH_PREFIX + "status", HttpHeaderNames.CONTENT_TYPE, MediaType.JSON_PATCH), - "[{ \"op\": \"replace\", \"path\": \"/writable\", \"value\": false }," + - " { \"op\": \"replace\", \"path\": \"/replicating\", \"value\": false }]").aggregate().join(); + "[" + writable(false) + "," + replicating(false) + "]").aggregate().join(); assertThat(res.status()).isEqualTo(HttpStatus.OK); assertThatJson(res.contentUtf8()).isEqualTo( @@ -90,8 +89,7 @@ void updateStatus_setWritableAndNonReplicating() { final AggregatedHttpResponse res = client.execute( RequestHeaders.of(HttpMethod.PATCH, API_V1_PATH_PREFIX + "status", HttpHeaderNames.CONTENT_TYPE, MediaType.JSON_PATCH), - "[{ \"op\": \"replace\", \"path\": \"/writable\", \"value\": true }," + - " { \"op\": \"replace\", \"path\": \"/replicating\", \"value\": false }]").aggregate().join(); + "[" + writable(true) + "," + replicating(false) + "]").aggregate().join(); assertThat(res.status()).isEqualTo(HttpStatus.BAD_REQUEST); } @@ -102,7 +100,7 @@ void redundantUpdateStatus_Writable() { final AggregatedHttpResponse res = client.execute( RequestHeaders.of(HttpMethod.PATCH, API_V1_PATH_PREFIX + "status", HttpHeaderNames.CONTENT_TYPE, MediaType.JSON_PATCH), - "[{ \"op\": \"replace\", \"path\": \"/writable\", \"value\": true }]").aggregate().join(); + "[" + writable(true) + "]").aggregate().join(); assertThat(res.status()).isEqualTo(HttpStatus.NOT_MODIFIED); } @@ -113,7 +111,7 @@ void redundantUpdateStatus_Replicating() { final AggregatedHttpResponse res = client.execute( RequestHeaders.of(HttpMethod.PATCH, API_V1_PATH_PREFIX + "status", HttpHeaderNames.CONTENT_TYPE, MediaType.JSON_PATCH), - "[{ \"op\": \"replace\", \"path\": \"/replicating\", \"value\": true }]").aggregate().join(); + "[" + replicating(true) + "]").aggregate().join(); assertThat(res.status()).isEqualTo(HttpStatus.NOT_MODIFIED); } @@ -127,10 +125,66 @@ void updateStatus_leaveReadOnlyMode() { final AggregatedHttpResponse res = client.execute( RequestHeaders.of(HttpMethod.PATCH, API_V1_PATH_PREFIX + "status", HttpHeaderNames.CONTENT_TYPE, MediaType.JSON_PATCH), - "[{ \"op\": \"replace\", \"path\": \"/writable\", \"value\": true }]").aggregate().join(); + "[" + writable(true) + "]").aggregate().join(); assertThat(res.status()).isEqualTo(HttpStatus.OK); assertThatJson(res.contentUtf8()).isEqualTo( "{ \"writable\": true, \"replicating\": true }"); } + + @Test + void updateStatus_enableReplicatingWithReadOnlyMode() { + final WebClient client = dogma.httpClient(); + + // Try to enter read-only mode with replication disabled. + AggregatedHttpResponse res = + client.execute(RequestHeaders.of(HttpMethod.PATCH, API_V1_PATH_PREFIX + "status", + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON_PATCH), + "[" + writable(false) + "," + replicating(false) + "]") + .aggregate() + .join(); + assertThat(res.status()).isEqualTo(HttpStatus.OK); + assertThatJson(res.contentUtf8()).isEqualTo("{ \"writable\": false, \"replicating\": false }"); + + // Try to enable replication. + res = client.execute(RequestHeaders.of(HttpMethod.PATCH, API_V1_PATH_PREFIX + "status", + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON_PATCH), + "[" + replicating(true) + "]") + .aggregate() + .join(); + assertThat(res.status()).isEqualTo(HttpStatus.OK); + assertThatJson(res.contentUtf8()).isEqualTo("{ \"writable\": false, \"replicating\": true }"); + } + + @Test + void updateStatus_disableReplicatingWithReadOnlyMode() { + final WebClient client = dogma.httpClient(); + + // Try to enter read-only mode with replication enabled. + AggregatedHttpResponse res = + client.execute(RequestHeaders.of(HttpMethod.PATCH, API_V1_PATH_PREFIX + "status", + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON_PATCH), + "[" + writable(false) + "," + replicating(true) + "]") + .aggregate() + .join(); + assertThat(res.status()).isEqualTo(HttpStatus.OK); + assertThatJson(res.contentUtf8()).isEqualTo("{ \"writable\": false, \"replicating\": true }"); + + // Try to disable replication. + res = client.execute(RequestHeaders.of(HttpMethod.PATCH, API_V1_PATH_PREFIX + "status", + HttpHeaderNames.CONTENT_TYPE, MediaType.JSON_PATCH), + "[" + replicating(false) + "]") + .aggregate() + .join(); + assertThat(res.status()).isEqualTo(HttpStatus.OK); + assertThatJson(res.contentUtf8()).isEqualTo("{ \"writable\": false, \"replicating\": false }"); + } + + private static String writable(boolean writable) { + return "{ \"op\": \"replace\", \"path\": \"/writable\", \"value\": " + writable + " }"; + } + + private static String replicating(boolean replicating) { + return "{ \"op\": \"replace\", \"path\": \"/replicating\", \"value\": " + replicating + " }"; + } }