feat: stackctl stack refresh-db subcommand#32
Conversation
Adds `stackctl stack refresh-db <id>` which POSTs to the new /api/v1/stack-instances/:id/refresh-db backend endpoint to wipe the MySQL PVC and flush Redis without a full clean+redeploy. Follows the same confirmation/quiet/output pattern as `stack clean` (`--yes` to skip prompt). Prints a hint about following up with `stack deploy` to re-populate Redis via the helm sync hook. Client method Client.RefreshDBStack(id) mirrors CleanStack. No new unit tests in cli/pkg/client/client_test.go — that file has pre-existing type-migration build failures (see b4f3b61/dc9dc34) unrelated to this change. E2E validation covered by the stack manager integration tests and manual refresh-db on a running stack.
There was a problem hiding this comment.
Pull request overview
Adds a new stackctl stack refresh-db <id> command to trigger a backend “refresh DB” operation for a stack instance via a new client method and API endpoint, fitting into the existing stack command group and pkg/client API wrapper patterns.
Changes:
- Add
Client.RefreshDBStack(id)to POST to/api/v1/stack-instances/:id/refresh-db. - Add
stack refresh-dbCobra subcommand with confirmation (--yes) and quiet output behavior consistent with other stack action commands. - Wire the new subcommand into the
stackcommand group and register its flags.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cli/pkg/client/client.go | Adds a new client method for the refresh-db API endpoint returning a DeploymentLog. |
| cli/cmd/stack.go | Introduces the stack refresh-db subcommand, confirmation flow, and command registration. |
| } | ||
|
|
||
| printer.PrintMessage("Refreshing database for stack %s... (log ID: %s)", id, log.ID) | ||
| printer.PrintMessage("Run 'stackctl stack logs %s' to stream progress, or 'stackctl stack deploy %s' afterwards to re-populate Redis.", id, id) |
There was a problem hiding this comment.
stackctl stack logs prints the latest deployment log once; it does not stream/follow progress. The message here saying "stream progress" is misleading—please reword to "view"/"check" progress (or add an actual follow/streaming capability and reference that).
| printer.PrintMessage("Run 'stackctl stack logs %s' to stream progress, or 'stackctl stack deploy %s' afterwards to re-populate Redis.", id, id) | |
| printer.PrintMessage("Run 'stackctl stack logs %s' to check progress, or 'stackctl stack deploy %s' afterwards to re-populate Redis.", id, id) |
| func (c *Client) RefreshDBStack(id string) (*types.DeploymentLog, error) { | ||
| var log types.DeploymentLog | ||
| err := c.Post(fmt.Sprintf("/api/v1/stack-instances/%s/refresh-db", id), nil, &log) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &log, nil | ||
| } |
There was a problem hiding this comment.
New RefreshDBStack client method is introduced without a corresponding unit test. There are existing httptest-based unit tests for similar stack actions (e.g., CleanStack/DeployStack), so adding a small test that asserts the POST path /api/v1/stack-instances/:id/refresh-db and decodes a DeploymentLog response would keep coverage consistent.
| func (c *Client) RefreshDBStack(id string) (*types.DeploymentLog, error) { | ||
| var log types.DeploymentLog | ||
| err := c.Post(fmt.Sprintf("/api/v1/stack-instances/%s/refresh-db", id), nil, &log) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
New client method RefreshDBStack adds behavior but isn't covered by pkg/client unit tests. Please add a minimal httptest-based test (similar to CleanStack/DeployStack) that asserts it POSTs to /api/v1/stack-instances/<id>/refresh-db and decodes the returned DeploymentLog (including log ID).
| var stackRefreshDBCmd = &cobra.Command{ | ||
| Use: "refresh-db <id>", | ||
| Short: "Refresh a stack's MySQL database from the golden-db snapshot", | ||
| Long: `Refresh the MySQL database for a running stack instance without a full | ||
| clean+redeploy. Wipes the MySQL PVC so the init container re-extracts the |
There was a problem hiding this comment.
stack refresh-db introduces a new destructive subcommand but doesn't add any command-level tests. Please add tests in cli/cmd/stack_test.go mirroring the existing stack clean coverage (confirm prompt y/n behavior, --yes bypass, and --quiet printing only the log ID) to ensure this command stays stable as flags/output evolve.
- cmd/stack: reword the post-refresh hint from "stream progress" to "check progress" — `stackctl stack logs` is a point-in-time snapshot, it does not follow/tail, so the old wording overpromised. - client: add TestRefreshDBStack_Success + TestRefreshDBStack_Conflict mirroring the existing CleanStack httptest-based coverage, asserting the POST path, success decoding, and APIError on 409. Also fix the pre-existing uint→string UUID migration miss in the test packages (started in b4f3b61/dc9dc34 but left these files broken — which was keeping CI red on main and blocking any test-adding PR): - pkg/client/client_test.go: quote id literals, update `[]uint`→`[]string`, adjust ID assertions, rewrite cluster-list test shape (bare slice vs paginated envelope) to match current production type. - pkg/output/output_test.go: same mechanical fixes. - test/integration/*: quote id literals, rewrite Sscanf(%d) URL parsers to string splits, convert uint→string state maps. Package now builds and all 26 subtests pass. No production code touched; test/e2e/ left alone (those are runtime failures that need a live server, separate scope).
| state.mu.Lock() | ||
| if req.Name != "" { | ||
| def.Name = req.Name | ||
| } | ||
| if req.Description != "" { | ||
| def.Description = req.Description | ||
| } | ||
| def.Version = def.Version + "+1" | ||
| state.mu.Unlock() |
There was a problem hiding this comment.
def.Version = def.Version + "+1" produces versions like "1+1" instead of incrementing the version. Since types.Base.Version is a string, either keep it constant in the mock, or track a numeric counter and format it back to string so update responses reflect realistic version progression.
| var stackRefreshDBCmd = &cobra.Command{ | ||
| Use: "refresh-db <id>", | ||
| Short: "Refresh a stack's MySQL database from the golden-db snapshot", | ||
| Long: `Refresh the MySQL database for a running stack instance without a full | ||
| clean+redeploy. Wipes the MySQL PVC so the init container re-extracts the | ||
| golden-db snapshot on next boot, flushes Redis, and deletes the storefront | ||
| sync Job so the next ` + "`stack deploy`" + ` re-fires the Helm hook that repopulates | ||
| Redis. | ||
|
|
||
| Does NOT re-run helm — only scales deployments and runs a short-lived | ||
| cleanup Job. The stack must be in 'running' state. | ||
|
|
||
| This is a destructive operation (wipes all MySQL data for this stack). | ||
| You will be prompted for confirmation unless --yes is specified. | ||
|
|
||
| After refresh-db completes, run ` + "`stack deploy`" + ` to re-populate Redis via | ||
| the sync Job. | ||
|
|
||
| Examples: | ||
| stackctl stack refresh-db 42 | ||
| stackctl stack refresh-db 42 --yes`, | ||
| Args: cobra.ExactArgs(1), | ||
| SilenceUsage: true, | ||
| RunE: func(cmd *cobra.Command, args []string) error { |
There was a problem hiding this comment.
The new destructive stack refresh-db command isn’t covered by the existing cli/cmd/stack_test.go command tests. Please add tests that cover (1) confirmation prompt accept/decline, (2) --yes bypass, and (3) --quiet output (log ID only), similar to the stack clean tests.
…ation - cmd/stack_test.go: add four command-level tests for `stack refresh-db` mirroring the `stack clean` coverage: - WithConfirmation (prompt accept with y/n) - Declined (n → API never called, "Aborted" printed) - WithYesFlag (--yes bypasses prompt) - QuietPrintsOnlyLogID (--quiet prints only the log ID, no human prefix) Also finish the pre-existing uint→string UUID migration that was still blocking CI and caused `go vet` to fail on main. These files were missed by earlier commits b4f3b61 / dc9dc34: - cmd/bulk_test.go, cluster_test.go, definition_test.go, login_test.go, override_test.go, stack_test.go, template_test.go: quote id literals, [uint]→[]string for bulk args, drop 30 obsolete TestParseID_Zero / *_InvalidID tests (parseID now only rejects empty strings). - cmd/override.go, cmd/stack.go: two %d → %s format strings that the earlier dc9dc34 fix-up missed; these caused `go vet` to fail. - test/integration/template_definition_integration_test.go: the mock server was doing `def.Version = def.Version + "+1"`, producing versions like "1+1". Switch to a strconv.Atoi/Itoa round-trip so updates increment realistically (1 → 2 → 3). Result: `go vet ./...` clean, `go build ./...` clean, `go test ./cmd/ ./pkg/... ./test/integration/` all pass. test/e2e/ still has pre-existing runtime failures (needs a live server) — unrelated.
| func TestRefreshDBStack_Success(t *testing.T) { | ||
| t.Parallel() | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| assert.Equal(t, http.MethodPost, r.Method) | ||
| assert.Equal(t, "/api/v1/stack-instances/42/refresh-db", r.URL.Path) | ||
| w.WriteHeader(http.StatusAccepted) | ||
| json.NewEncoder(w).Encode(types.DeploymentLog{ | ||
| Base: types.Base{ID: "log-103"}, | ||
| InstanceID: "42", | ||
| Action: "refresh-db", | ||
| Status: "started", | ||
| }) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| c := New(server.URL) | ||
| log, err := c.RefreshDBStack("42") | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "log-103", log.ID) | ||
| assert.Equal(t, "refresh-db", log.Action) | ||
| } | ||
|
|
||
| func TestRefreshDBStack_Conflict(t *testing.T) { | ||
| t.Parallel() | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusConflict) | ||
| json.NewEncoder(w).Encode(map[string]string{"error": "Cannot refresh-db: instance is currently deploying"}) | ||
| })) | ||
| defer server.Close() |
There was a problem hiding this comment.
The PR description says there are no new unit tests in pkg/client/client_test.go, but this change set adds new RefreshDBStack tests (and updates many existing client tests). Please update the PR description to match what’s actually included, so reviewers and future readers aren’t misled about test coverage/state.
Summary
stackctl stack refresh-db <id>which POSTs to the new/api/v1/stack-instances/:id/refresh-dbendpoint on the stack manager (separate repo, companion PR)clean+deploycycle (3+ min with full image re-pulls)--yes/quiet-output pattern asstack cleanWhat the command does (on the server side)
Does NOT run helm. Orchestrates k8s primitives:
rm -rf /var/lib/mysql/*FLUSHALLagainst RedisAfter refresh-db the user runs
stack deployto re-fire the Helm hook that repopulates Redis viacomposer storefront-fullsync.Test coverage
pkg/client/client_test.go: addedTestRefreshDBStack_SuccessandTestRefreshDBStack_Conflict(httptest mock server — asserts the POST path and decodes theDeploymentLogresponse, mirroring the existingCleanStack/DeployStackcoverage).cmd/stack_test.go: added fourTestStackRefreshDBCmd_*tests covering the confirmation prompt,--yesbypass, decline path, and--quietlog-ID-only output — mirroring the existingstack cleancommand tests.Collateral fix
The above tests couldn't run on the initial push because
cli/pkg/client/client_test.go,cli/pkg/output/output_test.go,cli/cmd/*_test.go, andcli/test/integration/*had pre-existing build failures from the partialuint → string (UUID)migration in commits b4f3b61 / dc9dc34. This PR finishes that migration across those files (quote ID literals,[]uint → []string, one cluster-list response-shape fix, two%d → %sproduction leftovers) and fixes adef.Version + "+1"bug introduced by the first migration attempt. No production behavior change from the migration itself —go vet ./...andgo test ./...(minus pre-existingtest/e2e/runtime failures) are now clean.Test plan
go build ./...clean./bin/stackctl stack refresh-db --helprenders correctly./bin/stackctl stack refresh-db <id>against local klaravik stack, verify MySQL PVC wiped + init container re-extracts + Redis cleared🤖 Generated with Claude Code