diff --git a/charts/cozystack/templates/_helpers.tpl b/charts/cozystack/templates/_helpers.tpl index 52d88035..c06ecf0d 100644 --- a/charts/cozystack/templates/_helpers.tpl +++ b/charts/cozystack/templates/_helpers.tpl @@ -443,12 +443,35 @@ cluster: {{- end }} {{- end }} +{{- define "talos.config.zfs" }} +{{- $zfs := .Values.zfs }} +{{- if kindIs "invalid" $zfs }} +{{- $zfs = dict }} +{{- end }} +{{- if not (kindIs "map" $zfs) }} +{{- fail (printf "talm: zfs must be a mapping (got %s). Example: zfs: { exportOnShutdown: false }." (kindOf $zfs)) }} +{{- end }} +{{- $export := $zfs.exportOnShutdown }} +{{- if and (not (kindIs "invalid" $export)) (not (kindIs "bool" $export)) }} +{{- fail (printf "talm: zfs.exportOnShutdown must be true or false (got %s %q). This field decides whether a reboot can hang on the pool export, so an ambiguous YAML scalar such as a quoted \"false\" is refused." (kindOf $export) (printf "%v" $export)) }} +{{- end }} +{{- if not $export }} +--- +apiVersion: v1alpha1 +kind: ExtensionServiceConfig +name: zfs-service +environment: + - ZFS_EXPORT_TIMEOUT=0 +{{- end }} +{{- end }} + {{- define "talos.config.legacy" }} {{- include "talos.config.machine.common" . }} {{- include "talm.config.registries.legacy" . }} {{- include "talos.config.network.legacy" . }} {{- include "talos.config.cluster" . }} +{{- include "talos.config.zfs" . }} {{- end }} {{- define "talos.config.multidoc" }} @@ -457,4 +480,5 @@ cluster: {{- include "talos.config.cluster" . }} {{- include "talm.config.registries.multidoc" . }} {{- include "talos.config.network.multidoc" . }} +{{- include "talos.config.zfs" . }} {{- end }} diff --git a/charts/cozystack/values.yaml b/charts/cozystack/values.yaml index 35b58daa..e8506a26 100644 --- a/charts/cozystack/values.yaml +++ b/charts/cozystack/values.yaml @@ -115,6 +115,32 @@ allocateNodeCIDRs: true # detection is worth the shorter idle timeout. tcpKeepaliveTuning: false +zfs: + # Whether zfs-service runs `zpool export -a` on shutdown. Off by default: + # while DRBD holds the zvols the export blocks in the kernel where SIGKILL + # cannot end it, the service never stops, and the node hangs in `rebooting` + # on every reboot, upgrade and reset. With the export off the chart emits an + # ExtensionServiceConfig setting ZFS_EXPORT_TIMEOUT=0, and pools are + # re-imported with `zpool import -f` on the next boot. Set true only when + # the pool sits on dm-crypt/LUKS devices (the mapping cannot close while + # the pool holds it) or when several hosts can import the pool. LUKS on top + # of zvols and native ZFS encryption keep raw disks under the pool, so they + # work with the default. + # + # Talos v1.13.0 added the export and v1.13.4 the variable, so on + # v1.13.0-v1.13.3 the document has no effect and the hang remains. + # + # A running zfs-service keeps the environment it started with: the first + # reboot after the document is applied, including the one inside + # `talm upgrade`, still exports. For that one reboot take DRBD down by hand + # (drain, `drbdadm down all` in the node's linstor-satellite, check + # /sys/block/zd*/holders is empty, reboot); later reboots need nothing. + # + # To pick a non-zero timeout, set this to true and ship your own + # zfs-service document: a ZFS_EXPORT_TIMEOUT in the node body is appended + # next to the chart's entry instead of replacing it. + exportOnShutdown: false + # Control-plane etcd tuning. Emitted only on controlplane nodes. etcd: # Max etcd backend DB size before etcd raises a NOSPACE alarm and diff --git a/docs/configuration/talos-versions.md b/docs/configuration/talos-versions.md index d975ef18..761cfde9 100644 --- a/docs/configuration/talos-versions.md +++ b/docs/configuration/talos-versions.md @@ -39,7 +39,7 @@ A project created before those pins existed carries empty values for this key an Selected via `Chart.yaml` (`templateOptions.talosVersion`) or `--talos-version`: -- **Talos < v1.12** — single YAML document with `machine.network` and `machine.registries` sections (the shape shown in [Node files](node-files.md)). +- **Talos < v1.12** — the machine config is a single v1alpha1 document with `machine.network` and `machine.registries` sections (the shape shown in [Node files](node-files.md)). Standalone documents that older Talos already accepts, such as the cozystack preset's `ExtensionServiceConfig`, follow it. - **Talos >= v1.12** — multi-document format with separate typed documents instead of the deprecated monolithic network and registry fields. - **Talos >= v1.14** — a second round of the same move, this time for the Kubernetes settings: `machine.kubelet`, `machine.nodeLabels` and the control-plane components each get a document of their own, and a config carrying both shapes is rejected. The shipped charts still write the v1alpha1 fields, which is why the presets pin below this. @@ -54,6 +54,7 @@ For v1.12+ multi-doc output, one document is emitted per configurable link on th - `BridgeConfig` — bridges, symmetric to `BondConfig` for bonds. Ports discovered via `spec.slaveKind == "bridge"` + `spec.masterIndex`; STP / VLAN-filtering settings reach the output when the bridge controller reports them on `spec.bridgeMaster`. - `Layer2VIPConfig` — one per VIP. The `floatingIP` shorthand emits it on controlplane nodes; each `vips` entry emits one on any node, so a storage VIP works on a worker. - `RegistryMirrorConfig` and `RegistryTLSConfig` — from the `registryMirrors` and `registryTLS` values, available on both charts. +- `ExtensionServiceConfig` for `zfs-service` — cozystack chart only, emitted on the legacy format too. It sets `ZFS_EXPORT_TIMEOUT=0` so the ZFS extension skips the pool export on shutdown, and is dropped by `zfs.exportOnShutdown: true`. ### Per-link emission rules diff --git a/docs/manual-test-plan.md b/docs/manual-test-plan.md index d5a30175..edaa2ec2 100644 --- a/docs/manual-test-plan.md +++ b/docs/manual-test-plan.md @@ -506,6 +506,22 @@ Collision check: with `tcpKeepaliveTuning: true`, set `extraSysctls: { "net.ipv4 Regression anchor: `TestContract_Machine_Sysctls_DRBDTuning_Cozystack`, `TestContract_Machine_Sysctls_TCPKeepalive_*`, and `TestContract_Cluster_Etcd_QuotaBackendBytes_*` pin every branch above; `TestContract_Machine_Sysctls_DRBDTuning_AbsentOnGeneric` / `TestContract_Cluster_Etcd_QuotaBackendBytes_AbsentOnGeneric` pin that the generic preset stays free of these opinions. +### B9a. ZFS pool export on shutdown (cozystack) + +Render the cozystack preset at defaults, once with a `talosVersion` below v1.12 and once at v1.12 or later: + +```bash +talm template -f nodes/controlplane-0.yaml | yq 'select(.kind == "ExtensionServiceConfig")' +``` + +Expected on both schemas: one document with `name: zfs-service` and `environment: [ZFS_EXPORT_TIMEOUT=0]`. + +Set `zfs: { exportOnShutdown: true }` in `values.yaml` and re-render. Expected: the same query prints nothing. + +On a live node with the document applied, run one reboot first: a running `ext-zfs-service` keeps the environment it started with, so this reboot still exports and hangs while DRBD holds the zvols. Before it, take DRBD down as the `zfs` comment in `charts/cozystack/values.yaml` describes: drain the node, run `drbdadm down all` in the node's linstor-satellite, and check that `/sys/block/zd*/holders` is empty. After that reboot, `talosctl read /proc//environ` contains `ZFS_EXPORT_TIMEOUT=0`. With DRBD holding the zvols (`/sys/block/zd*/holders` lists `drbd*`), drain the node and run `talosctl reboot`. Expected: the node comes back within minutes instead of staying in `rebooting`, the pool is imported, and `linstor resource list` shows the node's resources `UpToDate`. + +Regression anchor: `TestContract_Machine_ZFSExport_*` pins the default, the opt-out, the refusal of a non-boolean `exportOnShutdown` or a non-mapping `zfs`, the generic preset staying free of the document, and a render that loads in machinery after a node body restates the document. + ### B10. Values knobs for richer instance description These knobs let a node's specifics live in values instead of a forked or hand-edited template. All default empty, so a stock render is unchanged — the golden snapshots (`TestGoldenRender`) stay byte-identical and act as the backward-compat guard. diff --git a/pkg/engine/contract_machine_test.go b/pkg/engine/contract_machine_test.go index 29f31873..3e20ac3f 100644 --- a/pkg/engine/contract_machine_test.go +++ b/pkg/engine/contract_machine_test.go @@ -29,9 +29,12 @@ package engine import ( "encoding/base64" + "os" + "path/filepath" "strings" "testing" + "github.com/siderolabs/talos/pkg/machinery/config/configloader" "gopkg.in/yaml.v3" ) @@ -1340,3 +1343,150 @@ func TestContract_Machine_RegistryTLS_PlainHostStaysBare_Multidoc_Cozystack(t *t }) assertContains(t, out, "name: mirror.example.invalid:5000") } + +// Contract: cozystack turns off the zfs-service shutdown export by +// default. `zpool export` blocks in D-state while DRBD holds the zvols; +// SIGKILL cannot end it, the process keeps the service's PID namespace +// alive, and the node hangs in `rebooting` on every reboot, upgrade and +// reset. +func TestContract_Machine_ZFSExport_DisabledByDefault_Cozystack(t *testing.T) { + for _, cell := range cozystackCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "kind: ExtensionServiceConfig") + assertContains(t, out, "name: zfs-service") + assertContains(t, out, "ZFS_EXPORT_TIMEOUT=0") + }) + } +} + +// Contract: zfs.exportOnShutdown: true drops the document, restoring the +// extension's own export for pools that sit on dm-crypt devices. +func TestContract_Machine_ZFSExport_DocumentAbsentWhenEnabled_Cozystack(t *testing.T) { + out := renderCozystackWith(t, helmEngineEmptyLookup, map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "zfs": map[string]any{"exportOnShutdown": true}, + }) + assertNotContains(t, out, "ExtensionServiceConfig") + assertNotContains(t, out, "ZFS_EXPORT_TIMEOUT") +} + +func TestContract_Machine_ZFSExport_AbsentOnGeneric(t *testing.T) { + for _, cell := range genericCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertNotContains(t, out, "ExtensionServiceConfig") + }) + } +} + +// Contract: the document survives the apply pipeline on both schemas and +// the result loads in machinery. A node body that restates the document, +// as one written by `talm template -I` does, must not produce a second +// one: Talos rejects duplicate identities. +func TestContract_Machine_ZFSExport_LoadsThroughApplyPath_Cozystack(t *testing.T) { + const nodeBody = `apiVersion: v1alpha1 +kind: ExtensionServiceConfig +name: zfs-service +environment: + - ZFS_EXPORT_TIMEOUT=0 +` + + for _, cell := range cozystackControlplaneCells() { + contract := cell.talosVersion + if contract == "" { + contract = "v1.11" + } + + t.Run(cell.name, func(t *testing.T) { + chart := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + + rendered, err := applyPatchesAndRenderConfig( + Options{KubernetesVersion: "v1.34.3", TalosVersion: contract, Full: true}, + []string{chart}) + if err != nil { + t.Fatalf("applyPatchesAndRenderConfig: %v", err) + } + + nonFull, err := applyPatchesAndRenderConfig( + Options{KubernetesVersion: "v1.34.3", TalosVersion: contract}, + []string{chart}) + if err != nil { + t.Fatalf("applyPatchesAndRenderConfig (non-full): %v", err) + } + + bodyFile := filepath.Join(t.TempDir(), "node.yaml") + if err := os.WriteFile(bodyFile, []byte(nodeBody), 0o600); err != nil { + t.Fatal(err) + } + + merged, err := MergeFileAsPatch(rendered, bodyFile) + if err != nil { + t.Fatalf("MergeFileAsPatch: %v", err) + } + + for name, out := range map[string][]byte{"rendered": rendered, "non-full": nonFull, "merged": merged} { + if got := strings.Count(string(out), "kind: ExtensionServiceConfig"); got != 1 { + t.Errorf("%s config carries %d ExtensionServiceConfig documents, want 1\n%s", name, got, out) + } + } + + for name, out := range map[string][]byte{"rendered": rendered, "merged": merged} { + if _, err := configloader.NewFromBytes(out); err != nil { + t.Errorf("Talos refuses the %s config: %v", name, err) + } + } + }) + } +} + +// Contract: the chart has no values schema, so a non-boolean +// exportOnShutdown is refused instead of read by template truthiness, +// where a quoted "false" is truthy and would drop the document. +func TestContract_Machine_ZFSExport_NonBoolFails_Cozystack(t *testing.T) { + for name, v := range map[string]any{"quoted false": "false", "quoted true": "true", "number": 0} { + t.Run(name, func(t *testing.T) { + err := renderCozystackExpectError(t, helmEngineEmptyLookup, map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "zfs": map[string]any{"exportOnShutdown": v}, + }) + if err == nil { + t.Fatalf("expected a fail-fast for exportOnShutdown %#v", v) + } + if !strings.Contains(err.Error(), "zfs.exportOnShutdown must be true or false") { + t.Errorf("error should name the field and the accepted values, got %v", err) + } + }) + } +} + +func TestContract_Machine_ZFSExport_NonMapZFSFails_Cozystack(t *testing.T) { + for name, v := range map[string]any{"string": "foo", "false": false, "list": []any{}} { + t.Run(name, func(t *testing.T) { + err := renderCozystackExpectError(t, helmEngineEmptyLookup, map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "zfs": v, + }) + if err == nil { + t.Fatalf("expected a fail-fast for zfs %#v", v) + } + if !strings.Contains(err.Error(), "zfs must be a mapping") { + t.Errorf("error should explain zfs must be a mapping, got %v", err) + } + }) + } +} + +// Contract: an unset knob keeps the default, whether zfs is absent, +// empty, or left as `zfs:` with nothing after it. +func TestContract_Machine_ZFSExport_UnsetKeepsDefault_Cozystack(t *testing.T) { + for name, v := range map[string]any{"nil": nil, "empty map": map[string]any{}, "nil knob": map[string]any{"exportOnShutdown": nil}} { + t.Run(name, func(t *testing.T) { + out := renderCozystackWith(t, helmEngineEmptyLookup, map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + "zfs": v, + }) + assertContains(t, out, "ZFS_EXPORT_TIMEOUT=0") + }) + } +} diff --git a/pkg/engine/contract_schema_test.go b/pkg/engine/contract_schema_test.go index 48bcd964..d4da0d05 100644 --- a/pkg/engine/contract_schema_test.go +++ b/pkg/engine/contract_schema_test.go @@ -133,23 +133,37 @@ func TestContract_Schema_Version112PreReleasesRenderMultidoc(t *testing.T) { } } -// Contract: legacy schema produces a single YAML document — no `---` -// separators in the body. Talos's legacy parser expects exactly one -// document. (Helm always prepends one leading newline; we check that -// no internal separator appears.) -func TestContract_Schema_LegacyIsSingleDocument(t *testing.T) { +// Contract: the legacy schema keeps the machine config in one v1alpha1 +// document. The only documents allowed after it are standalone kinds +// that pre-v1.12 Talos already loads next to v1alpha1. +func TestContract_Schema_LegacyKeepsSingleV1alpha1Document(t *testing.T) { + legacyStandaloneKinds := map[string]bool{"ExtensionServiceConfig": true} + for _, chartPath := range []string{cozystackChartPath, genericChartPath} { t.Run(chartPath, func(t *testing.T) { out := renderChartTemplate(t, chartPath, controlplaneTpl) - // Match a `---` token surrounded by ANY newline form - // (\n or \r\n) — Windows-rendered output uses CRLF and - // pinning `\n---\n` would falsely pass on Windows. - // Scan line-by-line: a single line that is exactly `---` - // means an internal document separator. + + // Split line by line on an exact `---`: CRLF output from a + // Windows render would slip past a `\n---\n` match. + docs := [][]string{nil} for line := range strings.SplitSeq(out, "\n") { - if strings.TrimRight(line, "\r") == "---" { - t.Errorf("legacy render must not contain `---` separator:\n%s", out) - break + line = strings.TrimRight(line, "\r") + if line == "---" { + docs = append(docs, nil) + continue + } + docs[len(docs)-1] = append(docs[len(docs)-1], line) + } + + for i, doc := range docs[1:] { + kind := "" + for _, line := range doc { + if k, ok := strings.CutPrefix(line, "kind: "); ok { + kind = k + } + } + if !legacyStandaloneKinds[kind] { + t.Errorf("legacy render document %d has kind %q; only the v1alpha1 document and %v may appear:\n%s", i+2, kind, legacyStandaloneKinds, out) } } }) diff --git a/pkg/engine/testdata/golden/cozystack-controlplane-legacy.golden.yaml b/pkg/engine/testdata/golden/cozystack-controlplane-legacy.golden.yaml index 0cb9a3e6..a5b28cc4 100644 --- a/pkg/engine/testdata/golden/cozystack-controlplane-legacy.golden.yaml +++ b/pkg/engine/testdata/golden/cozystack-controlplane-legacy.golden.yaml @@ -112,3 +112,9 @@ cluster: - 192.168.1.0/24 extraArgs: quota-backend-bytes: "8589934592" +--- +apiVersion: v1alpha1 +kind: ExtensionServiceConfig +name: zfs-service +environment: + - ZFS_EXPORT_TIMEOUT=0 diff --git a/pkg/engine/testdata/golden/cozystack-controlplane-multidoc.golden.yaml b/pkg/engine/testdata/golden/cozystack-controlplane-multidoc.golden.yaml index fd03b73c..9b80b260 100644 --- a/pkg/engine/testdata/golden/cozystack-controlplane-multidoc.golden.yaml +++ b/pkg/engine/testdata/golden/cozystack-controlplane-multidoc.golden.yaml @@ -120,3 +120,9 @@ addresses: - address: 192.168.201.10/24 routes: - gateway: 192.168.201.1 +--- +apiVersion: v1alpha1 +kind: ExtensionServiceConfig +name: zfs-service +environment: + - ZFS_EXPORT_TIMEOUT=0 diff --git a/pkg/engine/testdata/golden/cozystack-worker-legacy.golden.yaml b/pkg/engine/testdata/golden/cozystack-worker-legacy.golden.yaml index 9f88403a..10490999 100644 --- a/pkg/engine/testdata/golden/cozystack-worker-legacy.golden.yaml +++ b/pkg/engine/testdata/golden/cozystack-worker-legacy.golden.yaml @@ -88,3 +88,9 @@ cluster: clusterName: "cozystack" controlPlane: endpoint: "https://talm-test.invalid:6443" +--- +apiVersion: v1alpha1 +kind: ExtensionServiceConfig +name: zfs-service +environment: + - ZFS_EXPORT_TIMEOUT=0 diff --git a/pkg/engine/testdata/golden/cozystack-worker-multidoc.golden.yaml b/pkg/engine/testdata/golden/cozystack-worker-multidoc.golden.yaml index 6a52db2e..601898fa 100644 --- a/pkg/engine/testdata/golden/cozystack-worker-multidoc.golden.yaml +++ b/pkg/engine/testdata/golden/cozystack-worker-multidoc.golden.yaml @@ -96,3 +96,9 @@ addresses: - address: 192.168.201.10/24 routes: - gateway: 192.168.201.1 +--- +apiVersion: v1alpha1 +kind: ExtensionServiceConfig +name: zfs-service +environment: + - ZFS_EXPORT_TIMEOUT=0