From c49b433c0895052b39cb3cf0af0d0d39dc8012a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=83=A1=E7=8E=AE=E6=96=87?= Date: Wed, 30 Apr 2025 14:56:15 +0800 Subject: [PATCH] disk: don't get VGS to check for an unsupported parameter We should not support this in the first place. This will affect user cost, and should not be allowed to set on PVC. --- pkg/disk/group_volume_snapshot_utils.go | 53 ++------------------ pkg/disk/group_volume_snapshot_utils_test.go | 19 +------ pkg/disk/groupcontrollerserver.go | 2 +- 3 files changed, 6 insertions(+), 68 deletions(-) diff --git a/pkg/disk/group_volume_snapshot_utils.go b/pkg/disk/group_volume_snapshot_utils.go index 94eb6d439..7718dbe4e 100644 --- a/pkg/disk/group_volume_snapshot_utils.go +++ b/pkg/disk/group_volume_snapshot_utils.go @@ -1,7 +1,6 @@ package disk import ( - "context" "fmt" "slices" "strings" @@ -10,48 +9,16 @@ import ( "github.com/aliyun/alibaba-cloud-sdk-go/services/ecs" "github.com/container-storage-interface/spec/lib/go/csi" "github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/common" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/timestamppb" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/klog/v2" ) -func getVolumeGroupSnapshotConfig(req *csi.CreateVolumeGroupSnapshotRequest) (*createGroupSnapshotParams, error) { - var ecsParams createGroupSnapshotParams - if req.Parameters != nil { - err := parseGroupSnapshotParameters(req.Parameters, &ecsParams) - if err != nil { - klog.Errorf("CreateSnapshot:: Snapshot name[%s], parse config failed: %v", req.Name, err) - return nil, err - } - } - - vsName := req.Parameters[common.VolumeGroupSnapshotNameKey] - vsNameSpace := req.Parameters[common.VolumeGroupSnapshotNamespaceKey] - // volumesnapshot not in parameters, just return - if vsName == "" || vsNameSpace == "" { - return &ecsParams, nil - } - - volumeGroupSnapshot, err := GlobalConfigVar.SnapClient.GroupsnapshotV1alpha1().VolumeGroupSnapshots(vsNameSpace).Get(context.Background(), vsName, metav1.GetOptions{}) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to get VolumeGroupSnapshot: %s/%s: %v", vsNameSpace, vsName, err) - } - err = parseGroupSnapshotAnnotations(volumeGroupSnapshot.Annotations, &ecsParams) - if err != nil { - klog.Errorf("CreateVolumeGroupSnapshot:: Snapshot name[%s], parse annotation failed: %v", req.Name, err) - return nil, err - } - return &ecsParams, nil -} - -func parseGroupSnapshotParameters(params map[string]string, ecsParams *createGroupSnapshotParams) (err error) { +func parseGroupSnapshotParameters(params map[string]string) (*createGroupSnapshotParams, error) { + ecsParams := &createGroupSnapshotParams{} tags := make(map[string]string) for k, v := range params { switch k { case RETENTIONDAYS: - return fmt.Errorf("groupSnapshot do not support retentionDays: %w", err) + return nil, fmt.Errorf("groupSnapshot do not support retentionDays") case SNAPSHOTRESOURCEGROUPID: ecsParams.ResourceGroupID = v case common.VolumeGroupSnapshotNameKey: @@ -81,19 +48,7 @@ func parseGroupSnapshotParameters(params map[string]string, ecsParams *createGro }) } } - return nil -} - -// if volumesnapshot have Annotations, use it first. -// storage.alibabacloud.com/snapshot-ttl -func parseGroupSnapshotAnnotations(anno map[string]string, ecsParams *createGroupSnapshotParams) error { - snapshotTTL := anno[common.SnapshotTTLKey] - - if snapshotTTL != "" { - return fmt.Errorf("groupSnapshot do not support retentionDays") - } - - return nil + return ecsParams, nil } func formatGroupSnapshot(groupSnapshot *ecs.SnapshotGroup) (*csi.VolumeGroupSnapshot, error) { diff --git a/pkg/disk/group_volume_snapshot_utils_test.go b/pkg/disk/group_volume_snapshot_utils_test.go index c7d874138..ff9603b83 100644 --- a/pkg/disk/group_volume_snapshot_utils_test.go +++ b/pkg/disk/group_volume_snapshot_utils_test.go @@ -12,21 +12,6 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" ) -func Test_parseGroupSnapshotAnnotations(t *testing.T) { - annotations := make(map[string]string) - ecsParams := &createGroupSnapshotParams{} - - // Test case 1: snapshotTTL is empty - annotations[common.SnapshotTTLKey] = "" - err := parseGroupSnapshotAnnotations(annotations, ecsParams) - assert.Nil(t, err) - - // Test case 2: snapshotTTL is not empty - annotations[common.SnapshotTTLKey] = "10" - err = parseGroupSnapshotAnnotations(annotations, ecsParams) - assert.Error(t, err) -} - func Test_formatGroupSnapshot(t *testing.T) { // Create a mock groupSnapshot object mockGroupSnapshot := &ecs.SnapshotGroup{ @@ -455,9 +440,7 @@ func Test_parseGroupSnapshotParameters(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ecsParams := &createGroupSnapshotParams{} - - err := parseGroupSnapshotParameters(tt.params, ecsParams) + ecsParams, err := parseGroupSnapshotParameters(tt.params) if tt.wantError { assert.Error(t, err) diff --git a/pkg/disk/groupcontrollerserver.go b/pkg/disk/groupcontrollerserver.go index 5838017c5..eccc9a3f5 100644 --- a/pkg/disk/groupcontrollerserver.go +++ b/pkg/disk/groupcontrollerserver.go @@ -105,7 +105,7 @@ func (cs *groupControllerServer) CreateVolumeGroupSnapshot(ctx context.Context, // todo: Do not check source disks here. If need, use `checkSourceVolumes` // init createSnapshotGroupRequest and parameters - params, err := getVolumeGroupSnapshotConfig(req) + params, err := parseGroupSnapshotParameters(req.Parameters) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "get volumeGroupSnapshot %s config failed: %v", req.GetName(), err) }