Merge pull request #1548 from hanyuel/snapshot-tag

Fix VolumeSnapshotClass tagging
kubernetes-sigs · Mar 28, 2023 · 67c0374 · 67c0374
2 parents 0734390 + ba2fb4a
commit 67c0374
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 4 deletions.
diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go
@@ -622,10 +622,6 @@ func (d *controllerService) CreateSnapshot(ctx context.Context, req *csi.CreateS
 		return nil, status.Errorf(codes.InvalidArgument, "Invalid tag value: %v", err)
 	}
 
-	for k, v := range addTags {
-		snapshotTags[k] = v
-	}
-
 	if d.driverOptions.kubernetesClusterID != "" {
 		resourceLifecycleTag := ResourceLifecycleTagPrefix + d.driverOptions.kubernetesClusterID
 		snapshotTags[resourceLifecycleTag] = ResourceLifecycleOwned
@@ -634,6 +630,11 @@ func (d *controllerService) CreateSnapshot(ctx context.Context, req *csi.CreateS
 	for k, v := range d.driverOptions.extraTags {
 		snapshotTags[k] = v
 	}
+
+	for k, v := range addTags {
+		snapshotTags[k] = v
+	}
+
 	opts := &cloud.SnapshotOptions{
 		Tags: snapshotTags,
 	}

diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go
@@ -2398,6 +2398,64 @@ func TestCreateSnapshot(t *testing.T) {
 					t.Fatalf("Unexpected error: %v", err)
 				}
 
+				if snap := resp.GetSnapshot(); snap == nil {
+					t.Fatalf("Expected snapshot %v, got nil", expSnapshot)
+				}
+			},
+		},
+		{
+			name: "success with VolumeSnapshotClass with Name tag and cluster id",
+			testFunc: func(t *testing.T) {
+				const (
+					snapshotName = "test-snapshot"
+					nameTagValue = "test-name-tag-value"
+					clusterId    = "test-cluster-id"
+				)
+
+				req := &csi.CreateSnapshotRequest{
+					Name: snapshotName,
+					Parameters: map[string]string{
+						"tagSpecification_1": NameTag + "=" + nameTagValue,
+					},
+					SourceVolumeId: "vol-test",
+				}
+				expSnapshot := &csi.Snapshot{
+					ReadyToUse: true,
+				}
+
+				ctx := context.Background()
+				mockSnapshot := &cloud.Snapshot{
+					SnapshotID:     fmt.Sprintf("snapshot-%d", rand.New(rand.NewSource(time.Now().UnixNano())).Uint64()),
+					SourceVolumeID: req.SourceVolumeId,
+					Size:           1,
+					CreationTime:   time.Now(),
+				}
+				mockCtl := gomock.NewController(t)
+				defer mockCtl.Finish()
+
+				snapshotOptions := &cloud.SnapshotOptions{
+					Tags: map[string]string{
+						cloud.SnapshotNameTagKey:               snapshotName,
+						cloud.AwsEbsDriverTagKey:               isManagedByDriver,
+						NameTag:                                nameTagValue,
+						ResourceLifecycleTagPrefix + clusterId: ResourceLifecycleOwned,
+					},
+				}
+
+				mockCloud := cloud.NewMockCloud(mockCtl)
+				mockCloud.EXPECT().CreateSnapshot(gomock.Eq(ctx), gomock.Eq(req.SourceVolumeId), gomock.Eq(snapshotOptions)).Return(mockSnapshot, nil)
+				mockCloud.EXPECT().GetSnapshotByName(gomock.Eq(ctx), gomock.Eq(req.GetName())).Return(nil, cloud.ErrNotFound)
+
+				awsDriver := controllerService{
+					cloud:         mockCloud,
+					inFlight:      internal.NewInFlight(),
+					driverOptions: &DriverOptions{kubernetesClusterID: clusterId},
+				}
+				resp, err := awsDriver.CreateSnapshot(context.Background(), req)
+				if err != nil {
+					t.Fatalf("Unexpected error: %v", err)
+				}
+
 				if snap := resp.GetSnapshot(); snap == nil {
 					t.Fatalf("Expected snapshot %v, got nil", expSnapshot)
 				}