Skip to content

Commit 020d209

Browse files
authored
Merge pull request #199 from cozystack/feat/reset-meta-safe-default
feat(commands): preserve META by default on talm reset
2 parents a00581e + a5d61ea commit 020d209

6 files changed

Lines changed: 529 additions & 19 deletions

File tree

README.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,23 @@ For example, to run a dashboard for three nodes:
224224
talm dashboard -f node1.yaml -f node2.yaml -f node3.yaml
225225
```
226226

227+
### `talm reset` — META-preserving default
228+
229+
`talm reset` diverges from upstream `talosctl reset` on one default. Upstream defaults to `--wipe-mode=all`, which wipes the Talos META partition along with STATE and EPHEMERAL — the node cannot self-recover and comes up in maintenance mode requiring a full re-apply. Talm instead populates `--system-labels-to-wipe=STATE,EPHEMERAL` when neither `--wipe-mode` nor `--system-labels-to-wipe` was passed, which preserves META so the node rejoins the cluster from its META-stored bootstrap config on the next boot.
230+
231+
Explicit operator intent is honored unchanged:
232+
233+
```bash
234+
# talm default — preserves META, node self-recovers.
235+
talm reset --reboot --graceful=true --nodes $NODE --endpoints $OTHER_NODE
236+
237+
# Explicit destructive opt-in (upstream's default).
238+
talm reset --wipe-mode=all --reboot --nodes $NODE --endpoints $OTHER_NODE
239+
240+
# Operator-specified narrower scope is honored byte-for-byte.
241+
talm reset --system-labels-to-wipe=STATE --reboot --nodes $NODE --endpoints $OTHER_NODE
242+
```
243+
227244
## Customization
228245

229246
You're free to edit template files in `./templates` directory.

docs/manual-test-plan.md

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -285,18 +285,72 @@ Expected: delete succeeds; read returns `NotFound`.
285285

286286
Expected: refuses with `etcd data directory is not empty`.
287287

288-
### H2. Reset a control-plane node (graceful + reboot, system labels only)
288+
### H2. Reset a control-plane node (talm safe default — preserves META)
289289

290-
⚠️ Destructive. Run only against a cluster you can afford to lose one node from. Requires `--system-labels-to-wipe=STATE` (and optionally `EPHEMERAL`) for a recoverable reset`--wipe-mode=all` (the default) removes META too, which makes self-recovery impossible.
290+
⚠️ Destructive. Run only against a cluster you can afford to lose one node from. The talm default populates `--system-labels-to-wipe=STATE,EPHEMERAL` automatically when neither `--wipe-mode` nor `--system-labels-to-wipe` was passed, so META survives and the node self-recovers on the next boot. Upstream `talosctl reset` defaults to `--wipe-mode=all`, which destroys META; that path is exposed in talm as the explicit `--wipe-mode=all` opt-in (see H2a).
291291

292292
```bash
293293
/tmp/talm-safety reset --graceful=true --reboot \
294-
--system-labels-to-wipe=STATE \
295-
--system-labels-to-wipe=EPHEMERAL \
296294
--nodes $NODE --endpoints $OTHER_NODE
297295
```
298296

299-
Expected: etcd member departs (`talm etcd members` from another node shows 2 members), node reboots, `post check passed`.
297+
Expected: etcd member departs (`talm etcd members` from another node shows 2 members), node reboots, `post check passed`. After the reboot the node returns to etcd as a fresh member with placeholder hostname `talos-XXXXX` within ~90s; the next `talm apply` refreshes the hostname.
298+
299+
Regression anchors:
300+
301+
- `talm reset --help` must show the talm-divergence note on both `--wipe-mode` ("preserves META") and `--system-labels-to-wipe` ("STATE,EPHEMERAL"). Without the help text, the default flip is invisible to operators reading the CLI surface.
302+
- The reset request must succeed without the operator having to type `--system-labels-to-wipe` manually. If the node comes back in maintenance mode requiring fresh apply, the wrapper did not apply the safe default and META was wiped — that is a regression.
303+
304+
### H2a. Reset with explicit destructive opt-in (`--wipe-mode=all` or `--wipe-mode=system-disk`)
305+
306+
⚠️ Highly destructive — META wiped, node CANNOT self-recover and requires fresh apply against `--insecure` maintenance mode. Run only on a cluster where the multi-day re-bootstrap cost is acceptable.
307+
308+
Two opt-out values land in the same destructive server-side branch: `--wipe-mode=all` (full system disk + user disks) and `--wipe-mode=system-disk` (system disk only). Both bypass the safety override and wipe META. `--wipe-mode=user-disks` is safe — it doesn't touch system partitions.
309+
310+
```bash
311+
# Equivalent destructive paths:
312+
/tmp/talm-safety reset --wipe-mode=all --graceful=true --reboot \
313+
--nodes $NODE --endpoints $OTHER_NODE
314+
/tmp/talm-safety reset --wipe-mode=system-disk --graceful=true --reboot \
315+
--nodes $NODE --endpoints $OTHER_NODE
316+
```
317+
318+
Expected: same as H2 up to the reboot; after the reboot the node comes up in maintenance mode (no machine config). `talm get hostnames -i --nodes $NODE` succeeds via the insecure path but the node is not yet a cluster member.
319+
320+
Regression anchor: when EITHER of these commands is run the wrapper MUST NOT silently add `--system-labels-to-wipe=STATE,EPHEMERAL` (which would override the operator's stated intent and quietly turn a destructive reset into a selective one). Verify via `talm reset --wipe-mode=all --help` or by observing that the reset request actually destroys META.
321+
322+
### H2b. Reset with operator-specified narrower scope (`--system-labels-to-wipe=STATE` only)
323+
324+
```bash
325+
/tmp/talm-safety reset --system-labels-to-wipe=STATE --graceful=true --reboot \
326+
--nodes $NODE --endpoints $OTHER_NODE
327+
```
328+
329+
Expected: only STATE wiped, EPHEMERAL kept (containerd image cache survives the reset), node returns. The operator's explicit narrower list must be honored byte-for-byte; the wrapper MUST NOT silently expand to `STATE,EPHEMERAL`.
330+
331+
Regression anchor: after the node returns, `talm dmesg --nodes $NODE | grep -i ephemeral` should show no fresh-format markers for the EPHEMERAL partition. If the wrapper silently expanded the operator's list, EPHEMERAL would have been wiped too.
332+
333+
### H2c. Reset with `--graceful=false` (ungraceful, preserves safe default)
334+
335+
```bash
336+
/tmp/talm-safety reset --graceful=false --reboot \
337+
--nodes $NODE --endpoints $OTHER_NODE
338+
```
339+
340+
Expected: ungraceful reset (no etcd leave), but the wrapper's safe default still fires (STATE+EPHEMERAL labels populated by talm because no wipe flag was passed). Node reboots; etcd cluster recovers via remaining quorum; rejoining member appears within ~120s.
341+
342+
Regression anchor: the default-flip MUST be independent of `--graceful`. A change that conditions the flip on `--graceful=true` is a regression — operators on ungraceful reset are the ones who most need the safe default.
343+
344+
### H2d. Reset triggered from modeline-bearing project root
345+
346+
```bash
347+
cd $PROJECT # directory with nodes/$NODE.yaml carrying the modeline
348+
/tmp/talm-safety reset --reboot --graceful=true
349+
```
350+
351+
Expected: same outcome as H2 — modeline supplies `--nodes` / `--endpoints` from `nodes/$NODE.yaml`, no wipe flags on the CLI, wrapper applies the safe default, META preserved.
352+
353+
Regression anchor: the default-flip is gated on `Changed("wipe-mode") && Changed("system-labels-to-wipe")` only — it is independent of where in the PreRunE chain it runs. A refactor that reorders the dispatch chain must keep this path working (modeline-supplied `--nodes` / `--endpoints` plus no operator-supplied wipe flags must still produce the safe default).
300354

301355
### H3. Etcd quorum after reset
302356

pkg/commands/init.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
"github.com/cozystack/talm/pkg/generated"
3232
"github.com/cozystack/talm/pkg/secureperm"
3333
"github.com/spf13/cobra"
34-
"golang.org/x/term"
3534
"gopkg.in/yaml.v3"
3635

3736
"github.com/siderolabs/talos/cmd/talosctl/cmd/mgmt/gen"
@@ -956,19 +955,6 @@ const (
956955
overwritePolicyNonInteractive
957956
)
958957

959-
// stdinIsTTY reports whether process stdin is connected to a
960-
// terminal. Var-typed so the unit tests can swap a fake.
961-
//
962-
// term.IsTerminal correctly returns false for /dev/null and pipes —
963-
// the naive os.Stdin.Stat()&ModeCharDevice check accepted /dev/null
964-
// (it's a character device) and led the previous version to prompt
965-
// in cron / scripted shells, EOFing the read.
966-
//
967-
//nolint:gochecknoglobals // injection seam for testability; matches stdinReader below.
968-
var stdinIsTTY = func() bool {
969-
return term.IsTerminal(int(os.Stdin.Fd()))
970-
}
971-
972958
// stdinReader is the io.Reader the interactive prompt reads from.
973959
// Var-typed so unit tests can supply canned input.
974960
//

pkg/commands/reset_handler.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Copyright Cozystack Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package commands
16+
17+
import (
18+
"github.com/cockroachdb/errors"
19+
"github.com/spf13/cobra"
20+
)
21+
22+
const (
23+
resetCmdName = "reset"
24+
25+
// resetSafeDefaultLabels are the system partition labels talm's
26+
// wrapper populates into `--system-labels-to-wipe` when an
27+
// operator runs `talm reset` without explicitly choosing a wipe
28+
// scope. Wiping STATE clears node-specific persistent state
29+
// (machine config, identity); wiping EPHEMERAL clears the
30+
// container/runtime layer. Leaving META untouched is the key
31+
// property: META carries the bootstrap config Talos uses to
32+
// rejoin the cluster on the next boot, so a reset with only
33+
// these two labels self-recovers without operator intervention.
34+
resetSafeDefaultLabels = "STATE,EPHEMERAL"
35+
)
36+
37+
// wrapResetCommand flips talm's `talm reset` default away from
38+
// upstream's destructive `--wipe-mode=all` toward the META-preserving
39+
// selective-wipe recipe. The flip only fires when the operator passed
40+
// neither `--wipe-mode` nor `--system-labels-to-wipe` on the CLI:
41+
//
42+
// - No wipe flags: PreRunE pre-populates
43+
// `--system-labels-to-wipe=STATE,EPHEMERAL`. The server-side
44+
// reset codepath, when SystemPartitionsToWipe is non-empty,
45+
// takes the label-driven path and "keep[s] other partitions
46+
// intact" per upstream's `--system-labels-to-wipe` flag doc in
47+
// `cmd/talosctl/cmd/talos/reset.go`. META survives; on the next
48+
// boot Talos rejoins the cluster from META without operator
49+
// intervention.
50+
// - Operator passed `--wipe-mode=...`: the safety override is
51+
// skipped. `--wipe-mode=all` remains the explicit destructive
52+
// opt-in; `--wipe-mode=system-disk` / `--wipe-mode=user-disks`
53+
// also bypass the flip on the assumption that the operator
54+
// stated wipe-scope intent.
55+
// - Operator passed `--system-labels-to-wipe=...`: the operator's
56+
// list is honored byte-for-byte. The wrapper does not silently
57+
// expand a narrower selection (e.g. STATE alone) to the safe
58+
// default — operators choosing a narrower scope are doing so
59+
// deliberately.
60+
//
61+
// Help-text overrides on both flags spell out the divergence so
62+
// `talm reset --help` carries the operator-facing story.
63+
//
64+
// Chain order: capture the wrapTalosCommand-installed PreRunE first,
65+
// run the flip BEFORE chaining. Order is not load-bearing here
66+
// (modeline does not touch wipe flags), but matching the shape of
67+
// the crashdump / rotate-ca wrappers keeps the dispatch site
68+
// readable.
69+
func wrapResetCommand(wrappedCmd *cobra.Command) {
70+
if wipeFlag := wrappedCmd.Flag("wipe-mode"); wipeFlag != nil {
71+
wipeFlag.Usage = "disk reset mode (talm default: --system-labels-to-wipe=" + resetSafeDefaultLabels +
72+
" preserves META so the node self-recovers; pass --wipe-mode=all or --wipe-mode=system-disk explicitly for upstream's destructive behaviour — both destroy META)"
73+
}
74+
75+
if labelsFlag := wrappedCmd.Flag("system-labels-to-wipe"); labelsFlag != nil {
76+
labelsFlag.Usage = "wipe selected system disk partitions by label, keeping others intact (talm default when no wipe flag is set: " +
77+
resetSafeDefaultLabels + ")"
78+
}
79+
80+
originalPreRunE := wrappedCmd.PreRunE
81+
82+
wrappedCmd.PreRunE = func(cmd *cobra.Command, args []string) error {
83+
if !cmd.Flags().Changed("wipe-mode") && !cmd.Flags().Changed("system-labels-to-wipe") {
84+
if err := cmd.Flags().Set("system-labels-to-wipe", resetSafeDefaultLabels); err != nil {
85+
return errors.WithHint(
86+
errors.Wrap(err, "applying talm safe-default wipe labels"),
87+
"this should not happen at runtime; if it does, fall back to passing --system-labels-to-wipe=STATE,EPHEMERAL explicitly",
88+
)
89+
}
90+
}
91+
92+
if originalPreRunE != nil {
93+
return originalPreRunE(cmd, args)
94+
}
95+
96+
return nil
97+
}
98+
}

0 commit comments

Comments
 (0)