From 94e7c8cd53c55c8d5b3bc49613b0de7c45c4be9e Mon Sep 17 00:00:00 2001 From: Vicente Cheng Date: Wed, 18 Sep 2024 11:13:52 +0800 Subject: [PATCH] controller: refact the duplcated codes for the clone operation - also correct the description of command Signed-off-by: Vicente Cheng --- cmd/provisioner/createsnap.go | 6 +-- cmd/provisioner/deletesnap.go | 4 +- pkg/lvm/controllerserver.go | 81 +++++++++++++---------------------- 3 files changed, 34 insertions(+), 57 deletions(-) diff --git a/cmd/provisioner/createsnap.go b/cmd/provisioner/createsnap.go index c9521761..7dcb456b 100644 --- a/cmd/provisioner/createsnap.go +++ b/cmd/provisioner/createsnap.go @@ -16,7 +16,7 @@ func createSnapCmd() *cli.Command { Flags: []cli.Flag{ &cli.StringFlag{ Name: flagSnapName, - Usage: "Required. Specify lv name.", + Usage: "Required. Specify snapshot name.", }, &cli.Int64Flag{ Name: flagLVSize, @@ -28,7 +28,7 @@ func createSnapCmd() *cli.Command { }, &cli.StringFlag{ Name: flagLVName, - Usage: "Required. the name of the volumegroup", + Usage: "Required. Specify the logical volume name.", }, &cli.StringFlag{ Name: flagLVMType, @@ -60,7 +60,7 @@ func createSnap(c *cli.Context) error { } snapName := c.String(flagSnapName) if snapName == "" { - return fmt.Errorf("invalid empty flag %v", flagLVMType) + return fmt.Errorf("invalid empty flag %v", flagSnapName) } lvType := c.String(flagLVMType) if lvType == "" { diff --git a/cmd/provisioner/deletesnap.go b/cmd/provisioner/deletesnap.go index 9abc7ef2..87a0fedf 100644 --- a/cmd/provisioner/deletesnap.go +++ b/cmd/provisioner/deletesnap.go @@ -16,7 +16,7 @@ func deleteSnapCmd() *cli.Command { Flags: []cli.Flag{ &cli.StringFlag{ Name: flagSnapName, - Usage: "Required. Specify lv name.", + Usage: "Required. Specify snapshot name.", }, &cli.StringFlag{ Name: flagVGName, @@ -40,7 +40,7 @@ func deleteSnap(c *cli.Context) error { } snapName := c.String(flagSnapName) if snapName == "" { - return fmt.Errorf("invalid empty flag %v", flagLVMType) + return fmt.Errorf("invalid empty flag %v", flagSnapName) } klog.Infof("delete snapshot: %s/%s", vgName, snapName) diff --git a/pkg/lvm/controllerserver.go b/pkg/lvm/controllerserver.go index ecc443a2..5613f3b6 100644 --- a/pkg/lvm/controllerserver.go +++ b/pkg/lvm/controllerserver.go @@ -195,36 +195,27 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol }, 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{}) - if err != nil { - klog.Errorf("error getting volume: %v", err) - return err - } +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"] - srcVgType := srcVol.Spec.CSI.VolumeAttributes["type"] - restoreSize := *snapContent.Status.RestoreSize + srcType := srcVol.Spec.CSI.VolumeAttributes["type"] - snapshotLVName := fmt.Sprintf("lvm-%s", *snapContent.Status.SnapshotHandle) - //srcSnapDev := fmt.Sprintf("/dev/%s/%s", srcVgName, snapshotLVName) srcInfo := &srcInfo{ - srcLVName: snapshotLVName, + srcLVName: srcLVName, srcVGName: srcVgName, - srcType: srcVgType, + srcType: srcType, } - klog.V(4).Infof("cloning volume from %s/%s ", srcVgName, snapshotLVName) + klog.V(4).Infof("cloning volume from %s/%s ", srcVgName, srcLVName) - if restoreSize > dstSize { - return status.Error(codes.InvalidArgument, "source volume size is larger than destination volume size") + if 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 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) } - va := volumeAction{ + return volumeAction{ action: actionTypeClone, name: dstName, nodeName: dstNode, @@ -237,7 +228,23 @@ func (cs *controllerServer) cloneFromSnapshot(ctx context.Context, snapContent * vgName: dstVGName, hostWritePath: cs.hostWritePath, srcInfo: srcInfo, + }, 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{}) + if err != nil { + klog.Errorf("error getting volume: %v", err) + return err } + restoreSize := *snapContent.Status.RestoreSize + snapshotLVName := fmt.Sprintf("lvm-%s", *snapContent.Status.SnapshotHandle) + va, err := cs.generateVolumeActionForClone(srcVol, snapshotLVName, dstName, dstNode, dstLVType, dstVGName, restoreSize, dstSize) + if err != nil { + return err + } + if err := createProvisionerPod(ctx, va); err != nil { klog.Errorf("error creating provisioner pod :%v", err) return err @@ -247,47 +254,17 @@ func (cs *controllerServer) cloneFromSnapshot(ctx context.Context, snapContent * } func (cs *controllerServer) cloneFromVolume(ctx context.Context, srcVol *v1.PersistentVolume, dstName, dstNode, dstLVType, dstVGName string, dstSize int64) 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"] srcSizeStr := srcVol.Spec.CSI.VolumeAttributes["RequiredBytes"] srcSize, err := strconv.ParseInt(srcSizeStr, 10, 64) if err != nil { - klog.Errorf("error parsing srcSize: %v", err) - return err + return status.Errorf(codes.InvalidArgument, "error parsing srcSize: %v", err) } - - //srcDev := fmt.Sprintf("/dev/%s/%s", srcVgName, srcVol.GetName()) srcLVName := srcVol.GetName() - srcInfo := &srcInfo{ - srcLVName: srcLVName, - srcVGName: srcVgName, - srcType: srcType, - } - klog.V(4).Infof("cloning volume from %s/%s ", srcVgName, srcLVName) - - if srcSize > dstSize { - return status.Error(codes.InvalidArgument, "source volume size is larger than destination volume size") - } - if srcNode != dstNode { - return status.Errorf(codes.InvalidArgument, "source (%s) and destination (%s) nodes are different (not supported)", srcNode, dstNode) + va, err := cs.generateVolumeActionForClone(srcVol, srcLVName, dstName, dstNode, dstLVType, dstVGName, srcSize, dstSize) + if err != nil { + return err } - va := volumeAction{ - action: actionTypeClone, - name: dstName, - nodeName: dstNode, - size: dstSize, - lvmType: dstLVType, - pullPolicy: cs.pullPolicy, - provisionerImage: cs.provisionerImage, - kubeClient: cs.kubeClient, - namespace: cs.namespace, - vgName: dstVGName, - hostWritePath: cs.hostWritePath, - srcInfo: srcInfo, - } if err := createProvisionerPod(ctx, va); err != nil { klog.Errorf("error creating provisioner pod :%v", err) return err