disk: add tags to ECS snapshot

Automatically add 3 more tags to every ECS snapshot: - ack.aliyun.com: current ACK cluster ID - csi.alibabacloud.com/snapshot/name - csi.alibabacloud.com/snapshot/namespace Also allow users to add custom tags. User should specify "snapshotTags/tagKey: tagValue" in VolumeSnapshotClass to add tag "tagKey: tagValue". This design allows arbitrary keys and values. Compared with the design of disk tags, which does not allow "," in key or value, or ":" in key. If this design works fine, we can also implement this on disk tags.
kubernetes-sigs · Oct 26, 2023 · 620ee91 · 620ee91
1 parent e8ae906
commit 620ee91
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 49 deletions.
diff --git a/pkg/common/constants.go b/pkg/common/constants.go
@@ -13,6 +13,12 @@ const (
 	PVNameKey = "csi.storage.k8s.io/pv/name"
 )
 
+// constants of keys in volume snapshot parameters
+const (
+	VolumeSnapshotNamespaceKey = "csi.storage.k8s.io/volumesnapshot/namespace"
+	VolumeSnapshotNameKey      = "csi.storage.k8s.io/volumesnapshot/name"
+)
+
 const (
 	// PVCNameTag is tag applied to provisioned alibaba cloud disk for compatibility
 	// with in-tree volume plugin. Value of the tag is PVC name. It is applied only when
@@ -32,3 +38,9 @@ const (
 	// thus provides such metadata to the CSI driver.
 	PVNameTag = "kubernetes.io/created-for/pv/name"
 )
+
+// Tags that will be added to ECS snapshots
+const (
+	VolumeSnapshotNameTag      = "csi.alibabacloud.com/snapshot/name"
+	VolumeSnapshotNamespaceTag = "csi.alibabacloud.com/snapshot/namespace"
+)
diff --git a/pkg/disk/cloud.go b/pkg/disk/cloud.go
@@ -779,6 +779,7 @@ type createSnapshotParams struct {
 	RetentionDays              int
 	InstantAccessRetentionDays int
 	InstantAccess              bool
+	SnapshotTags               []ecs.CreateSnapshotTag
 }
 
 func requestAndCreateSnapshot(ecsClient *ecs.Client, params *createSnapshotParams) (*ecs.CreateSnapshotResponse, error) {
@@ -794,11 +795,14 @@ func requestAndCreateSnapshot(ecsClient *ecs.Client, params *createSnapshotParam
 	createSnapshotRequest.ResourceGroupId = params.ResourceGroupID
 
 	// Set tags
-	snapshotTags := []ecs.CreateSnapshotTag{}
-	tag1 := ecs.CreateSnapshotTag{Key: DISKTAGKEY2, Value: DISKTAGVALUE2}
-	snapshotTags = append(snapshotTags, tag1)
-	tag2 := ecs.CreateSnapshotTag{Key: SNAPSHOTTAGKEY1, Value: "true"}
-	snapshotTags = append(snapshotTags, tag2)
+	snapshotTags := []ecs.CreateSnapshotTag{
+		{Key: DISKTAGKEY2, Value: DISKTAGVALUE2},
+		{Key: SNAPSHOTTAGKEY1, Value: "true"},
+	}
+	if GlobalConfigVar.ClusterID != "" {
+		snapshotTags = append(snapshotTags, ecs.CreateSnapshotTag{Key: DISKTAGKEY3, Value: GlobalConfigVar.ClusterID})
+	}
+	snapshotTags = append(snapshotTags, params.SnapshotTags...)
 	createSnapshotRequest.Tag = &snapshotTags
 
 	// Do Snapshot create

diff --git a/pkg/disk/constants.go b/pkg/disk/constants.go
@@ -21,22 +21,21 @@ const (
 	DISKTAGKEY3 = "ack.aliyun.com"
 	// ECS snapshot tag from old version, keep it for compatibility
 	SNAPSHOTTAGKEY1 = "force.delete.snapshot.k8s.aliyun.com"
-	// SNAPSHOTTYPE ...
-	SNAPSHOTTYPE = "snapshotType"
-	// INSTANTACCESS ...
-	INSTANTACCESS = "InstantAccess"
-	// RETENTIONDAYS ...
-	RETENTIONDAYS = "retentionDays"
-	// INSTANTACCESSRETENTIONDAYS ...
+)
+
+// keys used in CreateSnapshotRequest.Parameters
+const (
+	SNAPSHOTTYPE               = "snapshotType"
+	INSTANTACCESS              = "InstantAccess"
+	RETENTIONDAYS              = "retentionDays"
 	INSTANTACCESSRETENTIONDAYS = "instantAccessRetentionDays"
-	// SNAPSHOTRESOURCEGROUPID ...
-	SNAPSHOTRESOURCEGROUPID = "resourceGroupId"
+	SNAPSHOTRESOURCEGROUPID    = "resourceGroupId"
+	SNAPSHOT_TAG_PREFIX        = "snapshotTags/"
+)
+
+const (
 	// DiskSnapshotID means snapshot id
 	DiskSnapshotID = "csi.alibabacloud.com/disk-snapshot-id"
-	// VolumeSnapshotNamespace namespace
-	VolumeSnapshotNamespace = "csi.storage.k8s.io/volumesnapshot/namespace"
-	// VolumeSnapshotName tag
-	VolumeSnapshotName = "csi.storage.k8s.io/volumesnapshot/name"
 	// IAVolumeSnapshotKey tag
 	IAVolumeSnapshotKey = "csi.alibabacloud.com/snapshot-ia"
 	// SnapshotRequestTag interval limit

diff --git a/pkg/disk/controllerserver.go b/pkg/disk/controllerserver.go
@@ -37,6 +37,7 @@ import (
 	csicommon "github.com/kubernetes-csi/drivers/pkg/csi-common"
 	volumeSnasphotV1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
 	snapClientset "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned"
+	"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/common"
 	"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/disk/crds"
 	log "github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/log"
 	"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/utils"
@@ -444,8 +445,8 @@ func getVolumeSnapshotConfig(req *csi.CreateSnapshotRequest) (*createSnapshotPar
 		}
 	}
 
-	vsName := req.Parameters[VolumeSnapshotName]
-	vsNameSpace := req.Parameters[VolumeSnapshotNamespace]
+	vsName := req.Parameters[common.VolumeSnapshotNameKey]
+	vsNameSpace := req.Parameters[common.VolumeSnapshotNamespaceKey]
 	// volumesnapshot not in parameters, just retrun
 	if vsName == "" || vsNameSpace == "" {
 		return nil, nil
@@ -483,6 +484,23 @@ func parseSnapshotParameters(params map[string]string, ecsParams *createSnapshot
 			}
 		case SNAPSHOTRESOURCEGROUPID:
 			ecsParams.ResourceGroupID = v
+		case common.VolumeSnapshotNameKey:
+			ecsParams.SnapshotTags = append(ecsParams.SnapshotTags, ecs.CreateSnapshotTag{
+				Key:   common.VolumeSnapshotNameTag,
+				Value: v,
+			})
+		case common.VolumeSnapshotNamespaceKey:
+			ecsParams.SnapshotTags = append(ecsParams.SnapshotTags, ecs.CreateSnapshotTag{
+				Key:   common.VolumeSnapshotNamespaceTag,
+				Value: v,
+			})
+		default:
+			if strings.HasPrefix(k, SNAPSHOT_TAG_PREFIX) {
+				ecsParams.SnapshotTags = append(ecsParams.SnapshotTags, ecs.CreateSnapshotTag{
+					Key:   k[len(SNAPSHOT_TAG_PREFIX):],
+					Value: v,
+				})
+			}
 		}
 	}
 	return nil
@@ -529,8 +547,8 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
 	SnapshotRequestMap[req.Name] = cur
 
 	// used for snapshot events
-	snapshotName := req.Parameters[VolumeSnapshotName]
-	snapshotNamespace := req.Parameters[VolumeSnapshotNamespace]
+	snapshotName := req.Parameters[common.VolumeSnapshotNameKey]
+	snapshotNamespace := req.Parameters[common.VolumeSnapshotNamespaceKey]
 
 	ref := &v1.ObjectReference{
 		Kind:      "VolumeSnapshot",
@@ -703,33 +721,6 @@ func snapshotBeforeDelete(volumeID string, ecsClient *ecs.Client) error {
 	return createStaticSnap(volumeID, resp.SnapshotId, GlobalConfigVar.SnapClient)
 }
 
-func updateSnapshotIAStatus(req *csi.CreateSnapshotRequest, status string) error {
-	volumeSnapshotName := req.Parameters[VolumeSnapshotName]
-	volumeSnapshotNameSpace := req.Parameters[VolumeSnapshotNamespace]
-	if volumeSnapshotName == "" || volumeSnapshotNameSpace == "" {
-		log.Log.Infof("CreateSnapshot: cannot get volumesnapshot name and namespace: %s, %s, %s", volumeSnapshotName, volumeSnapshotNameSpace, req.Name)
-		return nil
-	}
-
-	volumeSnapshot, err := GlobalConfigVar.SnapClient.SnapshotV1().VolumeSnapshots(volumeSnapshotNameSpace).Get(context.Background(), volumeSnapshotName, metav1.GetOptions{})
-	if err != nil {
-		log.Log.Warnf("CreateSnapshot: get volumeSnapshot(%s/%s) labels error: %s", volumeSnapshotNameSpace, volumeSnapshotName, err.Error())
-		return err
-	}
-	if volumeSnapshot.Labels == nil {
-		volumeSnapshot.Labels = map[string]string{}
-	}
-	volumeSnapshot.Labels[IAVolumeSnapshotKey] = status
-
-	_, err = GlobalConfigVar.SnapClient.SnapshotV1().VolumeSnapshots(volumeSnapshotNameSpace).Update(context.Background(), volumeSnapshot, metav1.UpdateOptions{})
-	if err != nil {
-		log.Log.Warnf("CreateSnapshot: Update VolumeSnapshot(%s/%s) IA Status error: %s", volumeSnapshotNameSpace, volumeSnapshotName, err.Error())
-		return err
-	}
-	log.Log.Infof("CreateSnapshot: updateSnapshot(%s/%s) IA Status successful %s", volumeSnapshotNameSpace, volumeSnapshotName, req.Name)
-	return nil
-}
-
 // DeleteSnapshot ...
 func (cs *controllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequest) (*csi.DeleteSnapshotResponse, error) {