diff --git a/docs/linstor-migration.md b/docs/linstor-migration.md index d482de36..f9aead48 100644 --- a/docs/linstor-migration.md +++ b/docs/linstor-migration.md @@ -11,6 +11,7 @@ Migrated: nodes, storage pools, resource groups, resource definitions (+ volume **NOT migrated — read the report on stderr for the exact list per cluster:** - **LUKS passphrases.** LUKS-encrypted volumes convert with their layer stack intact (an RD whose `layer_stack` column is empty takes the stack its replicas were built with, read from `LAYER_RESOURCE_IDS`), but the per-volume passphrase (encrypted with the LINSTOR master key) is not decrypted. You must provision `spec.encryption` for those RDs by hand before the encrypted volumes can be opened. Each affected RD is reported. +- **Definitions whose replicas disagree on the layer stack** are skipped with their replicas and reported: replicas built two ways, or with their layers in another order, under a definition with no stack of its own; a diskful replica the dump shows no layer chain for, under such a definition; or a replica built otherwise than the stack the definition names. Adopted as they are, such a replica would be brought up under a stack it was not built with, DRBD metadata over a LUKS volume's ciphertext for one. Settle the stack in LINSTOR and convert again. - **Backup-shipping remotes**, unknown/runtime flag bits, and DELETE'd / FAILED_DEPLOYMENT rows are skipped and reported. None affect live data availability. - **DRBD TCP ports** are absent from most LINSTOR 1.33 dumps — see "Capture live DRBD ports" below. diff --git a/pkg/linstormigrate/convert.go b/pkg/linstormigrate/convert.go index 3e24060e..8bedf5e9 100644 --- a/pkg/linstormigrate/convert.go +++ b/pkg/linstormigrate/convert.go @@ -133,6 +133,11 @@ type converter struct { // resource_name, then by the joined stack, to the stack itself. diskStacks map[string]map[string][]string + // chained is every live replica indexDiskStacks found a layer chain + // for. A diskful replica without one was built under a stack the + // dump does not show. + chained map[replicaKey]bool + // referential-integrity sets, populated as each kind converts so // later kinds can drop rows that would dangle (a replica of a // skipped/absent RD, a replica on an unknown node, ...). @@ -214,6 +219,7 @@ func ConvertWithOptions(dump *Dump, opts Options) (*Result, error) { storVol: map[volumeReplicaKey]LayerStorageVolumeRow{}, luksVol: map[volumeReplicaKey]LayerLuksVolumeRow{}, diskStacks: map[string]map[string][]string{}, + chained: map[replicaKey]bool{}, convertedRD: map[string]bool{}, convertedNode: map[string]bool{}, convertedRG: map[string]bool{}, @@ -439,6 +445,8 @@ func (c *converter) indexDiskStacks() { stack = append(stack, cur.LayerResourceKind) } + c.chained[replicaKey{node: row.NodeName, rd: row.ResourceName}] = true + if c.diskStacks[row.ResourceName] == nil { c.diskStacks[row.ResourceName] = map[string][]string{} } @@ -447,13 +455,80 @@ func (c *converter) indexDiskStacks() { } } +// stackDisagreement says why the RD's replicas cannot be adopted under one +// stack, or "" when they can: replicas built under different stacks when +// the RD row names none, or a replica built under another stack than the +// one the row names. It reads the row's stack without reporting a parse +// failure; rdLayerStack reports that once. +func (c *converter) stackDisagreement(row *ResourceDefinitionRow) string { + var stack []string + if !emptyList(row.LayerStack) { + _ = json.Unmarshal([]byte(row.LayerStack), &stack) + } + + // With no stack of its own, the RD takes the one its replicas were + // built with, so every diskful replica has to show it; one the dump + // carries no chain for leaves that stack unknown. A stack the row names + // is what LINSTOR built the replicas under, so a missing chain there + // says nothing new. + if len(stack) == 0 { + if node := c.unchainedReplica(row.ResourceName); node != "" { + return "no layer stack of its own, and the replica on " + c.displayNode(node) + + " has no layer chain in the dump, so the stack it was built with is unknown" + } + } + + keys := make([]string, 0, len(c.diskStacks[row.ResourceName])) + for key := range c.diskStacks[row.ResourceName] { + keys = append(keys, key) + } + + slices.Sort(keys) + + if len(stack) == 0 { + if len(keys) > 1 { + return "no layer stack of its own, and its replicas were built as " + strings.Join(keys, " and ") + } + + return "" + } + + for _, key := range keys { + if key != strings.Join(stack, ",") { + return "layer stack " + strings.Join(stack, ",") + ", but a replica was built as " + key + } + } + + return "" +} + +// unchainedReplica names the node of a live diskful replica of rdName that +// the dump carries no layer chain for, or "" when every one has its chain. +// Such a replica was built under a stack nobody can read, and taking it as +// agreeing with the others is how a LUKS volume ends up adopted as plain. +// The filter is indexDiskStacks's: no snapshot, not diskless. +func (c *converter) unchainedReplica(rdName string) string { + for i := range c.dump.Resources { + res := &c.dump.Resources[i] + if res.ResourceName != rdName || res.SnapshotName != "" || res.ResourceFlags&resourceFlagDiskless != 0 { + continue + } + + if !c.chained[replicaKey{node: res.NodeName, rd: res.ResourceName}] { + return res.NodeName + } + } + + return "" +} + // rdLayerStack is the layer stack a converted RD carries. An RD row // that names one keeps it; an empty one takes the stack its replicas // were built with, so the satellite adopts each volume as it is rather // than as the default, and the group's stack, which blockstor would -// otherwise pin, is never laid over data built without it. Replicas -// that disagree, or a stack the replicas were not built with, are -// reported for the operator to settle before adopting. +// otherwise pin, is never laid over data built without it. An RD whose +// replicas disagree, or whose stack its replicas were not built with, +// never gets here: rdConvertible skips it (stackDisagreement). func (c *converter) rdLayerStack(row *ResourceDefinitionRow, dsp string) []string { stack := c.parseList(row.LayerStack, "resource definition "+dsp+" layer_stack") onDisk := c.diskStacks[row.ResourceName] @@ -466,25 +541,14 @@ func (c *converter) rdLayerStack(row *ResourceDefinitionRow, dsp string) []strin slices.Sort(keys) if len(stack) > 0 { - for _, key := range keys { - if key != strings.Join(stack, ",") { - c.warnf("resource definition %s: layer stack %s, but a replica was built as %s — that replica will not come up as it is", dsp, strings.Join(stack, ","), key) - } - } - return stack } - switch len(keys) { - case 0: - return nil - case 1: + if len(keys) == 1 { return slices.Clone(onDisk[keys[0]]) - default: - c.warnf("resource definition %s: no layer stack of its own, and its replicas were built as %s — left unset; set spec.layerStack before adopting", dsp, strings.Join(keys, " and ")) - - return nil } + + return nil } func (c *converter) convertNodes() []crdv1alpha1.Node { @@ -1081,6 +1145,14 @@ func (c *converter) rdConvertible(row *ResourceDefinitionRow, dsp string) bool { c.warnf("resource definition %s: unhandled flags bitmask %d dropped", dsp, row.ResourceFlags) } + if reason := c.stackDisagreement(row); reason != "" { + c.warnf("resource definition %s: %s — skipped with its replicas; adopted as it is, a replica would be "+ + "brought up under a stack it was not built with, which for a LUKS volume writes DRBD metadata "+ + "over its ciphertext; settle the stack in LINSTOR and convert again", dsp, reason) + + return false + } + if reason := c.volumelessReason(row.ResourceName); reason != "" { c.warnf("resource definition %s: %s — skipped; there is no volume to adopt, and migrating it leaves the controller placing replicas the satellite can never bring up", dsp, reason) @@ -1668,7 +1740,7 @@ func (c *converter) snapshotNodesFor(rdName, snapName, snapDsp string) []string // (`"[\"DRBD\",\"STORAGE\"]"`). Empty input, `[]` and `null` all yield // nil; a malformed value is reported and yields nil. func (c *converter) parseList(raw, context string) []string { - if raw == "" || raw == "[]" || raw == "null" { + if emptyList(raw) { return nil } @@ -1688,6 +1760,11 @@ func (c *converter) parseList(raw, context string) []string { return out } +// emptyList reports a list column that holds no list: empty, `[]` or `null`. +func emptyList(raw string) bool { + return raw == "" || raw == "[]" || raw == "null" +} + func (c *converter) displayNode(name string) string { return displayName(c.nodeDsp[name], name) } diff --git a/pkg/linstormigrate/convert_test.go b/pkg/linstormigrate/convert_test.go index 030b6d4d..18f5ee73 100644 --- a/pkg/linstormigrate/convert_test.go +++ b/pkg/linstormigrate/convert_test.go @@ -1763,7 +1763,7 @@ func TestBothLatchReasonsAreReported(t *testing.T) { {NodeName: "NODE-A", PoolName: "P2", DriverName: "ZFS", UUID: "sp-2"}, }, ResourceDefinitions: []ResourceDefinitionRow{ - {ResourceName: "PVC-BOTH", ResourceDspName: "pvc-both", LayerStack: `["DRBD","STORAGE"]`, UUID: "rd-b"}, + {ResourceName: "PVC-BOTH", ResourceDspName: "pvc-both", LayerStack: `["STORAGE"]`, UUID: "rd-b"}, }, VolumeDefinitions: []VolumeDefinitionRow{ {ResourceName: "PVC-BOTH", VlmNr: 0, VlmSize: 1048576, UUID: "vd-b"}, diff --git a/pkg/linstormigrate/layer_stack_test.go b/pkg/linstormigrate/layer_stack_test.go index 908005db..5e8eff4b 100644 --- a/pkg/linstormigrate/layer_stack_test.go +++ b/pkg/linstormigrate/layer_stack_test.go @@ -153,39 +153,76 @@ func TestADisklessReplicaDoesNotShapeTheStack(t *testing.T) { } } -// Replicas built two ways leave nothing to read: the stack stays unset -// and the operator is told which ways. -func TestReplicasThatDisagreeLeaveTheStackUnset(t *testing.T) { - t.Parallel() +// withReplicas adds a diskful replica of PVC-S on both nodes, so a test can +// see whether the replicas of a refused definition still convert. +func withReplicas(dump *Dump) *Dump { + dump.Resources = append(dump.Resources, + ResourceRow{NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + ResourceRow{NodeName: "NODE-B", ResourceName: "PVC-S", UUID: "r-b"}, + ) - got, res := convertedStack(t, stackDump("[]", - layerChain(1, "NODE-A", "PVC-S", "DRBD", "LUKS", "STORAGE"), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE"))) - if len(got) != 0 { - t.Fatalf("stack %v, want unset", got) - } - - if !hasWarning(res, "built as DRBD,LUKS,STORAGE and DRBD,STORAGE") { - t.Errorf("disagreement not reported; warnings %v", res.Warnings) - } + return dump } -// A stack the RD row names is kept, and a replica built otherwise is -// reported: the satellite would try to rebuild it. -func TestAReplicaBuiltOtherwiseThanItsRDIsReported(t *testing.T) { +// A definition whose replicas cannot be adopted under one stack is not +// converted, nor are its replicas: adopted as it is, a replica would come up +// under a stack it was not built with, DRBD metadata over a LUKS volume's +// ciphertext among them. Every shape is refused and named: replicas built +// two ways, or with their layers in another order, under a row with no +// stack; a replica built otherwise than the stack the row names; and, under +// a row with no stack, a diskful replica the dump shows no chain for, alone +// or next to one that has one. +func TestADefinitionWhoseReplicasDisagreeOnTheStackIsNotConverted(t *testing.T) { t.Parallel() - got, res := convertedStack(t, stackDump(`["DRBD","LUKS","STORAGE"]`, - layerChain(1, "NODE-A", "PVC-S", "DRBD", "LUKS", "STORAGE"), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE"))) - if !slices.Equal(got, []string{"DRBD", "LUKS", "STORAGE"}) { - t.Fatalf("stack %v, want the RD row's", got) - } + luks := layerChain(1, "NODE-A", "PVC-S", "DRBD", "LUKS", "STORAGE") - if !hasWarning(res, "a replica was built as DRBD,STORAGE") { - t.Errorf("mismatch not reported; warnings %v", res.Warnings) - } + for name, tc := range map[string]struct { + rowStack, reason string + chains [][]LayerResourceIDRow + }{ + "replicas built two ways": { + rowStack: "[]", reason: "built as DRBD,LUKS,STORAGE and DRBD,STORAGE", + chains: [][]LayerResourceIDRow{luks, layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE")}, + }, + "layers in another order": { + rowStack: "[]", reason: "built as DRBD,LUKS,STORAGE and LUKS,DRBD,STORAGE", + chains: [][]LayerResourceIDRow{luks, layerChain(10, "NODE-B", "PVC-S", "LUKS", "DRBD", "STORAGE")}, + }, + "a replica built otherwise": { + rowStack: `["DRBD","LUKS","STORAGE"]`, reason: "a replica was built as DRBD,STORAGE", + chains: [][]LayerResourceIDRow{luks, layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE")}, + }, + "no chain at all": {rowStack: "[]", reason: "has no layer chain in the dump"}, + "one chain missing": { + rowStack: "[]", reason: "the replica on node-b has no layer chain in the dump", + chains: [][]LayerResourceIDRow{luks}, + }, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + res, err := Convert(withReplicas(stackDump(tc.rowStack, tc.chains...))) + if err != nil { + t.Fatalf("Convert: %v", err) + } - if hasWarning(res, "a replica was built as DRBD,LUKS,STORAGE") { - t.Errorf("a matching replica was reported; warnings %v", res.Warnings) + for i := range res.ResourceDefinitions { + if res.ResourceDefinitions[i].Name == "pvc-s" { + t.Fatalf("pvc-s converted with stack %v", res.ResourceDefinitions[i].Spec.LayerStack) + } + } + + for i := range res.Resources { + if res.Resources[i].Spec.ResourceDefinitionName == "pvc-s" { + t.Errorf("replica %s of a skipped definition converted", res.Resources[i].Name) + } + } + + if !hasWarning(res, tc.reason) || !hasWarning(res, "skipped with its replicas") { + t.Errorf("the refusal does not name %q; warnings %v", tc.reason, res.Warnings) + } + }) } }