diff --git a/README.md b/README.md index 2a42a904..f3bc7473 100644 --- a/README.md +++ b/README.md @@ -15,9 +15,44 @@ The Harvester-CSI-Driver-LVM provides the following features: - Support Volume Expansion. - Support Volume Snapshot. - Support Volume Clone. +- Support Encryption at Rest (LUKS2 / dm-crypt). **NOTE**: The Snapshot/Clone feature only works on the same nodes. Clone works for different Volume Groups. +### Encryption at Rest + +Volumes can be transparently encrypted at rest with LUKS2 (dm-crypt). Set +`encrypted: "true"` on the StorageClass and reference a CSI secret that follows +the same `CRYPTO_KEY_*` convention as Longhorn encrypted volumes — the passphrase +lives in `CRYPTO_KEY_VALUE`. Using the platform's existing encryption-secret +schema means the Harvester admission webhook and UI accept the StorageClass +unchanged: + +```yaml +parameters: + type: dm-thin + vgName: vg01 + encrypted: "true" + csi.storage.k8s.io/provisioner-secret-name: ${pvc.name}-luks + csi.storage.k8s.io/provisioner-secret-namespace: ${pvc.namespace} + csi.storage.k8s.io/node-publish-secret-name: ${pvc.name}-luks + csi.storage.k8s.io/node-publish-secret-namespace: ${pvc.namespace} +``` + +The secret must carry `CRYPTO_KEY_VALUE` (the passphrase); the optional +`CRYPTO_KEY_CIPHER`, `CRYPTO_KEY_HASH`, `CRYPTO_KEY_SIZE` and `CRYPTO_PBKDF` +fields tune `luksFormat` and default to `aes-xts-plain64` / `sha256` / `256` / +`argon2i` (Longhorn's defaults) when omitted. + +On first `NodePublishVolume` the logical volume is LUKS2-formatted and opened as +`/dev/mapper/csi-lvm-`; the filesystem (or raw block bind-mount) is placed +on the mapper so all data on the backing LV is encrypted. The passphrase is fed +to `cryptsetup` over stdin and never appears in the host process list. The +mapping is torn down on `NodeUnpublishVolume` and grown on `NodeExpandVolume`. + +See `examples/storageclass-dm-thin-encrypted.yaml`. **Losing the passphrase +makes the data unrecoverable** — manage it with a KMS-backed secret store. + ## Installation ## You can use Helm to install the Harvester-CSI-Driver-LVM by remote repo or local helm chart files. diff --git a/cmd/provisioner/clonelv.go b/cmd/provisioner/clonelv.go index a7c83b31..174267de 100644 --- a/cmd/provisioner/clonelv.go +++ b/cmd/provisioner/clonelv.go @@ -5,7 +5,6 @@ import ( "os" "strings" "syscall" - "time" "github.com/urfave/cli/v2" "k8s.io/klog/v2" @@ -88,12 +87,8 @@ func clonelv(c *cli.Context) error { klog.Infof("Clone from src: %s, to dst: %s/%s", srcLvName, dstVGName, dstLV) - if !lvm.VgExists(dstVGName) { - lvm.VgActivate() - time.Sleep(1 * time.Second) // jitter - if !lvm.VgExists(dstVGName) { - return fmt.Errorf("vg %s does not exist, please check the corresponding VG is created", dstVGName) - } + if err := ensureCloneVolumeGroups(srcVgName, dstVGName); err != nil { + return err } klog.Infof("clone lv %s, vg: %s, type: %s", srcLvName, srcVgName, srcType) @@ -139,3 +134,16 @@ func clonelv(c *cli.Context) error { return nil } + +func ensureCloneVolumeGroups(srcVGName, dstVGName string) error { + if err := lvm.EnsureVG(srcVGName); err != nil { + return fmt.Errorf("source volume group is unavailable: %w", err) + } + if dstVGName == srcVGName { + return nil + } + if err := lvm.EnsureVG(dstVGName); err != nil { + return fmt.Errorf("destination volume group is unavailable: %w", err) + } + return nil +} diff --git a/cmd/provisioner/createlv.go b/cmd/provisioner/createlv.go index 85af54f8..83b67b91 100644 --- a/cmd/provisioner/createlv.go +++ b/cmd/provisioner/createlv.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "time" "github.com/urfave/cli/v2" "k8s.io/klog/v2" @@ -61,12 +60,8 @@ func createLV(c *cli.Context) error { klog.Infof("create lv %s size:%d vg:%s type:%s", lvName, lvSize, vgName, lvmType) - if !lvm.VgExists(vgName) { - lvm.VgActivate() - time.Sleep(1 * time.Second) // jitter - if !lvm.VgExists(vgName) { - return fmt.Errorf("vg %s does not exist, please check the corresponding VG is created", vgName) - } + if err := lvm.EnsureVG(vgName); err != nil { + return err } output, err := lvm.CreateLVS(vgName, lvName, lvSize, lvmType) diff --git a/cmd/provisioner/createsnap.go b/cmd/provisioner/createsnap.go index 7dcb456b..2a8b4da9 100644 --- a/cmd/provisioner/createsnap.go +++ b/cmd/provisioner/createsnap.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "time" "github.com/urfave/cli/v2" "k8s.io/klog/v2" @@ -69,12 +68,8 @@ func createSnap(c *cli.Context) error { klog.Infof("create snapshot: %s source size: %d source lv: %s/%s", snapName, lvSize, vgName, lvName) - if !lvm.VgExists(vgName) { - lvm.VgActivate() - time.Sleep(1 * time.Second) // jitter - if !lvm.VgExists(vgName) { - return fmt.Errorf("vg %s does not exist, please check the corresponding VG is created", vgName) - } + if err := lvm.EnsureVG(vgName); err != nil { + return err } output, err := lvm.CreateSnapshot(snapName, lvName, vgName, lvSize, lvType, !createSnapshotForClone) diff --git a/cmd/provisioner/deletelv.go b/cmd/provisioner/deletelv.go index 51a458b6..2ee43d89 100644 --- a/cmd/provisioner/deletelv.go +++ b/cmd/provisioner/deletelv.go @@ -52,7 +52,7 @@ func deleteLV(c *cli.Context) error { klog.Infof("delete lv %s", lvName) - output, err := lvm.RemoveLVS(lvName) + output, err := lvm.RemoveLVSInVG(vgName, lvName) if err != nil { return fmt.Errorf("unable to delete lv: %w output:%s", err, output) } diff --git a/cmd/provisioner/deletesnap.go b/cmd/provisioner/deletesnap.go index 87a0fedf..d10b086c 100644 --- a/cmd/provisioner/deletesnap.go +++ b/cmd/provisioner/deletesnap.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "time" "github.com/urfave/cli/v2" "k8s.io/klog/v2" @@ -45,17 +44,9 @@ func deleteSnap(c *cli.Context) error { klog.Infof("delete snapshot: %s/%s", vgName, snapName) - if !lvm.VgExists(vgName) { - lvm.VgActivate() - time.Sleep(1 * time.Second) // jitter - if !lvm.VgExists(vgName) { - return fmt.Errorf("vg %s does not exist, please check the corresponding VG is created", vgName) - } - } - output, err := lvm.DeleteSnapshot(snapName, vgName) if err != nil { - return fmt.Errorf("unable to create Snapshot: %w output:%s", err, output) + return fmt.Errorf("unable to delete snapshot: %w output:%s", err, output) } klog.Infof("Snapshot: %s/%s deleted", vgName, snapName) return nil diff --git a/cmd/provisioner/main.go b/cmd/provisioner/main.go index e0891fd2..1778eb25 100644 --- a/cmd/provisioner/main.go +++ b/cmd/provisioner/main.go @@ -22,12 +22,8 @@ const ( snapshotPrefix = "lvm-snapshot-" ) -func cmdNotFound(_ *cli.Context, command string) { - panic(fmt.Errorf("unrecognized command: %s", command)) -} - -func onUsageError(_ *cli.Context, _ error, _ bool) error { - panic(fmt.Errorf("usage error, please check your command")) +func onUsageError(_ *cli.Context, err error, _ bool) error { + return fmt.Errorf("usage error: %w", err) } func main() { @@ -40,7 +36,6 @@ func main() { deleteSnapCmd(), cloneLVCmd(), } - p.CommandNotFound = cmdNotFound p.OnUsageError = onUsageError klog.Infof("starting csi-lvmplugin-provisioner") diff --git a/deploy/charts/templates/rbac.yaml b/deploy/charts/templates/rbac.yaml index e92b30b5..57c8630c 100644 --- a/deploy/charts/templates/rbac.yaml +++ b/deploy/charts/templates/rbac.yaml @@ -21,6 +21,13 @@ rules: - apiGroups: [""] resources: ["nodes"] verbs: ["list", "get", "watch"] + # Encryption at rest: the external-provisioner sidecar resolves the + # csi.storage.k8s.io/provisioner-secret-name/-namespace referenced by an + # encrypted StorageClass. With ${pvc.namespace} templating the secret can live + # in any namespace, so this must be cluster-wide (matches Longhorn's chart). + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get", "list", "watch"] - apiGroups: ["storage.k8s.io"] resources: ["csinodes"] verbs: ["get", "list", "watch"] @@ -50,6 +57,30 @@ roleRef: name: harvester-csi-driver-lvm apiGroup: rbac.authorization.k8s.io --- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: harvester-csi-driver-lvm-snapshot-locations + namespace: {{ .Release.Namespace }} +rules: + - apiGroups: [""] + resources: ["configmaps"] + verbs: ["get", "create", "delete"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: harvester-csi-driver-lvm-snapshot-locations + namespace: {{ .Release.Namespace }} +subjects: + - kind: ServiceAccount + name: harvester-csi-driver-lvm + namespace: {{ .Release.Namespace }} +roleRef: + kind: Role + name: harvester-csi-driver-lvm-snapshot-locations + apiGroup: rbac.authorization.k8s.io +--- apiVersion: v1 kind: ServiceAccount metadata: @@ -91,4 +122,4 @@ roleRef: subjects: - kind: ServiceAccount name: harvester-csi-driver-lvm-webhook - namespace: {{ .Release.Namespace }} \ No newline at end of file + namespace: {{ .Release.Namespace }} diff --git a/examples/pre-existing-volume-snapshot-source.yaml b/examples/pre-existing-volume-snapshot-source.yaml new file mode 100644 index 00000000..fcfa1f57 --- /dev/null +++ b/examples/pre-existing-volume-snapshot-source.yaml @@ -0,0 +1,71 @@ +apiVersion: snapshot.storage.k8s.io/v1 +deletionPolicy: Retain +driver: lvm.driver.harvesterhci.io +kind: VolumeSnapshotClass +metadata: + name: lvm-snapshot-retain +--- +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + annotations: + cdi.harvesterhci.io/storageProfileCloneStrategy: snapshot + cdi.harvesterhci.io/storageProfileVolumeModeAccessModes: '{"Block":["ReadWriteOnce"]}' + cdi.harvesterhci.io/storageProfileVolumeSnapshotClass: lvm-snapshot-retain + name: lvm-pre-existing-demo +parameters: + type: dm-thin + vgName: VOLUME_GROUP_NAME +provisioner: lvm.driver.harvesterhci.io +reclaimPolicy: Delete +volumeBindingMode: WaitForFirstConsumer +allowVolumeExpansion: false +allowedTopologies: +- matchLabelExpressions: + - key: topology.lvm.csi/node + values: + - NODE_THAT_HAS_THE_VOLUME_GROUP +--- +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: pre-existing-source-pvc + namespace: default +spec: + storageClassName: lvm-pre-existing-demo + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + volumeMode: Block +--- +apiVersion: v1 +kind: Pod +metadata: + name: pre-existing-source-pod + namespace: default +spec: + containers: + - name: ubuntu-jammy-container + image: ubuntu:jammy + securityContext: + privileged: true + command: ["/bin/bash", "-c", "--"] + args: ["while true; do sleep 30; done;"] + volumeDevices: + - devicePath: "/volumes/pre-existing-source-pvc" + name: pre-existing-source-pvc + affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: + - matchExpressions: + - key: topology.lvm.csi/node + operator: In + values: + - NODE_THAT_HAS_THE_VOLUME_GROUP + volumes: + - name: pre-existing-source-pvc + persistentVolumeClaim: + claimName: pre-existing-source-pvc diff --git a/examples/pre-existing-volume-snapshot.yaml b/examples/pre-existing-volume-snapshot.yaml new file mode 100644 index 00000000..1d23e092 --- /dev/null +++ b/examples/pre-existing-volume-snapshot.yaml @@ -0,0 +1,55 @@ +# Case 1: Explicit location annotations. Use this for legacy snapshots or +# snapshots created outside this driver when no persisted location record exists. +apiVersion: snapshot.storage.k8s.io/v1 +kind: VolumeSnapshotContent +metadata: + name: pre-existing-lvm-snapshot-content + annotations: + lvm.driver.harvesterhci.io/nodeName: NODE_THAT_HAS_THE_LV_SNAPSHOT + lvm.driver.harvesterhci.io/vgName: VOLUME_GROUP_NAME +spec: + deletionPolicy: Delete + driver: lvm.driver.harvesterhci.io + source: + snapshotHandle: SNAPSHOT_NAME_WITHOUT_LVM_PREFIX + volumeSnapshotClassName: lvm-snapshot + volumeSnapshotRef: + name: pre-existing-lvm-snapshot + namespace: default +--- +apiVersion: snapshot.storage.k8s.io/v1 +kind: VolumeSnapshot +metadata: + name: pre-existing-lvm-snapshot + namespace: default +spec: + source: + volumeSnapshotContentName: pre-existing-lvm-snapshot-content + volumeSnapshotClassName: lvm-snapshot +--- +# Case 2: No location annotations. This works when the snapshot was originally +# created by this driver and its persisted location record is still available, +# including after deleting a Retain-policy VolumeSnapshotContent. +apiVersion: snapshot.storage.k8s.io/v1 +kind: VolumeSnapshotContent +metadata: + name: pre-existing-recorded-lvm-snapshot-content +spec: + deletionPolicy: Delete + driver: lvm.driver.harvesterhci.io + source: + snapshotHandle: SNAPSHOT_HANDLE_FROM_LOCATION_RECORD + volumeSnapshotClassName: lvm-snapshot + volumeSnapshotRef: + name: pre-existing-recorded-lvm-snapshot + namespace: default +--- +apiVersion: snapshot.storage.k8s.io/v1 +kind: VolumeSnapshot +metadata: + name: pre-existing-recorded-lvm-snapshot + namespace: default +spec: + source: + volumeSnapshotContentName: pre-existing-recorded-lvm-snapshot-content + volumeSnapshotClassName: lvm-snapshot diff --git a/examples/storageclass-dm-thin-encrypted.yaml b/examples/storageclass-dm-thin-encrypted.yaml new file mode 100644 index 00000000..6bf817d1 --- /dev/null +++ b/examples/storageclass-dm-thin-encrypted.yaml @@ -0,0 +1,60 @@ +# Encrypted-at-rest dm-thin StorageClass (LUKS2 / dm-crypt). +# +# Each volume provisioned from this class is LUKS2-formatted and opened as a +# dm-crypt device on the node at NodePublishVolume, so all data written to the +# backing logical volume is encrypted at rest. The passphrase and LUKS tuning +# are supplied via a CSI secret that follows the same CRYPTO_KEY_* convention as +# Longhorn encrypted volumes, so the Harvester webhook and UI accept it +# unchanged. The passphrase is fed to cryptsetup over stdin, never via argv. +# +# IMPORTANT: +# - The referenced secret must contain CRYPTO_KEY_VALUE (the passphrase). The +# remaining CRYPTO_KEY_* fields are optional tuning knobs; when omitted the +# driver applies Longhorn's defaults (aes-xts-plain64 / sha256 / 256 / argon2i). +# - Losing the passphrase means the data is unrecoverable. +# - ${pvc.name}/${pvc.namespace} templating lets every PVC use its own secret; +# replace with a fixed name/namespace if you prefer a single shared key. +allowVolumeExpansion: true +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: lvm-dm-thin-encrypted + annotations: + # Clone via host-assisted copy, not snapshot smart-clone. Cloning an + # unencrypted source (e.g. a VM image) into this class must write the data + # THROUGH the target's dm-crypt mapper so it lands encrypted; a block-level + # smart-clone would bypass LUKS. Required for CDI image clones on Harvester. + cdi.harvesterhci.io/storageProfileCloneStrategy: copy +parameters: + type: dm-thin + vgName: vg01 + encrypted: "true" + csi.storage.k8s.io/provisioner-secret-name: ${pvc.name}-luks + csi.storage.k8s.io/provisioner-secret-namespace: ${pvc.namespace} + csi.storage.k8s.io/node-publish-secret-name: ${pvc.name}-luks + csi.storage.k8s.io/node-publish-secret-namespace: ${pvc.namespace} +provisioner: lvm.driver.harvesterhci.io +reclaimPolicy: Delete +volumeBindingMode: WaitForFirstConsumer +allowedTopologies: +- matchLabelExpressions: + - key: topology.lvm.csi/node + values: + - +--- +# Example encryption secret for a PVC named "my-encrypted-pvc". +# In production, generate a strong random passphrase and manage it out of band +# (e.g. sealed-secrets, external-secrets, or a KMS-backed provider). +apiVersion: v1 +kind: Secret +metadata: + name: my-encrypted-pvc-luks + namespace: default +type: Opaque +stringData: + CRYPTO_KEY_VALUE: "replace-with-a-strong-random-passphrase" + CRYPTO_KEY_PROVIDER: "secret" + CRYPTO_KEY_CIPHER: "aes-xts-plain64" + CRYPTO_KEY_HASH: "sha256" + CRYPTO_KEY_SIZE: "256" + CRYPTO_PBKDF: "argon2i" diff --git a/package/Dockerfile b/package/Dockerfile index 8a548d42..87bd6858 100644 --- a/package/Dockerfile +++ b/package/Dockerfile @@ -3,7 +3,7 @@ FROM registry.suse.com/bci/bci-base:16.0 RUN zypper -n rm container-suseconnect && \ - zypper -n install util-linux util-linux-systemd lvm2 e2fsprogs nvme-cli device-mapper xfsprogs && \ + zypper -n install util-linux util-linux-systemd lvm2 e2fsprogs nvme-cli device-mapper xfsprogs cryptsetup && \ zypper -n clean -a && rm -rf /tmp/* /var/tmp/* /usr/share/doc/packages/* ARG TARGETPLATFORM diff --git a/pkg/lvm/controllerserver.go b/pkg/lvm/controllerserver.go index 988677bd..3d004472 100644 --- a/pkg/lvm/controllerserver.go +++ b/pkg/lvm/controllerserver.go @@ -17,9 +17,8 @@ limitations under the License. package lvm import ( + "crypto/sha256" "fmt" - "strconv" - "strings" "time" "github.com/container-storage-interface/spec/lib/go/csi" @@ -41,13 +40,24 @@ type controllerServer struct { caps []*csi.ControllerServiceCapability nodeID string hostWritePath string - kubeClient kubernetes.Clientset + kubeClient kubernetes.Interface provisionerImage string pullPolicy v1.PullPolicy namespace string - snapClient *snapclient.Clientset + snapClient snapclient.Interface } +const ( + snapshotNodeAnnotation = "lvm.driver.harvesterhci.io/nodeName" + snapshotVGAnnotation = "lvm.driver.harvesterhci.io/vgName" + + snapshotLocationConfigMapPrefix = "csi-lvm-snapshot-location-" + snapshotLocationLabel = "lvm.driver.harvesterhci.io/snapshot-location" + snapshotLocationHandleKey = "snapshotHandle" + snapshotLocationNodeKey = "nodeName" + snapshotLocationVGKey = "vgName" +) + // NewControllerServer func newControllerServer(nodeID string, hostWritePath string, namespace string, provisionerImage string, pullPolicy v1.PullPolicy) (*controllerServer, error) { config, err := rest.InClusterConfig() @@ -76,7 +86,7 @@ func newControllerServer(nodeID string, hostWritePath string, namespace string, }), nodeID: nodeID, hostWritePath: hostWritePath, - kubeClient: *kubeClient, + kubeClient: kubeClient, namespace: namespace, provisionerImage: provisionerImage, pullPolicy: pullPolicy, @@ -94,100 +104,41 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol if len(req.GetName()) == 0 { return nil, status.Error(codes.InvalidArgument, "Name missing in request") } - caps := req.GetVolumeCapabilities() - if caps == nil { - return nil, status.Error(codes.InvalidArgument, "Volume Capabilities missing in request") + if err := validateVolumeCapabilities(req.GetVolumeCapabilities()); err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } - // Keep a record of the requested access types. - var accessTypeMount, accessTypeBlock bool - - for _, cap := range caps { - if cap.GetBlock() != nil { - accessTypeBlock = true - } - if cap.GetMount() != nil { - accessTypeMount = true - } + lvmType, vgName, err := parseLVMParameters(req.GetParameters()) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } - if accessTypeBlock && accessTypeMount { - return nil, status.Error(codes.InvalidArgument, "cannot have both block and mount access type") + requiredBytes, err := validateCapacityRange(req.GetCapacityRange()) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } - lvmType := req.GetParameters()["type"] - if lvmType != "striped" && lvmType != "dm-thin" { - return nil, status.Errorf(codes.Internal, "lvmType is incorrect: %s", lvmType) - } + volumeContext := buildVolumeContext(req.GetParameters(), requiredBytes) - vgName := req.GetParameters()["vgName"] - if vgName == "" { - return nil, status.Error(codes.InvalidArgument, "vgName is missing, please check the storage class") + node, topology, err := topologyFromAccessibility(req.GetAccessibilityRequirements()) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } - - volumeContext := req.GetParameters() - size := strconv.FormatInt(req.GetCapacityRange().GetRequiredBytes(), 10) - - volumeContext["RequiredBytes"] = size - - // schedulded node of the pod is the first entry in the preferred segment - node := req.GetAccessibilityRequirements().GetPreferred()[0].GetSegments()[topologyKeyNode] - topology := []*csi.Topology{{ - Segments: map[string]string{topologyKeyNode: node}, - }} klog.Infof("creating volume %s on node: %s", req.GetName(), node) - if req.GetVolumeContentSource() != nil { - klog.Infof("cloning volume with source: %v", req.GetVolumeContentSource()) - volumeSource := req.VolumeContentSource - switch volumeSource.Type.(type) { - case *csi.VolumeContentSource_Snapshot: - srcSnapID := volumeSource.GetSnapshot().GetSnapshotId() - snapContentName := convertSnapContentName(srcSnapID) - snapContent, err := cs.snapClient.SnapshotV1().VolumeSnapshotContents().Get(ctx, snapContentName, metav1.GetOptions{}) - if err != nil { - klog.Errorf("error getting snapshot content: %v", err) - return nil, err - } - if err := cs.cloneFromSnapshot(ctx, snapContent, req.GetName(), node, lvmType, vgName, req.GetCapacityRange().GetRequiredBytes()); err != nil { - return nil, err - } - case *csi.VolumeContentSource_Volume: - srcVolID := volumeSource.GetVolume().GetVolumeId() - srcVolume, err := cs.kubeClient.CoreV1().PersistentVolumes().Get(ctx, srcVolID, metav1.GetOptions{}) - if err != nil { - return nil, status.Errorf(codes.Unavailable, "source volume %s not found", srcVolID) - } - if err := cs.cloneFromVolume(ctx, srcVolume, req.GetName(), node, lvmType, vgName, req.GetCapacityRange().GetRequiredBytes()); err != nil { - return nil, err - } - default: - return nil, status.Errorf(codes.InvalidArgument, "%v not a proper volume source", volumeSource) - } - } else { - va := volumeAction{ - action: actionTypeCreate, - name: req.GetName(), - nodeName: node, - size: req.GetCapacityRange().GetRequiredBytes(), - lvmType: lvmType, - pullPolicy: cs.pullPolicy, - provisionerImage: cs.provisionerImage, - kubeClient: cs.kubeClient, - namespace: cs.namespace, - vgName: vgName, - hostWritePath: cs.hostWritePath, - } - if err := createProvisionerPod(ctx, va); err != nil { - klog.Errorf("error creating provisioner pod :%v", err) - return nil, err - } + // Encrypted volumes lose the LUKS2 header (16 MiB) to overhead, so grow the + // backing LV by that much to still expose the requested capacity. The volume + // still reports its requested (usable) size below. + lvBytes := backingLVBytes(requiredBytes, isEncrypted(req.GetParameters())) + + if err := cs.provisionVolume(ctx, req, node, lvmType, vgName, lvBytes); err != nil { + return nil, err } return &csi.CreateVolumeResponse{ Volume: &csi.Volume{ VolumeId: req.GetName(), - CapacityBytes: req.GetCapacityRange().GetRequiredBytes(), + CapacityBytes: requiredBytes, VolumeContext: volumeContext, ContentSource: req.GetVolumeContentSource(), AccessibleTopology: topology, @@ -195,24 +146,139 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol }, nil } -func (cs *controllerServer) generateVolumeActionForClone(srcVol *v1.PersistentVolume, srcLVName, dstName, dstNode, dstLVType, dstVGName string, srcSize, dstSize int64) (volumeAction, error) { - ns := srcVol.Spec.NodeAffinity.Required.NodeSelectorTerms - srcNode := ns[0].MatchExpressions[0].Values[0] - srcVgName := srcVol.Spec.CSI.VolumeAttributes["vgName"] - srcType := srcVol.Spec.CSI.VolumeAttributes["type"] +func (cs *controllerServer) provisionVolume( + ctx context.Context, + req *csi.CreateVolumeRequest, + node, lvmType, vgName string, + requiredBytes int64, +) error { + if source := req.GetVolumeContentSource(); source != nil { + klog.Infof("cloning volume with source: %v", source) + return cs.cloneFromContentSource(ctx, source, req.GetName(), node, lvmType, vgName, requiredBytes) + } + + action := cs.newCreateVolumeAction(req.GetName(), node, lvmType, vgName, requiredBytes) + if err := createProvisionerPod(ctx, action); err != nil { + klog.Errorf("error creating provisioner pod: %v", err) + return err + } + return nil +} + +func (cs *controllerServer) cloneFromContentSource( + ctx context.Context, + source *csi.VolumeContentSource, + dstName, dstNode, dstLVMType, dstVGName string, + dstSize int64, +) error { + if source == nil { + return status.Error(codes.InvalidArgument, "volume content source is nil") + } + switch source.Type.(type) { + case *csi.VolumeContentSource_Snapshot: + return cs.cloneFromSnapshotSource(ctx, source.GetSnapshot(), dstName, dstNode, dstLVMType, dstVGName, dstSize) + case *csi.VolumeContentSource_Volume: + return cs.cloneFromVolumeSource(ctx, source.GetVolume(), dstName, dstNode, dstLVMType, dstVGName, dstSize) + default: + return status.Errorf(codes.InvalidArgument, "%v not a proper volume source", source) + } +} + +func (cs *controllerServer) cloneFromSnapshotSource( + ctx context.Context, + source *csi.VolumeContentSource_SnapshotSource, + dstName, dstNode, dstLVMType, dstVGName string, + dstSize int64, +) error { + snapshotID := source.GetSnapshotId() + if snapshotID == "" { + return status.Error(codes.InvalidArgument, "source snapshot ID is empty") + } + + // Snapshot handles are not VolumeSnapshotContent object names. In particular, + // data movers create pre-provisioned contents with their own object names. + content, err := cs.getSnapshotContent(ctx, snapshotID) + if err != nil { + return err + } + if content == nil { + return status.Errorf(codes.NotFound, "source snapshot %s not found", snapshotID) + } + return cs.cloneFromSnapshot(ctx, content, dstName, dstNode, dstLVMType, dstVGName, dstSize) +} + +func (cs *controllerServer) cloneFromVolumeSource( + ctx context.Context, + source *csi.VolumeContentSource_VolumeSource, + dstName, dstNode, dstLVMType, dstVGName string, + dstSize int64, +) error { + volumeID := source.GetVolumeId() + if volumeID == "" { + return status.Error(codes.InvalidArgument, "source volume ID is empty") + } + + volume, err := cs.kubeClient.CoreV1().PersistentVolumes().Get(ctx, volumeID, metav1.GetOptions{}) + if k8serror.IsNotFound(err) { + return status.Errorf(codes.NotFound, "source volume %s not found", volumeID) + } + if err != nil { + return status.Errorf(codes.Unavailable, "failed to get source volume %s: %v", volumeID, err) + } + return cs.cloneFromVolume(ctx, volume, dstName, dstNode, dstLVMType, dstVGName, dstSize) +} + +func (cs *controllerServer) newCreateVolumeAction(name, node, lvmType, vgName string, size int64) volumeAction { + return volumeAction{ + action: actionTypeCreate, + name: name, + nodeName: node, + size: size, + lvmType: lvmType, + pullPolicy: cs.pullPolicy, + provisionerImage: cs.provisionerImage, + kubeClient: cs.kubeClient, + namespace: cs.namespace, + vgName: vgName, + hostWritePath: cs.hostWritePath, + } +} + +func (cs *controllerServer) generateVolumeActionForClone( + srcVol *v1.PersistentVolume, + srcLVName, dstName, dstNode, dstLVType, dstVGName string, + srcSize, dstSize int64, +) (volumeAction, error) { + srcNode, srcVGName, srcLVMType, err := metadataFromPV(srcVol) + if err != nil { + return volumeAction{}, status.Error(codes.FailedPrecondition, err.Error()) + } srcInfo := &srcInfo{ srcLVName: srcLVName, - srcVGName: srcVgName, - srcType: srcType, + srcVGName: srcVGName, + srcType: srcLVMType, } - klog.V(4).Infof("cloning volume from %s/%s ", srcVgName, srcLVName) + return cs.newCloneVolumeAction(srcInfo, srcNode, dstName, dstNode, dstLVType, dstVGName, srcSize, dstSize) +} + +func (cs *controllerServer) newCloneVolumeAction( + source *srcInfo, + srcNode, dstName, dstNode, dstLVType, dstVGName string, + srcSize, dstSize int64, +) (volumeAction, error) { + if source == nil { + return volumeAction{}, status.Error(codes.FailedPrecondition, "clone source is nil") + } + klog.V(4).Infof("cloning volume from %s/%s ", source.srcVGName, source.srcLVName) if srcSize > dstSize { - return volumeAction{}, status.Errorf(codes.InvalidArgument, "source/snapshot volume size(%v) is larger than destination volume size(%v)", srcSize, dstSize) + return volumeAction{}, status.Errorf(codes.InvalidArgument, + "source/snapshot volume size(%v) is larger than destination volume size(%v)", srcSize, dstSize) } if srcNode != dstNode { - return volumeAction{}, status.Errorf(codes.InvalidArgument, "source (%s) and destination (%s) nodes are different (not supported)", srcNode, dstNode) + return volumeAction{}, status.Errorf(codes.InvalidArgument, + "source (%s) and destination (%s) nodes are different (not supported)", srcNode, dstNode) } return volumeAction{ @@ -227,19 +293,45 @@ func (cs *controllerServer) generateVolumeActionForClone(srcVol *v1.PersistentVo namespace: cs.namespace, vgName: dstVGName, hostWritePath: cs.hostWritePath, - srcInfo: srcInfo, + srcInfo: source, }, nil } -func (cs *controllerServer) cloneFromSnapshot(ctx context.Context, snapContent *snapv1.VolumeSnapshotContent, dstName, dstNode, dstLVType, dstVGName string, dstSize int64) error { - srcVolID := *snapContent.Spec.Source.VolumeHandle - srcVol, err := cs.kubeClient.CoreV1().PersistentVolumes().Get(ctx, srcVolID, metav1.GetOptions{}) +func (cs *controllerServer) cloneFromSnapshot( + ctx context.Context, + snapContent *snapv1.VolumeSnapshotContent, + dstName, dstNode, dstLVType, dstVGName string, + dstSize int64, +) error { + if snapContent == nil { + return status.Error(codes.FailedPrecondition, "snapshot content is nil") + } + if snapContent.Spec.Source.VolumeHandle == nil { + return cs.cloneFromPreProvisionedSnapshot(ctx, snapContent, dstName, dstNode, dstLVType, dstVGName, dstSize) + } + return cs.cloneFromDynamicSnapshot(ctx, snapContent, dstName, dstNode, dstLVType, dstVGName, dstSize) +} + +func (cs *controllerServer) cloneFromDynamicSnapshot( + ctx context.Context, + snapContent *snapv1.VolumeSnapshotContent, + dstName, dstNode, dstLVType, dstVGName string, + dstSize int64, +) error { + sourceVolumeID, snapshotID, restoreSize, err := metadataFromSnapshotContent(snapContent) if err != nil { - klog.Errorf("error getting volume: %v", err) - return err + return status.Error(codes.FailedPrecondition, err.Error()) + } + + srcVol, err := cs.kubeClient.CoreV1().PersistentVolumes().Get(ctx, sourceVolumeID, metav1.GetOptions{}) + if k8serror.IsNotFound(err) { + return status.Errorf(codes.NotFound, "source volume %s not found", sourceVolumeID) + } + if err != nil { + return status.Errorf(codes.Unavailable, "failed to get source volume %s: %v", sourceVolumeID, err) } - restoreSize := *snapContent.Status.RestoreSize - snapshotLVName := fmt.Sprintf("lvm-%s", *snapContent.Status.SnapshotHandle) + + snapshotLVName := fmt.Sprintf("lvm-%s", snapshotID) va, err := cs.generateVolumeActionForClone(srcVol, snapshotLVName, dstName, dstNode, dstLVType, dstVGName, restoreSize, dstSize) if err != nil { return err @@ -253,11 +345,72 @@ func (cs *controllerServer) cloneFromSnapshot(ctx context.Context, snapContent * return nil } -func (cs *controllerServer) cloneFromVolume(ctx context.Context, srcVol *v1.PersistentVolume, dstName, dstNode, dstLVType, dstVGName string, dstSize int64) error { - srcSizeStr := srcVol.Spec.CSI.VolumeAttributes["RequiredBytes"] - srcSize, err := strconv.ParseInt(srcSizeStr, 10, 64) +func (cs *controllerServer) cloneFromPreProvisionedSnapshot( + ctx context.Context, + snapContent *snapv1.VolumeSnapshotContent, + dstName, dstNode, dstLVType, dstVGName string, + dstSize int64, +) error { + action, err := cs.preProvisionedSnapshotCloneAction( + snapContent, + dstName, + dstNode, + dstLVType, + dstVGName, + dstSize, + ) + if err != nil { + return err + } + if err := createProvisionerPod(ctx, action); err != nil { + klog.Errorf("error creating provisioner pod: %v", err) + return err + } + return nil +} + +func (cs *controllerServer) preProvisionedSnapshotCloneAction( + snapContent *snapv1.VolumeSnapshotContent, + dstName, dstNode, dstLVType, dstVGName string, + dstSize int64, +) (volumeAction, error) { + snapshotID, restoreSize, err := preProvisionedSnapshotMetadata(snapContent) if err != nil { - return status.Errorf(codes.InvalidArgument, "error parsing srcSize: %v", err) + return volumeAction{}, status.Error(codes.FailedPrecondition, err.Error()) + } + + srcNode := snapContent.Annotations[snapshotNodeAnnotation] + if srcNode == "" { + srcNode = dstNode + } + srcVGName := snapContent.Annotations[snapshotVGAnnotation] + if srcVGName == "" { + srcVGName = dstVGName + } + if restoreSize == 0 { + restoreSize = dstSize + } + + source := &srcInfo{ + srcLVName: fmt.Sprintf("lvm-%s", snapshotID), + srcVGName: srcVGName, + // Pre-provisioned contents do not carry the source LVM type. Restores + // use the destination StorageClass type, which is also what determines + // whether the optimized same-VG dm-thin clone path can be used. + srcType: dstLVType, + } + return cs.newCloneVolumeAction(source, srcNode, dstName, dstNode, dstLVType, dstVGName, restoreSize, dstSize) +} + +func (cs *controllerServer) cloneFromVolume( + ctx context.Context, + srcVol *v1.PersistentVolume, + dstName, dstNode, dstLVType, dstVGName string, + dstSize int64, +) error { + srcSize, err := requiredBytesFromPersistentVolume(srcVol) + if err != nil { + return status.Error(codes.FailedPrecondition, err.Error()) } srcLVName := srcVol.GetName() va, err := cs.generateVolumeActionForClone(srcVol, srcLVName, dstName, dstNode, dstLVType, dstVGName, srcSize, dstSize) @@ -274,11 +427,9 @@ func (cs *controllerServer) cloneFromVolume(ctx context.Context, srcVol *v1.Pers } func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) { - // Check arguments - if len(req.GetVolumeId()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Volume ID missing in request") + if err := validateDeleteVolumeRequest(req); err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } - if err := cs.validateControllerServiceRequest(csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME); err != nil { klog.V(3).Infof("invalid delete volume req: %v", req) return nil, err @@ -286,52 +437,80 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol volID := req.GetVolumeId() - volume, err := cs.kubeClient.CoreV1().PersistentVolumes().Get(ctx, volID, metav1.GetOptions{}) + volume, err := cs.persistentVolumeForDeletion(ctx, volID) if err != nil { - panic(err.Error()) + return nil, err + } + if volume == nil { + return &csi.DeleteVolumeResponse{}, nil } + klog.V(4).Infof("volume %s to be deleted", volume) - ns := volume.Spec.NodeAffinity.Required.NodeSelectorTerms - node := ns[0].MatchExpressions[0].Values[0] - srcVgName := volume.Spec.CSI.VolumeAttributes["vgName"] - srcType := volume.Spec.CSI.VolumeAttributes["type"] - srcInfo := &srcInfo{ - srcLVName: volID, - srcVGName: srcVgName, - srcType: srcType, + nodeName, vgName, lvmType, err := metadataFromPV(volume) + if err != nil { + return nil, status.Error(codes.FailedPrecondition, err.Error()) + } + + klog.V(4).Infof("from node %s ", nodeName) + nodeAvailable, err := cs.nodeAvailableForDeletion(ctx, nodeName, volID) + if err != nil { + return nil, err + } + if !nodeAvailable { + return &csi.DeleteVolumeResponse{}, nil } - klog.V(4).Infof("from node %s ", node) + va := cs.newDeleteVolumeAction(volID, nodeName, vgName, lvmType) + if err := createProvisionerPod(ctx, va); err != nil { + klog.Errorf("error creating provisioner pod :%v", err) + return nil, err + } + + klog.V(4).Infof("volume %v successfully deleted", volID) + return &csi.DeleteVolumeResponse{}, nil +} - _, err = cs.kubeClient.CoreV1().Nodes().Get(ctx, node, metav1.GetOptions{}) +func (cs *controllerServer) persistentVolumeForDeletion(ctx context.Context, volumeID string) (*v1.PersistentVolume, error) { + volume, err := cs.kubeClient.CoreV1().PersistentVolumes().Get(ctx, volumeID, metav1.GetOptions{}) + if k8serror.IsNotFound(err) { + klog.Infof("volume %s is already absent", volumeID) + return nil, nil + } + if err != nil { + return nil, status.Errorf(codes.Unavailable, "failed to get volume %s: %v", volumeID, err) + } + return volume, nil +} + +func (cs *controllerServer) nodeAvailableForDeletion(ctx context.Context, nodeName, volumeID string) (bool, error) { + _, err := cs.kubeClient.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{}) + if k8serror.IsNotFound(err) { + klog.Infof("node %s not found anymore. Assuming volume %s is gone for good.", nodeName, volumeID) + return false, nil + } if err != nil { - if k8serror.IsNotFound(err) { - klog.Infof("node %s not found anymore. Assuming volume %s is gone for good.", node, volID) - return &csi.DeleteVolumeResponse{}, nil - } klog.Errorf("error getting nodes: %v", err) - return nil, err + return false, status.Errorf(codes.Unavailable, "failed to get node %s: %v", nodeName, err) } + return true, nil +} - va := volumeAction{ +func (cs *controllerServer) newDeleteVolumeAction(volumeID, nodeName, vgName, lvmType string) volumeAction { + return volumeAction{ action: actionTypeDelete, - name: volID, - nodeName: node, + name: volumeID, + nodeName: nodeName, pullPolicy: cs.pullPolicy, provisionerImage: cs.provisionerImage, kubeClient: cs.kubeClient, namespace: cs.namespace, hostWritePath: cs.hostWritePath, - srcInfo: srcInfo, - } - if err := createProvisionerPod(ctx, va); err != nil { - klog.Errorf("error creating provisioner pod :%v", err) - return nil, err + srcInfo: &srcInfo{ + srcLVName: volumeID, + srcVGName: vgName, + srcType: lvmType, + }, } - - klog.V(4).Infof("volume %v successfully deleted", volID) - - return &csi.DeleteVolumeResponse{}, nil } func (cs *controllerServer) ControllerGetCapabilities(_ context.Context, _ *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) { @@ -350,13 +529,8 @@ func (cs *controllerServer) ValidateVolumeCapabilities(_ context.Context, req *c return nil, status.Error(codes.InvalidArgument, req.VolumeId) } - for _, cap := range req.GetVolumeCapabilities() { - if cap.GetMount() == nil && cap.GetBlock() == nil { - return nil, status.Error(codes.InvalidArgument, "cannot have both mount and block access type be undefined") - } - - // A real driver would check the capabilities of the given volume with - // the set of requested capabilities. + if err := validateVolumeCapabilities(req.GetVolumeCapabilities()); err != nil { + return &csi.ValidateVolumeCapabilitiesResponse{Message: err.Error()}, nil } return &csi.ValidateVolumeCapabilitiesResponse{ @@ -419,102 +593,362 @@ func (cs *controllerServer) ListVolumes(_ context.Context, _ *csi.ListVolumesReq func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequest) (*csi.CreateSnapshotResponse, error) { klog.Infof("CreateSnapshot req: %v", req) - volID := req.GetSourceVolumeId() - volume, err := cs.kubeClient.CoreV1().PersistentVolumes().Get(ctx, volID, metav1.GetOptions{}) + snapshotName, volumeID, err := validateCreateSnapshotRequest(req) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + + volume, err := cs.kubeClient.CoreV1().PersistentVolumes().Get(ctx, volumeID, metav1.GetOptions{}) + if k8serror.IsNotFound(err) { + return nil, status.Errorf(codes.NotFound, "source volume %s not found", volumeID) + } if err != nil { - panic(err.Error()) + return nil, status.Errorf(codes.Unavailable, "failed to get source volume %s: %v", volumeID, err) } + klog.V(4).Infof("taking snapshot with volume %s ", volume) - ns := volume.Spec.NodeAffinity.Required.NodeSelectorTerms - node := ns[0].MatchExpressions[0].Values[0] - vgName := volume.Spec.CSI.VolumeAttributes["vgName"] - lvType := volume.Spec.CSI.VolumeAttributes["type"] - snapSizeStr := volume.Spec.CSI.VolumeAttributes["RequiredBytes"] - snapSize, err := strconv.ParseInt(snapSizeStr, 10, 64) - if err != nil { - klog.Errorf("error parsing snapSize: %v", err) - return nil, err + nodeName, vgName, lvmType, err := metadataFromPV(volume) + if err != nil { + return nil, status.Error(codes.FailedPrecondition, err.Error()) } - snapshotName := req.GetName() - snapTimestamp := ×tamppb.Timestamp{ - Seconds: time.Now().Unix(), + + snapSize, err := requiredBytesFromPersistentVolume(volume) + if err != nil { + return nil, status.Error(codes.FailedPrecondition, err.Error()) } - sa := snapshotAction{ + // Keep the physical location independently of the VolumeSnapshotContent. A + // Retain-policy content can be deleted and later re-created as a + // pre-provisioned content that only carries the opaque snapshot handle. + if err := cs.recordSnapshotLocation(ctx, snapshotName, nodeName, vgName); err != nil { + return nil, status.Errorf(codes.Unavailable, "failed to record location for snapshot %s: %v", snapshotName, err) + } + + action := cs.newCreateSnapshotAction(snapshotName, volumeID, nodeName, vgName, lvmType, snapSize) + if err := createSnapshotterPod(ctx, action); err != nil { + klog.Errorf("error creating provisioner pod :%v", err) + return nil, err + } + + return newCreateSnapshotResponse(snapshotName, volumeID, snapSize), nil +} + +func (cs *controllerServer) newCreateSnapshotAction( + snapshotName, volumeID, nodeName, vgName, lvmType string, + size int64, +) snapshotAction { + return snapshotAction{ action: actionTypeCreate, - srcVolName: volID, + srcVolName: volumeID, snapshotName: snapshotName, - nodeName: node, - snapSize: snapSize, + nodeName: nodeName, + snapSize: size, vgName: vgName, - lvType: lvType, + lvType: lvmType, hostWritePath: cs.hostWritePath, kubeClient: cs.kubeClient, namespace: cs.namespace, provisionerImage: cs.provisionerImage, pullPolicy: cs.pullPolicy, } - if err := createSnapshotterPod(ctx, sa); err != nil { - klog.Errorf("error creating provisioner pod :%v", err) - return nil, err - } +} +func newCreateSnapshotResponse(snapshotName, volumeID string, size int64) *csi.CreateSnapshotResponse { return &csi.CreateSnapshotResponse{ Snapshot: &csi.Snapshot{ SnapshotId: snapshotName, - SourceVolumeId: volID, - SizeBytes: snapSize, - CreationTime: snapTimestamp, - ReadyToUse: true, + SourceVolumeId: volumeID, + SizeBytes: size, + CreationTime: ×tamppb.Timestamp{ + Seconds: time.Now().Unix(), + }, + ReadyToUse: true, }, - }, nil + } } func (cs *controllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequest) (*csi.DeleteSnapshotResponse, error) { klog.Infof("DeleteSnapshot req: %v", req) snapName := req.GetSnapshotId() - snapshotsList, err := cs.snapClient.SnapshotV1().VolumeSnapshotContents().List(ctx, metav1.ListOptions{}) + if snapName == "" { + return nil, status.Error(codes.InvalidArgument, "snapshot ID missing in request") + } + + snapContent, err := cs.getSnapshotContent(ctx, snapName) + if err != nil { + return nil, err + } + if snapContent == nil { + klog.Infof("snapshot %s is already absent", snapName) + return &csi.DeleteSnapshotResponse{}, nil + } + + action, err := cs.deleteSnapshotAction(ctx, snapName, snapContent) if err != nil { - klog.Errorf("error listing snapshots: %v", err) return nil, err } - volID := "" - for _, snap := range snapshotsList.Items { - if *snap.Status.SnapshotHandle == snapName { - volID = *snap.Spec.Source.VolumeHandle + if action == nil { + return &csi.DeleteSnapshotResponse{}, nil + } + + if err := createSnapshotterPod(ctx, *action); err != nil { + klog.Errorf("error creating provisioner pod :%v", err) + return nil, err + } + if err := cs.forgetSnapshotLocation(ctx, snapName); err != nil { + // The backend deletion succeeded, so a stale location record must not + // make this idempotent CSI operation fail. A later create with the same + // handle will detect and report conflicting metadata. + klog.Warningf("failed to remove location record for snapshot %s: %v", snapName, err) + } + + return &csi.DeleteSnapshotResponse{}, nil +} + +func (cs *controllerServer) getSnapshotContent(ctx context.Context, snapshotID string) (*snapv1.VolumeSnapshotContent, error) { + contents, err := cs.snapClient.SnapshotV1().VolumeSnapshotContents().List(ctx, metav1.ListOptions{}) + if err != nil { + return nil, status.Errorf(codes.Unavailable, "failed to list snapshot contents: %v", err) + } + + for i := range contents.Items { + content := &contents.Items[i] + if content.Status != nil && + content.Status.SnapshotHandle != nil && + *content.Status.SnapshotHandle == snapshotID { + return content, nil + } + if content.Spec.Source.SnapshotHandle != nil && + *content.Spec.Source.SnapshotHandle == snapshotID { + return content, nil + } + } + return nil, nil +} + +func (cs *controllerServer) deleteSnapshotAction( + ctx context.Context, + snapshotID string, + content *snapv1.VolumeSnapshotContent, +) (*snapshotAction, error) { + if content == nil { + return nil, status.Error(codes.FailedPrecondition, "snapshot content is nil") + } + if content.Spec.Source.VolumeHandle == nil { + return cs.deletePreExistingSnapshotAction(ctx, snapshotID, content) + } + return cs.deleteDynamicSnapshotAction(ctx, snapshotID, *content.Spec.Source.VolumeHandle) +} + +func (cs *controllerServer) deletePreExistingSnapshotAction( + ctx context.Context, + snapshotID string, + content *snapv1.VolumeSnapshotContent, +) (*snapshotAction, error) { + if content.Spec.Source.SnapshotHandle == nil || *content.Spec.Source.SnapshotHandle == "" { + return nil, status.Errorf(codes.FailedPrecondition, "snapshot content %s has no source handle", content.Name) + } + nodeName := content.Annotations[snapshotNodeAnnotation] + vgName := content.Annotations[snapshotVGAnnotation] + if (nodeName == "") != (vgName == "") { + return nil, status.Errorf( + codes.FailedPrecondition, + "pre-existing snapshot %s has incomplete location annotations; both %s and %s are required", + snapshotID, + snapshotNodeAnnotation, + snapshotVGAnnotation, + ) + } + if nodeName == "" { + var found bool + var err error + nodeName, vgName, found, err = cs.lookupSnapshotLocation(ctx, snapshotID) + if err != nil { + return nil, status.Errorf(codes.Unavailable, "failed to look up location for snapshot %s: %v", snapshotID, err) + } + if !found { + return nil, status.Errorf( + codes.FailedPrecondition, + "pre-existing snapshot %s has no recorded location; add annotations %s and %s", + snapshotID, + snapshotNodeAnnotation, + snapshotVGAnnotation, + ) } } - if volID == "" { - klog.Errorf("snapshot %s not found", snapName) - return nil, status.Error(codes.NotFound, "snapshot not found") + action := cs.newDeleteSnapshotAction(snapshotID, nodeName, vgName) + return &action, nil +} + +func (cs *controllerServer) deleteDynamicSnapshotAction( + ctx context.Context, + snapshotID, volumeID string, +) (*snapshotAction, error) { + if volumeID == "" { + return nil, status.Errorf(codes.FailedPrecondition, "snapshot %s has an empty source volume handle", snapshotID) + } + volume, err := cs.kubeClient.CoreV1().PersistentVolumes().Get(ctx, volumeID, metav1.GetOptions{}) + if k8serror.IsNotFound(err) { + klog.Warningf( + "source volume %s for snapshot %s is already absent; returning success to preserve delete idempotency", + volumeID, + snapshotID, + ) + return nil, nil + } + if err != nil { + return nil, status.Errorf(codes.Unavailable, "failed to get source volume %s: %v", volumeID, err) + } + nodeName, vgName, _, err := metadataFromPV(volume) + if err != nil { + return nil, status.Error(codes.FailedPrecondition, err.Error()) + } + action := cs.newDeleteSnapshotAction(snapshotID, nodeName, vgName) + return &action, nil +} + +func snapshotLocationConfigMapName(snapshotID string) string { + digest := sha256.Sum256([]byte(snapshotID)) + return fmt.Sprintf("%s%x", snapshotLocationConfigMapPrefix, digest) +} + +type snapshotLocation struct { + handle string + nodeName string + vgName string +} + +func (location snapshotLocation) validate() error { + if location.handle == "" || location.nodeName == "" || location.vgName == "" { + return fmt.Errorf("snapshot handle, node name, and volume group are required") + } + return nil +} + +func (location snapshotLocation) configMap(namespace string) *v1.ConfigMap { + immutable := true + return &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: snapshotLocationConfigMapName(location.handle), + Namespace: namespace, + Labels: map[string]string{snapshotLocationLabel: "true"}, + }, + Immutable: &immutable, + Data: map[string]string{ + snapshotLocationHandleKey: location.handle, + snapshotLocationNodeKey: location.nodeName, + snapshotLocationVGKey: location.vgName, + }, + } +} + +// recordSnapshotLocation creates one immutable ConfigMap per snapshot. The +// object deliberately has no owner reference: it must outlive a +// Retain-policy VolumeSnapshotContent so a later pre-provisioned content can +// still resolve the backend location. +func (cs *controllerServer) recordSnapshotLocation(ctx context.Context, snapshotID, nodeName, vgName string) error { + desired := snapshotLocation{handle: snapshotID, nodeName: nodeName, vgName: vgName} + if err := desired.validate(); err != nil { + return err + } + + configMaps := cs.kubeClient.CoreV1().ConfigMaps(cs.namespace) + _, err := configMaps.Create(ctx, desired.configMap(cs.namespace), metav1.CreateOptions{}) + if err == nil { + return nil + } + if !k8serror.IsAlreadyExists(err) { + return err + } + + existing, err := configMaps.Get(ctx, snapshotLocationConfigMapName(snapshotID), metav1.GetOptions{}) + if err != nil { + return err + } + actual, err := snapshotLocationFromConfigMap(existing, snapshotID) + if err != nil { + return err + } + if actual != desired { + return fmt.Errorf( + "snapshot %s already has a conflicting location record %q/%q", + snapshotID, + actual.nodeName, + actual.vgName, + ) + } + return nil +} + +func (cs *controllerServer) lookupSnapshotLocation( + ctx context.Context, + snapshotID string, +) (nodeName, vgName string, found bool, err error) { + name := snapshotLocationConfigMapName(snapshotID) + location, err := cs.kubeClient.CoreV1().ConfigMaps(cs.namespace).Get(ctx, name, metav1.GetOptions{}) + if k8serror.IsNotFound(err) { + return "", "", false, nil + } + if err != nil { + return "", "", false, err } - volume, err := cs.kubeClient.CoreV1().PersistentVolumes().Get(ctx, volID, metav1.GetOptions{}) + recorded, err := snapshotLocationFromConfigMap(location, snapshotID) if err != nil { - panic(err.Error()) + return "", "", false, err } - klog.V(4).Infof("deleting snapshot with volume %s ", snapName) - ns := volume.Spec.NodeAffinity.Required.NodeSelectorTerms - node := ns[0].MatchExpressions[0].Values[0] - vgName := volume.Spec.CSI.VolumeAttributes["vgName"] + return recorded.nodeName, recorded.vgName, true, nil +} - sa := snapshotAction{ +func snapshotLocationFromConfigMap(configMap *v1.ConfigMap, snapshotID string) (snapshotLocation, error) { + if configMap == nil { + return snapshotLocation{}, fmt.Errorf("snapshot location ConfigMap is nil") + } + if configMap.Labels[snapshotLocationLabel] != "true" { + return snapshotLocation{}, fmt.Errorf("ConfigMap %s is not a snapshot location record", configMap.Name) + } + recorded := snapshotLocation{ + handle: configMap.Data[snapshotLocationHandleKey], + nodeName: configMap.Data[snapshotLocationNodeKey], + vgName: configMap.Data[snapshotLocationVGKey], + } + if recorded.handle != snapshotID { + return snapshotLocation{}, fmt.Errorf( + "ConfigMap %s records snapshot handle %q, expected %q", + configMap.Name, + recorded.handle, + snapshotID, + ) + } + if err := recorded.validate(); err != nil { + return snapshotLocation{}, fmt.Errorf("ConfigMap %s has an invalid snapshot location: %w", configMap.Name, err) + } + return recorded, nil +} + +func (cs *controllerServer) forgetSnapshotLocation(ctx context.Context, snapshotID string) error { + err := cs.kubeClient.CoreV1().ConfigMaps(cs.namespace).Delete( + ctx, + snapshotLocationConfigMapName(snapshotID), + metav1.DeleteOptions{}, + ) + if k8serror.IsNotFound(err) { + return nil + } + return err +} + +func (cs *controllerServer) newDeleteSnapshotAction(snapshotID, nodeName, vgName string) snapshotAction { + return snapshotAction{ action: actionTypeDelete, - snapshotName: snapName, - nodeName: node, + snapshotName: snapshotID, + nodeName: nodeName, vgName: vgName, - lvType: "", // not used hostWritePath: cs.hostWritePath, kubeClient: cs.kubeClient, namespace: cs.namespace, provisionerImage: cs.provisionerImage, pullPolicy: cs.pullPolicy, } - if err := createSnapshotterPod(ctx, sa); err != nil { - klog.Errorf("error creating provisioner pod :%v", err) - return nil, err - } - - return &csi.DeleteSnapshotResponse{}, nil } func (cs *controllerServer) ListSnapshots(_ context.Context, _ *csi.ListSnapshotsRequest) (*csi.ListSnapshotsResponse, error) { @@ -532,9 +966,3 @@ func (cs *controllerServer) ControllerGetVolume(_ context.Context, _ *csi.Contro func (cs *controllerServer) ControllerModifyVolume(_ context.Context, _ *csi.ControllerModifyVolumeRequest) (*csi.ControllerModifyVolumeResponse, error) { return nil, status.Error(codes.Unimplemented, "") } - -func convertSnapContentName(snapID string) string { - // snapshotID is in the form of "snapshot-" - // snapshotContentName is in the form of "snapshotcontent-" - return strings.Replace(snapID, "snapshot-", "snapcontent-", 1) -} diff --git a/pkg/lvm/controllerserver_test.go b/pkg/lvm/controllerserver_test.go new file mode 100644 index 00000000..e0591afa --- /dev/null +++ b/pkg/lvm/controllerserver_test.go @@ -0,0 +1,637 @@ +package lvm + +import ( + "context" + "testing" + + "github.com/container-storage-interface/spec/lib/go/csi" + snapv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" + snapclient "github.com/kubernetes-csi/external-snapshotter/client/v8/clientset/versioned" + snaptypedv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/clientset/versioned/typed/volumesnapshot/v1" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + v1 "k8s.io/api/core/v1" + k8serror "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/kubernetes" + corev1 "k8s.io/client-go/kubernetes/typed/core/v1" +) + +func strPointer(value string) *string { + return &value +} + +type fakeKubeClient struct { + kubernetes.Interface + volumes map[string]*v1.PersistentVolume + nodes map[string]*v1.Node + configMaps map[string]*v1.ConfigMap +} + +func (f *fakeKubeClient) CoreV1() corev1.CoreV1Interface { + return &fakeCoreV1{volumes: f.volumes, nodes: f.nodes, configMaps: f.configMaps} +} + +type fakeCoreV1 struct { + corev1.CoreV1Interface + volumes map[string]*v1.PersistentVolume + nodes map[string]*v1.Node + configMaps map[string]*v1.ConfigMap +} + +func (f *fakeCoreV1) PersistentVolumes() corev1.PersistentVolumeInterface { + return &fakePersistentVolumes{volumes: f.volumes} +} + +func (f *fakeCoreV1) Nodes() corev1.NodeInterface { + return &fakeNodes{nodes: f.nodes} +} + +func (f *fakeCoreV1) ConfigMaps(_ string) corev1.ConfigMapInterface { + return &fakeConfigMaps{configMaps: f.configMaps} +} + +type fakePersistentVolumes struct { + corev1.PersistentVolumeInterface + volumes map[string]*v1.PersistentVolume +} + +type fakeNodes struct { + corev1.NodeInterface + nodes map[string]*v1.Node +} + +type fakeConfigMaps struct { + corev1.ConfigMapInterface + configMaps map[string]*v1.ConfigMap +} + +func (f *fakeConfigMaps) Get( + _ context.Context, + name string, + _ metav1.GetOptions, +) (*v1.ConfigMap, error) { + if configMap := f.configMaps[name]; configMap != nil { + return configMap.DeepCopy(), nil + } + return nil, k8serror.NewNotFound(schema.GroupResource{Resource: "configmaps"}, name) +} + +func (f *fakeConfigMaps) Create( + _ context.Context, + configMap *v1.ConfigMap, + _ metav1.CreateOptions, +) (*v1.ConfigMap, error) { + if f.configMaps[configMap.Name] != nil { + return nil, k8serror.NewAlreadyExists(schema.GroupResource{Resource: "configmaps"}, configMap.Name) + } + f.configMaps[configMap.Name] = configMap.DeepCopy() + return configMap.DeepCopy(), nil +} + +func (f *fakeConfigMaps) Delete( + _ context.Context, + name string, + _ metav1.DeleteOptions, +) error { + if f.configMaps[name] == nil { + return k8serror.NewNotFound(schema.GroupResource{Resource: "configmaps"}, name) + } + delete(f.configMaps, name) + return nil +} + +func (f *fakeNodes) Get( + _ context.Context, + name string, + _ metav1.GetOptions, +) (*v1.Node, error) { + if node := f.nodes[name]; node != nil { + return node.DeepCopy(), nil + } + return nil, k8serror.NewNotFound(schema.GroupResource{Resource: "nodes"}, name) +} + +func (f *fakePersistentVolumes) Get( + _ context.Context, + name string, + _ metav1.GetOptions, +) (*v1.PersistentVolume, error) { + if volume := f.volumes[name]; volume != nil { + return volume.DeepCopy(), nil + } + return nil, k8serror.NewNotFound(schema.GroupResource{Resource: "persistentvolumes"}, name) +} + +type fakeSnapshotClient struct { + snapclient.Interface + contents []snapv1.VolumeSnapshotContent +} + +func (f *fakeSnapshotClient) SnapshotV1() snaptypedv1.SnapshotV1Interface { + return &fakeSnapshotV1{contents: f.contents} +} + +type fakeSnapshotV1 struct { + snaptypedv1.SnapshotV1Interface + contents []snapv1.VolumeSnapshotContent +} + +func (f *fakeSnapshotV1) VolumeSnapshotContents() snaptypedv1.VolumeSnapshotContentInterface { + return &fakeSnapshotContents{contents: f.contents} +} + +type fakeSnapshotContents struct { + snaptypedv1.VolumeSnapshotContentInterface + contents []snapv1.VolumeSnapshotContent +} + +func (f *fakeSnapshotContents) List( + _ context.Context, + _ metav1.ListOptions, +) (*snapv1.VolumeSnapshotContentList, error) { + return &snapv1.VolumeSnapshotContentList{Items: append([]snapv1.VolumeSnapshotContent(nil), f.contents...)}, nil +} + +func (f *fakeSnapshotContents) Watch( + _ context.Context, + _ metav1.ListOptions, +) (watch.Interface, error) { + return watch.NewEmptyWatch(), nil +} + +func controllerWithFakeClients() *controllerServer { + return &controllerServer{ + caps: getControllerServiceCapabilities([]csi.ControllerServiceCapability_RPC_Type{ + csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, + csi.ControllerServiceCapability_RPC_CREATE_DELETE_SNAPSHOT, + }), + kubeClient: &fakeKubeClient{ + volumes: map[string]*v1.PersistentVolume{}, + nodes: map[string]*v1.Node{}, + configMaps: map[string]*v1.ConfigMap{}, + }, + snapClient: &fakeSnapshotClient{}, + } +} + +func TestCreateVolumeRejectsMissingTopology(t *testing.T) { + cs := controllerWithFakeClients() + _, err := cs.CreateVolume(context.Background(), &csi.CreateVolumeRequest{ + Name: "volume", + CapacityRange: &csi.CapacityRange{RequiredBytes: 1048576}, + VolumeCapabilities: []*csi.VolumeCapability{mountCapability(csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER)}, + Parameters: map[string]string{ + "type": DmThinType, + "vgName": "vg", + }, + }) + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("expected InvalidArgument, got %v", err) + } +} + +func TestDeleteVolumeIsIdempotentWhenPVIsAbsent(t *testing.T) { + cs := controllerWithFakeClients() + if _, err := cs.DeleteVolume(context.Background(), &csi.DeleteVolumeRequest{VolumeId: "missing"}); err != nil { + t.Fatalf("idempotent delete failed: %v", err) + } +} + +func TestNodeAvailableForDeletion(t *testing.T) { + cs := controllerWithFakeClients() + + available, err := cs.nodeAvailableForDeletion(context.Background(), "missing", "volume") + if err != nil || available { + t.Fatalf("expected missing node to be unavailable without error, got available=%t err=%v", available, err) + } + + cs.kubeClient.(*fakeKubeClient).nodes["node-a"] = &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-a"}} + available, err = cs.nodeAvailableForDeletion(context.Background(), "node-a", "volume") + if err != nil || !available { + t.Fatalf("expected existing node to be available, got available=%t err=%v", available, err) + } +} + +func TestNewDeleteVolumeAction(t *testing.T) { + cs := controllerWithFakeClients() + action := cs.newDeleteVolumeAction("volume", "node-a", "vg-a", DmThinType) + + if action.action != actionTypeDelete || action.name != "volume" || action.nodeName != "node-a" { + t.Fatalf("unexpected delete action: %#v", action) + } + if action.srcInfo == nil || action.srcInfo.srcLVName != "volume" || action.srcInfo.srcVGName != "vg-a" || action.srcInfo.srcType != DmThinType { + t.Fatalf("unexpected delete source info: %#v", action.srcInfo) + } +} + +func TestCreateSnapshotDoesNotPanicWhenPVIsAbsent(t *testing.T) { + cs := controllerWithFakeClients() + _, err := cs.CreateSnapshot(context.Background(), &csi.CreateSnapshotRequest{ + Name: "snapshot-id", + SourceVolumeId: "missing", + }) + if status.Code(err) != codes.NotFound { + t.Fatalf("expected NotFound, got %v", err) + } +} + +func TestCloneFromSnapshotRejectsIncompleteContent(t *testing.T) { + cs := controllerWithFakeClients() + err := cs.cloneFromSnapshot( + context.Background(), + &snapv1.VolumeSnapshotContent{ObjectMeta: metav1.ObjectMeta{Name: "incomplete"}}, + "destination", + "node-a", + DmThinType, + "vg", + 1048576, + ) + if status.Code(err) != codes.FailedPrecondition { + t.Fatalf("expected FailedPrecondition, got %v", err) + } +} + +func TestCloneFromVolumeRejectsMalformedPersistentVolume(t *testing.T) { + cs := controllerWithFakeClients() + err := cs.cloneFromVolume( + context.Background(), + &v1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Name: "malformed"}}, + "destination", + "node-a", + DmThinType, + "vg", + 1048576, + ) + if status.Code(err) != codes.FailedPrecondition { + t.Fatalf("expected FailedPrecondition, got %v", err) + } +} + +func TestCloneFromContentSourceRejectsInvalidSources(t *testing.T) { + cs := controllerWithFakeClients() + tests := []struct { + name string + source *csi.VolumeContentSource + }{ + {name: "nil source"}, + {name: "missing source type", source: &csi.VolumeContentSource{}}, + { + name: "empty snapshot ID", + source: &csi.VolumeContentSource{Type: &csi.VolumeContentSource_Snapshot{ + Snapshot: &csi.VolumeContentSource_SnapshotSource{}, + }}, + }, + { + name: "empty volume ID", + source: &csi.VolumeContentSource{Type: &csi.VolumeContentSource_Volume{ + Volume: &csi.VolumeContentSource_VolumeSource{}, + }}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := cs.cloneFromContentSource( + context.Background(), + tt.source, + "destination", + "node-a", + DmThinType, + "vg", + 1048576, + ) + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("expected InvalidArgument, got %v", err) + } + }) + } +} + +func TestDeleteSnapshotIsNilSafeAndIdempotent(t *testing.T) { + t.Run("unrelated incomplete contents are ignored", func(t *testing.T) { + cs := controllerWithFakeClients() + cs.snapClient = &fakeSnapshotClient{contents: []snapv1.VolumeSnapshotContent{ + { + ObjectMeta: metav1.ObjectMeta{Name: "no-status"}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "nil-handle"}, + Status: &snapv1.VolumeSnapshotContentStatus{}, + }, + }} + + if _, err := cs.DeleteSnapshot( + context.Background(), + &csi.DeleteSnapshotRequest{SnapshotId: "missing"}, + ); err != nil { + t.Fatalf("idempotent snapshot delete failed: %v", err) + } + }) + + t.Run("missing source PV is idempotent", func(t *testing.T) { + cs := controllerWithFakeClients() + cs.snapClient = &fakeSnapshotClient{contents: []snapv1.VolumeSnapshotContent{{ + ObjectMeta: metav1.ObjectMeta{Name: "dynamic"}, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{ + VolumeHandle: strPointer("missing-volume"), + }, + }, + Status: &snapv1.VolumeSnapshotContentStatus{ + SnapshotHandle: strPointer("snapshot-id"), + }, + }}} + + if _, err := cs.DeleteSnapshot( + context.Background(), + &csi.DeleteSnapshotRequest{SnapshotId: "snapshot-id"}, + ); err != nil { + t.Fatalf("snapshot delete with missing source PV failed: %v", err) + } + }) +} + +func TestPreExistingSnapshotResolvesLocation(t *testing.T) { + cs := controllerWithFakeClients() + if _, err := cs.deleteSnapshotAction(context.Background(), "snapshot-id", nil); status.Code(err) != codes.FailedPrecondition { + t.Fatalf("expected nil content to return FailedPrecondition, got %v", err) + } + + content := &snapv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{Name: "pre-existing"}, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{ + SnapshotHandle: strPointer("snapshot-id"), + }, + }, + } + if _, err := cs.deleteSnapshotAction(context.Background(), "snapshot-id", content); status.Code(err) != codes.FailedPrecondition { + t.Fatalf("expected unrecorded snapshot to return FailedPrecondition, got %v", err) + } + + content.Annotations = map[string]string{snapshotNodeAnnotation: "node-a"} + if _, err := cs.deleteSnapshotAction(context.Background(), "snapshot-id", content); status.Code(err) != codes.FailedPrecondition { + t.Fatalf("expected incomplete annotations to return FailedPrecondition, got %v", err) + } + + content.Annotations = map[string]string{ + snapshotNodeAnnotation: "node-a", + snapshotVGAnnotation: "vg", + } + action, err := cs.deleteSnapshotAction(context.Background(), "snapshot-id", content) + if err != nil { + t.Fatalf("annotated pre-existing snapshot failed: %v", err) + } + if action.nodeName != "node-a" || action.vgName != "vg" { + t.Fatalf("unexpected action: %#v", action) + } + + content.Annotations = nil + if err := cs.recordSnapshotLocation(context.Background(), "snapshot-id", "recorded-node", "recorded-vg"); err != nil { + t.Fatalf("failed to record snapshot location: %v", err) + } + action, err = cs.deleteSnapshotAction(context.Background(), "snapshot-id", content) + if err != nil { + t.Fatalf("recorded pre-existing snapshot failed: %v", err) + } + if action.nodeName != "recorded-node" || action.vgName != "recorded-vg" { + t.Fatalf("unexpected recorded-location action: %#v", action) + } + + content.Annotations = map[string]string{ + snapshotNodeAnnotation: "override-node", + snapshotVGAnnotation: "override-vg", + } + action, err = cs.deleteSnapshotAction(context.Background(), "snapshot-id", content) + if err != nil { + t.Fatalf("annotation override failed: %v", err) + } + if action.nodeName != "override-node" || action.vgName != "override-vg" { + t.Fatalf("annotations did not override the recorded location: %#v", action) + } +} + +func TestSnapshotLocationRecordLifecycle(t *testing.T) { + cs := controllerWithFakeClients() + ctx := context.Background() + handle := "snapshot/with unicode 雪 and characters + not valid in a ConfigMap key" + name := snapshotLocationConfigMapName(handle) + + if name == handle || len(name) > 253 { + t.Fatalf("snapshot location name is not a safe derived name: %q", name) + } + if err := cs.recordSnapshotLocation(ctx, handle, "node-a", "vg-a"); err != nil { + t.Fatalf("recordSnapshotLocation failed: %v", err) + } + location := cs.kubeClient.(*fakeKubeClient).configMaps[name] + if location == nil || location.Immutable == nil || !*location.Immutable { + t.Fatalf("snapshot location ConfigMap is not immutable: %#v", location) + } + // CreateSnapshot is idempotent, so recording the same location again must be + // idempotent as well. + if err := cs.recordSnapshotLocation(ctx, handle, "node-a", "vg-a"); err != nil { + t.Fatalf("idempotent recordSnapshotLocation failed: %v", err) + } + + nodeName, vgName, found, err := cs.lookupSnapshotLocation(ctx, handle) + if err != nil || !found || nodeName != "node-a" || vgName != "vg-a" { + t.Fatalf("lookupSnapshotLocation = (%q, %q, %t, %v)", nodeName, vgName, found, err) + } + + if err := cs.recordSnapshotLocation(ctx, handle, "node-b", "vg-b"); err == nil { + t.Fatal("expected a conflicting location record to fail") + } + if err := cs.forgetSnapshotLocation(ctx, handle); err != nil { + t.Fatalf("forgetSnapshotLocation failed: %v", err) + } + if _, _, found, err := cs.lookupSnapshotLocation(ctx, handle); err != nil || found { + t.Fatalf("expected forgotten location to be absent, found=%t err=%v", found, err) + } + if err := cs.forgetSnapshotLocation(ctx, handle); err != nil { + t.Fatalf("idempotent forgetSnapshotLocation failed: %v", err) + } +} + +func TestSnapshotLocationRejectsMalformedRecord(t *testing.T) { + cs := controllerWithFakeClients() + handle := "snapshot-id" + cs.kubeClient.(*fakeKubeClient).configMaps[snapshotLocationConfigMapName(handle)] = &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: snapshotLocationConfigMapName(handle), + Labels: map[string]string{snapshotLocationLabel: "true"}, + }, + Data: map[string]string{ + snapshotLocationHandleKey: handle, + snapshotLocationNodeKey: "node-a", + }, + } + + if _, _, _, err := cs.lookupSnapshotLocation(context.Background(), handle); err == nil { + t.Fatal("expected an incomplete location record to fail") + } +} + +func TestCloneFromSnapshotSourceResolvesContentByHandle(t *testing.T) { + cs := controllerWithFakeClients() + restoreSize := int64(2 << 30) + cs.snapClient = &fakeSnapshotClient{contents: []snapv1.VolumeSnapshotContent{{ + ObjectMeta: metav1.ObjectMeta{Name: "data-mover-content"}, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{SnapshotHandle: strPointer("snapshot-id")}, + }, + Status: &snapv1.VolumeSnapshotContentStatus{ + SnapshotHandle: strPointer("snapshot-id"), + RestoreSize: &restoreSize, + }, + }}} + + err := cs.cloneFromContentSource( + context.Background(), + &csi.VolumeContentSource{Type: &csi.VolumeContentSource_Snapshot{ + Snapshot: &csi.VolumeContentSource_SnapshotSource{SnapshotId: "snapshot-id"}, + }}, + "destination", + "node-a", + DmThinType, + "vg", + 1<<30, + ) + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("expected size validation after handle-based resolution, got %v", err) + } +} + +func TestCloneFromSnapshotSourceReturnsNotFound(t *testing.T) { + cs := controllerWithFakeClients() + err := cs.cloneFromContentSource( + context.Background(), + &csi.VolumeContentSource{Type: &csi.VolumeContentSource_Snapshot{ + Snapshot: &csi.VolumeContentSource_SnapshotSource{SnapshotId: "missing"}, + }}, + "destination", + "node-a", + DmThinType, + "vg", + 1<<30, + ) + if status.Code(err) != codes.NotFound { + t.Fatalf("expected NotFound, got %v", err) + } +} + +func TestPreProvisionedSnapshotCloneAction(t *testing.T) { + cs := controllerWithFakeClients() + restoreSize := int64(1 << 30) + content := &snapv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pre-existing", + Annotations: map[string]string{ + snapshotNodeAnnotation: "node-a", + snapshotVGAnnotation: "source-vg", + }, + }, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{SnapshotHandle: strPointer("snapshot-id")}, + }, + Status: &snapv1.VolumeSnapshotContentStatus{RestoreSize: &restoreSize}, + } + + action, err := cs.preProvisionedSnapshotCloneAction( + content, + "destination", + "node-a", + DmThinType, + "destination-vg", + 2<<30, + ) + if err != nil { + t.Fatalf("pre-provisioned clone action failed: %v", err) + } + if action.action != actionTypeClone || action.name != "destination" || action.size != 2<<30 { + t.Fatalf("unexpected clone action: %#v", action) + } + if action.srcInfo == nil || + action.srcInfo.srcLVName != "lvm-snapshot-id" || + action.srcInfo.srcVGName != "source-vg" || + action.srcInfo.srcType != DmThinType { + t.Fatalf("unexpected clone source: %#v", action.srcInfo) + } +} + +func TestPreProvisionedSnapshotCloneActionUsesDestinationLocationFallback(t *testing.T) { + cs := controllerWithFakeClients() + zeroSize := int64(0) + content := &snapv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{Name: "pre-provisioned"}, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{SnapshotHandle: strPointer("snapshot-id")}, + }, + Status: &snapv1.VolumeSnapshotContentStatus{RestoreSize: &zeroSize}, + } + + action, err := cs.preProvisionedSnapshotCloneAction( + content, + "destination", + "node-a", + DmThinType, + "vg", + 1<<30, + ) + if err != nil { + t.Fatalf("fallback clone action failed: %v", err) + } + if action.srcInfo == nil || action.srcInfo.srcVGName != "vg" || action.nodeName != "node-a" || action.size != 1<<30 { + t.Fatalf("unexpected fallback action: %#v", action) + } +} + +func TestPreProvisionedSnapshotCloneActionRejectsNodeMismatch(t *testing.T) { + cs := controllerWithFakeClients() + content := &snapv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pre-existing", + Annotations: map[string]string{snapshotNodeAnnotation: "node-b"}, + }, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{SnapshotHandle: strPointer("snapshot-id")}, + }, + } + + _, err := cs.preProvisionedSnapshotCloneAction( + content, + "destination", + "node-a", + DmThinType, + "vg", + 1<<30, + ) + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("expected node mismatch to fail, got %v", err) + } +} + +func TestCloneFromDynamicSnapshotRejectsIncompleteStatus(t *testing.T) { + cs := controllerWithFakeClients() + content := &snapv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{Name: "dynamic"}, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{VolumeHandle: strPointer("source-volume")}, + }, + } + + err := cs.cloneFromSnapshot( + context.Background(), + content, + "destination", + "node-a", + DmThinType, + "vg", + 1<<30, + ) + if status.Code(err) != codes.FailedPrecondition { + t.Fatalf("expected FailedPrecondition, got %v", err) + } +} diff --git a/pkg/lvm/encryption.go b/pkg/lvm/encryption.go new file mode 100644 index 00000000..8eb49878 --- /dev/null +++ b/pkg/lvm/encryption.go @@ -0,0 +1,313 @@ +/* +Copyright 2017 The Kubernetes Authors. + +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 lvm + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "strconv" + "strings" + + "k8s.io/klog/v2" +) + +const ( + // encryptedParam is the StorageClass parameter (propagated into the volume + // context via buildVolumeContext) that opts a volume into LUKS2 encryption + // at rest. + encryptedParam = "encrypted" + + // Longhorn / Kubernetes CSI encryption-secret convention. Harvester's + // admission webhook (harvester-webhook) validates that any StorageClass + // referencing a CSI encryption secret carries these fields, exactly as it + // does for Longhorn encrypted volumes. The LVM driver therefore reads the + // same schema so an encrypted LVM StorageClass is a drop-in with the + // platform's existing encrypted-volume workflow and UI. The passphrase + // lives in CRYPTO_KEY_VALUE; the remaining fields are LUKS2 tuning knobs. + cryptoKeyValue = "CRYPTO_KEY_VALUE" // the passphrase (required) + cryptoKeyProvider = "CRYPTO_KEY_PROVIDER" // key provider, only "secret" is supported + cryptoKeyCipher = "CRYPTO_KEY_CIPHER" // luksFormat --cipher + cryptoKeyHash = "CRYPTO_KEY_HASH" // luksFormat --hash + cryptoKeySize = "CRYPTO_KEY_SIZE" // luksFormat --key-size + cryptoPBKDF = "CRYPTO_PBKDF" // luksFormat --pbkdf + + // LUKS2 format defaults, matching Longhorn's defaults, applied when the + // secret omits the optional tuning fields. + defaultCryptoCipher = "aes-xts-plain64" + defaultCryptoHash = "sha256" + defaultCryptoKeySize = "256" + defaultCryptoPBKDF = "argon2i" + + // cryptMapperPrefix namespaces the dm-crypt mapper devices this driver + // creates under /dev/mapper so they are easy to identify and never collide + // with other consumers. + cryptMapperPrefix = "csi-lvm-" + + // cryptsetup exit codes we care about. See cryptsetup(8) EXIT STATUS. + cryptExitNotLuks = 1 // isLuks: device does not carry a LUKS header + cryptExitInactive = 4 // status: no such active mapping +) + +// cryptExecutor runs cryptsetup with the passphrase supplied on stdin so it +// never appears in the host process list (argv). It is a package variable so +// unit tests can substitute a fake without shelling out. This mirrors the +// newCommandExecutor pattern used for the LVM commands, but adds stdin support +// which the shared go-common executor does not provide. +type cryptExecutor interface { + Execute(command string, args []string, stdin string) (string, error) +} + +var newCryptExecutor = func() cryptExecutor { + return &execCryptExecutor{} +} + +type execCryptExecutor struct{} + +func (e *execCryptExecutor) Execute(command string, args []string, stdin string) (string, error) { + c := exec.Command(command, args...) + if stdin != "" { + // Feed the passphrase over stdin; keeping it out of argv avoids + // leaking it via /proc//cmdline and the host process list. + c.Stdin = strings.NewReader(stdin) + } + var buf bytes.Buffer + c.Stdout = &buf + c.Stderr = &buf + err := c.Run() + out := buf.String() + if err != nil { + // Wrap with %w so commandExitCode can recover the exec.ExitError and + // its ExitCode(). args never contain the passphrase, so logging them is + // safe. + return out, fmt.Errorf("command %s %v failed: %w", command, args, err) + } + return out, nil +} + +// isEncrypted reports whether the volume context opts into encryption at rest. +func isEncrypted(volumeContext map[string]string) bool { + value, ok := volumeContext[encryptedParam] + if !ok { + return false + } + enabled, err := strconv.ParseBool(value) + return err == nil && enabled +} + +// luks2HeaderBytes is the space cryptsetup's default LUKS2 format reserves ahead +// of the data payload for the header and keyslot area (the default data offset +// is 16 MiB). The dm-crypt mapper therefore exposes 16 MiB less than its backing +// block device. We never pass --offset to luksFormat, so this default always +// applies. +const luks2HeaderBytes int64 = 16 * 1024 * 1024 + +// backingLVBytes returns the backing LV size needed to expose usableBytes of +// (decrypted) capacity. For encrypted volumes that is usableBytes plus the LUKS2 +// header overhead, so the requested capacity is honored end-to-end (e.g. a 10Gi +// encrypted PVC yields a full 10Gi usable device, which exact-fit consumers such +// as CDI/KubeVirt image imports require). For plain volumes it is unchanged. +func backingLVBytes(usableBytes int64, encrypted bool) int64 { + if encrypted { + return usableBytes + luks2HeaderBytes + } + return usableBytes +} + +// cryptMapperName is the dm-crypt mapping name for a volume. +func cryptMapperName(volID string) string { + return cryptMapperPrefix + volID +} + +// cryptMapperPath is the /dev/mapper path of the opened dm-crypt device. +func cryptMapperPath(volID string) string { + return "/dev/mapper/" + cryptMapperName(volID) +} + +// mapperExists reports whether an open dm-crypt mapping node exists for the +// volume. It is a cheap filesystem check that lets the unpublish/expand paths +// (which have no volume context) skip cryptsetup entirely for plain, +// non-encrypted volumes — the mapper only exists while a LUKS device is open. +func mapperExists(volID string) bool { + _, err := os.Stat(cryptMapperPath(volID)) + return err == nil +} + +// cryptoParams captures the LUKS2 tuning read from a CRYPTO_KEY_* encryption +// secret. Only the passphrase is required; the rest fall back to Longhorn's +// defaults so a minimal secret still produces a Longhorn-compatible LUKS device. +type cryptoParams struct { + passphrase string + cipher string + hash string + keySize string + pbkdf string +} + +// extractCryptoParams pulls the passphrase and LUKS tuning out of the CSI +// encryption secret, following the Longhorn CRYPTO_KEY_* convention that the +// Harvester webhook enforces. +func extractCryptoParams(secrets map[string]string) (*cryptoParams, error) { + passphrase := secrets[cryptoKeyValue] + if passphrase == "" { + return nil, fmt.Errorf( + "encrypted volume requires a non-empty %q entry in the encryption secret", + cryptoKeyValue, + ) + } + return &cryptoParams{ + passphrase: passphrase, + cipher: valueOrDefault(secrets[cryptoKeyCipher], defaultCryptoCipher), + hash: valueOrDefault(secrets[cryptoKeyHash], defaultCryptoHash), + keySize: valueOrDefault(secrets[cryptoKeySize], defaultCryptoKeySize), + pbkdf: valueOrDefault(secrets[cryptoPBKDF], defaultCryptoPBKDF), + }, nil +} + +func valueOrDefault(value, fallback string) string { + if value == "" { + return fallback + } + return value +} + +// openEncryptedDevice ensures the block device at devicePath carries a LUKS2 +// header (formatting it on first use) and opens it, returning the resulting +// /dev/mapper path to be mounted or bind-mounted. It is idempotent: a device +// that is already open is reused, so repeated NodePublishVolume calls are safe. +func openEncryptedDevice(devicePath, volID string, params *cryptoParams) (string, error) { + executor := newCryptExecutor() + mapperName := cryptMapperName(volID) + mapperPath := cryptMapperPath(volID) + + // Idempotent re-publish: if the mapping is already open, reuse it. + if mapperExists(volID) { + klog.Infof("dm-crypt device %s already open, reusing", mapperName) + return mapperPath, nil + } + + formatted, err := isLuks(executor, devicePath) + if err != nil { + return "", err + } + if !formatted { + klog.Infof("formatting %s as LUKS2 for encrypted volume %s", devicePath, volID) + if out, err := luksFormat(executor, devicePath, params); err != nil { + return "", fmt.Errorf("unable to LUKS-format %s: %w output:%s", devicePath, err, out) + } + } + + if out, err := luksOpen(executor, devicePath, mapperName, params.passphrase); err != nil { + return "", fmt.Errorf("unable to open LUKS device %s: %w output:%s", devicePath, err, out) + } + klog.Infof("opened dm-crypt device %s for volume %s", mapperName, volID) + return mapperPath, nil +} + +// closeEncryptedDevice tears down the dm-crypt mapping for a volume. It is +// idempotent and needs no passphrase: an already-closed (or never-encrypted) +// volume is a no-op. The caller must unmount the mapper first. +func closeEncryptedDevice(volID string) error { + if !mapperExists(volID) { + return nil + } + mapperName := cryptMapperName(volID) + if out, err := luksClose(newCryptExecutor(), mapperName); err != nil { + return fmt.Errorf("unable to close LUKS device %s: %w output:%s", mapperName, err, out) + } + klog.Infof("closed dm-crypt device %s for volume %s", mapperName, volID) + return nil +} + +// resizeEncryptedDevice grows the dm-crypt mapping to match the (already +// extended) backing LV. LUKS2 re-derives the volume key from a keyslot on +// resize unless the key is available in an accessible kernel keyring; in the +// CSI node plugin's mount namespace it is not, so cryptsetup would otherwise +// block on an interactive passphrase prompt. The passphrase is therefore fed +// on stdin (via --key-file -). Returns whether the mapping was active. +func resizeEncryptedDevice(volID, passphrase string) (bool, string, error) { + if !mapperExists(volID) { + return false, "", nil + } + mapperName := cryptMapperName(volID) + out, err := luksResize(newCryptExecutor(), mapperName, passphrase) + if err != nil { + return true, out, fmt.Errorf("unable to resize LUKS device %s: %w output:%s", mapperName, err, out) + } + return true, out, nil +} + +// encryptedVolumeActive reports whether an open dm-crypt mapping exists for the +// volume. Used by paths (expand, unpublish) that receive no volume context. +func encryptedVolumeActive(volID string) (bool, error) { + if !mapperExists(volID) { + return false, nil + } + return luksStatus(newCryptExecutor(), cryptMapperName(volID)) +} + +func isLuks(executor cryptExecutor, devicePath string) (bool, error) { + _, err := executor.Execute("cryptsetup", []string{"isLuks", devicePath}, "") + if err == nil { + return true, nil + } + if code, ok := commandExitCode(err); ok && code == cryptExitNotLuks { + return false, nil + } + return false, fmt.Errorf("unable to probe LUKS header on %s: %w", devicePath, err) +} + +func luksStatus(executor cryptExecutor, mapperName string) (bool, error) { + _, err := executor.Execute("cryptsetup", []string{"status", mapperName}, "") + if err == nil { + return true, nil + } + if code, ok := commandExitCode(err); ok && code == cryptExitInactive { + return false, nil + } + return false, fmt.Errorf("unable to query status of dm-crypt device %s: %w", mapperName, err) +} + +func luksFormat(executor cryptExecutor, devicePath string, params *cryptoParams) (string, error) { + return executor.Execute( + "cryptsetup", + []string{ + "luksFormat", "--type", "luks2", + "--cipher", params.cipher, + "--hash", params.hash, + "--key-size", params.keySize, + "--pbkdf", params.pbkdf, + "--batch-mode", devicePath, + }, + params.passphrase, + ) +} + +func luksOpen(executor cryptExecutor, devicePath, mapperName, passphrase string) (string, error) { + return executor.Execute("cryptsetup", []string{"luksOpen", devicePath, mapperName}, passphrase) +} + +func luksClose(executor cryptExecutor, mapperName string) (string, error) { + return executor.Execute("cryptsetup", []string{"luksClose", mapperName}, "") +} + +func luksResize(executor cryptExecutor, mapperName, passphrase string) (string, error) { + // Read the passphrase from stdin (--key-file -) so cryptsetup can unlock the + // keyslot non-interactively; keeping it off argv avoids leaking it via ps. + return executor.Execute("cryptsetup", []string{"resize", "--key-file", "-", mapperName}, passphrase) +} diff --git a/pkg/lvm/encryption_test.go b/pkg/lvm/encryption_test.go new file mode 100644 index 00000000..7cfa4780 --- /dev/null +++ b/pkg/lvm/encryption_test.go @@ -0,0 +1,277 @@ +package lvm + +import ( + "errors" + "reflect" + "strings" + "testing" +) + +type cryptCall struct { + command string + args []string + stdin string +} + +type cryptResult struct { + output string + err error +} + +type fakeCryptExecutor struct { + t *testing.T + results []cryptResult + calls []cryptCall +} + +func (f *fakeCryptExecutor) Execute(command string, args []string, stdin string) (string, error) { + f.t.Helper() + f.calls = append(f.calls, cryptCall{command: command, args: append([]string(nil), args...), stdin: stdin}) + if len(f.results) == 0 { + f.t.Fatalf("unexpected crypt command: %s %v", command, args) + } + result := f.results[0] + f.results = f.results[1:] + return result.output, result.err +} + +func useFakeCryptExecutor(t *testing.T, fake *fakeCryptExecutor) { + t.Helper() + original := newCryptExecutor + newCryptExecutor = func() cryptExecutor { + return fake + } + t.Cleanup(func() { + newCryptExecutor = original + }) +} + +func TestIsEncrypted(t *testing.T) { + cases := []struct { + name string + context map[string]string + want bool + }{ + {"absent", map[string]string{}, false}, + {"true", map[string]string{encryptedParam: "true"}, true}, + {"one", map[string]string{encryptedParam: "1"}, true}, + {"false", map[string]string{encryptedParam: "false"}, false}, + {"garbage", map[string]string{encryptedParam: "yesplease"}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := isEncrypted(tc.context); got != tc.want { + t.Fatalf("isEncrypted(%v)=%t, want %t", tc.context, got, tc.want) + } + }) + } +} + +func TestExtractCryptoParams(t *testing.T) { + if _, err := extractCryptoParams(map[string]string{}); err == nil { + t.Fatal("expected error for missing passphrase") + } + if _, err := extractCryptoParams(map[string]string{cryptoKeyValue: ""}); err == nil { + t.Fatal("expected error for empty passphrase") + } + + // Minimal secret: passphrase only -> Longhorn defaults fill the rest. + got, err := extractCryptoParams(map[string]string{cryptoKeyValue: "hunter2"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.passphrase != "hunter2" { + t.Fatalf("passphrase = %q, want %q", got.passphrase, "hunter2") + } + if got.cipher != defaultCryptoCipher || got.hash != defaultCryptoHash || + got.keySize != defaultCryptoKeySize || got.pbkdf != defaultCryptoPBKDF { + t.Fatalf("defaults not applied: %+v", got) + } + + // Full secret: explicit tuning is honored. + full, err := extractCryptoParams(map[string]string{ + cryptoKeyValue: "pw", + cryptoKeyCipher: "aes-cbc-essiv:sha256", + cryptoKeyHash: "sha512", + cryptoKeySize: "512", + cryptoPBKDF: "pbkdf2", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if full.cipher != "aes-cbc-essiv:sha256" || full.hash != "sha512" || + full.keySize != "512" || full.pbkdf != "pbkdf2" { + t.Fatalf("explicit tuning not honored: %+v", full) + } +} + +func TestCryptMapperNaming(t *testing.T) { + if got := cryptMapperName("abc"); got != "csi-lvm-abc" { + t.Fatalf("cryptMapperName = %q", got) + } + if got := cryptMapperPath("abc"); got != "/dev/mapper/csi-lvm-abc" { + t.Fatalf("cryptMapperPath = %q", got) + } +} + +func TestIsLuks(t *testing.T) { + t.Run("is luks", func(t *testing.T) { + fake := &fakeCryptExecutor{t: t, results: []cryptResult{{}}} + ok, err := isLuks(fake, "/dev/vg/lv") + if err != nil || !ok { + t.Fatalf("expected (true,nil), got (%t,%v)", ok, err) + } + }) + t.Run("not luks", func(t *testing.T) { + fake := &fakeCryptExecutor{t: t, results: []cryptResult{{err: commandExitError{code: cryptExitNotLuks}}}} + ok, err := isLuks(fake, "/dev/vg/lv") + if err != nil || ok { + t.Fatalf("expected (false,nil), got (%t,%v)", ok, err) + } + }) + t.Run("device error", func(t *testing.T) { + fake := &fakeCryptExecutor{t: t, results: []cryptResult{{err: commandExitError{code: 4}}}} + if _, err := isLuks(fake, "/dev/vg/lv"); err == nil { + t.Fatal("expected error for non-1 exit code") + } + }) +} + +func TestLuksStatus(t *testing.T) { + t.Run("active", func(t *testing.T) { + fake := &fakeCryptExecutor{t: t, results: []cryptResult{{}}} + ok, err := luksStatus(fake, "csi-lvm-x") + if err != nil || !ok { + t.Fatalf("expected (true,nil), got (%t,%v)", ok, err) + } + }) + t.Run("inactive", func(t *testing.T) { + fake := &fakeCryptExecutor{t: t, results: []cryptResult{{err: commandExitError{code: cryptExitInactive}}}} + ok, err := luksStatus(fake, "csi-lvm-x") + if err != nil || ok { + t.Fatalf("expected (false,nil), got (%t,%v)", ok, err) + } + }) + t.Run("other error", func(t *testing.T) { + fake := &fakeCryptExecutor{t: t, results: []cryptResult{{err: errors.New("boom")}}} + if _, err := luksStatus(fake, "csi-lvm-x"); err == nil { + t.Fatal("expected error") + } + }) +} + +// openEncryptedDevice on a fresh (never-formatted) device should probe, format +// with LUKS2, then open — and the passphrase must travel over stdin, never argv. +func TestOpenEncryptedDeviceFormatsWhenNotLuks(t *testing.T) { + const volID = "unit-open-fresh" // no /dev/mapper node exists for this in the test env + const passphrase = "s3cr3t-pass" + fake := &fakeCryptExecutor{ + t: t, + results: []cryptResult{ + {err: commandExitError{code: cryptExitNotLuks}}, // isLuks -> not luks + {}, // luksFormat + {}, // luksOpen + }, + } + useFakeCryptExecutor(t, fake) + + params := &cryptoParams{ + passphrase: passphrase, + cipher: defaultCryptoCipher, + hash: defaultCryptoHash, + keySize: defaultCryptoKeySize, + pbkdf: defaultCryptoPBKDF, + } + mapperPath, err := openEncryptedDevice("/dev/vg/"+volID, volID, params) + if err != nil { + t.Fatalf("openEncryptedDevice failed: %v", err) + } + if want := cryptMapperPath(volID); mapperPath != want { + t.Fatalf("mapperPath = %q, want %q", mapperPath, want) + } + if len(fake.calls) != 3 { + t.Fatalf("expected 3 cryptsetup calls, got %d: %#v", len(fake.calls), fake.calls) + } + assertCryptSubcommand(t, fake.calls[0], "isLuks", "") + assertCryptSubcommand(t, fake.calls[1], "luksFormat", passphrase) + assertCryptSubcommand(t, fake.calls[2], "luksOpen", passphrase) + + // The passphrase must never appear in argv. + for _, call := range fake.calls { + for _, arg := range call.args { + if strings.Contains(arg, passphrase) { + t.Fatalf("passphrase leaked into argv: %v", call.args) + } + } + } + // luksFormat must request LUKS2 in batch mode with the Longhorn-style tuning. + wantFormat := []string{ + "luksFormat", "--type", "luks2", + "--cipher", defaultCryptoCipher, + "--hash", defaultCryptoHash, + "--key-size", defaultCryptoKeySize, + "--pbkdf", defaultCryptoPBKDF, + "--batch-mode", "/dev/vg/" + volID, + } + if !reflect.DeepEqual(fake.calls[1].args, wantFormat) { + t.Fatalf("unexpected luksFormat args: %v", fake.calls[1].args) + } +} + +// A device that already carries a LUKS header must be opened, not reformatted. +func TestOpenEncryptedDeviceSkipsFormatWhenLuks(t *testing.T) { + const volID = "unit-open-existing" + fake := &fakeCryptExecutor{ + t: t, + results: []cryptResult{ + {}, // isLuks -> is luks + {}, // luksOpen + }, + } + useFakeCryptExecutor(t, fake) + + if _, err := openEncryptedDevice("/dev/vg/"+volID, volID, &cryptoParams{passphrase: "pw"}); err != nil { + t.Fatalf("openEncryptedDevice failed: %v", err) + } + if len(fake.calls) != 2 { + t.Fatalf("expected 2 calls (isLuks, luksOpen), got %#v", fake.calls) + } + assertCryptSubcommand(t, fake.calls[0], "isLuks", "") + assertCryptSubcommand(t, fake.calls[1], "luksOpen", "pw") +} + +// For a plain (non-encrypted) volume no dm-crypt mapper exists, so the +// unpublish/expand helpers must be no-ops that never invoke cryptsetup. +func TestCloseAndResizeNoopWithoutMapper(t *testing.T) { + const volID = "unit-plain-volume" + fake := &fakeCryptExecutor{t: t} // no results -> any call fails the test + useFakeCryptExecutor(t, fake) + + if err := closeEncryptedDevice(volID); err != nil { + t.Fatalf("closeEncryptedDevice should be a no-op, got %v", err) + } + active, _, err := resizeEncryptedDevice(volID, "unit-test-passphrase") + if err != nil || active { + t.Fatalf("resizeEncryptedDevice should be a no-op, got (active=%t, err=%v)", active, err) + } + got, err := encryptedVolumeActive(volID) + if err != nil || got { + t.Fatalf("encryptedVolumeActive should be false, got (%t, %v)", got, err) + } + if len(fake.calls) != 0 { + t.Fatalf("expected no cryptsetup calls, got %#v", fake.calls) + } +} + +func assertCryptSubcommand(t *testing.T, call cryptCall, subcommand, stdin string) { + t.Helper() + if call.command != "cryptsetup" { + t.Fatalf("expected cryptsetup, got %q", call.command) + } + if len(call.args) == 0 || call.args[0] != subcommand { + t.Fatalf("expected subcommand %q, got args %v", subcommand, call.args) + } + if call.stdin != stdin { + t.Fatalf("expected stdin %q for %s, got %q", stdin, subcommand, call.stdin) + } +} diff --git a/pkg/lvm/lvm.go b/pkg/lvm/lvm.go index 0d8decc6..dce29227 100644 --- a/pkg/lvm/lvm.go +++ b/pkg/lvm/lvm.go @@ -18,22 +18,25 @@ package lvm import ( "context" + "encoding/json" + "errors" "fmt" "os" "path/filepath" - "regexp" "strconv" "strings" "time" cmd "github.com/harvester/go-common/command" ioutil "github.com/harvester/go-common/io" + "golang.org/x/sys/unix" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" v1 "k8s.io/api/core/v1" k8serror "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" + corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/klog/v2" ) @@ -75,7 +78,7 @@ type volumeAction struct { lvmType string provisionerImage string pullPolicy v1.PullPolicy - kubeClient kubernetes.Clientset + kubeClient kubernetes.Interface namespace string vgName string hostWritePath string @@ -91,7 +94,7 @@ type snapshotAction struct { snapSize int64 provisionerImage string pullPolicy v1.PullPolicy - kubeClient kubernetes.Clientset + kubeClient kubernetes.Interface namespace string vgName string lvType string @@ -99,22 +102,55 @@ type snapshotAction struct { } const ( - ThinVolType = "thin" - ThinPoolType = "thin-pool" - StripedType = "striped" - DmThinType = "dm-thin" - actionTypeCreate = "create" - actionTypeDelete = "delete" - actionTypeClone = "clone" - pullIfNotPresent = "ifnotpresent" - fsTypeRegexpString = `TYPE="(\w+)"` - DefaultChunkSize = 4 * 1024 * 1024 + ThinVolType = "thin" + ThinPoolType = "thin-pool" + LinearType = "linear" + StripedType = "striped" + DmThinType = "dm-thin" + actionTypeCreate = "create" + actionTypeDelete = "delete" + actionTypeClone = "clone" + pullIfNotPresent = "ifnotpresent" + DefaultChunkSize = 4 * 1024 * 1024 + + // Keep these exit statuses synchronized with util-linux misc-utils/blkid.c. + // https://github.com/util-linux/util-linux/blob/master/misc-utils/blkid.c + blkidExitNotFound = 2 + blkidExitOther = 4 + blkidExitAmbiguous = 8 ) +type commandExecutor interface { + Execute(command string, args []string) (string, error) +} + var ( - fsTypeRegexp = regexp.MustCompile(fsTypeRegexpString) + newCommandExecutor = func() commandExecutor { + return cmd.NewExecutor() + } + unmountPath = unix.Unmount ) +type wipefsReport struct { + Signatures *[]struct { + Type string `json:"type"` + } `json:"signatures"` +} + +type logicalVolume struct { + Name string `json:"lv_name"` + VGName string `json:"vg_name"` + Size string `json:"lv_size"` + SegType string `json:"segtype"` + Origin string `json:"origin"` +} + +type logicalVolumeReport struct { + Reports []struct { + Volumes []logicalVolume `json:"lv"` + } `json:"report"` +} + // NewLvmDriver creates the driver func NewLvmDriver(driverName, nodeID, endpoint string, hostWritePath string, maxVolumesPerNode int64, version string, namespace string, provisionerImage string, pullPolicy string) (*Lvm, error) { if driverName == "" { @@ -159,7 +195,10 @@ func (lvm *Lvm) Run() error { var err error // Create GRPC servers lvm.ids = newIdentityServer(lvm.name, lvm.version) - lvm.ns = newNodeServer(lvm.nodeID, lvm.maxVolumesPerNode) + lvm.ns, err = newNodeServer(lvm.nodeID, lvm.maxVolumesPerNode) + if err != nil { + return err + } lvm.cs, err = newControllerServer(lvm.nodeID, lvm.hostWritePath, lvm.namespace, lvm.provisionerImage, lvm.pullPolicy) if err != nil { return err @@ -170,248 +209,516 @@ func (lvm *Lvm) Run() error { return nil } -func mountLV(lvname, mountPath string, vgName string, fsType string) (string, error) { - executor := cmd.NewExecutor() - lvPath := fmt.Sprintf("/dev/%s/%s", vgName, lvname) +// mountLV formats (when required) and mounts the device at lvPath onto +// mountPath. lvPath is the resolved block device to mount: for plain volumes +// this is /dev//, and for encrypted volumes it is the opened dm-crypt +// mapper (/dev/mapper/csi-lvm-) produced by NodePublishVolume. +func mountLV(lvPath, mountPath, fsType string, mountOptions []string, readOnly bool) (string, error) { + executor := newCommandExecutor() + fsType = defaultFilesystemType(fsType) + + formatOutput, err := ensureFilesystem(executor, lvPath, fsType) + if err != nil { + return formatOutput, err + } + + return mountFilesystem(executor, lvPath, mountPath, fsType, mountOptions, readOnly) +} - formatted := false - forceFormat := false +func defaultFilesystemType(fsType string) string { if fsType == "" { - fsType = "ext4" + return "ext4" } - out, err := executor.Execute("blkid", []string{lvPath}) + return fsType +} + +func ensureFilesystem(executor commandExecutor, lvPath, fsType string) (string, error) { + existingFSType, err := getFilesystemType(executor, lvPath) if err != nil { - klog.Infof("unable to check if %s is already formatted:%v", lvPath, err) + return "", err } - matches := fsTypeRegexp.FindStringSubmatch(out) - if len(matches) > 1 { - if matches[1] == "xfs_external_log" { // If old xfs signature was found - forceFormat = true - } else { - if matches[1] != fsType { - return out, fmt.Errorf("target fsType is %s but %s found", fsType, matches[1]) - } - formatted = true - } + if existingFSType != "" && existingFSType != fsType { + return "", fmt.Errorf("target fsType is %s but %s found", fsType, existingFSType) + } + if existingFSType != "" { + return "", nil } - if !formatted { - formatArgs := []string{} - if forceFormat { - formatArgs = append(formatArgs, "-f") - } - formatArgs = append(formatArgs, lvPath) + klog.Infof("formatting with mkfs.%s %s", fsType, lvPath) + out, err := executor.Execute(fmt.Sprintf("mkfs.%s", fsType), []string{lvPath}) + if err != nil { + return out, fmt.Errorf("unable to format lv:%s err:%w", lvPath, err) + } + return out, nil +} - klog.Infof("formatting with mkfs.%s %s", fsType, strings.Join(formatArgs, " ")) - out, err = executor.Execute(fmt.Sprintf("mkfs.%s", fsType), formatArgs) - if err != nil { - return out, fmt.Errorf("unable to format lv:%s err:%w", lvname, err) - } +func mountFilesystem(executor commandExecutor, lvPath, mountPath, fsType string, mountOptions []string, readOnly bool) (string, error) { + if err := os.MkdirAll(mountPath, 0777|os.ModeSetgid); err != nil { + return "", fmt.Errorf("unable to create mount directory for lv:%s err:%w", lvPath, err) } - err = os.MkdirAll(mountPath, 0777|os.ModeSetgid) + mountArgs := buildFilesystemMountArgs(lvPath, mountPath, fsType, mountOptions, readOnly) + out, err := performFilesystemMount(executor, mountArgs, lvPath, mountPath, readOnly) if err != nil { - return out, fmt.Errorf("unable to create mount directory for lv:%s err:%w", lvname, err) + return out, err + } + if readOnly { + return "", nil + } + if err := os.Chmod(mountPath, 0777|os.ModeSetgid); err != nil { + return "", fmt.Errorf("unable to change permissions of volume mount %s err:%w", mountPath, err) } + return "", nil +} +func buildFilesystemMountArgs(lvPath, mountPath, fsType string, mountOptions []string, readOnly bool) []string { // --make-shared is required that this mount is visible outside this container. - mountArgs := []string{"--make-shared", "-t", fsType, lvPath, mountPath} + mountArgs := []string{"--make-shared", "-t", fsType} + options := normalizeMountOptions(mountOptions, readOnly) + if len(options) > 0 { + mountArgs = append(mountArgs, "-o", strings.Join(options, ",")) + } + return append(mountArgs, lvPath, mountPath) +} + +func performFilesystemMount(executor commandExecutor, mountArgs []string, lvPath, mountPath string, readOnly bool) (string, error) { klog.Infof("mountlv command: mount %s", mountArgs) - out, err = executor.Execute("mount", mountArgs) + out, err := executor.Execute("mount", mountArgs) + if err == nil { + klog.Infof("mountlv output:%s", out) + return out, nil + } + + mountOutput := out + " " + err.Error() + if !strings.Contains(strings.ToLower(mountOutput), "already mounted") { + return out, fmt.Errorf("unable to mount %s to %s err:%w output:%s", lvPath, mountPath, err, out) + } + if err := validateExistingMount(executor, mountPath, readOnly); err != nil { + return out, fmt.Errorf("existing mount at %s is incompatible: %w", mountPath, err) + } + return out, nil +} + +func validateExistingMount(executor commandExecutor, mountPath string, readOnly bool) error { + out, err := executor.Execute( + "findmnt", + []string{"--noheadings", "--output", "OPTIONS", "--mountpoint", mountPath}, + ) if err != nil { - mountOutput := out - if !strings.Contains(mountOutput, "already mounted") { - return out, fmt.Errorf("unable to mount %s to %s err:%w output:%s", lvPath, mountPath, err, out) + return fmt.Errorf("unable to verify mount: %w output:%s", err, out) + } + if !readOnly { + return nil + } + for _, option := range strings.Split(strings.TrimSpace(out), ",") { + if option == "ro" { + return nil } } - err = os.Chmod(mountPath, 0777|os.ModeSetgid) - if err != nil { - return "", fmt.Errorf("unable to change permissions of volume mount %s err:%w", mountPath, err) + return fmt.Errorf("readonly was requested but existing mount options are %q", strings.TrimSpace(out)) +} + +func getFilesystemType(executor commandExecutor, lvPath string) (string, error) { + // Probe the device directly. lsblk can return an empty FSTYPE when the + // container's udev database is missing or stale, even for a mounted filesystem. + out, err := executor.Execute("blkid", []string{"-p", "-s", "TYPE", "-o", "value", lvPath}) + if err == nil { + fsType := strings.TrimSpace(out) + if fsType == "" { + return "", fmt.Errorf("blkid succeeded but returned no filesystem type for %s", lvPath) + } + return fsType, nil + } + + exitCode, ok := commandExitCode(err) + if !ok { + return "", fmt.Errorf("unable to determine filesystem type for %s with blkid: %w", lvPath, err) + } + + switch exitCode { + case blkidExitNotFound: + return getFilesystemTypeFromWipefs(executor, lvPath) + case blkidExitAmbiguous: + return "", fmt.Errorf("ambiguous filesystem signatures detected on %s by blkid: %w", lvPath, err) + case blkidExitOther: + return "", fmt.Errorf("blkid failed to inspect %s: %w", lvPath, err) + default: + return "", fmt.Errorf("blkid returned unexpected status %d for %s: %w", exitCode, lvPath, err) } - klog.Infof("mountlv output:%s", out) - return "", nil } -func bindMountLV(lvname, mountPath string, vgName string) (string, error) { - executor := cmd.NewExecutor() - lvPath := fmt.Sprintf("/dev/%s/%s", vgName, lvname) - _, err := os.Create(mountPath) - if err != nil { - return "", fmt.Errorf("unable to create mount directory for lv:%s err:%w", lvname, err) +func commandExitCode(err error) (int, bool) { + var exitError interface { + ExitCode() int } + if !errors.As(err, &exitError) { + return 0, false + } + return exitError.ExitCode(), true +} - // --make-shared is required that this mount is visible outside this container. - // --bind is required for raw block volumes to make them visible inside the pod. - mountArgs := []string{"--make-shared", "--bind", lvPath, mountPath} - klog.Infof("bindmountlv command: mount %s", mountArgs) - out, err := executor.Execute("mount", mountArgs) +func getFilesystemTypeFromWipefs(executor commandExecutor, lvPath string) (string, error) { + out, err := executor.Execute("wipefs", []string{"--no-act", "--json", "--output", "TYPE", lvPath}) if err != nil { - mountOutput := out - if !strings.Contains(mountOutput, "already mounted") { - return out, fmt.Errorf("unable to mount %s to %s err:%w output:%s", lvPath, mountPath, err, out) + return "", fmt.Errorf("unable to confirm filesystem signatures for %s with wipefs: %w", lvPath, err) + } + + report := wipefsReport{} + if err := json.Unmarshal([]byte(out), &report); err != nil { + return "", fmt.Errorf("unable to parse wipefs output for %s: %w", lvPath, err) + } + if report.Signatures == nil { + return "", fmt.Errorf("wipefs output for %s does not contain a signatures array", lvPath) + } + if len(*report.Signatures) == 0 { + return "", nil + } + if len(*report.Signatures) != 1 { + return "", fmt.Errorf("ambiguous filesystem signatures detected on %s by wipefs: found %d signatures", lvPath, len(*report.Signatures)) + } + + fsType := strings.TrimSpace((*report.Signatures)[0].Type) + if fsType == "" { + return "", fmt.Errorf("wipefs reported a signature without a type for %s", lvPath) + } + return fsType, nil +} + +func normalizeMountOptions(values []string, readOnly bool) []string { + options := make([]string, 0, len(values)+1) + hasReadOnly := false + for _, option := range strings.Split(strings.Join(values, ","), ",") { + option = strings.TrimSpace(option) + if option == "" || readOnly && option == "rw" { + continue + } + if option == "ro" { + hasReadOnly = true } + options = append(options, option) + } + if !readOnly || hasReadOnly { + return options + } + return append(options, "ro") +} + +// bindMountLV bind-mounts the raw block device at lvPath onto mountPath. lvPath +// is the resolved device: /dev// for plain volumes, or the opened +// dm-crypt mapper (/dev/mapper/csi-lvm-) for encrypted volumes. +func bindMountLV(lvPath, mountPath string, readOnly bool) (string, error) { + executor := newCommandExecutor() + if err := prepareBindMountTarget(lvPath, mountPath); err != nil { + return "", err } - err = os.Chmod(mountPath, 0777|os.ModeSetgid) + + out, err := performBindMount(executor, lvPath, mountPath) if err != nil { - return "", fmt.Errorf("unable to change permissions of volume mount %s err:%w", mountPath, err) + return out, err + } + if readOnly { + out, err = remountBindReadOnly(executor, mountPath) + if err != nil { + return out, err + } } klog.Infof("bindmountlv output:%s", out) return "", nil } -func umountLV(targetPath string) { - executor := cmd.NewExecutor() - executor.SetTimeout(30 * time.Second) - generalUmountArgs := []string{"--force", targetPath} - out, err := executor.Execute("umount", generalUmountArgs) - if err == cmd.ErrCmdTimeout { - klog.Infof("umount %s timeout, use lazy", targetPath) - lazyUmountArgs := []string{"--lazy", "--force", targetPath} - out, err := executor.Execute("umount", lazyUmountArgs) - if err != nil { - klog.Errorf("unable to umount %s output:%s err:%v", targetPath, out, err) - } - } else if err != nil { - klog.Errorf("unable to umount %s output:%s err:%v", targetPath, out, err) +func prepareBindMountTarget(lvName, mountPath string) error { + target, err := os.OpenFile(mountPath, os.O_CREATE|os.O_EXCL, 0600) + if os.IsExist(err) { + return validateExistingBindMountTarget(lvName, mountPath) + } + if err != nil { + return fmt.Errorf("unable to create mount target for lv:%s err:%w", lvName, err) + } + if err := target.Close(); err != nil { + return fmt.Errorf("unable to close mount target for lv:%s err:%w", lvName, err) + } + if err := os.Chmod(mountPath, 0777|os.ModeSetgid); err != nil { + return fmt.Errorf("unable to change permissions of volume mount %s err:%w", mountPath, err) + } + return nil +} + +func validateExistingBindMountTarget(lvName, mountPath string) error { + info, err := os.Lstat(mountPath) + if err != nil { + return fmt.Errorf("unable to inspect mount target for lv:%s err:%w", lvName, err) + } + if info.IsDir() || info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("mount target for lv:%s must be a file", lvName) } + return nil } -func createSnapshotterPod(ctx context.Context, sa snapshotAction) (err error) { +func performBindMount(executor commandExecutor, source, target string) (string, error) { + // --make-shared is required that this mount is visible outside this container. + // --bind is required for raw block volumes to make them visible inside the pod. + args := []string{"--make-shared", "--bind", source, target} + klog.Infof("bindmountlv command: mount %s", args) + out, err := executor.Execute("mount", args) + if err == nil || strings.Contains(strings.ToLower(out+" "+err.Error()), "already mounted") { + return out, nil + } + return out, fmt.Errorf("unable to mount %s to %s err:%w output:%s", source, target, err, out) +} + +func remountBindReadOnly(executor commandExecutor, target string) (string, error) { + args := []string{"-o", "remount,bind,ro", target} + klog.Infof("remounting bind mount read-only: mount %s", args) + out, err := executor.Execute("mount", args) + if err == nil { + return out, nil + } + _ = unmountTarget(target) + return out, fmt.Errorf("unable to remount %s read-only: %w output:%s", target, err, out) +} + +func unmountTarget(targetPath string) error { + err := unmountPath(targetPath, 0) + if isUnmountComplete(err) { + return nil + } + if errors.Is(err, unix.EBUSY) { + return lazyUnmountTarget(targetPath) + } + return fmt.Errorf("unable to unmount %s: %w", targetPath, err) +} + +func lazyUnmountTarget(targetPath string) error { + err := unmountPath(targetPath, unix.MNT_DETACH) + if isUnmountComplete(err) { + return nil + } + return fmt.Errorf("unable to lazily unmount %s: %w", targetPath, err) +} + +func isUnmountComplete(err error) bool { + return err == nil || errors.Is(err, unix.EINVAL) || errors.Is(err, unix.ENOENT) +} + +func createSnapshotterPod(ctx context.Context, sa snapshotAction) error { + args, err := snapshotProvisionerArgs(sa) + if err != nil { + return err + } + + klog.Infof("start snapshotterPod with args:%s", args) + action := fmt.Sprintf("snap-%s", sa.action) + pod := genProvisionerPodContent(action, sa.snapshotName, sa.nodeName, sa.hostWritePath, sa.provisionerImage, sa.pullPolicy, args) + if err := runProvisionerPod(ctx, sa.kubeClient.CoreV1().Pods(sa.namespace), pod, "snapshot", sa.action); err != nil { + return err + } + + klog.Infof("Snapshot %v has been %vd on %v", sa.snapshotName, sa.action, sa.nodeName) + return nil +} + +func snapshotProvisionerArgs(sa snapshotAction) ([]string, error) { if sa.snapshotName == "" || sa.nodeName == "" { klog.Errorf("invalid snapshotAction %v", sa) - return fmt.Errorf("invalid empty name or path or node") + return nil, fmt.Errorf("invalid empty name or path or node") } if sa.action == actionTypeCreate && sa.srcVolName == "" { klog.Errorf("invalid snapshotAction %v", sa) - return fmt.Errorf("createlv without srcVolName") + return nil, fmt.Errorf("createlv without srcVolName") } - args := []string{} + switch sa.action { case actionTypeCreate: - args = append(args, "createsnap", "--snapname", sa.snapshotName, "--lvname", sa.srcVolName, "--vgname", sa.vgName, "--lvsize", fmt.Sprintf("%d", sa.snapSize), "--lvmtype", sa.lvType) + return []string{"createsnap", "--snapname", sa.snapshotName, "--lvname", sa.srcVolName, "--vgname", sa.vgName, "--lvsize", fmt.Sprintf("%d", sa.snapSize), "--lvmtype", sa.lvType}, nil case actionTypeDelete: - args = append(args, "deletesnap", "--snapname", sa.snapshotName, "--vgname", sa.vgName) + return []string{"deletesnap", "--snapname", sa.snapshotName, "--vgname", sa.vgName}, nil default: - return fmt.Errorf("invalid action %v", sa.action) + return nil, fmt.Errorf("invalid action %v", sa.action) } +} - klog.Infof("start snapshotterPod with args:%s", args) - action := fmt.Sprintf("snap-%s", sa.action) - snapshotterPod := genProvisionerPodContent(action, sa.snapshotName, sa.nodeName, sa.hostWritePath, sa.provisionerImage, sa.pullPolicy, args) - - _, err = sa.kubeClient.CoreV1().Pods(sa.namespace).Create(ctx, snapshotterPod, metav1.CreateOptions{}) - if err != nil && !k8serror.IsAlreadyExists(err) { +func createProvisionerPod(ctx context.Context, va volumeAction) error { + args, err := volumeProvisionerArgs(va) + if err != nil { return err } - defer func() { - e := sa.kubeClient.CoreV1().Pods(sa.namespace).Delete(ctx, snapshotterPod.Name, metav1.DeleteOptions{}) - if e != nil { - klog.Errorf("unable to delete the snapshotter pod: %v", e) - } - }() - - completed := false - retrySeconds := 60 - for i := 0; i < retrySeconds; i++ { - pod, err := sa.kubeClient.CoreV1().Pods(sa.namespace).Get(ctx, snapshotterPod.Name, metav1.GetOptions{}) - if pod.Status.Phase == v1.PodFailed { - // pod terminated in time, but with failure - // return ResourceExhausted so the requesting pod can be rescheduled to anonther node - // see https://github.com/kubernetes-csi/external-provisioner/pull/405 - klog.Info("provisioner pod terminated with failure") - return status.Error(codes.ResourceExhausted, "Snapshot creation failed") - } - if err != nil { - klog.Errorf("error reading provisioner pod:%v", err) - } else if pod.Status.Phase == v1.PodSucceeded { - klog.Info("provisioner pod terminated successfully") - completed = true - break - } - klog.Infof("provisioner pod status:%s", pod.Status.Phase) - time.Sleep(1 * time.Second) - } - if !completed { - return fmt.Errorf("create process timeout after %v seconds", retrySeconds) + klog.Infof("start provisionerPod with args:%s", args) + action := fmt.Sprintf("lvm-%s", va.action) + pod := genProvisionerPodContent(action, va.name, va.nodeName, va.hostWritePath, va.provisionerImage, va.pullPolicy, args) + if err := runProvisionerPod(ctx, va.kubeClient.CoreV1().Pods(va.namespace), pod, "volume", va.action); err != nil { + return err } - klog.Infof("Snapshot %v has been %vd on %v", sa.snapshotName, sa.action, sa.nodeName) + klog.Infof("Volume %v has been %vd on %v", va.name, va.action, va.nodeName) return nil } -func createProvisionerPod(ctx context.Context, va volumeAction) (err error) { +func volumeProvisionerArgs(va volumeAction) ([]string, error) { if va.name == "" || va.nodeName == "" { - return fmt.Errorf("invalid empty name or path or node") + return nil, fmt.Errorf("invalid empty name or path or node") } if va.action == actionTypeCreate && va.lvmType == "" { - return fmt.Errorf("createlv without lvm type") + return nil, fmt.Errorf("createlv without lvm type") } - args := []string{} + var args []string switch va.action { case actionTypeCreate: args = append(args, "createlv", "--lvsize", fmt.Sprintf("%d", va.size), "--lvmtype", va.lvmType, "--vgname", va.vgName) case actionTypeDelete: + if va.srcInfo == nil { + return nil, fmt.Errorf("deletelv without source volume information") + } args = append(args, "deletelv", "--srcvgname", va.srcInfo.srcVGName, "--srctype", va.srcInfo.srcType) case actionTypeClone: + if va.srcInfo == nil { + return nil, fmt.Errorf("clonelv without source volume information") + } args = append(args, "clonelv", "--srclvname", va.srcInfo.srcLVName, "--srcvgname", va.srcInfo.srcVGName, "--srctype", va.srcInfo.srcType, "--lvsize", fmt.Sprintf("%d", va.size), "--vgname", va.vgName, "--lvmtype", va.lvmType) default: - return fmt.Errorf("invalid action %v", va.action) + return nil, fmt.Errorf("invalid action %v", va.action) } - args = append(args, "--lvname", va.name) + return append(args, "--lvname", va.name), nil +} - klog.Infof("start provisionerPod with args:%s", args) - action := fmt.Sprintf("lvm-%s", va.action) - provisionerPod := genProvisionerPodContent(action, va.name, va.nodeName, va.hostWritePath, va.provisionerImage, va.pullPolicy, args) +func runProvisionerPod(ctx context.Context, pods corev1.PodInterface, pod *v1.Pod, resource string, action actionType) error { + if err := createOrReuseProvisionerPod(ctx, pods, pod); err != nil { + return err + } - // If it already exists due to some previous errors, the pod will be cleaned up later automatically - // https://github.com/rancher/local-path-provisioner/issues/27 - _, err = va.kubeClient.CoreV1().Pods(va.namespace).Create(ctx, provisionerPod, metav1.CreateOptions{}) - if err != nil && !k8serror.IsAlreadyExists(err) { + terminal, err := waitForProvisionerPod(ctx, pods, pod.Name, resource, action) + if !terminal { + klog.Infof("retaining nonterminal provisioner pod %s for a later retry: %v", pod.Name, err) return err } - defer func() { - e := va.kubeClient.CoreV1().Pods(va.namespace).Delete(ctx, provisionerPod.Name, metav1.DeleteOptions{}) - if e != nil { - klog.Errorf("unable to delete the provisioner pod: %v", e) + deleteProvisionerPod(pods, pod.Name) + return err +} + +func createOrReuseProvisionerPod(ctx context.Context, pods corev1.PodInterface, pod *v1.Pod) error { + _, err := pods.Create(ctx, pod, metav1.CreateOptions{}) + if k8serror.IsAlreadyExists(err) { + klog.Infof("reusing existing provisioner pod %s", pod.Name) + return nil + } + return err +} + +func waitForProvisionerPod(ctx context.Context, pods corev1.PodInterface, podName, resource string, action actionType) (bool, error) { + const provisionerPodPollAttempts = 60 + for range provisionerPodPollAttempts { + pod, readErr := pods.Get(ctx, podName, metav1.GetOptions{}) + terminal, resultErr := provisionerPodResult(ctx, pod, readErr, resource, action) + if terminal || resultErr != nil { + return terminal, resultErr } - }() - - completed := false - retrySeconds := 60 - for i := 0; i < retrySeconds; i++ { - pod, err := va.kubeClient.CoreV1().Pods(va.namespace).Get(ctx, provisionerPod.Name, metav1.GetOptions{}) - if pod.Status.Phase == v1.PodFailed { - // pod terminated in time, but with failure - // return ResourceExhausted so the requesting pod can be rescheduled to anonther node - // see https://github.com/kubernetes-csi/external-provisioner/pull/405 - klog.Info("provisioner pod terminated with failure") - return status.Error(codes.ResourceExhausted, "volume creation failed") + if err := waitForRetry(ctx); err != nil { + return false, err } - if err != nil { - klog.Errorf("error reading provisioner pod:%v", err) - } else if pod.Status.Phase == v1.PodSucceeded { - klog.Info("provisioner pod terminated successfully") - completed = true - break + } + return false, fmt.Errorf("%s %s process timeout after %d polling attempts", resource, action, provisionerPodPollAttempts) +} + +func provisionerPodResult(ctx context.Context, pod *v1.Pod, readErr error, resource string, action actionType) (bool, error) { + if readErr != nil { + if ctx.Err() != nil { + return false, status.FromContextError(ctx.Err()).Err() } - klog.Infof("provisioner pod status:%s", pod.Status.Phase) - time.Sleep(1 * time.Second) + klog.Errorf("error reading provisioner pod: %v", readErr) + return false, nil } - if !completed { - return fmt.Errorf("create process timeout after %v seconds", retrySeconds) + if pod == nil { + return false, status.Error(codes.Internal, "Kubernetes API returned an empty provisioner pod") } - klog.Infof("Volume %v has been %vd on %v", va.name, va.action, va.nodeName) - return nil + switch pod.Status.Phase { + case v1.PodFailed: + klog.Infof("provisioner pod %s terminated with failure", pod.Name) + return true, provisionerPodFailure(pod, resource, action) + case v1.PodSucceeded: + klog.Infof("provisioner pod %s terminated successfully", pod.Name) + return true, nil + default: + klog.Infof("provisioner pod %s status:%s", pod.Name, pod.Status.Phase) + return false, nil + } +} + +func provisionerPodFailure(pod *v1.Pod, resource string, action actionType) error { + details := make([]string, 0, len(pod.Status.ContainerStatuses)+1) + if pod.Status.Reason != "" || pod.Status.Message != "" { + details = append(details, fmt.Sprintf( + "pod reason=%s message=%s", + valueOrUnknown(pod.Status.Reason), + valueOrUnknown(compactErrorMessage(pod.Status.Message)), + )) + } + for _, containerStatus := range pod.Status.ContainerStatuses { + terminated := containerStatus.State.Terminated + if terminated == nil { + continue + } + detail := fmt.Sprintf( + "container %s exited with code %d reason=%s", + containerStatus.Name, + terminated.ExitCode, + valueOrUnknown(terminated.Reason), + ) + if message := compactErrorMessage(terminated.Message); message != "" { + detail += " message=" + message + } + details = append(details, detail) + } + + message := fmt.Sprintf("%s %s helper pod %s failed", resource, action, pod.Name) + if len(details) > 0 { + message += ": " + strings.Join(details, "; ") + } + return status.Error(codes.Internal, message) +} + +func compactErrorMessage(message string) string { + const maxLength = 1024 + message = strings.Join(strings.Fields(message), " ") + runes := []rune(message) + if len(runes) <= maxLength { + return message + } + return string(runes[:maxLength]) + "..." +} + +func valueOrUnknown(value string) string { + if value == "" { + return "unknown" + } + return value +} + +func deleteProvisionerPod(pods corev1.PodInterface, podName string) { + cleanupCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + if err := pods.Delete(cleanupCtx, podName, metav1.DeleteOptions{}); err != nil && !k8serror.IsNotFound(err) { + klog.Errorf("unable to delete provisioner pod %s: %v", podName, err) + } +} + +func waitForRetry(ctx context.Context) error { + timer := time.NewTimer(time.Second) + defer timer.Stop() + select { + case <-ctx.Done(): + return status.FromContextError(ctx.Err()).Err() + case <-timer.C: + return nil + } } // VgExists checks if the given volume group exists func VgExists(vgname string) bool { - executor := cmd.NewExecutor() + executor := newCommandExecutor() out, err := executor.Execute("vgs", []string{vgname, "--noheadings", "-o", "vg_name"}) if err != nil { klog.Infof("unable to list existing volumegroups:%v", err) @@ -420,53 +727,85 @@ func VgExists(vgname string) bool { return vgname == strings.TrimSpace(out) } -// VgActivate execute vgchange -ay to activate all volumes of the volume group -func VgActivate() { - executor := cmd.NewExecutor() - // scan for vgs and activate if any +// EnsureVG verifies that a volume group is discoverable and activates its LVs. +func EnsureVG(vgName string) error { + if err := ensureVGDiscovered(vgName); err != nil { + return err + } + if err := activateVolumeGroups(vgName); err != nil { + return fmt.Errorf("unable to activate volume group %s: %w", vgName, err) + } + return nil +} + +func ensureVGDiscovered(vgName string) error { + if VgExists(vgName) { + return nil + } + if err := scanVolumeGroups(); err != nil { + return err + } + if VgExists(vgName) { + return nil + } + return fmt.Errorf("volume group %s does not exist; ensure it is created on the target node", vgName) +} + +// VgActivate executes vgchange -ay to activate all volumes of all discovered +// volume groups. +func VgActivate() error { + if err := scanVolumeGroups(); err != nil { + return err + } + return activateVolumeGroups() +} + +func scanVolumeGroups() error { + executor := newCommandExecutor() out, err := executor.Execute("vgscan", []string{}) if err != nil { - klog.Infof("unable to scan for volumegroups:%s %v", out, err) + return fmt.Errorf("unable to scan for volume groups: %w output:%s", err, out) } - _, err = executor.Execute("vgchange", []string{"-ay"}) + return nil +} + +func activateVolumeGroups(vgNames ...string) error { + executor := newCommandExecutor() + args := append([]string{"-ay"}, vgNames...) + out, err := executor.Execute("vgchange", args) if err != nil { - klog.Infof("unable to activate volumegroups:%s %v", out, err) + return fmt.Errorf("unable to activate volume groups: %w output:%s", err, out) } + return nil } // CreateLVS creates the new volume, used by lvcreate provisioner pod func CreateLVS(vg string, name string, size uint64, lvmType string) (string, error) { - - if lvExists(vg, name) { - klog.Infof("logicalvolume: %s already exists\n", name) - return name, nil - } - if size == 0 { return "", fmt.Errorf("size must be greater than 0") } + if lvmType != StripedType && lvmType != DmThinType { + return "", fmt.Errorf("unsupported lvmtype: %s", lvmType) + } - // TODO: check available capacity, fail if request doesn't fit - - executor := cmd.NewExecutor() - thinPoolName := "" - // we need to create thin pool first if the lvmType is dm-thin - if lvmType == DmThinType { - thinPoolName = fmt.Sprintf("%s-thinpool", vg) - found, err := getThinPool(vg, thinPoolName) - if err != nil { - return "", fmt.Errorf("unable to determine if thinpool exists: %w", err) - } - if !found { - args := []string{"-l90%FREE", "--thinpool", thinPoolName, vg} - klog.Infof("lvcreate %s", args) - _, err := executor.Execute("lvcreate", args) - if err != nil { - return "", fmt.Errorf("unable to create thinpool: %w", err) - } + existing, found, err := getLogicalVolume(vg, name) + if err != nil { + return "", fmt.Errorf("unable to check existing logical volume %s/%s: %w", vg, name, err) + } + if found { + if err := validateExistingVolume(existing, size, lvmType); err != nil { + return "", err } + klog.Infof("logical volume %s/%s already exists and is compatible", vg, name) + return name, nil + } + + thinPoolName, err := prepareThinPool(vg, lvmType) + if err != nil { + return "", err } + executor := newCommandExecutor() args := []string{"-v", "--yes", "-n", name, "-W", "y"} pvs, err := pvCount(vg) @@ -483,8 +822,6 @@ func CreateLVS(vg string, name string, size uint64, lvmType string) (string, err args = append(args, "-L", fmt.Sprintf("%db", size), "--type", "striped", "--stripes", fmt.Sprintf("%d", pvs), vg) case DmThinType: args = append(args, "-V", fmt.Sprintf("%db", size), "--thin-pool", thinPoolName, vg) - default: - return "", fmt.Errorf("unsupported lvmtype: %s", lvmType) } tags := []string{"harvester-csi-lvm"} @@ -496,59 +833,246 @@ func CreateLVS(vg string, name string, size uint64, lvmType string) (string, err return out, err } -func lvExists(vg string, name string) bool { - executor := cmd.NewExecutor() - vgname := vg + "/" + name - out, err := executor.Execute("lvs", []string{vgname, "--noheadings", "-o", "lv_name"}) +func prepareThinPool(vgName, lvmType string) (string, error) { + if lvmType != DmThinType { + return "", nil + } + + thinPoolName := fmt.Sprintf("%s-thinpool", vgName) + found, err := getThinPool(vgName, thinPoolName) if err != nil { - klog.Infof("unable to list existing volumes:%v", err) - return false + return "", fmt.Errorf("unable to determine if thin pool %s/%s exists: %w", vgName, thinPoolName, err) + } + if found { + return thinPoolName, validateThinPool(vgName, thinPoolName) + } + + args := []string{"-l90%FREE", "--thinpool", thinPoolName, vgName} + klog.Infof("lvcreate %s", args) + if _, err := newCommandExecutor().Execute("lvcreate", args); err != nil { + return "", fmt.Errorf("unable to create thin pool %s/%s: %w", vgName, thinPoolName, err) } - return name == strings.TrimSpace(out) + return thinPoolName, nil } -func extendLVS(name string, size uint64, isBlock bool) (string, error) { - vgName, err := getRelatedVG(name) +func getLogicalVolume(vgName, lvName string) (logicalVolume, bool, error) { + volumes, err := listLogicalVolumes() if err != nil { - return "", fmt.Errorf("unable to get related vg for lv %s: %w", name, err) + return logicalVolume{}, false, err } - if !lvExists(vgName, name) { - return "", fmt.Errorf("logical volume %s does not exist", name) + for _, volume := range volumes { + if volume.VGName == vgName && volume.Name == lvName { + return volume, true, nil + } } + return logicalVolume{}, false, nil +} - lvSize, err := getLVSize(name, vgName) +func getLogicalVolumeByName(lvName string) (logicalVolume, error) { + volumes, err := listLogicalVolumes() if err != nil { - return "", fmt.Errorf("unable to get size of lv %s: %w", name, err) + return logicalVolume{}, err } - if lvSize == size { - klog.Infof("logical volume %s already has the requested size %d", name, size) - return "", nil + + var match *logicalVolume + for i := range volumes { + if volumes[i].Name != lvName { + continue + } + if match != nil { + return logicalVolume{}, fmt.Errorf( + "logical volume name %s is ambiguous across volume groups %s and %s", + lvName, + match.VGName, + volumes[i].VGName, + ) + } + match = &volumes[i] + } + if match == nil { + return logicalVolume{}, fmt.Errorf("logical volume %s does not exist", lvName) + } + return *match, nil +} + +func listLogicalVolumes() ([]logicalVolume, error) { + executor := newCommandExecutor() + args := []string{ + "--reportformat", "json", + "--units", "b", + "--nosuffix", + "--options", "lv_name,vg_name,lv_size,segtype,origin", + } + out, err := executor.Execute("lvs", args) + if err != nil { + return nil, err } - executor := cmd.NewExecutor() - args := []string{"-L", fmt.Sprintf("%db", size)} + report := logicalVolumeReport{} + if err := json.Unmarshal([]byte(out), &report); err != nil { + return nil, fmt.Errorf("unable to parse lvs output: %w", err) + } + + volumes := []logicalVolume{} + for _, item := range report.Reports { + for _, volume := range item.Volumes { + volumes = append(volumes, normalizeLogicalVolume(volume)) + } + } + return volumes, nil +} + +func normalizeLogicalVolume(volume logicalVolume) logicalVolume { + volume.Name = strings.TrimSpace(volume.Name) + volume.VGName = strings.TrimSpace(volume.VGName) + volume.Size = strings.TrimSpace(volume.Size) + volume.SegType = strings.TrimSpace(volume.SegType) + volume.Origin = strings.TrimSpace(volume.Origin) + return volume +} + +func validateExistingVolume(volume logicalVolume, requestedSize uint64, requestedType string) error { + actualSize, err := parseLogicalVolumeSize(volume) + if err != nil { + return err + } + if actualSize < requestedSize { + return fmt.Errorf( + "existing logical volume %s/%s has size %d, smaller than requested size %d", + volume.VGName, + volume.Name, + actualSize, + requestedSize, + ) + } + + typeMatches := requestedType == DmThinType && volume.SegType == ThinVolType || + requestedType == StripedType && (volume.SegType == StripedType || volume.SegType == LinearType) + if !typeMatches { + return fmt.Errorf( + "existing logical volume %s/%s has type %s, incompatible with requested type %s", + volume.VGName, + volume.Name, + volume.SegType, + requestedType, + ) + } + return nil +} + +func parseLogicalVolumeSize(volume logicalVolume) (uint64, error) { + size, err := strconv.ParseUint(volume.Size, 10, 64) + if err != nil { + return 0, fmt.Errorf("unable to parse size %q of logical volume %s/%s: %w", volume.Size, volume.VGName, volume.Name, err) + } + return size, nil +} + +func extendLVS(name string, size uint64, isBlock bool, volumePath string) (string, error) { + volume, err := getLogicalVolumeByName(name) + if err != nil { + return "", fmt.Errorf("unable to get logical volume %s: %w", name, err) + } + + executor := newCommandExecutor() + lvOutput, err := ensureLogicalVolumeSize(executor, volume, size) + if err != nil { + return lvOutput, err + } if isBlock { - args = append(args, "-n") - } else { - args = append(args, "-r") + return lvOutput, nil } - args = append(args, fmt.Sprintf("%s/%s", vgName, name)) + + devicePath := fmt.Sprintf("/dev/%s/%s", volume.VGName, volume.Name) + fsOutput, err := resizeFilesystem(executor, devicePath, volumePath) + return combineCommandOutput(lvOutput, fsOutput), err +} + +func ensureLogicalVolumeSize(executor commandExecutor, volume logicalVolume, size uint64) (string, error) { + lvSize, err := parseLogicalVolumeSize(volume) + if err != nil { + return "", err + } + if lvSize >= size { + klog.Infof("logical volume %s already has size %d, satisfying requested size %d", volume.Name, lvSize, size) + return "", nil + } + + args := buildLVExtendArgs(volume.VGName, volume.Name, size) klog.Infof("lvextend %s", args) - out, err := executor.Execute("lvextend", args) - return out, err + output, err := executor.Execute("lvextend", args) + if err != nil { + return output, fmt.Errorf("unable to extend logical volume %s/%s: %w", volume.VGName, volume.Name, err) + } + return output, nil +} + +func resizeFilesystem(executor commandExecutor, devicePath, volumePath string) (string, error) { + fsType, err := getFilesystemType(executor, devicePath) + if err != nil { + return "", fmt.Errorf("unable to detect filesystem on %s: %w", devicePath, err) + } + + var command string + var args []string + switch fsType { + case "ext2", "ext3", "ext4": + command = "resize2fs" + args = []string{devicePath} + case "xfs": + command = "xfs_growfs" + args = []string{"-d", volumePath} + default: + return "", fmt.Errorf("filesystem type %q on %s does not support expansion", fsType, devicePath) + } + + klog.Infof("%s %s", command, args) + output, err := executor.Execute(command, args) + if err != nil { + return output, fmt.Errorf("unable to resize %s filesystem on %s: %w", fsType, devicePath, err) + } + return output, nil +} + +func combineCommandOutput(outputs ...string) string { + nonEmpty := make([]string, 0, len(outputs)) + for _, output := range outputs { + if output = strings.TrimSpace(output); output != "" { + nonEmpty = append(nonEmpty, output) + } + } + return strings.Join(nonEmpty, "\n") +} + +func buildLVExtendArgs(vgName, lvName string, size uint64) []string { + return []string{ + "-L", fmt.Sprintf("%db", size), + "-n", + fmt.Sprintf("%s/%s", vgName, lvName), + } } // RemoveLVS executes lvremove func RemoveLVS(name string) (string, error) { - vgName, err := getRelatedVG(name) + volume, err := getLogicalVolumeByName(name) + if err != nil { + return "", fmt.Errorf("unable to get logical volume %s: %w", name, err) + } + return RemoveLVSInVG(volume.VGName, name) +} + +// RemoveLVSInVG removes a logical volume from the specified VG. It is +// idempotent: an already absent LV is treated as success. +func RemoveLVSInVG(vgName, name string) (string, error) { + _, found, err := getLogicalVolume(vgName, name) if err != nil { - return "", fmt.Errorf("unable to get related vg for lv %s: %w", name, err) + return "", fmt.Errorf("unable to check logical volume %s/%s: %w", vgName, name, err) } - if !lvExists(vgName, name) { + if !found { return fmt.Sprintf("logical volume %s does not exist. Assuming it has already been deleted.", name), nil } - executor := cmd.NewExecutor() + executor := newCommandExecutor() args := make([]string, 0, 3) args = append(args, "-q", "-y") args = append(args, fmt.Sprintf("%s/%s", vgName, name)) @@ -566,20 +1090,43 @@ func CreateSnapshot(snapshotName, srcVolName, vgName string, volSize int64, lvTy return "", fmt.Errorf("size must be greater than 0") } - if !lvExists(vgName, srcVolName) { + if _, found, err := getLogicalVolume(vgName, srcVolName); err != nil { + return "", fmt.Errorf("unable to check source logical volume %s/%s: %w", vgName, srcVolName, err) + } else if !found { return "", fmt.Errorf("logical volume %s does not exist", srcVolName) } - executor := cmd.NewExecutor() // Names starting "snapshot" are reserved for internal use by LVM // we patch new snapName as "lvm-" // parameters: -s, -y, -a n, -n, snapshotName, (-L, volSize), vgName/srcVolName + backendSnapshotName := snapshotName + if !forClone { + backendSnapshotName = fmt.Sprintf("lvm-%s", snapshotName) + } + existing, found, err := getLogicalVolume(vgName, backendSnapshotName) + if err != nil { + return "", fmt.Errorf("unable to check existing snapshot %s/%s: %w", vgName, backendSnapshotName, err) + } + if found { + if existing.Origin != srcVolName { + return "", fmt.Errorf( + "existing snapshot %s/%s has origin %s, expected %s", + vgName, + backendSnapshotName, + existing.Origin, + srcVolName, + ) + } + klog.Infof("snapshot %s/%s already exists and is compatible", vgName, backendSnapshotName) + return backendSnapshotName, nil + } + + executor := newCommandExecutor() args := []string{"-s", "-y"} if !forClone { args = append(args, "-a", "n") - snapshotName = fmt.Sprintf("lvm-%s", snapshotName) } - args = append(args, "-n", snapshotName) + args = append(args, "-n", backendSnapshotName) switch lvType { case StripedType: args = append(args, "-L", fmt.Sprintf("%db", volSize)) @@ -600,10 +1147,16 @@ func DeleteSnapshot(snapshotName, vgName string) (string, error) { return "", fmt.Errorf("invalid empty name") } - executor := cmd.NewExecutor() // Names starting "snapshot" are reserved for internal use by LVM // we patch new snapName as "lvm-" snapshotName = fmt.Sprintf("lvm-%s", snapshotName) + if _, found, err := getLogicalVolume(vgName, snapshotName); err != nil { + return "", fmt.Errorf("unable to check snapshot %s/%s: %w", vgName, snapshotName, err) + } else if !found { + return fmt.Sprintf("snapshot %s/%s does not exist. Assuming it has already been deleted.", vgName, snapshotName), nil + } + + executor := newCommandExecutor() args := make([]string, 0, 3) args = append(args, "-q", "-y") args = append(args, fmt.Sprintf("/dev/%s/%s", vgName, snapshotName)) @@ -631,7 +1184,7 @@ func RemoveThinPool(vgName string) error { klog.Infof("thinpool %s is not empty, skip remove!", targetThinPool) return nil } - _, err = RemoveLVS(targetThinPool) + _, err = RemoveLVSInVG(vgName, targetThinPool) if err != nil { return fmt.Errorf("unable to remove thinpool: %w", err) } @@ -639,7 +1192,7 @@ func RemoveThinPool(vgName string) error { } func pvCount(vgname string) (int, error) { - executor := cmd.NewExecutor() + executor := newCommandExecutor() out, err := executor.Execute("vgs", []string{vgname, "--noheadings", "-o", "pv_count"}) if err != nil { return 0, err @@ -653,11 +1206,13 @@ func pvCount(vgname string) (int, error) { } func getThinPoolAndCounts(vgName string) (map[string]int, error) { - executor := cmd.NewExecutor() + executor := newCommandExecutor() // we would like to get the segtype, name as below: - // thin thinvol01 <-- this is volume - // thin-pool vg02-thinpool 1 <-- this is thin-pool - args := []string{"--noheadings", "-o", "segtype,name,thin_count", vgName} + // vg02 thin thinvol01 <-- this is volume + // vg02 thin-pool vg02-thinpool 1 <-- this is thin-pool + // Query all VGs so an already removed VG produces an empty result rather + // than turning an idempotent delete into an error. + args := []string{"--noheadings", "-o", "vg_name,segtype,name,thin_count"} out, err := executor.Execute("lvs", args) if err != nil { klog.Infof("execute lvs %s, err: %v", args, err) @@ -673,18 +1228,18 @@ func getThinPoolAndCounts(vgName string) (map[string]int, error) { continue } parts := strings.Fields(line) - if len(parts) != 3 { + if len(parts) != 4 { klog.Infof("Skip thin info: %s", line) continue } // confirm again, we only care about thin-pool // thinInfo: map[]: - if parts[0] == ThinPoolType { - count, err := strconv.Atoi(parts[2]) + if parts[0] == vgName && parts[1] == ThinPoolType { + count, err := strconv.Atoi(parts[3]) if err != nil { return nil, err } - thinInfo[parts[1]] = count + thinInfo[parts[2]] = count } } return thinInfo, nil @@ -701,69 +1256,36 @@ func getThinPool(vgName, thinpoolName string) (bool, error) { return false, nil } -func getRelatedVG(lvname string) (string, error) { - executor := cmd.NewExecutor() - // we would like to get the lvname, vgname as below: - // pvc-2e08db0f-01d0-462a-9da7-7da06fefd206 vg01 - // pvc-b13348e4-3c0c-4757-b781-3e6485a16780 vg02 - out, err := executor.Execute("lvs", []string{"--noheadings", "-o", "lv_name,vg_name"}) - if err != nil { - return "", fmt.Errorf("unable to list existing volumes:%v", err) - } - lines := strings.Split(out, "\n") - lvVgPairs := make(map[string]string) - for _, line := range lines { - if line == "" { - continue - } - parts := strings.Fields(line) - if len(parts) != 2 { - klog.Warningf("unexpected output from lvs: %s", line) - continue - } - lvVgPairs[parts[0]] = parts[1] +func validateThinPool(vgName, thinpoolName string) error { + executor := newCommandExecutor() + args := []string{ + "--noheadings", + "-o", "lv_attr,lv_health_status", + fmt.Sprintf("%s/%s", vgName, thinpoolName), } - - if _, ok := lvVgPairs[lvname]; !ok { - return "", fmt.Errorf("logical volume %s does not exist", lvname) + out, err := executor.Execute("lvs", args) + if err != nil { + return fmt.Errorf("unable to inspect thin pool %s/%s: %w output:%s", vgName, thinpoolName, err, out) } - relatedVgName := lvVgPairs[lvname] - return relatedVgName, nil -} - -func getLVSize(lvName, vgName string) (uint64, error) { - var err error - if vgName == "" { - vgName, err = getRelatedVG(lvName) - if err != nil { - return 0, fmt.Errorf("unable to get related vg for lv %s: %w", lvName, err) - } - if !lvExists(vgName, lvName) { - return 0, fmt.Errorf("logical volume %s does not exist", lvName) - } + fields := strings.Fields(out) + if len(fields) == 0 || len(fields[0]) < 5 { + return fmt.Errorf("thin pool %s/%s returned invalid attributes %q", vgName, thinpoolName, strings.TrimSpace(out)) } - executor := cmd.NewExecutor() - targetLVName := fmt.Sprintf("%s/%s", vgName, lvName) - // check current lv size - args := make([]string, 0, 6) - args = append(args, "--noheadings", "--unit", "b", "-o", "Size") - args = append(args, targetLVName) - out, err := executor.Execute("lvs", args) - if err != nil { - return 0, fmt.Errorf("unable to get size of lv %s: %w", lvName, err) + if fields[0][4] != 'a' { + return fmt.Errorf("thin pool %s/%s is inactive (attributes %s)", vgName, thinpoolName, fields[0]) } - lvSizeStr := strings.TrimSpace(out) - lvSizeStr = strings.TrimSuffix(lvSizeStr, "B") - klog.Infof("current size of lv %s is %s", lvName, lvSizeStr) - lvSize, err := strconv.ParseUint(lvSizeStr, 10, 64) - if err != nil { - return 0, fmt.Errorf("unable to parse size of lv %s: %w", lvName, err) + if len(fields) > 1 { + return fmt.Errorf("thin pool %s/%s is unhealthy: %s", vgName, thinpoolName, strings.Join(fields[1:], " ")) } - return lvSize, nil + return nil } -func genProvisionerPodContent(action, name, targetNode, hostWritePath, provisionerImage string, pullPolicy v1.PullPolicy, args []string) *v1.Pod { +func genProvisionerPodContent( + action, name, targetNode, hostWritePath, provisionerImage string, + pullPolicy v1.PullPolicy, + args []string, +) *v1.Pod { hostPathTypeDirOrCreate := v1.HostPathDirectoryOrCreate hostPathTypeDirectory := v1.HostPathDirectory privileged := true @@ -827,8 +1349,9 @@ func genProvisionerPodContent(action, name, targetNode, hostWritePath, provision MountPath: "/run/udev", }, }, - TerminationMessagePath: "/termination.log", - ImagePullPolicy: pullPolicy, + TerminationMessagePath: "/termination.log", + TerminationMessagePolicy: v1.TerminationMessageFallbackToLogsOnError, + ImagePullPolicy: pullPolicy, SecurityContext: &v1.SecurityContext{ Privileged: &privileged, }, diff --git a/pkg/lvm/lvm_test.go b/pkg/lvm/lvm_test.go new file mode 100644 index 00000000..6d74d7c5 --- /dev/null +++ b/pkg/lvm/lvm_test.go @@ -0,0 +1,849 @@ +package lvm + +import ( + "errors" + "fmt" + "path/filepath" + "reflect" + "strings" + "testing" +) + +type commandCall struct { + command string + args []string +} + +type commandResult struct { + command string + output string + err error +} + +type commandExitError struct { + code int +} + +func (e commandExitError) Error() string { + return fmt.Sprintf("command exited with status %d", e.code) +} + +func (e commandExitError) ExitCode() int { + return e.code +} + +type fakeCommandExecutor struct { + t *testing.T + results []commandResult + calls []commandCall +} + +func (f *fakeCommandExecutor) Execute(command string, args []string) (string, error) { + f.t.Helper() + f.calls = append(f.calls, commandCall{command: command, args: append([]string(nil), args...)}) + if len(f.results) == 0 { + f.t.Fatalf("unexpected command: %s %v", command, args) + } + result := f.results[0] + f.results = f.results[1:] + if result.command != command { + f.t.Fatalf("expected command %q, got %q with args %v", result.command, command, args) + } + return result.output, result.err +} + +func useFakeCommandExecutor(t *testing.T, fake *fakeCommandExecutor) { + t.Helper() + original := newCommandExecutor + newCommandExecutor = func() commandExecutor { + return fake + } + t.Cleanup(func() { + newCommandExecutor = original + }) +} + +func TestMountLVDoesNotFormatWhenFilesystemProbeFails(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{ + command: "blkid", + err: errors.New("transient probe failure"), + }}, + } + useFakeCommandExecutor(t, fake) + + _, err := mountLV("/dev/vg/volume", filepath.Join(t.TempDir(), "mount"), "ext4", nil, false) + if err == nil || !strings.Contains(err.Error(), "unable to determine filesystem type") { + t.Fatalf("expected filesystem probe error, got %v", err) + } + if len(fake.calls) != 1 || fake.calls[0].command != "blkid" { + t.Fatalf("expected only blkid to run, got %#v", fake.calls) + } +} + +func TestMountLVDoesNotFormatWhenBlkidIsAmbiguous(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{ + command: "blkid", + err: commandExitError{code: 8}, + }}, + } + useFakeCommandExecutor(t, fake) + + _, err := mountLV("/dev/vg/volume", filepath.Join(t.TempDir(), "mount"), "ext4", nil, false) + if err == nil || !strings.Contains(err.Error(), "ambiguous filesystem signatures") { + t.Fatalf("expected ambiguous signature error, got %v", err) + } + if len(fake.calls) != 1 || fake.calls[0].command != "blkid" { + t.Fatalf("expected only blkid to run, got %#v", fake.calls) + } +} + +func TestMountLVDoesNotFormatWhenBlkidHasOperationalError(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{ + command: "blkid", + err: commandExitError{code: 4}, + }}, + } + useFakeCommandExecutor(t, fake) + + _, err := mountLV("/dev/vg/volume", filepath.Join(t.TempDir(), "mount"), "ext4", nil, false) + if err == nil || !strings.Contains(err.Error(), "blkid failed to inspect") { + t.Fatalf("expected blkid operational error, got %v", err) + } + if len(fake.calls) != 1 || fake.calls[0].command != "blkid" { + t.Fatalf("expected only blkid to run, got %#v", fake.calls) + } +} + +func TestMountLVUsesWipefsToConfirmExistingFilesystem(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "blkid", err: commandExitError{code: 2}}, + {command: "wipefs", output: `{"signatures":[{"type":"ext4"}]}`}, + {command: "mount"}, + }, + } + useFakeCommandExecutor(t, fake) + + if _, err := mountLV("/dev/vg/volume", filepath.Join(t.TempDir(), "mount"), "ext4", nil, false); err != nil { + t.Fatalf("mountLV failed: %v", err) + } + if want := []string{"-p", "-s", "TYPE", "-o", "value", "/dev/vg/volume"}; !reflect.DeepEqual(fake.calls[0].args, want) { + t.Fatalf("unexpected blkid arguments: want %#v, got %#v", want, fake.calls[0].args) + } + if want := []string{"--no-act", "--json", "--output", "TYPE", "/dev/vg/volume"}; !reflect.DeepEqual(fake.calls[1].args, want) { + t.Fatalf("unexpected wipefs arguments: want %#v, got %#v", want, fake.calls[1].args) + } + if got := []string{fake.calls[0].command, fake.calls[1].command, fake.calls[2].command}; !reflect.DeepEqual(got, []string{"blkid", "wipefs", "mount"}) { + t.Fatalf("unexpected commands: %#v", fake.calls) + } +} + +func TestMountLVDoesNotFormatWhenWipefsFails(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "blkid", err: commandExitError{code: 2}}, + {command: "wipefs", err: errors.New("device read failed")}, + }, + } + useFakeCommandExecutor(t, fake) + + _, err := mountLV("/dev/vg/volume", filepath.Join(t.TempDir(), "mount"), "ext4", nil, false) + if err == nil || !strings.Contains(err.Error(), "unable to confirm filesystem signatures") { + t.Fatalf("expected wipefs error, got %v", err) + } + if len(fake.calls) != 2 { + t.Fatalf("expected only blkid and wipefs to run, got %#v", fake.calls) + } +} + +func TestMountLVDoesNotFormatXFSExternalLogSignature(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{ + command: "blkid", + output: "xfs_external_log\n", + }}, + } + useFakeCommandExecutor(t, fake) + + _, err := mountLV("/dev/vg/volume", filepath.Join(t.TempDir(), "mount"), "xfs", nil, false) + if err == nil || !strings.Contains(err.Error(), "xfs_external_log found") { + t.Fatalf("expected existing signature error, got %v", err) + } + if len(fake.calls) != 1 || fake.calls[0].command != "blkid" { + t.Fatalf("expected only blkid to run, got %#v", fake.calls) + } +} + +func TestMountLVFormatsOnlyAfterWipefsConfirmsNoSignatures(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "blkid", err: commandExitError{code: 2}}, + {command: "wipefs", output: `{"signatures":[]}`}, + {command: "mkfs.ext4"}, + {command: "mount"}, + }, + } + useFakeCommandExecutor(t, fake) + + if _, err := mountLV("/dev/vg/volume", filepath.Join(t.TempDir(), "mount"), "ext4", nil, false); err != nil { + t.Fatalf("mountLV failed: %v", err) + } + commands := make([]string, 0, len(fake.calls)) + for _, call := range fake.calls { + commands = append(commands, call.command) + } + if want := []string{"blkid", "wipefs", "mkfs.ext4", "mount"}; !reflect.DeepEqual(commands, want) { + t.Fatalf("unexpected commands: want %#v, got %#v", want, commands) + } +} + +func TestMountLVPassesMountFlagsAndReadOnly(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "blkid", output: "ext4\n"}, + {command: "mount"}, + }, + } + useFakeCommandExecutor(t, fake) + + mountPath := filepath.Join(t.TempDir(), "mount") + if _, err := mountLV("/dev/vg/volume", mountPath, "ext4", []string{"noatime"}, true); err != nil { + t.Fatalf("mountLV failed: %v", err) + } + + want := []string{"--make-shared", "-t", "ext4", "-o", "noatime,ro", "/dev/vg/volume", mountPath} + if got := fake.calls[1].args; !reflect.DeepEqual(got, want) { + t.Fatalf("unexpected mount arguments:\nwant: %#v\n got: %#v", want, got) + } +} + +func TestNormalizeMountOptionsEnforcesReadOnly(t *testing.T) { + got := normalizeMountOptions([]string{"rw", "noatime,nosuid"}, true) + want := []string{"noatime", "nosuid", "ro"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("unexpected normalized mount options: want %#v, got %#v", want, got) + } +} + +func TestMountLVAcceptsAlreadyMountedError(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "blkid", output: "ext4\n"}, + {command: "mount", err: errors.New("mount: /target: already mounted")}, + {command: "findmnt", output: "rw,relatime"}, + }, + } + useFakeCommandExecutor(t, fake) + + if _, err := mountLV("/dev/vg/volume", filepath.Join(t.TempDir(), "mount"), "ext4", nil, false); err != nil { + t.Fatalf("idempotent mount retry failed: %v", err) + } +} + +func TestMountLVRejectsWritableExistingMountForReadOnlyRequest(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "blkid", output: "ext4\n"}, + {command: "mount", err: errors.New("mount: /target: already mounted")}, + {command: "findmnt", output: "rw,relatime"}, + }, + } + useFakeCommandExecutor(t, fake) + + _, err := mountLV("/dev/vg/volume", filepath.Join(t.TempDir(), "mount"), "ext4", nil, true) + if err == nil || !strings.Contains(err.Error(), "readonly was requested") { + t.Fatalf("expected incompatible existing mount error, got %v", err) + } +} + +func TestCreateLVSExistingVolumeCompatibility(t *testing.T) { + const report = `{ + "report": [{ + "lv": [{ + "lv_name": "volume", + "vg_name": "vg", + "lv_size": "2097152", + "segtype": "thin", + "origin": "" + }] + }] + }` + + t.Run("compatible retry succeeds without lvcreate", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{command: "lvs", output: report}}, + } + useFakeCommandExecutor(t, fake) + + if _, err := CreateLVS("vg", "volume", 1048576, DmThinType); err != nil { + t.Fatalf("compatible retry failed: %v", err) + } + if len(fake.calls) != 1 { + t.Fatalf("expected only compatibility lookup, got %#v", fake.calls) + } + }) + + t.Run("incompatible retry fails", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{command: "lvs", output: report}}, + } + useFakeCommandExecutor(t, fake) + + if _, err := CreateLVS("vg", "volume", 1048576, StripedType); err == nil { + t.Fatal("expected incompatible type error") + } + }) +} + +func TestCreateLVSValidatesExistingThinPool(t *testing.T) { + const noVolumes = `{"report":[{"lv":[]}]}` + const thinPool = "vg thin-pool vg-thinpool 0\n" + + t.Run("inactive pool fails before lvcreate", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "lvs", output: noVolumes}, + {command: "lvs", output: thinPool}, + {command: "lvs", output: "twi---tz--"}, + }, + } + useFakeCommandExecutor(t, fake) + + _, err := CreateLVS("vg", "volume", 1048576, DmThinType) + if err == nil || !strings.Contains(err.Error(), "thin pool vg/vg-thinpool is inactive") { + t.Fatalf("expected inactive thin-pool error, got %v", err) + } + if len(fake.calls) != 3 { + t.Fatalf("inactive pool should fail before lvcreate, got %#v", fake.calls) + } + }) + + t.Run("unhealthy pool fails before lvcreate", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "lvs", output: noVolumes}, + {command: "lvs", output: thinPool}, + {command: "lvs", output: "twi-a-tz-- partial"}, + }, + } + useFakeCommandExecutor(t, fake) + + _, err := CreateLVS("vg", "volume", 1048576, DmThinType) + if err == nil || !strings.Contains(err.Error(), "thin pool vg/vg-thinpool is unhealthy: partial") { + t.Fatalf("expected unhealthy thin-pool error, got %v", err) + } + if len(fake.calls) != 3 { + t.Fatalf("unhealthy pool should fail before lvcreate, got %#v", fake.calls) + } + }) + + t.Run("active healthy pool permits lvcreate", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "lvs", output: noVolumes}, + {command: "lvs", output: thinPool}, + {command: "lvs", output: "twi-a-tz--"}, + {command: "vgs", output: "1"}, + {command: "lvcreate", output: "created"}, + }, + } + useFakeCommandExecutor(t, fake) + + output, err := CreateLVS("vg", "volume", 1048576, DmThinType) + if err != nil || output != "created" { + t.Fatalf("expected active healthy pool to permit creation, output=%q err=%v", output, err) + } + }) + + t.Run("missing pool is created before the volume", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "lvs", output: noVolumes}, + {command: "lvs"}, + {command: "lvcreate", output: "pool created"}, + {command: "vgs", output: "1"}, + {command: "lvcreate", output: "volume created"}, + }, + } + useFakeCommandExecutor(t, fake) + + output, err := CreateLVS("vg", "volume", 1048576, DmThinType) + if err != nil || output != "volume created" { + t.Fatalf("expected thin pool and volume creation, output=%q err=%v", output, err) + } + wantPoolArgs := []string{"-l90%FREE", "--thinpool", "vg-thinpool", "vg"} + if !reflect.DeepEqual(fake.calls[2].args, wantPoolArgs) { + t.Fatalf("unexpected thin-pool creation arguments: want %#v, got %#v", wantPoolArgs, fake.calls[2].args) + } + }) +} + +func TestVgActivateReturnsCommandErrors(t *testing.T) { + t.Run("vgscan failure", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{ + command: "vgscan", + output: "scan output", + err: errors.New("scan failed"), + }}, + } + useFakeCommandExecutor(t, fake) + + err := VgActivate() + if err == nil || !strings.Contains(err.Error(), "scan output") || !strings.Contains(err.Error(), "scan failed") { + t.Fatalf("expected vgscan output and error, got %v", err) + } + if len(fake.calls) != 1 { + t.Fatalf("vgchange should not run after vgscan failure, got %#v", fake.calls) + } + }) + + t.Run("vgchange failure reports vgchange output", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "vgscan", output: "scan output"}, + {command: "vgchange", output: "activation output", err: errors.New("activation failed")}, + }, + } + useFakeCommandExecutor(t, fake) + + err := VgActivate() + if err == nil || !strings.Contains(err.Error(), "activation output") || !strings.Contains(err.Error(), "activation failed") { + t.Fatalf("expected vgchange output and error, got %v", err) + } + if strings.Contains(err.Error(), "scan output") { + t.Fatalf("vgchange failure reported stale vgscan output: %v", err) + } + }) +} + +func TestEnsureVG(t *testing.T) { + t.Run("existing VG is activated", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "vgs", output: " vg\n"}, + {command: "vgchange"}, + }, + } + useFakeCommandExecutor(t, fake) + + if err := EnsureVG("vg"); err != nil { + t.Fatalf("EnsureVG failed: %v", err) + } + if len(fake.calls) != 2 || fake.calls[1].command != "vgchange" { + t.Fatalf("existing VG should be activated, got %#v", fake.calls) + } + wantArgs := []string{"-ay", "vg"} + if !reflect.DeepEqual(fake.calls[1].args, wantArgs) { + t.Fatalf("unexpected targeted activation arguments: want %#v, got %#v", wantArgs, fake.calls[1].args) + } + }) + + t.Run("existing but unactivatable VG fails", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "vgs", output: "vg"}, + { + command: "vgchange", + output: "thin-pool metadata LV is active", + err: errors.New("activation prohibited"), + }, + }, + } + useFakeCommandExecutor(t, fake) + + err := EnsureVG("vg") + if err == nil || !strings.Contains(err.Error(), "unable to activate volume group vg") || + !strings.Contains(err.Error(), "thin-pool metadata LV is active") { + t.Fatalf("expected activation failure with LVM output, got %v", err) + } + }) + + t.Run("missing VG is scanned then activated", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "vgs"}, + {command: "vgscan"}, + {command: "vgs", output: "vg"}, + {command: "vgchange"}, + }, + } + useFakeCommandExecutor(t, fake) + + if err := EnsureVG("vg"); err != nil { + t.Fatalf("EnsureVG failed: %v", err) + } + want := []string{"vgs", "vgscan", "vgs", "vgchange"} + for i, command := range want { + if fake.calls[i].command != command { + t.Fatalf("unexpected command sequence: want %#v, got %#v", want, fake.calls) + } + } + }) + + t.Run("missing VG returns actionable error", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "vgs"}, + {command: "vgscan"}, + {command: "vgs"}, + }, + } + useFakeCommandExecutor(t, fake) + + err := EnsureVG("vg") + if err == nil || !strings.Contains(err.Error(), "volume group vg does not exist") { + t.Fatalf("expected missing VG error, got %v", err) + } + }) +} + +func TestBuildLVExtendArgs(t *testing.T) { + want := []string{"-L", "1048576b", "-n", "vg/volume"} + got := buildLVExtendArgs("vg", "volume", 1048576) + if !reflect.DeepEqual(got, want) { + t.Fatalf("unexpected lvextend arguments: want %#v, got %#v", want, got) + } +} + +func TestGetLogicalVolumeByName(t *testing.T) { + t.Run("returns matching volume", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{ + command: "lvs", + output: `{"report":[{"lv":[{"lv_name":" volume ","vg_name":" vg ","lv_size":" 1048576 ","segtype":"linear","origin":""}]}]}`, + }}, + } + useFakeCommandExecutor(t, fake) + + volume, err := getLogicalVolumeByName("volume") + if err != nil { + t.Fatalf("getLogicalVolumeByName failed: %v", err) + } + if volume.VGName != "vg" || volume.Size != "1048576" { + t.Fatalf("unexpected logical volume: %#v", volume) + } + }) + + t.Run("rejects ambiguous name", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{ + command: "lvs", + output: `{"report":[{"lv":[{"lv_name":"volume","vg_name":"vg-a","lv_size":"1"},{"lv_name":"volume","vg_name":"vg-b","lv_size":"1"}]}]}`, + }}, + } + useFakeCommandExecutor(t, fake) + + if _, err := getLogicalVolumeByName("volume"); err == nil || !strings.Contains(err.Error(), "ambiguous") { + t.Fatalf("expected ambiguous name error, got %v", err) + } + }) + + t.Run("rejects missing volume", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{command: "lvs", output: `{"report":[{"lv":[]}]}`}}, + } + useFakeCommandExecutor(t, fake) + + if _, err := getLogicalVolumeByName("missing"); err == nil { + t.Fatal("expected missing volume error") + } + }) +} + +func TestExtendLVSExpandsLogicalVolumeAndExt4Filesystem(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + { + command: "lvs", + output: `{"report":[{"lv":[{"lv_name":"volume","vg_name":"vg","lv_size":"1048576","segtype":"linear","origin":""}]}]}`, + }, + {command: "lvextend", output: "extended"}, + {command: "blkid", output: "ext4\n"}, + {command: "resize2fs", output: "resized"}, + }, + } + useFakeCommandExecutor(t, fake) + + out, err := extendLVS("volume", 2097152, false, "/mnt/volume") + if err != nil || out != "extended\nresized" { + t.Fatalf("extendLVS failed: output=%q err=%v", out, err) + } + wantCommands := []string{"lvs", "lvextend", "blkid", "resize2fs"} + if len(fake.calls) != len(wantCommands) { + t.Fatalf("unexpected command sequence: %#v", fake.calls) + } + for i, want := range wantCommands { + if fake.calls[i].command != want { + t.Fatalf("unexpected command sequence: %#v", fake.calls) + } + } + if want := []string{"-L", "2097152b", "-n", "vg/volume"}; !reflect.DeepEqual(fake.calls[1].args, want) { + t.Fatalf("unexpected lvextend arguments: want %#v, got %#v", want, fake.calls[1].args) + } +} + +func TestExtendLVSRetriesExt4ResizeWhenLogicalVolumeAlreadyExpanded(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + { + command: "lvs", + output: `{"report":[{"lv":[{"lv_name":"volume","vg_name":"vg","lv_size":"2097152","segtype":"linear","origin":""}]}]}`, + }, + {command: "blkid", output: "ext4"}, + {command: "resize2fs", output: "filesystem expanded"}, + }, + } + useFakeCommandExecutor(t, fake) + + out, err := extendLVS("volume", 2097152, false, "/mnt/volume") + if err != nil || out != "filesystem expanded" { + t.Fatalf("extendLVS retry failed: output=%q err=%v", out, err) + } + wantCommands := []string{"lvs", "blkid", "resize2fs"} + if len(fake.calls) != len(wantCommands) { + t.Fatalf("unexpected command sequence: %#v", fake.calls) + } + for i, want := range wantCommands { + if fake.calls[i].command != want { + t.Fatalf("unexpected command sequence: %#v", fake.calls) + } + } +} + +func TestExtendLVSExpandsBlockVolumeWithoutFilesystemCommands(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + { + command: "lvs", + output: `{"report":[{"lv":[{"lv_name":"volume","vg_name":"vg","lv_size":"1048576","segtype":"thin","origin":""}]}]}`, + }, + {command: "lvextend", output: "extended"}, + }, + } + useFakeCommandExecutor(t, fake) + + out, err := extendLVS("volume", 2097152, true, "/unused") + if err != nil || out != "extended" { + t.Fatalf("block extendLVS failed: output=%q err=%v", out, err) + } + if len(fake.calls) != 2 || fake.calls[1].command != "lvextend" { + t.Fatalf("unexpected block-volume command sequence: %#v", fake.calls) + } +} + +func TestExtendLVSUsesMountPathForXFS(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + { + command: "lvs", + output: `{"report":[{"lv":[{"lv_name":"volume","vg_name":"vg","lv_size":"2097152","segtype":"linear","origin":""}]}]}`, + }, + {command: "blkid", output: "xfs"}, + {command: "xfs_growfs", output: "grown"}, + }, + } + useFakeCommandExecutor(t, fake) + + const mountPath = "/mnt/xfs-volume" + if _, err := extendLVS("volume", 2097152, false, mountPath); err != nil { + t.Fatalf("XFS extendLVS failed: %v", err) + } + want := []string{"-d", mountPath} + if got := fake.calls[2].args; !reflect.DeepEqual(got, want) { + t.Fatalf("unexpected xfs_growfs arguments: want %#v, got %#v", want, got) + } +} + +func TestExtendLVSReturnsFilesystemResizeFailureAfterLVExpansion(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + { + command: "lvs", + output: `{"report":[{"lv":[{"lv_name":"volume","vg_name":"vg","lv_size":"1048576","segtype":"linear","origin":""}]}]}`, + }, + {command: "lvextend", output: "LV expanded"}, + {command: "blkid", output: "ext4"}, + {command: "resize2fs", output: "resize output", err: errors.New("resize failed")}, + }, + } + useFakeCommandExecutor(t, fake) + + out, err := extendLVS("volume", 2097152, false, "/mnt/volume") + if err == nil || !strings.Contains(err.Error(), "unable to resize ext4 filesystem") { + t.Fatalf("expected filesystem resize failure, got output=%q err=%v", out, err) + } + if out != "LV expanded\nresize output" { + t.Fatalf("expected both command outputs, got %q", out) + } +} + +func TestExtendLVSRejectsUnsupportedFilesystem(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + { + command: "lvs", + output: `{"report":[{"lv":[{"lv_name":"volume","vg_name":"vg","lv_size":"2097152","segtype":"linear","origin":""}]}]}`, + }, + {command: "blkid", output: "btrfs"}, + }, + } + useFakeCommandExecutor(t, fake) + + if _, err := extendLVS("volume", 2097152, false, "/mnt/volume"); err == nil || !strings.Contains(err.Error(), "does not support expansion") { + t.Fatalf("expected unsupported filesystem error, got %v", err) + } +} + +func TestSnapshotBackendOperationsAreIdempotent(t *testing.T) { + const report = `{ + "report": [{ + "lv": [ + { + "lv_name": "source", + "vg_name": "vg", + "lv_size": "1048576", + "segtype": "thin", + "origin": "" + }, + { + "lv_name": "lvm-snapshot-id", + "vg_name": "vg", + "lv_size": "1048576", + "segtype": "thin", + "origin": "source" + } + ] + }] + }` + + t.Run("create retry returns existing compatible snapshot", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "lvs", output: report}, + {command: "lvs", output: report}, + }, + } + useFakeCommandExecutor(t, fake) + + if _, err := CreateSnapshot("snapshot-id", "source", "vg", 1048576, DmThinType, false); err != nil { + t.Fatalf("snapshot retry failed: %v", err) + } + if len(fake.calls) != 2 { + t.Fatalf("expected only source and snapshot lookups, got %#v", fake.calls) + } + }) + + t.Run("delete retry accepts missing snapshot", func(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{command: "lvs", output: `{"report":[{"lv":[]}]}`}}, + } + useFakeCommandExecutor(t, fake) + + output, err := DeleteSnapshot("snapshot-id", "vg") + if err != nil { + t.Fatalf("snapshot delete retry failed: %v", err) + } + if !strings.Contains(output, "already been deleted") { + t.Fatalf("unexpected idempotent delete output: %q", output) + } + }) +} + +func TestRemoveThinPoolIsIdempotentWhenVGIsAbsent(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{command: "lvs", output: ""}}, + } + useFakeCommandExecutor(t, fake) + + if err := RemoveThinPool("missing-vg"); err != nil { + t.Fatalf("missing VG should be treated as an already absent thin pool: %v", err) + } +} + +func TestGetThinPoolAndCountsFiltersVolumeGroup(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{{ + command: "lvs", + output: "vg-a thin volume-a\n" + + "vg-a thin-pool vg-a-thinpool 2\n" + + "vg-b thin-pool vg-b-thinpool 1\n", + }}, + } + useFakeCommandExecutor(t, fake) + + got, err := getThinPoolAndCounts("vg-a") + if err != nil { + t.Fatalf("getThinPoolAndCounts failed: %v", err) + } + want := map[string]int{"vg-a-thinpool": 2} + if !reflect.DeepEqual(got, want) { + t.Fatalf("unexpected thin pool counts: want %#v, got %#v", want, got) + } +} + +func TestGetFilesystemTypeRejectsAmbiguousWipefsOutput(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "blkid", err: commandExitError{code: 2}}, + {command: "wipefs", output: `{"signatures":[{"type":"ext4"},{"type":"xfs"}]}`}, + }, + } + + if _, err := getFilesystemType(fake, "/dev/vg/volume"); err == nil || !strings.Contains(err.Error(), "ambiguous filesystem signatures") { + t.Fatalf("expected ambiguous wipefs output to fail, got %v", err) + } +} + +func TestGetFilesystemTypeRejectsMissingWipefsSignatures(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "blkid", err: commandExitError{code: 2}}, + {command: "wipefs", output: `{}`}, + }, + } + + if _, err := getFilesystemType(fake, "/dev/vg/volume"); err == nil || !strings.Contains(err.Error(), "does not contain a signatures array") { + t.Fatalf("expected incomplete wipefs output to fail, got %v", err) + } +} diff --git a/pkg/lvm/nodeserver.go b/pkg/lvm/nodeserver.go index 132e2539..b2a54aab 100644 --- a/pkg/lvm/nodeserver.go +++ b/pkg/lvm/nodeserver.go @@ -22,6 +22,7 @@ import ( "os" "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/kubernetes-csi/csi-lib-utils/protosanitizer" "golang.org/x/sys/unix" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -36,90 +37,116 @@ type nodeServer struct { devicesPattern string } -func newNodeServer(nodeID string, maxVolumesPerNode int64) *nodeServer { - - VgActivate() +func newNodeServer(nodeID string, maxVolumesPerNode int64) (*nodeServer, error) { + if err := VgActivate(); err != nil { + return nil, fmt.Errorf("unable to initialize LVM volume groups: %w", err) + } return &nodeServer{ nodeID: nodeID, maxVolumesPerNode: maxVolumesPerNode, - } + }, nil } func (ns *nodeServer) NodePublishVolume(_ context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { - - // Check arguments - if req.GetVolumeCapability() == nil { - return nil, status.Error(codes.InvalidArgument, "Volume capability missing in request") - } - if len(req.GetVolumeId()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Volume ID missing in request") - } - if len(req.GetTargetPath()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Target path missing in request") - } - - volAttrs := req.GetVolumeContext() - targetPath := req.GetTargetPath() - vgName := volAttrs["vgName"] - - if req.GetVolumeCapability().GetBlock() != nil && - req.GetVolumeCapability().GetMount() != nil { - return nil, status.Error(codes.InvalidArgument, "cannot have both block and mount access type") - } - - var accessTypeMount, accessTypeBlock bool - volCap := req.GetVolumeCapability() - - if volCap.GetBlock() != nil { - accessTypeBlock = true - } - if volCap.GetMount() != nil { - accessTypeMount = true + vgName, err := validateNodePublishRequest(req) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } - // sanity checks (probably more sanity checks are needed later) - if accessTypeBlock && accessTypeMount { - return nil, status.Error(codes.InvalidArgument, "cannot have both block and mount access type") + // Resolve the block device to publish. For encrypted volumes this is the + // opened dm-crypt mapper; otherwise it is the bare logical volume. + devicePath := fmt.Sprintf("/dev/%s/%s", vgName, req.GetVolumeId()) + encrypted := isEncrypted(req.GetVolumeContext()) + if encrypted { + params, perr := extractCryptoParams(req.GetSecrets()) + if perr != nil { + return nil, status.Error(codes.InvalidArgument, perr.Error()) + } + mapperPath, oerr := openEncryptedDevice(devicePath, req.GetVolumeId(), params) + if oerr != nil { + return nil, status.Errorf(codes.Internal, "unable to open encrypted volume %s: %v", req.GetVolumeId(), oerr) + } + devicePath = mapperPath } if req.GetVolumeCapability().GetBlock() != nil { - - output, err := bindMountLV(req.GetVolumeId(), targetPath, vgName) - if err != nil { - return nil, fmt.Errorf("unable to bind mount lv: %w output:%s", err, output) - } - // FIXME: VolumeCapability is a struct and not the size - klog.Infof("block lv %s size:%s vg:%s devices:%s created at:%s", req.GetVolumeId(), req.GetVolumeCapability(), vgName, ns.devicesPattern, targetPath) - - } else if req.GetVolumeCapability().GetMount() != nil { - - output, err := mountLV(req.GetVolumeId(), targetPath, vgName, req.GetVolumeCapability().GetMount().GetFsType()) - if err != nil { - return nil, fmt.Errorf("unable to mount lv: %w output:%s", err, output) + err = ns.publishBlockVolume(req, devicePath) + } else { + err = ns.publishFilesystemVolume(req, devicePath) + } + if err != nil { + // Avoid leaking an open dm-crypt mapping if the mount step failed. + if encrypted { + if cerr := closeEncryptedDevice(req.GetVolumeId()); cerr != nil { + klog.Errorf("failed to close dm-crypt device for %s after publish error: %v", req.GetVolumeId(), cerr) + } } - // FIXME: VolumeCapability is a struct and not the size - klog.Infof("mounted lv %s size:%s vg:%s devices:%s created at:%s", req.GetVolumeId(), req.GetVolumeCapability(), vgName, ns.devicesPattern, targetPath) - + return nil, err } return &csi.NodePublishVolumeResponse{}, nil } -func (ns *nodeServer) NodeUnpublishVolume(_ context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) { +func (ns *nodeServer) publishBlockVolume(req *csi.NodePublishVolumeRequest, devicePath string) error { + output, err := bindMountLV(devicePath, req.GetTargetPath(), req.GetReadonly()) + if err != nil { + return fmt.Errorf("unable to bind mount lv: %w output:%s", err, output) + } + klog.Infof( + "block lv %s capability:%s device:%s devices:%s created at:%s", + req.GetVolumeId(), + req.GetVolumeCapability(), + devicePath, + ns.devicesPattern, + req.GetTargetPath(), + ) + return nil +} - volID := req.GetVolumeId() +func (ns *nodeServer) publishFilesystemVolume(req *csi.NodePublishVolumeRequest, devicePath string) error { + mount := req.GetVolumeCapability().GetMount() + output, err := mountLV( + devicePath, + req.GetTargetPath(), + mount.GetFsType(), + mount.GetMountFlags(), + req.GetReadonly(), + ) + if err != nil { + return fmt.Errorf("unable to mount lv: %w output:%s", err, output) + } + klog.Infof( + "mounted lv %s capability:%s device:%s devices:%s created at:%s", + req.GetVolumeId(), + req.GetVolumeCapability(), + devicePath, + ns.devicesPattern, + req.GetTargetPath(), + ) + return nil +} +func (ns *nodeServer) NodeUnpublishVolume(_ context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) { klog.Infof("NodeUnpublishRequest: %s", req) - // Check arguments - if len(volID) == 0 { - return nil, status.Error(codes.InvalidArgument, "Volume ID missing in request") + volID, targetPath, err := validateNodeUnpublishRequest(req) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } - if len(req.GetTargetPath()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Target path missing in request") + + if err := unmountTarget(targetPath); err != nil { + return nil, status.Errorf(codes.Internal, "failed to unmount volume %s: %v", volID, err) + } + if err := os.Remove(targetPath); err != nil && !os.IsNotExist(err) { + return nil, status.Errorf(codes.Internal, "failed to remove target path %s: %v", targetPath, err) } - umountLV(req.GetTargetPath()) + // NodeUnpublishVolume carries no volume context or secrets, so we cannot + // tell here whether the volume was encrypted. closeEncryptedDevice is a + // no-op when no dm-crypt mapping exists, so it is safe to always attempt. + if err := closeEncryptedDevice(volID); err != nil { + return nil, status.Errorf(codes.Internal, "failed to close encrypted volume %s: %v", volID, err) + } return &csi.NodeUnpublishVolumeResponse{}, nil } @@ -229,44 +256,72 @@ func (ns *nodeServer) NodeGetVolumeStats(_ context.Context, in *csi.NodeGetVolum } func (ns *nodeServer) NodeExpandVolume(_ context.Context, req *csi.NodeExpandVolumeRequest) (*csi.NodeExpandVolumeResponse, error) { - - klog.Infof("NodeExpandVolume: %s", req) - // Check arguments - if req.GetCapacityRange() == nil { - return nil, status.Error(codes.InvalidArgument, "Volume capability missing in request") - } - if len(req.GetVolumeId()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Volume ID missing in request") + // StripSecrets keeps the node-expand-secret passphrase out of the logs for + // encrypted volumes (external-resizer populates req.Secrets for those). + klog.Infof("NodeExpandVolume: %s", protosanitizer.StripSecrets(req)) + volID, volPath, capacity, err := validateNodeExpandRequest(req) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } - capacity := int64(req.GetCapacityRange().GetRequiredBytes()) - volID := req.GetVolumeId() - volPath := req.GetVolumePath() - if len(volPath) == 0 { - return nil, status.Error(codes.InvalidArgument, "Volume path not provided") + isBlock, err := isBlockVolumePath(volPath) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } - info, err := os.Stat(volPath) + // Expand requests carry no volume context, so probe for an open dm-crypt + // mapping to decide whether this is an encrypted volume. + encrypted, err := encryptedVolumeActive(volID) if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "Could not get file information from %s: %v", volPath, err) + return nil, status.Errorf(codes.Internal, "unable to inspect encrypted volume %s: %v", volID, err) } - isBlock := false - m := info.Mode() - if !m.IsDir() { - klog.Warning("volume expand request on block device: filesystem resize has to be done externally") - isBlock = true + if !encrypted { + output, eerr := extendLVS(volID, uint64(capacity), isBlock, volPath) //nolint:gosec + if eerr != nil { + return nil, status.Errorf(codes.Internal, "unable to expand volume %s: %v output:%s", volID, eerr, output) + } + return &csi.NodeExpandVolumeResponse{CapacityBytes: capacity}, nil } - output, err := extendLVS(volID, uint64(capacity), isBlock) //nolint:gosec - - if err != nil { - return nil, fmt.Errorf("unable to umount lv: %w output:%s", err, output) + // Encrypted: the LUKS resize needs the passphrase, which reaches us only if + // the StorageClass wires csi.storage.k8s.io/node-expand-secret-name/-namespace + // so the external-resizer populates NodeExpandVolumeRequest.Secrets. + params, perr := extractCryptoParams(req.GetSecrets()) + if perr != nil { + return nil, status.Errorf(codes.InvalidArgument, "encrypted volume %s expand is missing its passphrase secret (StorageClass needs a node-expand-secret): %v", volID, perr) + } + // Encrypted: grow the backing LV only (the filesystem, if any, lives on the + // dm-crypt mapper, not the bare LV), then grow the crypt mapping, then the + // filesystem on the mapper. Grow the LV by the LUKS2 header overhead so the + // decrypted device reaches the full requested capacity (matching create). + if output, eerr := extendLVS(volID, uint64(backingLVBytes(capacity, true)), true, volPath); eerr != nil { //nolint:gosec + return nil, status.Errorf(codes.Internal, "unable to expand logical volume %s: %v output:%s", volID, eerr, output) + } + if _, output, rerr := resizeEncryptedDevice(volID, params.passphrase); rerr != nil { + return nil, status.Errorf(codes.Internal, "unable to resize encrypted volume %s: %v output:%s", volID, rerr, output) + } + if !isBlock { + if output, rerr := resizeFilesystem(newCommandExecutor(), cryptMapperPath(volID), volPath); rerr != nil { + return nil, status.Errorf(codes.Internal, "unable to resize filesystem for %s: %v output:%s", volID, rerr, output) + } } return &csi.NodeExpandVolumeResponse{ CapacityBytes: capacity, }, nil +} + +func isBlockVolumePath(volumePath string) (bool, error) { + info, err := os.Stat(volumePath) + if err != nil { + return false, fmt.Errorf("could not get file information from %s: %w", volumePath, err) + } + if info.IsDir() { + return false, nil + } + klog.Warning("volume expand request on block device: filesystem resize has to be done externally") + return true, nil } diff --git a/pkg/lvm/nodeserver_test.go b/pkg/lvm/nodeserver_test.go new file mode 100644 index 00000000..de9c727f --- /dev/null +++ b/pkg/lvm/nodeserver_test.go @@ -0,0 +1,198 @@ +package lvm + +import ( + "context" + "os" + "path/filepath" + "reflect" + "testing" + + "github.com/container-storage-interface/spec/lib/go/csi" + "golang.org/x/sys/unix" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestNodePublishRejectsUnsupportedAccessMode(t *testing.T) { + ns := newNodeServerForTest() + _, err := ns.NodePublishVolume(context.Background(), &csi.NodePublishVolumeRequest{ + VolumeId: "volume", + TargetPath: filepath.Join(t.TempDir(), "target"), + VolumeContext: map[string]string{ + "vgName": "vg", + }, + VolumeCapability: mountCapability(csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER), + }) + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("expected InvalidArgument, got %v", err) + } +} + +func TestNodeUnpublishRemovesTarget(t *testing.T) { + original := unmountPath + unmountPath = func(_ string, _ int) error { + return unix.EINVAL + } + t.Cleanup(func() { + unmountPath = original + }) + + ns := newNodeServerForTest() + target := filepath.Join(t.TempDir(), "target") + if err := os.WriteFile(target, nil, 0600); err != nil { + t.Fatal(err) + } + + if _, err := ns.NodeUnpublishVolume(context.Background(), &csi.NodeUnpublishVolumeRequest{ + VolumeId: "volume", + TargetPath: target, + }); err != nil { + t.Fatalf("NodeUnpublishVolume failed: %v", err) + } + if _, err := os.Stat(target); !os.IsNotExist(err) { + t.Fatalf("expected target to be removed, stat error: %v", err) + } +} + +func TestNodeUnpublishReturnsUnmountFailure(t *testing.T) { + original := unmountPath + unmountPath = func(_ string, _ int) error { + return unix.EPERM + } + t.Cleanup(func() { + unmountPath = original + }) + + ns := newNodeServerForTest() + target := filepath.Join(t.TempDir(), "target") + if err := os.WriteFile(target, nil, 0600); err != nil { + t.Fatal(err) + } + + _, err := ns.NodeUnpublishVolume(context.Background(), &csi.NodeUnpublishVolumeRequest{ + VolumeId: "volume", + TargetPath: target, + }) + if status.Code(err) != codes.Internal { + t.Fatalf("expected Internal, got %v", err) + } + if _, statErr := os.Stat(target); statErr != nil { + t.Fatalf("target should remain after failed unmount: %v", statErr) + } +} + +func TestUnmountTargetFallsBackToLazyUnmountWhenBusy(t *testing.T) { + original := unmountPath + var flags []int + unmountPath = func(_ string, unmountFlags int) error { + flags = append(flags, unmountFlags) + if unmountFlags == 0 { + return unix.EBUSY + } + return nil + } + t.Cleanup(func() { + unmountPath = original + }) + + if err := unmountTarget("/target"); err != nil { + t.Fatalf("unmountTarget failed: %v", err) + } + want := []int{0, unix.MNT_DETACH} + if !reflect.DeepEqual(flags, want) { + t.Fatalf("unexpected unmount flags: want %#v, got %#v", want, flags) + } +} + +func TestUnmountTargetReturnsLazyUnmountFailure(t *testing.T) { + original := unmountPath + unmountPath = func(_ string, unmountFlags int) error { + if unmountFlags == 0 { + return unix.EBUSY + } + return unix.EPERM + } + t.Cleanup(func() { + unmountPath = original + }) + + if err := unmountTarget("/target"); err == nil { + t.Fatal("expected lazy unmount failure") + } +} + +func TestIsBlockVolumePath(t *testing.T) { + t.Run("directory is filesystem volume", func(t *testing.T) { + isBlock, err := isBlockVolumePath(t.TempDir()) + if err != nil || isBlock { + t.Fatalf("expected filesystem volume, got isBlock=%t err=%v", isBlock, err) + } + }) + + t.Run("file is block volume", func(t *testing.T) { + volumePath := filepath.Join(t.TempDir(), "volume") + if err := os.WriteFile(volumePath, nil, 0600); err != nil { + t.Fatal(err) + } + + isBlock, err := isBlockVolumePath(volumePath) + if err != nil || !isBlock { + t.Fatalf("expected block volume, got isBlock=%t err=%v", isBlock, err) + } + }) + + t.Run("missing path fails", func(t *testing.T) { + if _, err := isBlockVolumePath(filepath.Join(t.TempDir(), "missing")); err == nil { + t.Fatal("expected missing path to fail") + } + }) +} + +func TestBindMountReadOnlyUsesRemount(t *testing.T) { + fake := &fakeCommandExecutor{ + t: t, + results: []commandResult{ + {command: "mount"}, + {command: "mount"}, + }, + } + useFakeCommandExecutor(t, fake) + + target := filepath.Join(t.TempDir(), "target") + if _, err := bindMountLV("/dev/vg/volume", target, true); err != nil { + t.Fatalf("bindMountLV failed: %v", err) + } + + want := []string{"-o", "remount,bind,ro", target} + if got := fake.calls[1].args; !reflect.DeepEqual(got, want) { + t.Fatalf("unexpected readonly remount arguments: want %#v, got %#v", want, got) + } +} + +func TestPrepareBindMountTargetDoesNotChangeExistingTargetPermissions(t *testing.T) { + target := filepath.Join(t.TempDir(), "target") + if err := os.WriteFile(target, nil, 0600); err != nil { + t.Fatal(err) + } + + if err := prepareBindMountTarget("volume", target); err != nil { + t.Fatalf("prepareBindMountTarget failed: %v", err) + } + info, err := os.Stat(target) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != 0600 { + t.Fatalf("existing target permissions changed: got %o", got) + } +} + +func TestPrepareBindMountTargetRejectsDirectory(t *testing.T) { + if err := prepareBindMountTarget("volume", t.TempDir()); err == nil { + t.Fatal("expected directory target to fail") + } +} + +func newNodeServerForTest() *nodeServer { + return &nodeServer{nodeID: "node-a"} +} diff --git a/pkg/lvm/provisioner_pod_test.go b/pkg/lvm/provisioner_pod_test.go new file mode 100644 index 00000000..fa80c366 --- /dev/null +++ b/pkg/lvm/provisioner_pod_test.go @@ -0,0 +1,278 @@ +package lvm + +import ( + "context" + "errors" + "reflect" + "strings" + "testing" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + v1 "k8s.io/api/core/v1" + k8serror "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + corev1 "k8s.io/client-go/kubernetes/typed/core/v1" +) + +type fakeProvisionerPods struct { + corev1.PodInterface + createErr error + getErr error + phase v1.PodPhase + podStatus v1.PodStatus + deleteErr error + deleted bool + deleteContextErr error + onGet func() +} + +func (f *fakeProvisionerPods) Create( + _ context.Context, + pod *v1.Pod, + _ metav1.CreateOptions, +) (*v1.Pod, error) { + return pod, f.createErr +} + +func (f *fakeProvisionerPods) Get( + _ context.Context, + name string, + _ metav1.GetOptions, +) (*v1.Pod, error) { + if f.onGet != nil { + f.onGet() + f.onGet = nil + } + status := f.podStatus + if status.Phase == "" { + status.Phase = f.phase + } + return &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: status, + }, f.getErr +} + +func (f *fakeProvisionerPods) Delete( + ctx context.Context, + _ string, + _ metav1.DeleteOptions, +) error { + f.deleted = true + f.deleteContextErr = ctx.Err() + return f.deleteErr +} + +func TestVolumeProvisionerArgs(t *testing.T) { + t.Run("create", func(t *testing.T) { + args, err := volumeProvisionerArgs(volumeAction{ + action: actionTypeCreate, + name: "volume", + nodeName: "node-a", + size: 1048576, + lvmType: DmThinType, + vgName: "vg", + }) + if err != nil { + t.Fatalf("volumeProvisionerArgs failed: %v", err) + } + want := []string{ + "createlv", "--lvsize", "1048576", "--lvmtype", DmThinType, + "--vgname", "vg", "--lvname", "volume", + } + if !reflect.DeepEqual(args, want) { + t.Fatalf("unexpected arguments: want %#v, got %#v", want, args) + } + }) + + t.Run("delete requires source metadata", func(t *testing.T) { + _, err := volumeProvisionerArgs(volumeAction{ + action: actionTypeDelete, + name: "volume", + nodeName: "node-a", + }) + if err == nil { + t.Fatal("expected missing source metadata to fail") + } + }) +} + +func TestSnapshotProvisionerArgs(t *testing.T) { + args, err := snapshotProvisionerArgs(snapshotAction{ + action: actionTypeDelete, + snapshotName: "snapshot", + nodeName: "node-a", + vgName: "vg", + }) + if err != nil { + t.Fatalf("snapshotProvisionerArgs failed: %v", err) + } + want := []string{"deletesnap", "--snapname", "snapshot", "--vgname", "vg"} + if !reflect.DeepEqual(args, want) { + t.Fatalf("unexpected arguments: want %#v, got %#v", want, args) + } +} + +func TestProvisionerPodFallsBackToLogsForTerminationMessage(t *testing.T) { + pod := genProvisionerPodContent( + "lvm-create", + "volume", + "node-a", + "/var/lib/lvm", + "provisioner:latest", + v1.PullIfNotPresent, + []string{"createlv"}, + ) + if len(pod.Spec.Containers) != 1 { + t.Fatalf("expected one provisioner container, got %d", len(pod.Spec.Containers)) + } + container := pod.Spec.Containers[0] + if container.TerminationMessagePath != "/termination.log" { + t.Fatalf("unexpected termination message path %q", container.TerminationMessagePath) + } + if container.TerminationMessagePolicy != v1.TerminationMessageFallbackToLogsOnError { + t.Fatalf("unexpected termination message policy %q", container.TerminationMessagePolicy) + } +} + +func TestRunProvisionerPod(t *testing.T) { + t.Run("success uses independent cleanup context", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + pods := &fakeProvisionerPods{ + phase: v1.PodSucceeded, + onGet: cancel, + } + + err := runProvisionerPod( + ctx, + pods, + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "helper"}}, + "volume", + actionTypeCreate, + ) + if err != nil { + t.Fatalf("runProvisionerPod failed: %v", err) + } + if !pods.deleted { + t.Fatal("expected helper pod cleanup") + } + if pods.deleteContextErr != nil { + t.Fatalf("cleanup inherited canceled request context: %v", pods.deleteContextErr) + } + }) + + t.Run("canceled request retains pending pod for retry", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + pods := &fakeProvisionerPods{ + phase: v1.PodPending, + onGet: cancel, + } + + err := runProvisionerPod( + ctx, + pods, + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "helper"}}, + "volume", + actionTypeDelete, + ) + if status.Code(err) != codes.Canceled { + t.Fatalf("expected canceled request, got %v", err) + } + if pods.deleted { + t.Fatal("pending helper pod should be retained") + } + + pods.createErr = k8serror.NewAlreadyExists( + schema.GroupResource{Resource: "pods"}, + "helper", + ) + pods.phase = v1.PodSucceeded + if err := runProvisionerPod( + context.Background(), + pods, + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "helper"}}, + "volume", + actionTypeDelete, + ); err != nil { + t.Fatalf("retry did not reuse retained helper pod: %v", err) + } + if !pods.deleted { + t.Fatal("terminal helper pod should be cleaned up") + } + }) + + t.Run("failed pod maps to internal and is cleaned up", func(t *testing.T) { + pods := &fakeProvisionerPods{podStatus: v1.PodStatus{ + Phase: v1.PodFailed, + Reason: "ContainerFailure", + Message: "helper container failed", + ContainerStatuses: []v1.ContainerStatus{{ + Name: "provisioner", + State: v1.ContainerState{Terminated: &v1.ContainerStateTerminated{ + ExitCode: 1, + Reason: "Error", + Message: "lvcreate failed: insufficient free space", + }}, + }}, + }} + err := runProvisionerPod( + context.Background(), + pods, + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "helper"}}, + "snapshot", + actionTypeDelete, + ) + if status.Code(err) != codes.Internal { + t.Fatalf("expected Internal, got %v", err) + } + for _, detail := range []string{ + "ContainerFailure", + "container provisioner exited with code 1", + "lvcreate failed: insufficient free space", + } { + if !strings.Contains(err.Error(), detail) { + t.Fatalf("expected failure to contain %q, got %v", detail, err) + } + } + if !pods.deleted { + t.Fatal("expected failed helper pod cleanup") + } + }) + + t.Run("existing helper pod is reused", func(t *testing.T) { + pods := &fakeProvisionerPods{ + createErr: k8serror.NewAlreadyExists( + schema.GroupResource{Resource: "pods"}, + "helper", + ), + phase: v1.PodSucceeded, + } + if err := runProvisionerPod( + context.Background(), + pods, + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "helper"}}, + "volume", + actionTypeCreate, + ); err != nil { + t.Fatalf("existing helper pod was not reused: %v", err) + } + }) + + t.Run("create failure does not attempt cleanup", func(t *testing.T) { + pods := &fakeProvisionerPods{createErr: errors.New("api unavailable")} + if err := runProvisionerPod( + context.Background(), + pods, + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "helper"}}, + "volume", + actionTypeCreate, + ); err == nil { + t.Fatal("expected create error") + } + if pods.deleted { + t.Fatal("unexpected cleanup for a pod that was not created") + } + }) +} diff --git a/pkg/lvm/validation.go b/pkg/lvm/validation.go new file mode 100644 index 00000000..070ca227 --- /dev/null +++ b/pkg/lvm/validation.go @@ -0,0 +1,363 @@ +package lvm + +import ( + "fmt" + "path/filepath" + "strconv" + + "github.com/container-storage-interface/spec/lib/go/csi" + snapv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" + v1 "k8s.io/api/core/v1" +) + +func parseLVMParameters(parameters map[string]string) (string, string, error) { + lvmType := parameters["type"] + if lvmType != StripedType && lvmType != DmThinType { + return "", "", fmt.Errorf("lvmType is incorrect: %s", lvmType) + } + + vgName := parameters["vgName"] + if vgName == "" { + return "", "", fmt.Errorf("vgName is missing, please check the storage class") + } + + return lvmType, vgName, nil +} + +func validateCapacityRange(capacityRange *csi.CapacityRange) (int64, error) { + if capacityRange == nil || capacityRange.GetRequiredBytes() <= 0 { + return 0, fmt.Errorf("capacity range with required bytes greater than zero is required") + } + return capacityRange.GetRequiredBytes(), nil +} + +func validateCreateSnapshotRequest(req *csi.CreateSnapshotRequest) (string, string, error) { + if req.GetSourceVolumeId() == "" { + return "", "", fmt.Errorf("source volume ID missing in request") + } + if req.GetName() == "" { + return "", "", fmt.Errorf("snapshot name missing in request") + } + return req.GetName(), req.GetSourceVolumeId(), nil +} + +func validateDeleteVolumeRequest(req *csi.DeleteVolumeRequest) error { + if req.GetVolumeId() == "" { + return fmt.Errorf("volume ID missing in request") + } + return nil +} + +func buildVolumeContext(parameters map[string]string, requiredBytes int64) map[string]string { + volumeContext := make(map[string]string, len(parameters)+1) + for key, value := range parameters { + volumeContext[key] = value + } + volumeContext["RequiredBytes"] = strconv.FormatInt(requiredBytes, 10) + return volumeContext +} + +func validateVolumeCapabilities(capabilities []*csi.VolumeCapability) error { + if len(capabilities) == 0 { + return fmt.Errorf("volume capabilities are required") + } + + var hasBlock, hasMount bool + for _, capability := range capabilities { + if capability == nil { + return fmt.Errorf("volume capability must not be nil") + } + + block := capability.GetBlock() != nil + mount := capability.GetMount() != nil + if block == mount { + return fmt.Errorf("volume capability must specify exactly one of block or mount access type") + } + hasBlock = hasBlock || block + hasMount = hasMount || mount + + if capability.GetAccessMode() == nil { + return fmt.Errorf("volume access mode is required") + } + switch capability.GetAccessMode().GetMode() { + case csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY, + csi.VolumeCapability_AccessMode_SINGLE_NODE_SINGLE_WRITER: + default: + return fmt.Errorf("unsupported access mode %s", capability.GetAccessMode().GetMode()) + } + } + + if hasBlock && hasMount { + return fmt.Errorf("cannot combine block and mount access types") + } + return nil +} + +func validateNodePublishCapability(capability *csi.VolumeCapability, readOnly bool) error { + if err := validateVolumeCapabilities([]*csi.VolumeCapability{capability}); err != nil { + return err + } + if capability.GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY && !readOnly { + return fmt.Errorf("SINGLE_NODE_READER_ONLY requires readonly=true") + } + return nil +} + +func validateNodePublishRequest(req *csi.NodePublishVolumeRequest) (string, error) { + if req.GetVolumeCapability() == nil { + return "", fmt.Errorf("volume capability missing in request") + } + if req.GetVolumeId() == "" { + return "", fmt.Errorf("volume ID missing in request") + } + if req.GetTargetPath() == "" { + return "", fmt.Errorf("target path missing in request") + } + if !filepath.IsAbs(req.GetTargetPath()) { + return "", fmt.Errorf("target path must be absolute") + } + if err := validateNodePublishCapability(req.GetVolumeCapability(), req.GetReadonly()); err != nil { + return "", err + } + + vgName := req.GetVolumeContext()["vgName"] + if vgName == "" { + return "", fmt.Errorf("vgName is missing from volume context") + } + return vgName, nil +} + +func validateNodeUnpublishRequest(req *csi.NodeUnpublishVolumeRequest) (string, string, error) { + volumeID := req.GetVolumeId() + if volumeID == "" { + return "", "", fmt.Errorf("volume ID missing in request") + } + + targetPath := req.GetTargetPath() + if targetPath == "" { + return "", "", fmt.Errorf("target path missing in request") + } + if !filepath.IsAbs(targetPath) { + return "", "", fmt.Errorf("target path must be absolute") + } + return volumeID, targetPath, nil +} + +func validateNodeExpandRequest(req *csi.NodeExpandVolumeRequest) (string, string, int64, error) { + volumeID := req.GetVolumeId() + if volumeID == "" { + return "", "", 0, fmt.Errorf("volume ID missing in request") + } + + volumePath := req.GetVolumePath() + if volumePath == "" { + return "", "", 0, fmt.Errorf("volume path not provided") + } + + capacity, err := validateCapacityRange(req.GetCapacityRange()) + if err != nil { + return "", "", 0, err + } + return volumeID, volumePath, capacity, nil +} + +func nodeFromAccessibility(requirement *csi.TopologyRequirement) (string, error) { + if requirement == nil { + return "", fmt.Errorf("accessibility requirements are required for local LVM volumes") + } + + requisiteNodes := map[string]struct{}{} + for _, topology := range requirement.GetRequisite() { + if node := topology.GetSegments()[topologyKeyNode]; node != "" { + requisiteNodes[node] = struct{}{} + } + } + + for _, topology := range requirement.GetPreferred() { + node := topology.GetSegments()[topologyKeyNode] + if node == "" { + continue + } + _, allowed := requisiteNodes[node] + if len(requisiteNodes) > 0 && !allowed { + return "", fmt.Errorf("preferred node %s is not present in requisite topologies", node) + } + return node, nil + } + + if len(requisiteNodes) == 0 { + return "", fmt.Errorf("accessibility requirements do not contain %s", topologyKeyNode) + } + if len(requisiteNodes) > 1 { + return "", fmt.Errorf("multiple requisite nodes are unsupported without a preferred node") + } + for node := range requisiteNodes { + return node, nil + } + return "", fmt.Errorf("unable to select a topology node") +} + +func topologyFromAccessibility(requirement *csi.TopologyRequirement) (string, []*csi.Topology, error) { + node, err := nodeFromAccessibility(requirement) + if err != nil { + return "", nil, err + } + return node, []*csi.Topology{{ + Segments: map[string]string{topologyKeyNode: node}, + }}, nil +} + +func metadataFromPV(volume *v1.PersistentVolume) (string, string, string, error) { + if volume == nil { + return "", "", "", fmt.Errorf("persistent volume is nil") + } + + vgName, lvmType, err := lvmAttributesFromPV(volume.Name, volume.Spec.CSI) + if err != nil { + return "", "", "", err + } + nodeName, err := nodeFromPVAffinity(volume.Name, volume.Spec.NodeAffinity) + if err != nil { + return "", "", "", err + } + return nodeName, vgName, lvmType, nil +} + +func lvmAttributesFromPV(name string, source *v1.CSIPersistentVolumeSource) (string, string, error) { + if source == nil { + return "", "", fmt.Errorf("persistent volume %s has no CSI source", name) + } + + vgName := source.VolumeAttributes["vgName"] + if vgName == "" { + return "", "", fmt.Errorf("persistent volume %s has no vgName attribute", name) + } + lvmType := source.VolumeAttributes["type"] + if lvmType != StripedType && lvmType != DmThinType { + return "", "", fmt.Errorf("persistent volume %s has invalid LVM type %q", name, lvmType) + } + return vgName, lvmType, nil +} + +func nodeFromPVAffinity(name string, affinity *v1.VolumeNodeAffinity) (string, error) { + if affinity == nil || affinity.Required == nil { + return "", fmt.Errorf("persistent volume %s has no required node affinity", name) + } + nodes := map[string]struct{}{} + for _, expression := range matchExpressionsFromTerms(affinity.Required.NodeSelectorTerms) { + if expression.Key != topologyKeyNode { + continue + } + if expression.Operator != v1.NodeSelectorOpIn { + return "", fmt.Errorf( + "persistent volume %s topology expression must use operator In", + name, + ) + } + for _, node := range expression.Values { + if node == "" { + continue + } + nodes[node] = struct{}{} + } + } + if len(nodes) != 1 { + return "", fmt.Errorf( + "persistent volume %s must reference exactly one %s node, found %d", + name, + topologyKeyNode, + len(nodes), + ) + } + + for node := range nodes { + return node, nil + } + return "", fmt.Errorf("unable to select node for persistent volume %s", name) +} + +func matchExpressionsFromTerms(terms []v1.NodeSelectorTerm) []v1.NodeSelectorRequirement { + expressions := make([]v1.NodeSelectorRequirement, 0) + for _, term := range terms { + expressions = append(expressions, term.MatchExpressions...) + } + return expressions +} + +func requiredBytesFromPersistentVolume(volume *v1.PersistentVolume) (int64, error) { + if volume == nil { + return 0, fmt.Errorf("persistent volume is nil") + } + if volume.Spec.CSI == nil { + return 0, fmt.Errorf("persistent volume %s has no CSI source", volume.Name) + } + value := volume.Spec.CSI.VolumeAttributes["RequiredBytes"] + size, err := strconv.ParseInt(value, 10, 64) + if err != nil || size <= 0 { + return 0, fmt.Errorf("persistent volume %s has invalid RequiredBytes attribute %q", volume.Name, value) + } + return size, nil +} + +func metadataFromSnapshotContent(content *snapv1.VolumeSnapshotContent) (string, string, int64, error) { + if content == nil { + return "", "", 0, fmt.Errorf("snapshot content is nil") + } + if content.Spec.Source.VolumeHandle == nil || *content.Spec.Source.VolumeHandle == "" { + return "", "", 0, fmt.Errorf("snapshot content %s has no source volume handle", content.Name) + } + if content.Status == nil || + content.Status.RestoreSize == nil || + content.Status.SnapshotHandle == nil || + *content.Status.SnapshotHandle == "" || + *content.Status.RestoreSize <= 0 { + return "", "", 0, fmt.Errorf("snapshot content %s is not ready to restore", content.Name) + } + return *content.Spec.Source.VolumeHandle, + *content.Status.SnapshotHandle, + *content.Status.RestoreSize, + nil +} + +func preProvisionedSnapshotMetadata(content *snapv1.VolumeSnapshotContent) (string, int64, error) { + if content == nil { + return "", 0, fmt.Errorf("snapshot content is nil") + } + + statusHandle := "" + if content.Status != nil && content.Status.SnapshotHandle != nil { + statusHandle = *content.Status.SnapshotHandle + } + sourceHandle := "" + if content.Spec.Source.SnapshotHandle != nil { + sourceHandle = *content.Spec.Source.SnapshotHandle + } + if statusHandle != "" && sourceHandle != "" && statusHandle != sourceHandle { + return "", 0, fmt.Errorf( + "pre-provisioned snapshot content %s has conflicting snapshot handles %q and %q", + content.Name, + statusHandle, + sourceHandle, + ) + } + + snapshotID := statusHandle + if snapshotID == "" { + snapshotID = sourceHandle + } + if snapshotID == "" { + return "", 0, fmt.Errorf("pre-provisioned snapshot content %s has no snapshot handle", content.Name) + } + + if content.Status == nil || content.Status.RestoreSize == nil { + return snapshotID, 0, nil + } + // The snapshot controller may publish zero when the size of a + // pre-provisioned snapshot is unknown. Let the restore path fall back to + // the destination PVC size in that case. + if *content.Status.RestoreSize < 0 { + return "", 0, fmt.Errorf("pre-provisioned snapshot content %s has invalid restore size %d", content.Name, *content.Status.RestoreSize) + } + return snapshotID, *content.Status.RestoreSize, nil +} diff --git a/pkg/lvm/validation_test.go b/pkg/lvm/validation_test.go new file mode 100644 index 00000000..b4a64ae5 --- /dev/null +++ b/pkg/lvm/validation_test.go @@ -0,0 +1,512 @@ +package lvm + +import ( + "testing" + + "github.com/container-storage-interface/spec/lib/go/csi" + snapv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func mountCapability(mode csi.VolumeCapability_AccessMode_Mode) *csi.VolumeCapability { + return &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{FsType: "ext4"}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{Mode: mode}, + } +} + +func validPersistentVolume(name string) *v1.PersistentVolume { + return &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: v1.PersistentVolumeSpec{ + PersistentVolumeSource: v1.PersistentVolumeSource{ + CSI: &v1.CSIPersistentVolumeSource{ + Driver: "lvm.driver.harvesterhci.io", + VolumeAttributes: map[string]string{ + "vgName": "vg", + "type": DmThinType, + }, + }, + }, + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{{ + MatchExpressions: []v1.NodeSelectorRequirement{{ + Key: topologyKeyNode, + Operator: v1.NodeSelectorOpIn, + Values: []string{"node-a"}, + }}, + }}, + }, + }, + }, + } +} + +func TestParseLVMParameters(t *testing.T) { + tests := []struct { + name string + parameters map[string]string + wantType string + wantVG string + wantErr bool + }{ + { + name: "striped", + parameters: map[string]string{"type": StripedType, "vgName": "vg-a"}, + wantType: StripedType, + wantVG: "vg-a", + }, + { + name: "dm-thin", + parameters: map[string]string{"type": DmThinType, "vgName": "vg-b"}, + wantType: DmThinType, + wantVG: "vg-b", + }, + { + name: "missing type", + parameters: map[string]string{"vgName": "vg-a"}, + wantErr: true, + }, + { + name: "unsupported type", + parameters: map[string]string{"type": "linear", "vgName": "vg-a"}, + wantErr: true, + }, + { + name: "missing vg", + parameters: map[string]string{"type": DmThinType}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotType, gotVG, err := parseLVMParameters(tt.parameters) + if tt.wantErr { + if err == nil { + t.Fatalf("expected validation error, got type=%q vg=%q", gotType, gotVG) + } + return + } + if err != nil { + t.Fatalf("parseLVMParameters failed: %v", err) + } + if gotType != tt.wantType || gotVG != tt.wantVG { + t.Fatalf( + "unexpected parameters: want type=%q vg=%q, got type=%q vg=%q", + tt.wantType, + tt.wantVG, + gotType, + gotVG, + ) + } + }) + } +} + +func TestValidateCapacityRange(t *testing.T) { + tests := []struct { + name string + capacityRange *csi.CapacityRange + want int64 + wantErr bool + }{ + {name: "nil range", wantErr: true}, + { + name: "zero required bytes", + capacityRange: &csi.CapacityRange{}, + wantErr: true, + }, + { + name: "negative required bytes", + capacityRange: &csi.CapacityRange{RequiredBytes: -1}, + wantErr: true, + }, + { + name: "valid required bytes", + capacityRange: &csi.CapacityRange{RequiredBytes: 1048576}, + want: 1048576, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := validateCapacityRange(tt.capacityRange) + if tt.wantErr { + if err == nil { + t.Fatalf("expected validation error, got %d", got) + } + return + } + if err != nil { + t.Fatalf("validateCapacityRange failed: %v", err) + } + if got != tt.want { + t.Fatalf("unexpected capacity: want %d, got %d", tt.want, got) + } + }) + } +} + +func TestValidateCreateSnapshotRequest(t *testing.T) { + tests := []struct { + name string + request *csi.CreateSnapshotRequest + wantSnapshot string + wantVolume string + wantErr bool + }{ + {name: "nil request", wantErr: true}, + { + name: "missing source volume", + request: &csi.CreateSnapshotRequest{Name: "snapshot"}, + wantErr: true, + }, + { + name: "missing snapshot name", + request: &csi.CreateSnapshotRequest{SourceVolumeId: "volume"}, + wantErr: true, + }, + { + name: "valid request", + request: &csi.CreateSnapshotRequest{ + Name: "snapshot", + SourceVolumeId: "volume", + }, + wantSnapshot: "snapshot", + wantVolume: "volume", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + snapshotName, volumeID, err := validateCreateSnapshotRequest(tt.request) + if tt.wantErr { + if err == nil { + t.Fatalf("expected validation error, got snapshot=%q volume=%q", snapshotName, volumeID) + } + return + } + if err != nil { + t.Fatalf("validateCreateSnapshotRequest failed: %v", err) + } + if snapshotName != tt.wantSnapshot || volumeID != tt.wantVolume { + t.Fatalf( + "unexpected values: want snapshot=%q volume=%q, got snapshot=%q volume=%q", + tt.wantSnapshot, + tt.wantVolume, + snapshotName, + volumeID, + ) + } + }) + } +} + +func TestBuildVolumeContext(t *testing.T) { + parameters := map[string]string{ + "type": DmThinType, + "vgName": "vg-a", + } + + got := buildVolumeContext(parameters, 1048576) + if got["type"] != DmThinType || + got["vgName"] != "vg-a" || + got["RequiredBytes"] != "1048576" { + t.Fatalf("unexpected volume context: %#v", got) + } + if _, exists := parameters["RequiredBytes"]; exists { + t.Fatal("buildVolumeContext must not mutate request parameters") + } +} + +func TestValidateVolumeCapabilitiesRejectsMultiNodeModes(t *testing.T) { + err := validateVolumeCapabilities([]*csi.VolumeCapability{ + mountCapability(csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER), + }) + if err == nil { + t.Fatal("expected multi-node access mode to be rejected") + } +} + +func TestValidateNodePublishCapabilityRequiresReadonlyForReader(t *testing.T) { + capability := mountCapability(csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY) + if err := validateNodePublishCapability(capability, false); err == nil { + t.Fatal("expected readonly=false to be rejected for SINGLE_NODE_READER_ONLY") + } + if err := validateNodePublishCapability(capability, true); err != nil { + t.Fatalf("expected readonly reader capability to be accepted: %v", err) + } +} + +func TestValidateNodePublishRequest(t *testing.T) { + validRequest := func() *csi.NodePublishVolumeRequest { + return &csi.NodePublishVolumeRequest{ + VolumeId: "volume", + TargetPath: "/var/lib/kubelet/pods/target", + VolumeCapability: mountCapability(csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER), + VolumeContext: map[string]string{"vgName": "vg-a"}, + } + } + + t.Run("valid", func(t *testing.T) { + vgName, err := validateNodePublishRequest(validRequest()) + if err != nil || vgName != "vg-a" { + t.Fatalf("expected vg-a, got vg=%q err=%v", vgName, err) + } + }) + + t.Run("missing volume group", func(t *testing.T) { + req := validRequest() + delete(req.VolumeContext, "vgName") + if _, err := validateNodePublishRequest(req); err == nil { + t.Fatal("expected missing volume group to fail") + } + }) + + t.Run("relative target path", func(t *testing.T) { + req := validRequest() + req.TargetPath = "relative/path" + if _, err := validateNodePublishRequest(req); err == nil { + t.Fatal("expected relative target path to fail") + } + }) +} + +func TestValidateNodeUnpublishRequest(t *testing.T) { + t.Run("valid", func(t *testing.T) { + volumeID, targetPath, err := validateNodeUnpublishRequest(&csi.NodeUnpublishVolumeRequest{ + VolumeId: "volume", + TargetPath: "/var/lib/kubelet/pods/target", + }) + if err != nil || volumeID != "volume" || targetPath != "/var/lib/kubelet/pods/target" { + t.Fatalf("unexpected result: volumeID=%q targetPath=%q err=%v", volumeID, targetPath, err) + } + }) + + tests := map[string]*csi.NodeUnpublishVolumeRequest{ + "missing volume ID": {TargetPath: "/target"}, + "missing target path": {VolumeId: "volume"}, + "relative target path": {VolumeId: "volume", TargetPath: "relative/path"}, + } + for name, req := range tests { + t.Run(name, func(t *testing.T) { + if _, _, err := validateNodeUnpublishRequest(req); err == nil { + t.Fatal("expected validation to fail") + } + }) + } +} + +func TestValidateNodeExpandRequest(t *testing.T) { + t.Run("valid", func(t *testing.T) { + volumeID, volumePath, capacity, err := validateNodeExpandRequest(&csi.NodeExpandVolumeRequest{ + VolumeId: "volume", + VolumePath: "/var/lib/kubelet/pods/volume", + CapacityRange: &csi.CapacityRange{RequiredBytes: 1048576}, + }) + if err != nil || volumeID != "volume" || volumePath != "/var/lib/kubelet/pods/volume" || capacity != 1048576 { + t.Fatalf("unexpected result: volumeID=%q volumePath=%q capacity=%d err=%v", volumeID, volumePath, capacity, err) + } + }) + + tests := map[string]*csi.NodeExpandVolumeRequest{ + "missing volume ID": { + VolumePath: "/volume", + CapacityRange: &csi.CapacityRange{RequiredBytes: 1048576}, + }, + "missing volume path": { + VolumeId: "volume", + CapacityRange: &csi.CapacityRange{RequiredBytes: 1048576}, + }, + "missing capacity": { + VolumeId: "volume", + VolumePath: "/volume", + }, + "non-positive capacity": { + VolumeId: "volume", + VolumePath: "/volume", + CapacityRange: &csi.CapacityRange{}, + }, + } + for name, req := range tests { + t.Run(name, func(t *testing.T) { + if _, _, _, err := validateNodeExpandRequest(req); err == nil { + t.Fatal("expected validation to fail") + } + }) + } +} + +func TestValidateDeleteVolumeRequest(t *testing.T) { + if err := validateDeleteVolumeRequest(&csi.DeleteVolumeRequest{VolumeId: "volume"}); err != nil { + t.Fatalf("valid request failed: %v", err) + } + if err := validateDeleteVolumeRequest(&csi.DeleteVolumeRequest{}); err == nil { + t.Fatal("expected missing volume ID to fail") + } +} + +func TestNodeFromAccessibility(t *testing.T) { + t.Run("preferred node", func(t *testing.T) { + node, err := nodeFromAccessibility(&csi.TopologyRequirement{ + Preferred: []*csi.Topology{{ + Segments: map[string]string{topologyKeyNode: "node-a"}, + }}, + }) + if err != nil || node != "node-a" { + t.Fatalf("expected node-a, got node=%q err=%v", node, err) + } + }) + + t.Run("single requisite fallback", func(t *testing.T) { + node, err := nodeFromAccessibility(&csi.TopologyRequirement{ + Requisite: []*csi.Topology{{ + Segments: map[string]string{topologyKeyNode: "node-b"}, + }}, + }) + if err != nil || node != "node-b" { + t.Fatalf("expected node-b, got node=%q err=%v", node, err) + } + }) + + t.Run("missing requirement", func(t *testing.T) { + if _, err := nodeFromAccessibility(nil); err == nil { + t.Fatal("expected nil requirement to fail") + } + }) + + t.Run("preferred node must be requisite", func(t *testing.T) { + _, err := nodeFromAccessibility(&csi.TopologyRequirement{ + Preferred: []*csi.Topology{{ + Segments: map[string]string{topologyKeyNode: "node-a"}, + }}, + Requisite: []*csi.Topology{{ + Segments: map[string]string{topologyKeyNode: "node-b"}, + }}, + }) + if err == nil { + t.Fatal("expected preferred node outside requisite topology to fail") + } + }) +} + +func TestTopologyFromAccessibility(t *testing.T) { + node, topology, err := topologyFromAccessibility(&csi.TopologyRequirement{ + Preferred: []*csi.Topology{{ + Segments: map[string]string{topologyKeyNode: "node-a"}, + }}, + }) + if err != nil { + t.Fatalf("topologyFromAccessibility failed: %v", err) + } + if node != "node-a" { + t.Fatalf("expected node-a, got %q", node) + } + if len(topology) != 1 || topology[0].GetSegments()[topologyKeyNode] != "node-a" { + t.Fatalf("unexpected accessible topology: %#v", topology) + } +} + +func TestMetadataFromPersistentVolume(t *testing.T) { + nodeName, vgName, lvmType, err := metadataFromPV(validPersistentVolume("volume")) + if err != nil { + t.Fatalf("valid metadata failed: %v", err) + } + if nodeName != "node-a" || vgName != "vg" || lvmType != DmThinType { + t.Fatalf("unexpected metadata: node=%q vg=%q type=%q", nodeName, vgName, lvmType) + } + + malformed := validPersistentVolume("malformed") + malformed.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions[0].Values = nil + if _, _, _, err := metadataFromPV(malformed); err == nil { + t.Fatal("expected empty topology values to fail") + } +} + +func TestPreProvisionedSnapshotMetadata(t *testing.T) { + restoreSize := int64(2 << 30) + + t.Run("returns matching status handle and restore size", func(t *testing.T) { + content := &snapv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{Name: "content"}, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{SnapshotHandle: strPointer("snapshot-id")}, + }, + Status: &snapv1.VolumeSnapshotContentStatus{ + SnapshotHandle: strPointer("snapshot-id"), + RestoreSize: &restoreSize, + }, + } + handle, size, err := preProvisionedSnapshotMetadata(content) + if err != nil || handle != "snapshot-id" || size != restoreSize { + t.Fatalf("unexpected metadata: handle=%q size=%d err=%v", handle, size, err) + } + }) + + t.Run("falls back to source handle without restore size", func(t *testing.T) { + content := &snapv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{Name: "content"}, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{SnapshotHandle: strPointer("snapshot-id")}, + }, + } + handle, size, err := preProvisionedSnapshotMetadata(content) + if err != nil || handle != "snapshot-id" || size != 0 { + t.Fatalf("unexpected fallback metadata: handle=%q size=%d err=%v", handle, size, err) + } + }) + + t.Run("falls back to destination size when restore size is zero", func(t *testing.T) { + zeroSize := int64(0) + content := &snapv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{Name: "content"}, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{SnapshotHandle: strPointer("snapshot-id")}, + }, + Status: &snapv1.VolumeSnapshotContentStatus{RestoreSize: &zeroSize}, + } + handle, size, err := preProvisionedSnapshotMetadata(content) + if err != nil || handle != "snapshot-id" || size != 0 { + t.Fatalf("unexpected zero-size metadata: handle=%q size=%d err=%v", handle, size, err) + } + }) + + t.Run("rejects conflicting handles", func(t *testing.T) { + content := &snapv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{Name: "content"}, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{SnapshotHandle: strPointer("spec-handle")}, + }, + Status: &snapv1.VolumeSnapshotContentStatus{SnapshotHandle: strPointer("status-handle")}, + } + if _, _, err := preProvisionedSnapshotMetadata(content); err == nil { + t.Fatal("expected conflicting handles to fail") + } + }) + + t.Run("rejects missing handle", func(t *testing.T) { + content := &snapv1.VolumeSnapshotContent{ObjectMeta: metav1.ObjectMeta{Name: "content"}} + if _, _, err := preProvisionedSnapshotMetadata(content); err == nil { + t.Fatal("expected missing handle to fail") + } + }) + + t.Run("rejects invalid restore size", func(t *testing.T) { + invalidSize := int64(-1) + content := &snapv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{Name: "content"}, + Spec: snapv1.VolumeSnapshotContentSpec{ + Source: snapv1.VolumeSnapshotContentSource{SnapshotHandle: strPointer("snapshot-id")}, + }, + Status: &snapv1.VolumeSnapshotContentStatus{RestoreSize: &invalidSize}, + } + if _, _, err := preProvisionedSnapshotMetadata(content); err == nil { + t.Fatal("expected invalid restore size to fail") + } + }) +}