From e17b8c477349a5abdb85db7819d0865b8ebd640c Mon Sep 17 00:00:00 2001 From: Minseok Kim Date: Thu, 6 Aug 2026 23:38:28 +0900 Subject: [PATCH] Plan cleanup work instead of letting clients derive it The teardown order and its preconditions are the part every operational client reimplements: replication off before disable or the standby stops tracking, unlink before drop or the metadata dangles, purge only for what is already gone. Getting one wrong breaks a cluster, so the rule belongs in one place. POST /control/jobs answers what an action would do. Three choices are worth naming. Targets are a list. A per-table endpoint is what makes each client build its own queue, and then makes the next client build it again. dryRun is a flag on the endpoint that will execute, not a separate preview endpoint. Two code paths computing the same plan is how a plan and its execution come to disagree - the plan an operator reviewed has to be the plan that runs. Until authorization lands, dryRun=false is refused. Refusals are typed. An operator facing a disabled action needs to know which precondition to satisfy, and a test needs to assert that the right one was named, so Precondition is an enum with the human detail beside it. These are predictions: the data plane enforces these invariants and stays the one that says no. Fanout is partial - a pair that has drifted apart is brought together rather than refused - and metadata actions stay on the active side, because metadata is not replicated. Part of #370 Co-Authored-By: Claude Opus 5 (1M context) --- .../control/cleanup/ControlJobController.kt | 36 +++ .../control/cleanup/CleanupJobService.kt | 72 +++++ .../server/control/cleanup/CleanupPlan.kt | 140 +++++++++ .../server/control/cleanup/CleanupPlanner.kt | 175 +++++++++++ .../cluster/ClusterClientConfiguration.kt | 4 + .../api/control/cleanup/ControlJobE2ETest.kt | 85 ++++++ .../control/cleanup/CleanupPlannerTest.kt | 280 ++++++++++++++++++ .../filter/ReadOnlyRequestFilterTest.kt | 11 +- .../filter/ServerRoleRequestFilterTest.kt | 7 +- 9 files changed, 807 insertions(+), 3 deletions(-) create mode 100644 server/src/main/kotlin/com/kakao/actionbase/server/api/control/cleanup/ControlJobController.kt create mode 100644 server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupJobService.kt create mode 100644 server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlan.kt create mode 100644 server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlanner.kt create mode 100644 server/src/test/kotlin/com/kakao/actionbase/server/api/control/cleanup/ControlJobE2ETest.kt create mode 100644 server/src/test/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlannerTest.kt diff --git a/server/src/main/kotlin/com/kakao/actionbase/server/api/control/cleanup/ControlJobController.kt b/server/src/main/kotlin/com/kakao/actionbase/server/api/control/cleanup/ControlJobController.kt new file mode 100644 index 000000000..51167ed61 --- /dev/null +++ b/server/src/main/kotlin/com/kakao/actionbase/server/api/control/cleanup/ControlJobController.kt @@ -0,0 +1,36 @@ +package com.kakao.actionbase.server.api.control.cleanup + +import com.kakao.actionbase.server.configuration.ConditionalOnControlRole +import com.kakao.actionbase.server.configuration.HttpHeaderConstants +import com.kakao.actionbase.server.control.cleanup.CleanupJobService +import com.kakao.actionbase.server.control.cleanup.JobRequest +import com.kakao.actionbase.server.control.cleanup.JobView +import com.kakao.actionbase.server.control.cluster.CallerHeaders + +import org.springframework.http.HttpHeaders +import org.springframework.web.bind.annotation.PostMapping +import org.springframework.web.bind.annotation.RequestBody +import org.springframework.web.bind.annotation.RequestHeader +import org.springframework.web.bind.annotation.RestController + +import reactor.core.publisher.Mono + +/** + * Cleanup work, planned. `dryRun=true` is the only mode for now and returns the steps the action + * would take, or the precondition that stops it. + * + * Execution will be this same endpoint with `dryRun=false`, so that the plan an operator reviewed is + * the plan that runs - two code paths computing the same thing is how they come to disagree. + */ +@RestController +@ConditionalOnControlRole +class ControlJobController( + private val jobService: CleanupJobService, +) { + @PostMapping("/control/jobs") + fun submit( + @RequestBody request: JobRequest, + @RequestHeader(value = HttpHeaders.AUTHORIZATION, required = false) authorization: String?, + @RequestHeader(value = HttpHeaderConstants.ACTOR_ROLE, required = false) actorRole: String?, + ): Mono = jobService.submit(request, CallerHeaders.forwarded(authorization, actorRole)) +} diff --git a/server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupJobService.kt b/server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupJobService.kt new file mode 100644 index 000000000..25f4cb0c6 --- /dev/null +++ b/server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupJobService.kt @@ -0,0 +1,72 @@ +package com.kakao.actionbase.server.control.cleanup + +import reactor.core.publisher.Flux +import reactor.core.publisher.Mono + +/** + * Plans a batch of cleanup work. + * + * Targets arrive as a list on purpose. A per-table endpoint is what makes every client build its own + * queue, and then build it again for the next client. + * + * Only dry runs are answered for now: execution waits for authorization, so that a cross-cluster + * drop never becomes reachable before there is something checking who asked for it. + */ +class CleanupJobService( + private val cleanupService: CleanupService, +) { + fun submit( + request: JobRequest, + headers: Map, + ): Mono { + val action = CleanupAction.of(request.action) + require(request.targets.isNotEmpty()) { "targets is empty: name at least one table to plan for" } + if (!request.dryRun) { + throw UnsupportedOperationException( + "execution is not available yet, only dryRun=true. The plan this returns is what execution will run.", + ) + } + + return Flux + .fromIterable(request.targets.map { it.tenant }.distinct()) + .flatMapSequential { cleanupService.htables(it, null, headers) } + .collectList() + .map { views -> view(action, request, views) } + } + + private fun view( + action: CleanupAction, + request: JobRequest, + views: List, + ): JobView { + val tables = views.flatMap { it.tables }.associateBy { it.tenant to it.name } + + return JobView( + dryRun = true, + action = action, + fanout = request.fanout, + plans = + request.targets.distinct().map { target -> + tables[target.tenant to target.table] + ?.let { CleanupPlanner.plan(action, it, request.fanout) } + ?: unseen(target, action) + }, + failures = views.flatMap { it.failures }, + ) + } + + /** + * A table nobody reported. It may be gone, or its cluster may be the one that did not answer - + * which is why the failures travel in the same response. + */ + private fun unseen( + target: JobTarget, + action: CleanupAction, + ) = CleanupPlan( + target = target, + action = action, + ok = false, + refusal = Precondition.TABLE_ALREADY_GONE, + detail = "not seen on any side, and nothing binds it - check failures before concluding it is gone", + ) +} diff --git a/server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlan.kt b/server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlan.kt new file mode 100644 index 000000000..6efe8f9a9 --- /dev/null +++ b/server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlan.kt @@ -0,0 +1,140 @@ +package com.kakao.actionbase.server.control.cleanup + +import com.kakao.actionbase.server.control.topology.Side + +/** + * The lifecycle of a datastore table, as operators run it. + * + * Teardown goes `REPLICATION_OFF -> UNLINK -> DISABLE_TABLE -> DROP_TABLE -> DELETE_METADATA`; + * setup goes `ENABLE_TABLE -> REPLICATION_ON`. The order is not a style preference - disabling a + * table while replication is still on breaks standby sync, and dropping one a collection still + * points at leaves the metadata dangling. + * + * Per #470 `table` is the physical HBase table and `collection` the logical v3 entity, so a name + * carrying `TABLE` never means metadata. + */ +enum class CleanupAction { + /** Replication scope 1 -> 0. */ + REPLICATION_OFF, + + /** Replication scope 0 -> 1. */ + REPLICATION_ON, + + /** Deactivate the collection and storage bound to the table. */ + UNLINK, + + DISABLE_TABLE, + + ENABLE_TABLE, + + DROP_TABLE, + + /** Delete the metadata left behind once the table is gone. */ + DELETE_METADATA, + ; + + /** Metadata is not replicated, so the actions that touch it only ever run on the active side. */ + val metadataOnly: Boolean get() = this == UNLINK || this == DELETE_METADATA + + /** + * Whether the action takes something that cannot be put back: [DROP_TABLE] loses the rows, + * [DELETE_METADATA] the definition. Everything else is a staging step an operator can undo, + * which is the line a confirmation prompt belongs on. + */ + val destructive: Boolean get() = this == DROP_TABLE || this == DELETE_METADATA + + companion object { + fun of(value: String): CleanupAction = + entries.firstOrNull { it.name.equals(value, ignoreCase = true) } + ?: throw IllegalArgumentException("unknown action '$value', expected one of ${entries.map { it.name.lowercase() }}") + } +} + +/** A single call the executor will make. */ +enum class StepOp { + REPLICATION_DISABLE, + REPLICATION_ENABLE, + DEACTIVATE_COLLECTION, + DEACTIVATE_STORAGE, + DISABLE_TABLE, + ENABLE_TABLE, + DROP_TABLE, + DELETE_COLLECTION, + DELETE_STORAGE, +} + +/** + * Why a plan is empty. Typed rather than prose: an operator staring at a disabled button needs to + * know which precondition to satisfy, and a test needs to assert that it named the right one. + */ +enum class Precondition { + /** Every side is already where the action would take it. */ + ALREADY_SATISFIED, + + /** `DISABLE_TABLE` and `DROP_TABLE` need replication off first, or the standby stops tracking. */ + NEEDS_LOCAL_REPLICATION, + + /** `REPLICATION_ON` needs the table enabled first. */ + NEEDS_ENABLED, + + /** `DROP_TABLE` needs the metadata unlinked first. */ + NEEDS_UNLINK, + + /** `DROP_TABLE` needs the table disabled first. */ + NEEDS_DISABLE, + + /** `DELETE_METADATA` is for what is already gone. */ + TABLE_STILL_EXISTS, + + /** `DROP_TABLE` is for what still exists. */ + TABLE_ALREADY_GONE, + + /** Another tenant's metadata still points at this table. */ + HELD_BY_ANOTHER_TENANT, + + /** Nothing binds the table, so there is nothing to unlink or purge. */ + NOTHING_BOUND, +} + +data class JobTarget( + val tenant: String, + val table: String, +) + +data class PlanStep( + val side: Side, + val op: StepOp, + /** The metadata entity a metadata step acts on. */ + val name: String? = null, +) + +/** + * What would happen, or why nothing would. + * + * A refusal is a *prediction*: the data plane enforces these invariants itself and remains the one + * that says no. Repeating the checks here is how a client learns before it asks, not a second source + * of truth. + */ +data class CleanupPlan( + val target: JobTarget, + val action: CleanupAction, + val ok: Boolean, + val steps: List = emptyList(), + val refusal: Precondition? = null, + val detail: String? = null, +) + +data class JobRequest( + val targets: List = emptyList(), + val action: String, + val fanout: Boolean = false, + val dryRun: Boolean = true, +) + +data class JobView( + val dryRun: Boolean, + val action: CleanupAction, + val fanout: Boolean, + val plans: List, + val failures: List, +) diff --git a/server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlanner.kt b/server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlanner.kt new file mode 100644 index 000000000..0b41224e6 --- /dev/null +++ b/server/src/main/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlanner.kt @@ -0,0 +1,175 @@ +package com.kakao.actionbase.server.control.cleanup + +import com.kakao.actionbase.server.control.topology.Side +import com.kakao.actionbase.v2.engine.service.ddl.DatastoreTableReference + +/** + * Decides which calls an action needs, in which order, on which sides. + * + * Pure: everything it knows comes from the projected [TableView]. Two properties matter more than + * the individual rules. + * + * Fanout is partial. Only the sides not already at the target state get a step, so an active/standby + * pair that has drifted apart is brought together rather than refused - an operator should not have + * to fix the drift by hand before asking for the action. + * + * A refusal is a prediction. The data plane enforces these invariants and stays the one that says + * no; predicting them here is how a client learns before it asks. + */ +object CleanupPlanner { + fun plan( + action: CleanupAction, + table: TableView, + fanout: Boolean, + ): CleanupPlan { + val target = JobTarget(table.tenant, table.name) + val sides = table.sidesFor(action, fanout) + + return when (action) { + CleanupAction.REPLICATION_OFF -> table.replication(target, action, sides, from = 1, to = StepOp.REPLICATION_DISABLE) + CleanupAction.REPLICATION_ON -> table.toReplicationOn(target, action, sides) + CleanupAction.DISABLE_TABLE -> table.toDisabled(target, action, sides) + CleanupAction.ENABLE_TABLE -> table.toEnabled(target, action, sides) + CleanupAction.UNLINK -> table.toUnlinked(target, action, sides) + CleanupAction.DROP_TABLE -> table.toDropped(target, action, sides) + CleanupAction.DELETE_METADATA -> table.toMetadataDeleted(target, action, sides) + } + } + + /** Which sides an action may touch: metadata lives with the active, table state can fan out. */ + private fun TableView.sidesFor( + action: CleanupAction, + fanout: Boolean, + ): List = + when { + action.metadataOnly -> listOf(Side.ACTIVE).filter { it in sides } + fanout -> Side.entries.filter { it in sides } + else -> listOf(Side.ACTIVE).filter { it in sides } + } + + private fun TableView.replication( + target: JobTarget, + action: CleanupAction, + sides: List, + from: Int, + to: StepOp, + ): CleanupPlan { + val targets = sides.filter { state(it).present && state(it).replicationScope == from } + if (targets.isEmpty()) return refuse(target, action, Precondition.ALREADY_SATISFIED, "every side is already there") + return CleanupPlan(target, action, ok = true, steps = targets.map { PlanStep(it, to) }) + } + + private fun TableView.toReplicationOn( + target: JobTarget, + action: CleanupAction, + sides: List, + ): CleanupPlan { + val targets = sides.filter { state(it).present && state(it).replicationScope == 0 } + if (targets.isEmpty()) return refuse(target, action, Precondition.ALREADY_SATISFIED, "every side already replicates") + targets.firstOrNull { state(it).enabled == false }?.let { + return refuse(target, action, Precondition.NEEDS_ENABLED, "$it is disabled: enable it before turning replication on") + } + return CleanupPlan(target, action, ok = true, steps = targets.map { PlanStep(it, StepOp.REPLICATION_ENABLE) }) + } + + private fun TableView.toDisabled( + target: JobTarget, + action: CleanupAction, + sides: List, + ): CleanupPlan { + val targets = sides.filter { state(it).enabled == true } + if (targets.isEmpty()) return refuse(target, action, Precondition.ALREADY_SATISFIED, "every side is already disabled") + heldByAnother()?.let { return refuse(target, action, Precondition.HELD_BY_ANOTHER_TENANT, it) } + targets.firstOrNull { state(it).replicationScope != 0 }?.let { + return refuse( + target, + action, + Precondition.NEEDS_LOCAL_REPLICATION, + "$it still replicates: disabling with replication on breaks standby sync, so turn replication off first", + ) + } + return CleanupPlan(target, action, ok = true, steps = targets.map { PlanStep(it, StepOp.DISABLE_TABLE) }) + } + + private fun TableView.toEnabled( + target: JobTarget, + action: CleanupAction, + sides: List, + ): CleanupPlan { + val targets = sides.filter { state(it).enabled == false } + if (targets.isEmpty()) return refuse(target, action, Precondition.ALREADY_SATISFIED, "every side is already enabled") + heldByAnother()?.let { return refuse(target, action, Precondition.HELD_BY_ANOTHER_TENANT, it) } + return CleanupPlan(target, action, ok = true, steps = targets.map { PlanStep(it, StepOp.ENABLE_TABLE) }) + } + + private fun TableView.toUnlinked( + target: JobTarget, + action: CleanupAction, + sides: List, + ): CleanupPlan { + val side = sides.firstOrNull() ?: return refuse(target, action, Precondition.NOTHING_BOUND, "no active side to unlink on") + val live = references.filter { it.active } + if (live.isEmpty()) return refuse(target, action, Precondition.ALREADY_SATISFIED, "nothing active still binds this table") + // Labels first: a storage that a label still reaches through cannot be deactivated yet. + return CleanupPlan(target, action, ok = true, steps = live.stepsOn(side, StepOp.DEACTIVATE_COLLECTION, StepOp.DEACTIVATE_STORAGE)) + } + + private fun TableView.toDropped( + target: JobTarget, + action: CleanupAction, + sides: List, + ): CleanupPlan { + val targets = sides.filter { state(it).present } + if (targets.isEmpty()) return refuse(target, action, Precondition.TABLE_ALREADY_GONE, "nothing left to drop: delete the metadata instead") + heldByAnother()?.let { return refuse(target, action, Precondition.HELD_BY_ANOTHER_TENANT, it) } + if (references.any { it.active }) { + return refuse(target, action, Precondition.NEEDS_UNLINK, "metadata still binds this table: unlink first") + } + targets.firstOrNull { state(it).replicationScope != 0 }?.let { + return refuse(target, action, Precondition.NEEDS_LOCAL_REPLICATION, "$it still replicates: turn replication off first") + } + targets.firstOrNull { state(it).enabled != false }?.let { + return refuse(target, action, Precondition.NEEDS_DISABLE, "$it is still enabled: disable it first") + } + return CleanupPlan(target, action, ok = true, steps = targets.map { PlanStep(it, StepOp.DROP_TABLE) }) + } + + private fun TableView.toMetadataDeleted( + target: JobTarget, + action: CleanupAction, + sides: List, + ): CleanupPlan { + val side = sides.firstOrNull() ?: return refuse(target, action, Precondition.NOTHING_BOUND, "no active side to delete metadata on") + this.sides.entries.firstOrNull { it.value.present }?.let { + return refuse(target, action, Precondition.TABLE_STILL_EXISTS, "${it.key} still has the table: drop it first") + } + if (references.isEmpty()) return refuse(target, action, Precondition.ALREADY_SATISFIED, "no metadata left to delete") + return CleanupPlan(target, action, ok = true, steps = references.stepsOn(side, StepOp.DELETE_COLLECTION, StepOp.DELETE_STORAGE)) + } + + /** Collections before storages: the storage is reached through the collection, so it goes second. */ + private fun List.stepsOn( + side: Side, + collectionOp: StepOp, + storageOp: StepOp, + ): List = + filter { it.kind == DatastoreTableReference.Kind.LABEL }.map { PlanStep(side, collectionOp, it.name) } + + filter { it.kind == DatastoreTableReference.Kind.STORAGE }.map { PlanStep(side, storageOp, it.name) } + + private fun TableView.heldByAnother(): String? = + references + .filter { it.active && it.tenant != tenant } + .map { it.tenant } + .distinct() + .takeIf { it.isNotEmpty() } + ?.let { "still bound by $it: that tenant has to release it first" } + + private fun TableView.state(side: Side): SideState = sides.getValue(side) + + private fun refuse( + target: JobTarget, + action: CleanupAction, + precondition: Precondition, + detail: String, + ) = CleanupPlan(target, action, ok = false, refusal = precondition, detail = detail) +} diff --git a/server/src/main/kotlin/com/kakao/actionbase/server/control/cluster/ClusterClientConfiguration.kt b/server/src/main/kotlin/com/kakao/actionbase/server/control/cluster/ClusterClientConfiguration.kt index 289597d62..eac7e090f 100644 --- a/server/src/main/kotlin/com/kakao/actionbase/server/control/cluster/ClusterClientConfiguration.kt +++ b/server/src/main/kotlin/com/kakao/actionbase/server/control/cluster/ClusterClientConfiguration.kt @@ -1,6 +1,7 @@ package com.kakao.actionbase.server.control.cluster import com.kakao.actionbase.server.configuration.ConditionalOnControlRole +import com.kakao.actionbase.server.control.cleanup.CleanupJobService import com.kakao.actionbase.server.control.cleanup.CleanupService import com.kakao.actionbase.server.control.topology.Topology import com.kakao.actionbase.server.control.topology.TopologyProperties @@ -24,4 +25,7 @@ class ClusterClientConfiguration { topology: Topology, clusterClient: ClusterClient, ): CleanupService = CleanupService(topology, clusterClient) + + @Bean + fun cleanupJobService(cleanupService: CleanupService): CleanupJobService = CleanupJobService(cleanupService) } diff --git a/server/src/test/kotlin/com/kakao/actionbase/server/api/control/cleanup/ControlJobE2ETest.kt b/server/src/test/kotlin/com/kakao/actionbase/server/api/control/cleanup/ControlJobE2ETest.kt new file mode 100644 index 000000000..342819446 --- /dev/null +++ b/server/src/test/kotlin/com/kakao/actionbase/server/api/control/cleanup/ControlJobE2ETest.kt @@ -0,0 +1,85 @@ +package com.kakao.actionbase.server.api.control.cleanup + +import com.kakao.actionbase.server.test.E2ETestBase + +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.springframework.boot.test.context.SpringBootTest + +/** The job endpoint over http, including the refusal of anything that would take effect. */ +@SpringBootTest( + properties = [ + "actionbase.role=CONTROL", + "actionbase.control.request-timeout=2s", + "actionbase.control.tenants.alpha.env=prod", + "actionbase.control.tenants.alpha.namespace=ab_alpha", + "actionbase.control.tenants.alpha.active-url=http://127.0.0.1:1", + ], +) +class ControlJobE2ETest : E2ETestBase() { + @Test + fun `refuses to execute until authorization is in place`() { + client + .post() + .uri("/control/jobs") + .bodyValue(mapOf("targets" to listOf(target()), "action" to "drop_table", "dryRun" to false)) + .exchange() + .expectStatus() + .isBadRequest + .expectBody() + .jsonPath("$.message") + .value { assertTrue(it.contains("dryRun"), it) } + } + + @Test + fun `plans a dry run and reports the cluster it could not read`() { + client + .post() + .uri("/control/jobs") + .bodyValue(mapOf("targets" to listOf(target()), "action" to "drop_table", "dryRun" to true)) + .exchange() + .expectStatus() + .isOk + .expectBody() + .jsonPath("$.dryRun") + .isEqualTo(true) + .jsonPath("$.action") + .isEqualTo("DROP_TABLE") + .jsonPath("$.plans.length()") + .isEqualTo(1) + // The cluster never answered, so the table was not seen - and the failure says why. + .jsonPath("$.plans[0].ok") + .isEqualTo(false) + .jsonPath("$.plans[0].refusal") + .isEqualTo("TABLE_ALREADY_GONE") + .jsonPath("$.failures.length()") + .isEqualTo(1) + } + + @Test + fun `an unknown action names the ones that exist`() { + client + .post() + .uri("/control/jobs") + .bodyValue(mapOf("targets" to listOf(target()), "action" to "obliterate")) + .exchange() + .expectStatus() + .isBadRequest + .expectBody() + .jsonPath("$.message") + .value { assertTrue(it.contains("delete_metadata"), it) } + } + + @Test + fun `an empty target list is a bad request`() { + client + .post() + .uri("/control/jobs") + .bodyValue(mapOf("targets" to emptyList(), "action" to "drop_table")) + .exchange() + .expectStatus() + .isBadRequest + } + + private fun target() = mapOf("tenant" to "alpha", "table" to "ab_alpha:t1") +} diff --git a/server/src/test/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlannerTest.kt b/server/src/test/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlannerTest.kt new file mode 100644 index 000000000..0b9356a69 --- /dev/null +++ b/server/src/test/kotlin/com/kakao/actionbase/server/control/cleanup/CleanupPlannerTest.kt @@ -0,0 +1,280 @@ +package com.kakao.actionbase.server.control.cleanup + +import com.kakao.actionbase.server.control.topology.Side +import com.kakao.actionbase.v2.engine.service.ddl.DatastoreTableReference + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * The plan is dry, so the whole grid can be walked here: every refusal asserts *which* precondition + * stopped it, because that string is the only thing an operator has to go on. + */ +class CleanupPlannerTest { + @Test + fun `going local turns replication off on the sides that have it on`() { + val plan = + plan( + CleanupAction.REPLICATION_OFF, + table(active = state(scope = 1), standby = state(scope = 0)), + fanout = true, + ) + + assertTrue(plan.ok) + assertEquals(listOf(PlanStep(Side.ACTIVE, StepOp.REPLICATION_DISABLE)), plan.steps) + } + + // Partial fanout: a pair that drifted apart is brought together, not refused. + @Test + fun `a side already at the target state is skipped rather than refused`() { + val plan = + plan( + CleanupAction.DISABLE_TABLE, + table(active = state(enabled = true, scope = 0), standby = state(enabled = false, scope = 0)), + fanout = true, + ) + + assertTrue(plan.ok) + assertEquals(listOf(Side.ACTIVE), plan.steps.map { it.side }) + } + + @Test + fun `nothing to do is a refusal that says so`() { + val plan = plan(CleanupAction.REPLICATION_OFF, table(active = state(scope = 0))) + + assertRefused(plan, Precondition.ALREADY_SATISFIED) + } + + @Test + fun `disable needs replication off first`() { + val plan = plan(CleanupAction.DISABLE_TABLE, table(active = state(enabled = true, scope = 1))) + + assertRefused(plan, Precondition.NEEDS_LOCAL_REPLICATION) + assertTrue(plan.detail!!.contains("standby sync"), plan.detail) + } + + @Test + fun `going global needs the table enabled first`() { + val plan = plan(CleanupAction.REPLICATION_ON, table(active = state(enabled = false, scope = 0))) + + assertRefused(plan, Precondition.NEEDS_ENABLED) + } + + @Test + fun `dropping needs the metadata unlinked first`() { + val plan = + plan( + CleanupAction.DROP_TABLE, + table(active = state(enabled = false, scope = 0), references = listOf(label("svc.label", active = true))), + ) + + assertRefused(plan, Precondition.NEEDS_UNLINK) + } + + @Test + fun `dropping needs the table disabled first`() { + val plan = plan(CleanupAction.DROP_TABLE, table(active = state(enabled = true, scope = 0))) + + assertRefused(plan, Precondition.NEEDS_DISABLE) + } + + @Test + fun `dropping needs replication off first`() { + val plan = plan(CleanupAction.DROP_TABLE, table(active = state(enabled = false, scope = 1))) + + assertRefused(plan, Precondition.NEEDS_LOCAL_REPLICATION) + } + + @Test + fun `dropping runs once every step before it is done`() { + val plan = + plan( + CleanupAction.DROP_TABLE, + table(active = state(enabled = false, scope = 0), standby = state(enabled = false, scope = 0)), + fanout = true, + ) + + assertTrue(plan.ok) + assertEquals(listOf(Side.ACTIVE, Side.STANDBY), plan.steps.map { it.side }) + assertTrue(plan.steps.all { it.op == StepOp.DROP_TABLE }) + } + + // drop and purge are complementary: whichever applies, the other refuses. + @Test + fun `dropping a table that is already gone points at purge`() { + val plan = plan(CleanupAction.DROP_TABLE, table(active = state(present = false))) + + assertRefused(plan, Precondition.TABLE_ALREADY_GONE) + assertTrue(plan.detail!!.contains("delete the metadata"), plan.detail) + } + + @Test + fun `purging a table that still exists points at drop`() { + val plan = plan(CleanupAction.DELETE_METADATA, table(active = state(enabled = false, scope = 0))) + + assertRefused(plan, Precondition.TABLE_STILL_EXISTS) + assertTrue(plan.detail!!.contains("drop"), plan.detail) + } + + @Test + fun `unlinking deactivates labels before storages`() { + val plan = + plan( + CleanupAction.UNLINK, + table( + active = state(), + references = listOf(storage("svc.store", active = true), label("svc.label", active = true)), + ), + ) + + assertTrue(plan.ok) + assertEquals( + listOf(StepOp.DEACTIVATE_COLLECTION to "svc.label", StepOp.DEACTIVATE_STORAGE to "svc.store"), + plan.steps.map { it.op to it.name }, + ) + } + + @Test + fun `purging deletes labels before storages`() { + val plan = + plan( + CleanupAction.DELETE_METADATA, + table( + active = state(present = false), + references = listOf(storage("svc.store", active = false), label("svc.label", active = false)), + ), + ) + + assertTrue(plan.ok) + assertEquals( + listOf(StepOp.DELETE_COLLECTION to "svc.label", StepOp.DELETE_STORAGE to "svc.store"), + plan.steps.map { it.op to it.name }, + ) + } + + // Metadata is not replicated, so it is only ever touched on the active side. + @Test + fun `unlinking stays on the active side even with fanout asked for`() { + val plan = + plan( + CleanupAction.UNLINK, + table(active = state(), standby = state(), references = listOf(label("svc.label", active = true))), + fanout = true, + ) + + assertEquals(listOf(Side.ACTIVE), plan.steps.map { it.side }.distinct()) + } + + @Test + fun `unlinking with nothing bound is already satisfied`() { + val plan = plan(CleanupAction.UNLINK, table(active = state(), references = listOf(label("svc.label", active = false)))) + + assertRefused(plan, Precondition.ALREADY_SATISFIED) + } + + // Only the control plane can see this, so only the control plane can warn about it. + @Test + fun `a table another tenant still binds is refused by name`() { + val plan = + plan( + CleanupAction.DISABLE_TABLE, + table( + active = state(enabled = true, scope = 0), + references = listOf(label("other.label", active = true, tenant = "talk")), + ), + ) + + assertRefused(plan, Precondition.HELD_BY_ANOTHER_TENANT) + assertTrue(plan.detail!!.contains("talk"), plan.detail) + } + + @Test + fun `without fanout only the active side is planned`() { + val plan = + plan( + CleanupAction.REPLICATION_OFF, + table(active = state(scope = 1), standby = state(scope = 1)), + fanout = false, + ) + + assertEquals(listOf(Side.ACTIVE), plan.steps.map { it.side }) + } + + @Test + fun `a table living only on the standby is still droppable there`() { + val plan = + plan( + CleanupAction.DROP_TABLE, + table(active = state(present = false), standby = state(enabled = false, scope = 0)), + fanout = true, + ) + + assertTrue(plan.ok) + assertEquals(listOf(PlanStep(Side.STANDBY, StepOp.DROP_TABLE)), plan.steps) + } + + @Test + fun `an unknown action names the ones that exist`() { + val thrown = runCatching { CleanupAction.of("obliterate") }.exceptionOrNull() + + assertTrue(thrown is IllegalArgumentException) + assertTrue(thrown.message!!.contains("delete_metadata"), thrown.message) + } + + private fun assertRefused( + plan: CleanupPlan, + expected: Precondition, + ) { + assertFalse(plan.ok, "expected a refusal, got ${plan.steps}") + assertEquals(expected, plan.refusal) + assertTrue(plan.steps.isEmpty()) + assertTrue(!plan.detail.isNullOrBlank()) + } + + private fun plan( + action: CleanupAction, + table: TableView, + fanout: Boolean = false, + ) = CleanupPlanner.plan(action, table, fanout) + + private fun state( + present: Boolean = true, + enabled: Boolean = true, + scope: Int = 0, + ) = SideState( + cluster = "cluster:80", + present = present, + enabled = if (present) enabled else null, + replicationScope = if (present) scope else null, + ) + + private fun table( + active: SideState? = null, + standby: SideState? = null, + references: List = emptyList(), + ) = TableView( + tenant = "kc", + name = "ns:t", + sides = + buildMap { + active?.let { put(Side.ACTIVE, it) } + standby?.let { put(Side.STANDBY, it) } + }, + references = references, + sharedWith = emptyList(), + ) + + private fun label( + name: String, + active: Boolean, + tenant: String = "kc", + ) = ReferenceView(tenant, Side.ACTIVE, DatastoreTableReference.Kind.LABEL, name, active) + + private fun storage( + name: String, + active: Boolean, + tenant: String = "kc", + ) = ReferenceView(tenant, Side.ACTIVE, DatastoreTableReference.Kind.STORAGE, name, active) +} diff --git a/server/src/test/kotlin/com/kakao/actionbase/server/filter/ReadOnlyRequestFilterTest.kt b/server/src/test/kotlin/com/kakao/actionbase/server/filter/ReadOnlyRequestFilterTest.kt index 0d6012584..645114feb 100644 --- a/server/src/test/kotlin/com/kakao/actionbase/server/filter/ReadOnlyRequestFilterTest.kt +++ b/server/src/test/kotlin/com/kakao/actionbase/server/filter/ReadOnlyRequestFilterTest.kt @@ -34,7 +34,7 @@ class ReadOnlyRequestFilterTest { @Test fun `declared endpoints must match scanned controller annotations`() { val scanned = EndpointScanner.scan("com.kakao.actionbase.server.api").map { (m, p) -> "$m $p" }.toSet() - val declared = READ_ENDPOINTS + WRITE_ENDPOINTS + CONTROL_ENDPOINTS + NON_GRAPH_ENDPOINTS + val declared = READ_ENDPOINTS + WRITE_ENDPOINTS + CONTROL_ENDPOINTS + CONTROL_WRITE_ENDPOINTS + NON_GRAPH_ENDPOINTS val missing = scanned - declared val stale = declared - scanned @@ -263,6 +263,13 @@ class ReadOnlyRequestFilterTest { "GET /control/htables", ) + // Operational writes: refused by a read-only instance like any other write, but served by a + // control instance - which is why they cannot simply join WRITE_ENDPOINTS. + val CONTROL_WRITE_ENDPOINTS = + setOf( + "POST /control/jobs", + ) + val NON_GRAPH_ENDPOINTS = setOf( "GET /", @@ -284,7 +291,7 @@ class ReadOnlyRequestFilterTest { fun readEndpoints(): Stream = (READ_ENDPOINTS + CONTROL_ENDPOINTS).sorted().map { toTestArgs(it) }.stream() @JvmStatic - fun writeEndpoints(): Stream = WRITE_ENDPOINTS.sorted().map { toTestArgs(it) }.stream() + fun writeEndpoints(): Stream = (WRITE_ENDPOINTS + CONTROL_WRITE_ENDPOINTS).sorted().map { toTestArgs(it) }.stream() @JvmStatic fun nonGraphEndpoints(): Stream = diff --git a/server/src/test/kotlin/com/kakao/actionbase/server/filter/ServerRoleRequestFilterTest.kt b/server/src/test/kotlin/com/kakao/actionbase/server/filter/ServerRoleRequestFilterTest.kt index 189e6f0df..4e4ad8829 100644 --- a/server/src/test/kotlin/com/kakao/actionbase/server/filter/ServerRoleRequestFilterTest.kt +++ b/server/src/test/kotlin/com/kakao/actionbase/server/filter/ServerRoleRequestFilterTest.kt @@ -2,6 +2,7 @@ package com.kakao.actionbase.server.filter import com.kakao.actionbase.server.configuration.ServerRole import com.kakao.actionbase.server.filter.ReadOnlyRequestFilterTest.Companion.CONTROL_ENDPOINTS +import com.kakao.actionbase.server.filter.ReadOnlyRequestFilterTest.Companion.CONTROL_WRITE_ENDPOINTS import com.kakao.actionbase.server.filter.ReadOnlyRequestFilterTest.Companion.NON_GRAPH_ENDPOINTS import com.kakao.actionbase.server.filter.ReadOnlyRequestFilterTest.Companion.READ_ENDPOINTS import com.kakao.actionbase.server.filter.ReadOnlyRequestFilterTest.Companion.WRITE_ENDPOINTS @@ -139,7 +140,11 @@ class ServerRoleRequestFilterTest { fun dataEndpoints(): Stream = (READ_ENDPOINTS + WRITE_ENDPOINTS).sorted().map { ReadOnlyRequestFilterTest.toTestArgs(it) }.stream() @JvmStatic - fun controlEndpoints(): Stream = CONTROL_ENDPOINTS.sorted().map { ReadOnlyRequestFilterTest.toTestArgs(it) }.stream() + fun controlEndpoints(): Stream = + (CONTROL_ENDPOINTS + CONTROL_WRITE_ENDPOINTS) + .sorted() + .map { ReadOnlyRequestFilterTest.toTestArgs(it) } + .stream() @JvmStatic fun neutralEndpoints(): Stream = NON_GRAPH_ENDPOINTS.sorted().map { ReadOnlyRequestFilterTest.toTestArgs(it) }.stream()