diff --git a/docs/linstor-migration.md b/docs/linstor-migration.md index 88c0f5cc..800625c8 100644 --- a/docs/linstor-migration.md +++ b/docs/linstor-migration.md @@ -11,6 +11,8 @@ 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 key (encrypted with the LINSTOR master key) is not decrypted. blockstor opens every LUKS volume with the cluster passphrase kept in its Secret, so add that passphrase to each such volume as a key slot before adopting it: pre-flight step 7 says how. Each affected RD is reported. +- **Definitions with external DRBD metadata** are skipped with their replicas and reported: a replica with a `.meta` layer child or a meta pool on its DRBD volumes, or a replica the dump has no layer chain for while `StorPoolNameDrbdMeta` names a pool in any scope LINSTOR reads it from (volume definition, volume group, replica, definition, resource group, node). blockstor keeps DRBD metadata on the data device and does not adopt a separate metadata volume, so such a replica would be brought up with internal metadata written over the end of its data. Changing the property does not move existing metadata, so copy the data to a volume with internal metadata in LINSTOR and convert that one. For a replica without a chain, check in LINSTOR where its metadata is first: if it is on the data device, remove the property and convert again. +- **`StorPoolNameDrbdMeta` over internal metadata** does not stop the definition: when every replica's chain has no `.meta` child, the metadata is on the data device (LINSTOR applies the property only to replicas created after it was set, and `.internal` asks for internal metadata). The definition converts, and the property is not carried over to the definition or its replicas, the two places blockstor reads it from, so the satellite does not look for a separate metadata volume. The report names the scope the property was set on. - **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. A `layer_stack` column the converter cannot read counts as no stack, and the warning names its value. 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 d88ac8ee..9db8bd61 100644 --- a/pkg/linstormigrate/convert.go +++ b/pkg/linstormigrate/convert.go @@ -21,6 +21,7 @@ package linstormigrate import ( "encoding/json" "fmt" + "maps" "slices" "sort" "strconv" @@ -138,6 +139,12 @@ type converter struct { // dump does not show. chained map[replicaKey]bool + // metaPoolDropped names the definitions whose StorPoolNameDrbdMeta is + // left off the converted definition and its replicas: their replicas + // keep their metadata on the data device, and the dispatcher would + // otherwise read the property and look for a metadata volume. + metaPoolDropped map[string]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, ...). @@ -220,6 +227,7 @@ func ConvertWithOptions(dump *Dump, opts Options) (*Result, error) { luksVol: map[volumeReplicaKey]LayerLuksVolumeRow{}, diskStacks: map[string]map[string][]string{}, chained: map[replicaKey]bool{}, + metaPoolDropped: map[string]bool{}, convertedRD: map[string]bool{}, convertedNode: map[string]bool{}, convertedRG: map[string]bool{}, @@ -1113,7 +1121,7 @@ func (c *converter) convertResourceDefinitions() []crdv1alpha1.ResourceDefinitio rgRef = "" } - typed, residual, extra := k8sstore.SplitProps(c.props.ResourceDefinition(row.ResourceName)) + typed, residual, extra := k8sstore.SplitProps(c.withoutDroppedMetaPool(row.ResourceName, c.props.ResourceDefinition(row.ResourceName))) latch := c.rdInitializedLatch(row.ResourceName) @@ -1186,15 +1194,350 @@ func (c *converter) rdConvertible(row *ResourceDefinitionRow, dsp string) bool { return false } + reason, remedy, internal := c.externalMetadataReason(row) + if reason != "" { + c.warnf("resource definition %s: %s — skipped with its replicas; %s", dsp, reason, remedy) + + 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) return false } + if internal != "" { + c.metaPoolDropped[row.ResourceName] = true + c.warnf("resource definition %s: %s", dsp, internal) + } + return true } +// externalMetadataReason answers why the replicas of row cannot be adopted +// for where LINSTOR keeps their DRBD metadata, and what the operator does +// about it, both "" when nothing stands in the way. The third answer is a +// note for a definition that converts although a meta pool is set for it. +// +// LINSTOR builds a replica with external metadata with a ".meta" STORAGE +// child under its DRBD layer and records the meta pool on each DRBD volume. +// blockstor cannot adopt that layout: the satellite looks for the metadata +// under its own volume name and does not provision one, so a replica +// converted without it comes up with internal metadata and create-md lands +// on the tail of the data. +// +// StorPoolNameDrbdMeta alone is not that evidence: LINSTOR applies it to +// replicas created after it is set, and decides from the ".meta" child when +// it loads its database. A replica the dump has a chain for and no ".meta" +// keeps its metadata on the data device and converts; the property is left +// off the converted definition and replicas, the two scopes the dispatcher +// reads it from, and the other scopes convert onto objects nothing reads it +// from. A replica the dump has no chain for is decided by the property +// alone, read through every scope LINSTOR takes it from, per volume. +func (c *converter) externalMetadataReason(row *ResourceDefinitionRow) (string, string, string) { + rdName := row.ResourceName + + if node := c.externalMetadataReplica(rdName); node != "" { + return "the replica on " + c.displayNode(node) + " keeps its DRBD metadata on a separate volume", + "blockstor keeps DRBD metadata on the data device, so adopted as it is the satellite would write " + + "new metadata over the end of the volume's data; copy its data to a volume with internal metadata " + + "in LINSTOR and convert that one", "" + } + + if !c.runsDRBD(row) { + return "", "", "" + } + + var internal string + + for i := range c.dump.Resources { + res := &c.dump.Resources[i] + if res.ResourceName != rdName || res.SnapshotName != "" || + res.ResourceFlags&(resourceFlagDiskless|resourceFlagDelete) != 0 { + continue + } + + pools := c.drbdMetaPools(row, res.NodeName) + if len(pools) == 0 { + continue + } + + if !c.chained[replicaKey{node: res.NodeName, rd: rdName}] { + for _, mp := range pools { + if isExternalMetaPool(mp.pool) { + return "the dump has no layer chain for the replica on " + c.displayNode(res.NodeName) + ", and " + + storPoolNameDrbdMeta + " " + mp.pool + " on " + mp.scope + + " would put its DRBD metadata on a separate volume", + "check in LINSTOR where that replica keeps its metadata: if on a separate volume, copy its data " + + "to a volume with internal metadata and convert that one; if on the data device, remove " + + "the property and convert again", "" + } + } + } + + if internal == "" { + internal = "its replicas keep their DRBD metadata on the data device, though " + storPoolNameDrbdMeta + + " " + pools[0].pool + " is set on " + pools[0].scope + "; the property is not carried over to the " + + "definition or its replicas, so blockstor does not look for a separate metadata volume" + } + } + + return "", "", internal +} + +// runsDRBD says whether the replicas of row come up with a DRBD layer, and +// so whether a meta pool property means anything for them. A stack the row +// names decides; an empty one is the stack the replicas were built with, +// which stackDisagreement has already found to be one; with neither, the +// controller pins DRBD over STORAGE. +func (c *converter) runsDRBD(row *ResourceDefinitionRow) bool { + // An unreadable column counts as no stack, as everywhere else in the + // converter: a partial decode of it says nothing about DRBD. + stack, _ := rowLayerStack(row) + + if len(stack) == 0 { + for _, onDisk := range c.diskStacks[row.ResourceName] { + stack = onDisk + } + } + + return len(stack) == 0 || slices.ContainsFunc(stack, func(kind string) bool { return strings.EqualFold(kind, "DRBD") }) +} + +// externalMetadataReplica names the node of a replica of rdName that keeps +// its DRBD metadata on a separate volume, by its ".meta" layer child or by +// the meta pool on one of its DRBD volumes, or "" when none does. Only what +// converts counts: a replica marked DELETE, and a volume marked DELETE, do +// not. A ".meta" child the dump has no storage rows for still counts, since +// nothing says which volumes it holds. +func (c *converter) externalMetadataReplica(rdName string) string { + deleted, live := c.deletedReplicaNodes(rdName), c.liveVolumes(rdName) + + byID := map[int32]*LayerResourceIDRow{} + metaChildren := map[int32]*LayerResourceIDRow{} + + for i := range c.dump.LayerResourceIDs { + lri := &c.dump.LayerResourceIDs[i] + if lri.ResourceName != rdName || lri.SnapshotName != "" || deleted[lri.NodeName] { + continue + } + + if lri.LayerResourceSuffix == drbdMetaSuffix { + metaChildren[lri.LayerResourceID] = lri + + continue + } + + byID[lri.LayerResourceID] = lri + } + + if node := c.liveMetaChild(metaChildren, live); node != "" { + return node + } + + for i := range c.dump.LayerDrbdVolumes { + vol := &c.dump.LayerDrbdVolumes[i] + if lri, ok := byID[vol.LayerResourceID]; ok && live[vol.VlmNr] && isExternalMetaPool(vol.PoolName) { + return lri.NodeName + } + } + + return "" +} + +// deletedReplicaNodes is the nodes whose replica of rdName is marked DELETE. +func (c *converter) deletedReplicaNodes(rdName string) map[string]bool { + deleted := map[string]bool{} + + for i := range c.dump.Resources { + res := &c.dump.Resources[i] + if res.ResourceName == rdName && res.SnapshotName == "" && res.ResourceFlags&resourceFlagDelete != 0 { + deleted[res.NodeName] = true + } + } + + return deleted +} + +// liveVolumes is the volume numbers of rdName that are not marked DELETE. +func (c *converter) liveVolumes(rdName string) map[int32]bool { + live := map[int32]bool{} + + for i := range c.dump.VolumeDefinitions { + vd := &c.dump.VolumeDefinitions[i] + if vd.ResourceName == rdName && vd.SnapshotName == "" && vd.VlmFlags&resourceFlagDelete == 0 { + live[vd.VlmNr] = true + } + } + + return live +} + +// liveMetaChild names the node of a ".meta" child that holds a volume in +// live, or that the dump shows no volume for. +func (c *converter) liveMetaChild(children map[int32]*LayerResourceIDRow, live map[int32]bool) string { + if len(children) == 0 { + return "" + } + + vols := map[int32][]int32{} + + for i := range c.dump.LayerStorageVolumes { + row := &c.dump.LayerStorageVolumes[i] + if _, ok := children[row.LayerResourceID]; ok { + vols[row.LayerResourceID] = append(vols[row.LayerResourceID], row.VlmNr) + } + } + + ids := make([]int32, 0, len(children)) + for id := range children { + ids = append(ids, id) + } + + slices.Sort(ids) + + for _, id := range ids { + if len(vols[id]) == 0 || slices.ContainsFunc(vols[id], func(nr int32) bool { return live[nr] }) { + return children[id].NodeName + } + } + + return "" +} + +// metaPool is the StorPoolNameDrbdMeta one volume resolves to, and the +// scope it came from. +type metaPool struct { + pool string + scope string +} + +// drbdMetaPools is the StorPoolNameDrbdMeta LINSTOR resolves for each volume +// of the replica of row on node, for the volumes that resolve one. Each +// volume is resolved on its own, most specific scope first: volume +// definition, volume group, resource, resource definition, resource group, +// node (getExtMetaDataStorPoolName in LINSTOR's RscDrbdLayerHelper). +func (c *converter) drbdMetaPools(row *ResourceDefinitionRow, node string) []metaPool { + rd, rg := row.ResourceName, row.ResourceGroupName + + shared := func() (metaPool, bool) { + for _, scope := range []struct { + props map[string]string + name string + }{ + {c.props.Resource(node, rd), "the replica on " + c.displayNode(node)}, + {c.props.ResourceDefinition(rd), "the definition"}, + {c.resourceGroupProps(rg), "resource group " + rg}, + {c.props.Node(node), "node " + c.displayNode(node)}, + } { + if v := scope.props[storPoolNameDrbdMeta]; v != "" { + return metaPool{pool: v, scope: scope.name}, true + } + } + + return metaPool{}, false + } + + var ( + out []metaPool + vols int + ) + + for i := range c.dump.VolumeDefinitions { + vd := &c.dump.VolumeDefinitions[i] + // A volume marked DELETE is not converted, so its metadata + // setting does not decide anything for the live ones. + if vd.ResourceName != rd || vd.SnapshotName != "" || vd.VlmFlags&resourceFlagDelete != 0 { + continue + } + + vols++ + + vlm := strconv.Itoa(int(vd.VlmNr)) + if v := c.props.VolumeDefinition(rd, vlm)[storPoolNameDrbdMeta]; v != "" { + out = append(out, metaPool{pool: v, scope: "volume definition " + c.displayRD(rd) + "/" + vlm}) + + continue + } + + if v := c.resourceGroupVolumeProps(rg, vlm)[storPoolNameDrbdMeta]; v != "" { + out = append(out, metaPool{pool: v, scope: "volume group " + rg + "/" + vlm}) + + continue + } + + if mp, ok := shared(); ok { + out = append(out, mp) + } + } + + if vols == 0 { + if mp, ok := shared(); ok { + out = append(out, mp) + } + } + + return out +} + +func (c *converter) resourceGroupProps(rg string) map[string]string { + if rg == "" { + return nil + } + + return c.props.ResourceGroup(rg) +} + +func (c *converter) resourceGroupVolumeProps(rg, vlm string) map[string]string { + if rg == "" { + return nil + } + + return c.props.VolumeGroup(rg, vlm) +} + +// withoutDroppedMetaPool takes StorPoolNameDrbdMeta out of the props of a +// definition named in metaPoolDropped, or of one of its replicas. It runs +// before SplitProps, which routes a non-DRBD key to the residual props and to +// the overflow map both: the store merges the overflow back into the props on +// every write, so a copy left there returns with the next property update. +func (c *converter) withoutDroppedMetaPool(rdName string, props map[string]string) map[string]string { + if !c.metaPoolDropped[rdName] { + return props + } + + if _, ok := props[storPoolNameDrbdMeta]; !ok { + return props + } + + out := maps.Clone(props) + delete(out, storPoolNameDrbdMeta) + + if len(out) == 0 { + return nil + } + + return out +} + +// isExternalMetaPool says whether a meta pool value names a separate pool, +// read as LINSTOR v1.33.0 reads it: blank is unset, and ".internal" is matched +// in any case but untrimmed, so a value with spaces around it is a pool name. +func isExternalMetaPool(pool string) bool { + return strings.TrimSpace(pool) != "" && !strings.EqualFold(pool, drbdMetaInternal) +} + +// The layer suffix and the property LINSTOR marks external DRBD metadata +// with. +const ( + drbdMetaSuffix = ".meta" + storPoolNameDrbdMeta = "StorPoolNameDrbdMeta" + // drbdMetaInternal is the StorPoolNameDrbdMeta value that asks for + // internal metadata. + drbdMetaInternal = ".internal" +) + // rdHasVolumeDefinitions reports whether this resource definition owns a // volume that will convert. // @@ -1562,7 +1905,7 @@ func (c *converter) buildResource(row *ResourceRow, rdDsp, nodeDsp, replicaName c.reportReplicaVolumeFlags(row, replicaName) - typed, residual, extra := k8sstore.SplitProps(c.props.Resource(row.NodeName, row.ResourceName)) + typed, residual, extra := k8sstore.SplitProps(c.withoutDroppedMetaPool(row.ResourceName, c.props.Resource(row.NodeName, row.ResourceName))) resource := crdv1alpha1.Resource{ TypeMeta: typeMeta("Resource"), diff --git a/pkg/linstormigrate/external_metadata_test.go b/pkg/linstormigrate/external_metadata_test.go new file mode 100644 index 00000000..c3d1326e --- /dev/null +++ b/pkg/linstormigrate/external_metadata_test.go @@ -0,0 +1,591 @@ +// SPDX-License-Identifier: Apache-2.0 + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package linstormigrate + +import ( + "os" + "path/filepath" + "slices" + "testing" +) + +// A replica whose DRBD metadata lives on a separate volume is never adopted: +// blockstor would bring it up with internal metadata and write it over the +// end of the data. The definition and all its replicas stay behind, and the +// report says which replica and why. The evidence is a ".meta" layer child, +// a meta pool on a DRBD volume, or, for a replica the dump has no chain +// for, StorPoolNameDrbdMeta in any scope LINSTOR reads it from. +func TestADefinitionWithExternalMetadataIsSkipped(t *testing.T) { + t.Parallel() + + meta := LayerResourceIDRow{ + LayerResourceID: 20, LayerResourceKind: "STORAGE", LayerResourceParentID: ptr(int32(1)), + LayerResourceSuffix: ".meta", NodeName: "NODE-A", ResourceName: "PVC-S", + } + chained := func() *Dump { + return stackDump("[]", layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE")) + } + // The definition names its own stack, so #223 lets replicas with no + // chain through, and only the property can tell where their metadata is. + unchained := func(instance string) *Dump { + return withProps(stackDump(`["DRBD","STORAGE"]`), + PropsContainerRow{PropsInstance: instance, PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}) + } + + for name, tc := range map[string]struct { + dump *Dump + want string + }{ + "meta layer": { + dump: stackDump("[]", append(layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), meta), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE")), + want: "the replica on node-a keeps its DRBD metadata on a separate volume", + }, + "meta pool on a DRBD volume": { + dump: func() *Dump { + d := chained() + d.LayerDrbdVolumes = []LayerDrbdVolumeRow{{LayerResourceID: 10, VlmNr: 0, NodeName: "NODE-B", PoolName: "meta"}} + + return d + }(), + want: "the replica on node-b keeps its DRBD metadata on a separate volume", + }, + "no chain, pool on the volume definition": {dump: unchained("/VLM_DFNS/PVC-S/0"), want: "on volume definition pvc-s/0"}, + "no chain, pool on the volume group": {dump: unchained("/VLM_GRPS/RG/0"), want: "on volume group RG/0"}, + "no chain, pool on the replica": {dump: unchained("/RSCS/NODE-A/PVC-S"), want: "on the replica on node-a"}, + "no chain, pool on the definition": {dump: unchained("/RSC_DFNS/PVC-S"), want: "on the definition"}, + "no chain, pool on the resource group": {dump: unchained("/RSC_GRPS/RG"), want: "on resource group RG"}, + "no chain, pool on the node": {dump: unchained("/NODES/NODE-B"), want: "on node node-b"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + res := convertTwoReplicas(t, tc.dump) + + if len(res.ResourceDefinitions) != 0 || len(res.Resources) != 0 { + t.Fatalf("converted %d definitions and %d replicas, want none; warnings %v", + len(res.ResourceDefinitions), len(res.Resources), res.Warnings) + } + + if !hasWarning(res, tc.want) { + t.Errorf("the skip does not say %q; warnings %v", tc.want, res.Warnings) + } + + if !hasWarning(res, "copy its data to a volume with internal metadata") { + t.Errorf("the skip does not say how to move the data; warnings %v", res.Warnings) + } + }) + } +} + +// StorPoolNameDrbdMeta on replicas whose chains have no ".meta" child does +// not move their metadata: LINSTOR applies it to replicas created later, and +// ".internal" asks for internal metadata anyway. The definition converts, +// the property is left off the definition and its replicas, the scopes the +// dispatcher reads it from, and the report says the metadata stays where it +// is. +func TestAMetaPoolOverInternalMetadataConvertsWithoutIt(t *testing.T) { + t.Parallel() + + for name, prop := range map[string]PropsContainerRow{ + "pool on the volume definition": {PropsInstance: "/VLM_DFNS/PVC-S/0", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}, + "pool on the volume group": {PropsInstance: "/VLM_GRPS/RG/0", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}, + "pool on a replica": {PropsInstance: "/RSCS/NODE-B/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}, + "pool on the definition": {PropsInstance: "/RSC_DFNS/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}, + "pool on the resource group": {PropsInstance: "/RSC_GRPS/RG", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}, + "pool on a node": {PropsInstance: "/NODES/NODE-A", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}, + "internal on the definition": {PropsInstance: "/RSC_DFNS/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: ".internal"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + res := convertTwoReplicas(t, withProps(stackDump("[]", + layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE")), prop)) + + if len(res.ResourceDefinitions) != 1 || len(res.Resources) != 2 { + t.Fatalf("converted %d definitions and %d replicas, want the definition and both replicas; warnings %v", + len(res.ResourceDefinitions), len(res.Resources), res.Warnings) + } + + if v, ok := res.ResourceDefinitions[0].Spec.Props[storPoolNameDrbdMeta]; ok { + t.Errorf("the definition carries %s %q", storPoolNameDrbdMeta, v) + } + + for i := range res.Resources { + if v, ok := res.Resources[i].Spec.Props[storPoolNameDrbdMeta]; ok { + t.Errorf("replica %s carries %s %q", res.Resources[i].Name, storPoolNameDrbdMeta, v) + } + } + + if !hasWarning(res, "keep their DRBD metadata on the data device") || !hasWarning(res, "not carried over") { + t.Errorf("the report does not say the metadata stays internal; warnings %v", res.Warnings) + } + + if hasWarning(res, "separate volume") || hasWarning(res, "copy its data") || hasWarning(res, "skipped") { + t.Errorf("the report says the definition was skipped or the metadata is external; warnings %v", res.Warnings) + } + }) + } +} + +// LINSTOR resolves the meta pool per volume, and matches ".internal" in any +// case but untrimmed: spaces around it make it a pool name. A replica without +// a chain is external when any of its volumes resolves to a separate pool, +// and internal when every one asks for internal metadata. +func TestAMetaPoolIsResolvedPerVolumeAsLINSTORReadsIt(t *testing.T) { + t.Parallel() + + twoVolumes := func(props ...PropsContainerRow) *Dump { + d := withProps(stackDump(`["DRBD","STORAGE"]`), props...) + d.VolumeDefinitions = append(d.VolumeDefinitions, VolumeDefinitionRow{ResourceName: "PVC-S", VlmNr: 1, VlmSize: 1048576, UUID: "vd-s1"}) + + return d + } + + t.Run("internal on volume 0, a pool on the definition", func(t *testing.T) { + t.Parallel() + + res := convertTwoReplicas(t, twoVolumes( + PropsContainerRow{PropsInstance: "/VLM_DFNS/PVC-S/0", PropKey: "StorPoolNameDrbdMeta", PropValue: ".internal"}, + PropsContainerRow{PropsInstance: "/RSC_DFNS/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"})) + + if len(res.ResourceDefinitions) != 0 { + t.Fatalf("converted %d definitions, want none: volume 1 keeps its metadata on meta; warnings %v", + len(res.ResourceDefinitions), res.Warnings) + } + + if !hasWarning(res, "meta on the definition would put its DRBD metadata on a separate volume") { + t.Errorf("the skip does not name the pool volume 1 resolves to; warnings %v", res.Warnings) + } + }) + + t.Run("internal in another case", func(t *testing.T) { + t.Parallel() + + res := convertTwoReplicas(t, twoVolumes( + PropsContainerRow{PropsInstance: "/RSC_DFNS/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: ".INTERNAL"})) + + if len(res.ResourceDefinitions) != 1 || len(res.Resources) != 2 { + t.Fatalf("converted %d definitions and %d replicas, want the definition and both replicas; warnings %v", + len(res.ResourceDefinitions), len(res.Resources), res.Warnings) + } + }) + + t.Run("internal with spaces around it is a pool name", func(t *testing.T) { + t.Parallel() + + res := convertTwoReplicas(t, twoVolumes( + PropsContainerRow{PropsInstance: "/RSC_DFNS/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: " .INTERNAL "})) + + if len(res.ResourceDefinitions) != 0 { + t.Fatalf("converted %d definitions, want none: LINSTOR reads %q as a pool; warnings %v", + len(res.ResourceDefinitions), " .INTERNAL ", res.Warnings) + } + }) + + t.Run("a blank value is unset", func(t *testing.T) { + t.Parallel() + + res := convertTwoReplicas(t, twoVolumes( + PropsContainerRow{PropsInstance: "/RSC_DFNS/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: " "})) + + if len(res.ResourceDefinitions) != 1 || len(res.Resources) != 2 { + t.Fatalf("converted %d definitions and %d replicas, want the definition and both replicas; warnings %v", + len(res.ResourceDefinitions), len(res.Resources), res.Warnings) + } + }) +} + +// Rows that do not convert say nothing about where a live replica keeps its +// metadata: a diskless replica, a replica marked DELETE and a snapshot's +// rows, whatever their layout or property. +func TestRowsThatDoNotConvertDoNotDecideTheMetadata(t *testing.T) { + t.Parallel() + + meta := func(node string, id int32) LayerResourceIDRow { + return LayerResourceIDRow{ + LayerResourceID: id + 1, LayerResourceKind: "STORAGE", LayerResourceParentID: ptr(id), + LayerResourceSuffix: ".meta", NodeName: node, ResourceName: "PVC-S", + } + } + // The definition names its stack, so a replica without a chain is let + // through by #223 and only the metadata check can stop it. + named := func(chains ...[]LayerResourceIDRow) *Dump { return stackDump(`["DRBD","STORAGE"]`, chains...) } + + for name, tc := range map[string]struct { + dump *Dump + replicas []ResourceRow + }{ + "diskless replica with a pool and no chain": { + dump: withProps(named(layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE")), + PropsContainerRow{PropsInstance: "/RSCS/NODE-B/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}), + replicas: []ResourceRow{ + {NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + {NodeName: "NODE-B", ResourceName: "PVC-S", ResourceFlags: resourceFlagDiskless, UUID: "r-b"}, + }, + }, + "snapshot placement with a pool and no chain": { + dump: withProps(named(layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE")), + PropsContainerRow{PropsInstance: "/RSCS/NODE-B/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}), + replicas: []ResourceRow{ + {NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + {NodeName: "NODE-B", ResourceName: "PVC-S", SnapshotName: "SNAP", UUID: "s-b"}, + }, + }, + "snapshot volume definition with a pool": { + dump: func() *Dump { + d := withProps(named(layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE")), + PropsContainerRow{PropsInstance: "/VLM_DFNS/PVC-S/7", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}) + d.VolumeDefinitions = append(d.VolumeDefinitions, + VolumeDefinitionRow{ResourceName: "PVC-S", SnapshotName: "SNAP", VlmNr: 7, VlmSize: 1048576, UUID: "svd"}) + + return d + }(), + replicas: []ResourceRow{ + {NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + {NodeName: "NODE-B", ResourceName: "PVC-S", UUID: "r-b"}, + }, + }, + "snapshot layer row with a meta child": { + dump: func() *Dump { + d := named(layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE")) + m := meta("NODE-A", 40) + m.SnapshotName = "SNAP" + d.LayerResourceIDs = append(d.LayerResourceIDs, m) + + return d + }(), + replicas: []ResourceRow{ + {NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + {NodeName: "NODE-B", ResourceName: "PVC-S", UUID: "r-b"}, + }, + }, + "meta child on a replica marked DELETE": { + dump: named(layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), + append(layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE"), meta("NODE-B", 10))), + replicas: []ResourceRow{ + {NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + {NodeName: "NODE-B", ResourceName: "PVC-S", ResourceFlags: resourceFlagDelete, UUID: "r-b"}, + }, + }, + "internal pool and no chain": { + dump: withProps(named(layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE")), + PropsContainerRow{PropsInstance: "/RSCS/NODE-B/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: ".internal"}), + replicas: []ResourceRow{ + {NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + {NodeName: "NODE-B", ResourceName: "PVC-S", UUID: "r-b"}, + }, + }, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + tc.dump.Resources = tc.replicas + + res, err := Convert(tc.dump) + if err != nil { + t.Fatalf("Convert: %v", err) + } + + if len(res.ResourceDefinitions) != 1 { + t.Fatalf("converted %d definitions, want pvc-s; warnings %v", len(res.ResourceDefinitions), res.Warnings) + } + + if hasWarning(res, "separate volume") { + t.Errorf("the metadata was reported external; warnings %v", res.Warnings) + } + }) + } +} + +// The meta pool of a DRBD volume loads from the dump as kubectl prints it, +// a null pool included. +func TestTheDRBDVolumeMetaPoolLoadsFromTheDump(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + body := `{"items":[` + + `{"spec":{"layer_resource_id":10,"vlm_nr":0,"node_name":"NODE-B","pool_name":"meta"}},` + + `{"spec":{"layer_resource_id":11,"vlm_nr":0,"node_name":null,"pool_name":null}}]}` + + for table, data := range map[string]string{ + "layerdrbdvolumes": body, + "nodes": `{"items":[{"spec":{"node_name":"NODE-B","node_dsp_name":"node-b","node_type":2,"uuid":"n-b"}}]}`, + } { + err := os.WriteFile(filepath.Join(dir, table+dumpFileSuffix), []byte(data), 0o600) + if err != nil { + t.Fatal(err) + } + } + + dump, err := LoadDump(dir) + if err != nil { + t.Fatalf("LoadDump: %v", err) + } + + want := []LayerDrbdVolumeRow{{LayerResourceID: 10, NodeName: "NODE-B", PoolName: "meta"}, {LayerResourceID: 11}} + if !slices.Equal(dump.LayerDrbdVolumes, want) { + t.Fatalf("loaded %+v, want %+v", dump.LayerDrbdVolumes, want) + } +} + +// One definition's metadata says nothing about another's: a neighbour on +// other nodes with internal metadata still converts. +func TestExternalMetadataSkipsOnlyItsOwnDefinition(t *testing.T) { + t.Parallel() + + meta := LayerResourceIDRow{ + LayerResourceID: 20, LayerResourceKind: "STORAGE", LayerResourceParentID: ptr(int32(1)), + LayerResourceSuffix: ".meta", NodeName: "NODE-A", ResourceName: "PVC-S", + } + + for name, dump := range map[string]*Dump{ + "meta layer on the other definition": stackDump("[]", + append(layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), meta), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE")), + // PVC-T has no replica on NODE-B, so the node's property is not its. + // PVC-S names its stack and has no chain on NODE-B, so the property + // decides for it. + "meta pool on a node the other definition is not on": withProps(stackDump(`["DRBD","STORAGE"]`, + layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE")), + PropsContainerRow{PropsInstance: "/NODES/NODE-B", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}), + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + dump.ResourceDefinitions = append(dump.ResourceDefinitions, + ResourceDefinitionRow{ResourceName: "PVC-T", ResourceDspName: "pvc-t", ResourceGroupName: "RG", LayerStack: "[]", UUID: "rd-t"}) + dump.VolumeDefinitions = append(dump.VolumeDefinitions, VolumeDefinitionRow{ResourceName: "PVC-T", VlmNr: 0, VlmSize: 1048576, UUID: "vd-t"}) + dump.LayerResourceIDs = append(dump.LayerResourceIDs, layerChain(30, "NODE-A", "PVC-T", "DRBD", "STORAGE")...) + dump.Resources = []ResourceRow{ + {NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + {NodeName: "NODE-B", ResourceName: "PVC-S", UUID: "r-b"}, + {NodeName: "NODE-A", ResourceName: "PVC-T", UUID: "r-t"}, + } + + res, err := Convert(dump) + if err != nil { + t.Fatalf("Convert: %v", err) + } + + if len(res.ResourceDefinitions) != 1 || res.ResourceDefinitions[0].Name != "pvc-t" || len(res.Resources) != 1 { + t.Fatalf("converted %d definitions and %d replicas, want pvc-t and its replica; warnings %v", + len(res.ResourceDefinitions), len(res.Resources), res.Warnings) + } + }) + } +} + +// convertTwoReplicas places a replica of PVC-S on each node of dump and +// converts it. +func convertTwoReplicas(t *testing.T, dump *Dump) *Result { + t.Helper() + + dump.Resources = []ResourceRow{ + {NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + {NodeName: "NODE-B", ResourceName: "PVC-S", UUID: "r-b"}, + } + + res, err := Convert(dump) + if err != nil { + t.Fatalf("Convert: %v", err) + } + + return res +} + +func withProps(dump *Dump, rows ...PropsContainerRow) *Dump { + dump.PropsContainers = append(dump.PropsContainers, rows...) + + return dump +} + +// A meta pool says nothing about a replica that is going away or a +// definition with no DRBD layer: neither keeps DRBD metadata, so the +// definition converts. +func TestAMetaPoolWithoutDRBDMetadataBehindItConverts(t *testing.T) { + t.Parallel() + + for name, tc := range map[string]struct { + dump *Dump + replicas []ResourceRow + }{ + "on a replica marked DELETE": { + // NODE-B has no chain, so without the DELETE filter its property + // would decide alone. + dump: withProps(stackDump(`["DRBD","STORAGE"]`, layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE")), + PropsContainerRow{PropsInstance: "/RSCS/NODE-B/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}), + replicas: []ResourceRow{ + {NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + {NodeName: "NODE-B", ResourceName: "PVC-S", ResourceFlags: resourceFlagDelete, UUID: "r-b"}, + }, + }, + "on a definition without DRBD": { + dump: withProps(stackDump(`["STORAGE"]`), + PropsContainerRow{PropsInstance: "/RSC_GRPS/RG", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}), + replicas: []ResourceRow{ + {NodeName: "NODE-A", ResourceName: "PVC-S", UUID: "r-a"}, + }, + }, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + tc.dump.Resources = tc.replicas + + res, err := Convert(tc.dump) + if err != nil { + t.Fatalf("Convert: %v", err) + } + + if len(res.ResourceDefinitions) != 1 || len(res.Resources) != 1 { + t.Fatalf("converted %d definitions and %d replicas, want the definition and its live replica; warnings %v", + len(res.ResourceDefinitions), len(res.Resources), res.Warnings) + } + + if hasWarning(res, storPoolNameDrbdMeta) { + t.Errorf("the meta pool was reported; warnings %v", res.Warnings) + } + }) + } +} + +// An unreadable layer_stack counts as no stack here too, so the replicas' +// chains decide: DRBD over STORAGE with internal metadata, and the meta pool +// property is dropped instead of surviving a partial decode of the column. +func TestAnUnreadableStackStillDropsTheMetaPool(t *testing.T) { + t.Parallel() + + res := convertTwoReplicas(t, withProps(stackDump(`["STORAGE",4]`, + layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE")), + PropsContainerRow{PropsInstance: "/RSC_DFNS/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"})) + + if len(res.ResourceDefinitions) != 1 || len(res.Resources) != 2 { + t.Fatalf("converted %d definitions and %d replicas, want the definition and both replicas; warnings %v", + len(res.ResourceDefinitions), len(res.Resources), res.Warnings) + } + + rd := res.ResourceDefinitions[0] + if _, ok := rd.Spec.Props[storPoolNameDrbdMeta]; ok { + t.Errorf("the definition's props carry %s", storPoolNameDrbdMeta) + } + + if _, ok := rd.Spec.ExtraProps[storPoolNameDrbdMeta]; ok { + t.Errorf("the definition's extra props carry %s", storPoolNameDrbdMeta) + } +} + +// A volume marked DELETE is not converted, so its meta pool does not stop the +// live volumes: the definition converts with the live volume alone. +func TestADeletedVolumesMetaPoolDoesNotStopTheLiveOnes(t *testing.T) { + t.Parallel() + + dump := withProps(stackDump(`["DRBD","STORAGE"]`), + PropsContainerRow{PropsInstance: "/VLM_DFNS/PVC-S/0", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}, + PropsContainerRow{PropsInstance: "/VLM_DFNS/PVC-S/1", PropKey: "StorPoolNameDrbdMeta", PropValue: ".internal"}) + dump.VolumeDefinitions[0].VlmFlags = resourceFlagDelete + dump.VolumeDefinitions = append(dump.VolumeDefinitions, VolumeDefinitionRow{ResourceName: "PVC-S", VlmNr: 1, VlmSize: 1048576, UUID: "vd-s1"}) + + res := convertTwoReplicas(t, dump) + if len(res.ResourceDefinitions) != 1 || len(res.Resources) != 2 { + t.Fatalf("converted %d definitions and %d replicas, want the definition and both replicas; warnings %v", + len(res.ResourceDefinitions), len(res.Resources), res.Warnings) + } + + vds := res.ResourceDefinitions[0].Spec.VolumeDefinitions + if len(vds) != 1 || vds[0].VolumeNumber != 1 { + t.Errorf("volumes %+v, want volume 1 alone", vds) + } +} + +// The layout of a volume marked DELETE does not stop its live neighbour +// either: neither a meta pool on its DRBD volume nor a ".meta" child that +// holds it alone. +func TestADeletedVolumesLayoutDoesNotStopTheLiveOnes(t *testing.T) { + t.Parallel() + + twoVolumes := func(d *Dump) *Dump { + d.VolumeDefinitions[0].VlmFlags = resourceFlagDelete + d.VolumeDefinitions = append(d.VolumeDefinitions, VolumeDefinitionRow{ResourceName: "PVC-S", VlmNr: 1, VlmSize: 1048576, UUID: "vd-s1"}) + + return d + } + + for name, dump := range map[string]*Dump{ + "meta pool on the deleted volume": func() *Dump { + d := twoVolumes(stackDump(`["DRBD","STORAGE"]`, + layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE"))) + d.LayerDrbdVolumes = []LayerDrbdVolumeRow{ + {LayerResourceID: 1, VlmNr: 0, NodeName: "NODE-A", PoolName: "meta"}, + {LayerResourceID: 1, VlmNr: 1}, + {LayerResourceID: 10, VlmNr: 0, NodeName: "NODE-B", PoolName: "meta"}, + {LayerResourceID: 10, VlmNr: 1}, + } + + return d + }(), + "meta child holding only the deleted volume": func() *Dump { + d := twoVolumes(stackDump(`["DRBD","STORAGE"]`, + layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE"))) + d.LayerResourceIDs = append(d.LayerResourceIDs, LayerResourceIDRow{ + LayerResourceID: 30, LayerResourceKind: "STORAGE", LayerResourceParentID: ptr(int32(1)), + LayerResourceSuffix: ".meta", NodeName: "NODE-A", ResourceName: "PVC-S", + }) + d.LayerStorageVolumes = append(d.LayerStorageVolumes, LayerStorageVolumeRow{LayerResourceID: 30, VlmNr: 0, NodeName: "NODE-A"}) + + return d + }(), + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + res := convertTwoReplicas(t, dump) + if len(res.ResourceDefinitions) != 1 || len(res.Resources) != 2 { + t.Fatalf("converted %d definitions and %d replicas, want the definition and both replicas; warnings %v", + len(res.ResourceDefinitions), len(res.Resources), res.Warnings) + } + + vds := res.ResourceDefinitions[0].Spec.VolumeDefinitions + if len(vds) != 1 || vds[0].VolumeNumber != 1 { + t.Errorf("volumes %+v, want volume 1 alone", vds) + } + }) + } +} + +// A ".meta" child that holds a live volume still stops the definition. +func TestAMetaChildHoldingALiveVolumeStillSkips(t *testing.T) { + t.Parallel() + + d := stackDump(`["DRBD","STORAGE"]`, + layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE")) + d.VolumeDefinitions[0].VlmFlags = resourceFlagDelete + d.VolumeDefinitions = append(d.VolumeDefinitions, VolumeDefinitionRow{ResourceName: "PVC-S", VlmNr: 1, VlmSize: 1048576, UUID: "vd-s1"}) + d.LayerResourceIDs = append(d.LayerResourceIDs, LayerResourceIDRow{ + LayerResourceID: 30, LayerResourceKind: "STORAGE", LayerResourceParentID: ptr(int32(1)), + LayerResourceSuffix: ".meta", NodeName: "NODE-A", ResourceName: "PVC-S", + }) + d.LayerStorageVolumes = append(d.LayerStorageVolumes, + LayerStorageVolumeRow{LayerResourceID: 30, VlmNr: 0, NodeName: "NODE-A"}, + LayerStorageVolumeRow{LayerResourceID: 30, VlmNr: 1, NodeName: "NODE-A"}) + + res := convertTwoReplicas(t, d) + if len(res.ResourceDefinitions) != 0 { + t.Fatalf("converted %d definitions, want none: volume 1 keeps its metadata on a separate volume; warnings %v", + len(res.ResourceDefinitions), res.Warnings) + } +} diff --git a/pkg/linstormigrate/layer_stack_test.go b/pkg/linstormigrate/layer_stack_test.go index 5e8eff4b..b7552539 100644 --- a/pkg/linstormigrate/layer_stack_test.go +++ b/pkg/linstormigrate/layer_stack_test.go @@ -110,15 +110,17 @@ func TestAnEmptyRDStackIsReadOffItsReplicas(t *testing.T) { } } -// Snapshot instances and suffixed children (DRBD external metadata) -// are not the replica's data path and do not shape its stack. +// Snapshot instances and suffixed children (a cache's sub-devices) are +// not the replica's data path and do not shape its stack. External DRBD +// metadata, the ".meta" child, refuses the definition instead +// (TestADefinitionWithExternalMetadataIsSkipped). func TestSnapshotAndSuffixedLayersDoNotShapeTheStack(t *testing.T) { t.Parallel() - meta := LayerResourceIDRow{LayerResourceID: 20, LayerResourceKind: "STORAGE", LayerResourceParentID: ptr(int32(1)), LayerResourceSuffix: ".meta", NodeName: "NODE-A", ResourceName: "PVC-S"} + cache := LayerResourceIDRow{LayerResourceID: 20, LayerResourceKind: "STORAGE", LayerResourceParentID: ptr(int32(1)), LayerResourceSuffix: ".dcache", NodeName: "NODE-A", ResourceName: "PVC-S"} snap := LayerResourceIDRow{LayerResourceID: 21, LayerResourceKind: "STORAGE", NodeName: "NODE-A", ResourceName: "PVC-S", SnapshotName: "SNAP"} - got, res := convertedStack(t, stackDump("[]", layerChain(1, "NODE-A", "PVC-S", "DRBD", "LUKS", "STORAGE"), []LayerResourceIDRow{meta, snap})) + got, res := convertedStack(t, stackDump("[]", layerChain(1, "NODE-A", "PVC-S", "DRBD", "LUKS", "STORAGE"), []LayerResourceIDRow{cache, snap})) if !slices.Equal(got, []string{"DRBD", "LUKS", "STORAGE"}) { t.Fatalf("stack %v; warnings %v", got, res.Warnings) } diff --git a/pkg/linstormigrate/meta_pool_round_trip_test.go b/pkg/linstormigrate/meta_pool_round_trip_test.go new file mode 100644 index 00000000..df5eb80e --- /dev/null +++ b/pkg/linstormigrate/meta_pool_round_trip_test.go @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: Apache-2.0 + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package linstormigrate + +import ( + "testing" + + "k8s.io/apimachinery/pkg/runtime" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + crdv1alpha1 "github.com/cozystack/blockstor/api/v1alpha1" + apiv1 "github.com/cozystack/blockstor/pkg/api/v1" + k8sstore "github.com/cozystack/blockstor/pkg/store/k8s" +) + +// A meta pool left off a converted definition and its replicas stays off +// after an ordinary property update through the store: the store merges the +// overflow props back into the props on every write, so a copy left there +// would return and send the adopted replica down the external-metadata path. +func TestADroppedMetaPoolStaysOffThroughAStoreUpdate(t *testing.T) { + t.Parallel() + + res := convertTwoReplicas(t, withProps(stackDump("[]", + layerChain(1, "NODE-A", "PVC-S", "DRBD", "STORAGE"), layerChain(10, "NODE-B", "PVC-S", "DRBD", "STORAGE")), + PropsContainerRow{PropsInstance: "/RSC_DFNS/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"}, + PropsContainerRow{PropsInstance: "/RSCS/NODE-A/PVC-S", PropKey: "StorPoolNameDrbdMeta", PropValue: "meta"})) + + if len(res.ResourceDefinitions) != 1 || len(res.Resources) != 2 { + t.Fatalf("converted %d definitions and %d replicas; warnings %v", len(res.ResourceDefinitions), len(res.Resources), res.Warnings) + } + + scheme := runtime.NewScheme() + if err := crdv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + + objs := make([]ctrlclient.Object, 0, 1+len(res.Resources)) + objs = append(objs, &res.ResourceDefinitions[0]) + for i := range res.Resources { + objs = append(objs, &res.Resources[i]) + } + + cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + st := k8sstore.New(cli) + ctx := t.Context() + + addAux := func(props map[string]string) map[string]string { + if props == nil { + props = map[string]string{} + } + + props["Aux/example"] = "value" + + return props + } + + if err := st.ResourceDefinitions().PatchResourceDefinitionSpec(ctx, "pvc-s", func(rd *apiv1.ResourceDefinition) error { + rd.Props = addAux(rd.Props) + + return nil + }); err != nil { + t.Fatalf("update the definition: %v", err) + } + + if err := st.Resources().PatchResourceSpec(ctx, "pvc-s", "node-a", func(r *apiv1.Resource) error { + r.Props = addAux(r.Props) + + return nil + }); err != nil { + t.Fatalf("update the replica: %v", err) + } + + // The dispatcher takes the meta pool from these two Props maps. + var rd crdv1alpha1.ResourceDefinition + if err := cli.Get(ctx, ctrlclient.ObjectKeyFromObject(&res.ResourceDefinitions[0]), &rd); err != nil { + t.Fatalf("read the definition back: %v", err) + } + + if v, ok := rd.Spec.Props[storPoolNameDrbdMeta]; ok { + t.Errorf("the definition carries %s %q after an update", storPoolNameDrbdMeta, v) + } + + for i := range res.Resources { + var r crdv1alpha1.Resource + if err := cli.Get(ctx, ctrlclient.ObjectKeyFromObject(&res.Resources[i]), &r); err != nil { + t.Fatalf("read replica %s back: %v", res.Resources[i].Name, err) + } + + if v, ok := r.Spec.Props[storPoolNameDrbdMeta]; ok { + t.Errorf("replica %s carries %s %q after an update", r.Name, storPoolNameDrbdMeta, v) + } + } +} diff --git a/pkg/linstormigrate/model.go b/pkg/linstormigrate/model.go index 6ecfe97e..fbc48506 100644 --- a/pkg/linstormigrate/model.go +++ b/pkg/linstormigrate/model.go @@ -206,6 +206,11 @@ type LayerDrbdVolumeDefinitionRow struct { type LayerDrbdVolumeRow struct { LayerResourceID int32 `json:"layer_resource_id"` VlmNr int32 `json:"vlm_nr"` + // NodeName and PoolName name the storage pool that holds the + // volume's external DRBD metadata; both are empty for internal + // metadata. + NodeName string `json:"node_name,omitempty"` + PoolName string `json:"pool_name,omitempty"` } // LayerStorageVolumeRow is one row of LAYER_STORAGE_VOLUMES — which diff --git a/pkg/linstormigrate/props.go b/pkg/linstormigrate/props.go index 004f2600..5ad31546 100644 --- a/pkg/linstormigrate/props.go +++ b/pkg/linstormigrate/props.go @@ -116,6 +116,11 @@ func (idx *PropsIndex) VolumeDefinition(rd, vlmNr string) map[string]string { return idx.get("VLM_DFNS", rd, vlmNr) } +// VolumeGroup returns the /VLM_GRPS// props. +func (idx *PropsIndex) VolumeGroup(rg, vlmNr string) map[string]string { + return idx.get("VLM_GRPS", rg, vlmNr) +} + // Resource returns the /RSCS// props. func (idx *PropsIndex) Resource(node, rd string) map[string]string { return idx.get("RSCS", node, rd) diff --git a/site/content/en/docs/migration.md b/site/content/en/docs/migration.md index 6d2ae096..5f489f64 100644 --- a/site/content/en/docs/migration.md +++ b/site/content/en/docs/migration.md @@ -78,6 +78,8 @@ linstor-migrate -in ./dump -drbd-ports ./drbd-ports.txt -out ./blockstor-manifes Read the whole report it prints. Each `warning:` line is something the tool refused to guess. Resolve every one of these before you go on: - `LUKS passphrase NOT migrated`. +- `keeps its DRBD metadata on a separate volume`, or `would put its DRBD metadata on a separate volume` for a replica the dump has no layer chain for. The definition is skipped with its replicas: blockstor keeps DRBD metadata on the data device, and adopted as it is the replica would get new metadata over the end of its data. Changing the property does not move existing metadata. For a replica without a chain, check in LINSTOR where its metadata is first. If it is on the data device, remove the property and convert again. Otherwise, copy the data to a volume with internal metadata in LINSTOR and convert that one. +- `keep their DRBD metadata on the data device, though StorPoolNameDrbdMeta ... is set`. Nothing to do: the metadata is internal, because LINSTOR applies the property only to replicas created after it was set, and `.internal` asks for internal metadata. The definition converts, and the property is not carried over to the definition or its replicas, so blockstor does not look for a separate metadata volume. - `no DRBD port`. - `skipped with its replicas`. The definition's replicas cannot be adopted under one layer stack: they were built two ways, a diskful replica has no layer chain in the dump, or a replica was built otherwise than the stack the definition names. The reason in the line says which. An unreadable `layer_stack` column counts as no stack, and the line names its value. Adopted as it is, such a replica would come 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. - `unreadable layer_stack`. The definition's `layer_stack` column is not a list the converter can read. When its replicas agree, it takes the stack they were built with and the line names it; check that this is the stack you expect. With no diskful replica the stack is left unset and the controller pins one; see [How the stack is pinned](../layer-stack/#how-the-stack-is-pinned).