diff --git a/client/hack/update-crd.sh b/client/hack/update-crd.sh index 2012837d1..1ad39e98f 100755 --- a/client/hack/update-crd.sh +++ b/client/hack/update-crd.sh @@ -28,7 +28,7 @@ then TMP_DIR=$(mktemp -d); cd $TMP_DIR; go mod init tmp; - go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.11.3; + go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.12.0; rm -rf $TMP_DIR; CONTROLLER_GEN=$(which controller-gen) fi diff --git a/cmd/csi-snapshotter/main.go b/cmd/csi-snapshotter/main.go index 7fc975b41..a53a0378e 100644 --- a/cmd/csi-snapshotter/main.go +++ b/cmd/csi-snapshotter/main.go @@ -79,14 +79,13 @@ var ( kubeAPIQPS = flag.Float64("kube-api-qps", 5, "QPS to use while communicating with the kubernetes apiserver. Defaults to 5.0.") kubeAPIBurst = flag.Int("kube-api-burst", 10, "Burst to use while communicating with the kubernetes apiserver. Defaults to 10.") - metricsAddress = flag.String("metrics-address", "", "(deprecated) The TCP network address where the prometheus metrics endpoint will listen (example: `:8080`). The default is empty string, which means metrics endpoint is disabled. Only one of `--metrics-address` and `--http-endpoint` can be set.") - httpEndpoint = flag.String("http-endpoint", "", "The TCP network address where the HTTP server for diagnostics, including metrics and leader election health check, will listen (example: `:8080`). The default is empty string, which means the server is disabled. Only one of `--metrics-address` and `--http-endpoint` can be set.") - metricsPath = flag.String("metrics-path", "/metrics", "The HTTP path where prometheus metrics will be exposed. Default is `/metrics`.") - retryIntervalStart = flag.Duration("retry-interval-start", time.Second, "Initial retry interval of failed volume snapshot creation or deletion. It doubles with each failure, up to retry-interval-max. Default is 1 second.") - retryIntervalMax = flag.Duration("retry-interval-max", 5*time.Minute, "Maximum retry interval of failed volume snapshot creation or deletion. Default is 5 minutes.") - enableNodeDeployment = flag.Bool("node-deployment", false, "Enables deploying the sidecar controller together with a CSI driver on nodes to manage snapshots for node-local volumes.") - // TODO(xing-yang): Enable enableVolumeGroupSnapshots when the feature is fully implemented - // enableVolumeGroupSnapshots = flag.Bool("enable-volume-group-snapshots", false, "Enables the volume group snapshot feature, allowing the user to create snapshots of groups of volumes.") + metricsAddress = flag.String("metrics-address", "", "(deprecated) The TCP network address where the prometheus metrics endpoint will listen (example: `:8080`). The default is empty string, which means metrics endpoint is disabled. Only one of `--metrics-address` and `--http-endpoint` can be set.") + httpEndpoint = flag.String("http-endpoint", "", "The TCP network address where the HTTP server for diagnostics, including metrics and leader election health check, will listen (example: `:8080`). The default is empty string, which means the server is disabled. Only one of `--metrics-address` and `--http-endpoint` can be set.") + metricsPath = flag.String("metrics-path", "/metrics", "The HTTP path where prometheus metrics will be exposed. Default is `/metrics`.") + retryIntervalStart = flag.Duration("retry-interval-start", time.Second, "Initial retry interval of failed volume snapshot creation or deletion. It doubles with each failure, up to retry-interval-max. Default is 1 second.") + retryIntervalMax = flag.Duration("retry-interval-max", 5*time.Minute, "Maximum retry interval of failed volume snapshot creation or deletion. Default is 5 minutes.") + enableNodeDeployment = flag.Bool("node-deployment", false, "Enables deploying the sidecar controller together with a CSI driver on nodes to manage snapshots for node-local volumes.") + enableVolumeGroupSnapshots = flag.Bool("enable-volume-group-snapshots", false, "Enables the volume group snapshot feature, allowing the user to create a snapshot of a group of volumes.") groupSnapshotNamePrefix = flag.String("groupsnapshot-name-prefix", "groupsnapshot", "Prefix to apply to the name of a created group snapshot") groupSnapshotNameUUIDLength = flag.Int("groupsnapshot-name-uuid-length", -1, "Length in characters for the generated uuid of a created group snapshot. Defaults behavior is to NOT truncate.") @@ -228,9 +227,6 @@ func main() { snapShotter := snapshotter.NewSnapshotter(csiConn) var groupSnapshotter group_snapshotter.GroupSnapshotter - // TODO(xing-yang): Remove the following line when the enableVolumeGroupSnapshots feature is enabled - bEnable := false - enableVolumeGroupSnapshots := &bEnable if *enableVolumeGroupSnapshots { supportsCreateVolumeGroupSnapshot, err := supportsGroupControllerCreateVolumeGroupSnapshot(ctx, csiConn) if err != nil { diff --git a/cmd/snapshot-controller/main.go b/cmd/snapshot-controller/main.go index f62fab7cc..e31b19b87 100644 --- a/cmd/snapshot-controller/main.go +++ b/cmd/snapshot-controller/main.go @@ -72,8 +72,7 @@ var ( retryIntervalMax = flag.Duration("retry-interval-max", 5*time.Minute, "Maximum retry interval of failed volume snapshot creation or deletion. Default is 5 minutes.") enableDistributedSnapshotting = flag.Bool("enable-distributed-snapshotting", false, "Enables each node to handle snapshotting for the local volumes created on that node") preventVolumeModeConversion = flag.Bool("prevent-volume-mode-conversion", false, "Prevents an unauthorised user from modifying the volume mode when creating a PVC from an existing VolumeSnapshot.") - // TODO(xing-yang): Enable enableVolumeGroupSnapshots when the feature is fully implemented - // enableVolumeGroupSnapshots = flag.Bool("enable-volume-group-snapshots", false, "Enables the volume group snapshot feature, allowing the user to create snapshots of groups of volumes.") + enableVolumeGroupSnapshots = flag.Bool("enable-volume-group-snapshots", false, "Enables the volume group snapshot feature, allowing the user to create a snapshot of a group of volumes.") retryCRDIntervalMax = flag.Duration("retry-crd-interval-max", 5*time.Second, "Maximum retry interval to wait for CRDs to appear. The default is 5 seconds.") ) @@ -208,9 +207,6 @@ func main() { klog.V(2).Infof("Start NewCSISnapshotController with kubeconfig [%s] resyncPeriod [%+v]", *kubeconfig, *resyncPeriod) - // TODO(xing-yang): Remove the following lines when the enableVolumeGroupSnapshots feature is enabled - bEnable := false - enableVolumeGroupSnapshots := &bEnable ctrl := controller.NewCSISnapshotCommonController( snapClient, kubeClient, diff --git a/pkg/common-controller/groupsnapshot_controller_helper.go b/pkg/common-controller/groupsnapshot_controller_helper.go index bc0b21c3c..543638eb3 100644 --- a/pkg/common-controller/groupsnapshot_controller_helper.go +++ b/pkg/common-controller/groupsnapshot_controller_helper.go @@ -1103,16 +1103,7 @@ func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimesta groupSnapshotContent = nil } - klog.V(5).Infof("processGroupSnapshotWithDeletionTimestamp[%s]: delete group snapshot content and remove finalizer from group snapshot if needed", utils.GroupSnapshotKey(groupSnapshot)) - - return ctrl.checkandRemoveGroupSnapshotFinalizersAndCheckandDeleteGroupSnapshotContent(groupSnapshot, groupSnapshotContent, deleteGroupSnapshotContent) -} - -// checkandRemoveGroupSnapshotFinalizersAndCheckandDeleteGroupSnapshotContent deletes -// the group snapshot content and removes group snapshot finalizers if needed -func (ctrl *csiSnapshotCommonController) checkandRemoveGroupSnapshotFinalizersAndCheckandDeleteGroupSnapshotContent(groupSnapshot *crdv1alpha1.VolumeGroupSnapshot, groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent, deleteGroupSnapshotContent bool) error { - klog.V(5).Infof("checkandRemoveGroupSnapshotFinalizersAndCheckandDeleteGroupSnapshotContent VolumeGroupSnapshot[%s]: %s", utils.GroupSnapshotKey(groupSnapshot), utils.GetGroupSnapshotStatusForLogging(groupSnapshot)) - + klog.V(5).Infof("processGroupSnapshotWithDeletionTimestamp[%s]: check if group snapshot is a candidate for deletion", utils.GroupSnapshotKey(groupSnapshot)) if !utils.IsGroupSnapshotDeletionCandidate(groupSnapshot) { return nil } @@ -1135,14 +1126,15 @@ func (ctrl *csiSnapshotCommonController) checkandRemoveGroupSnapshotFinalizersAn } - // regardless of the deletion policy, set the VolumeSnapshotBeingDeleted on - // content object, this is to allow snapshotter sidecar controller to conduct - // a delete operation whenever the content has deletion timestamp set. + // regardless of the deletion policy, set VolumeGroupSnapshotBeingDeleted on + // group snapshot content object, this is to allow snapshotter sidecar controller + // to conduct a delete operation whenever the group snapshot content has deletion + // timestamp set. if groupSnapshotContent != nil { - klog.V(5).Infof("checkandRemoveGroupSnapshotFinalizersAndCheckandDeleteGroupSnapshotContent[%s]: Set VolumeGroupSnapshotBeingDeleted annotation on the group snapshot content [%s]", utils.GroupSnapshotKey(groupSnapshot), groupSnapshotContent.Name) + klog.V(5).Infof("processGroupSnapshotWithDeletionTimestamp[%s]: Set VolumeGroupSnapshotBeingDeleted annotation on the group snapshot content [%s]", utils.GroupSnapshotKey(groupSnapshot), groupSnapshotContent.Name) updatedGroupSnapshotContent, err := ctrl.setAnnVolumeGroupSnapshotBeingDeleted(groupSnapshotContent) if err != nil { - klog.V(4).Infof("checkandRemoveGroupSnapshotFinalizersAndCheckandDeleteGroupSnapshotContent[%s]: failed to set VolumeGroupSnapshotBeingDeleted annotation on the group snapshot content [%s]", utils.GroupSnapshotKey(groupSnapshot), groupSnapshotContent.Name) + klog.V(4).Infof("processGroupSnapshotWithDeletionTimestamp[%s]: failed to set VolumeGroupSnapshotBeingDeleted annotation on the group snapshot content [%s]: %v", utils.GroupSnapshotKey(groupSnapshot), groupSnapshotContent.Name, err) return err } groupSnapshotContent = updatedGroupSnapshotContent @@ -1150,10 +1142,10 @@ func (ctrl *csiSnapshotCommonController) checkandRemoveGroupSnapshotFinalizersAn // VolumeGroupSnapshot should be deleted. Check and remove finalizers // If group snapshot content exists and has a deletion policy of Delete, set - // DeletionTimeStamp on the content; + // DeletionTimeStamp on the group snapshot content; // VolumeGroupSnapshotContent won't be deleted immediately due to the VolumeGroupSnapshotContentFinalizer if groupSnapshotContent != nil && deleteGroupSnapshotContent { - klog.V(5).Infof("checkandRemoveGroupSnapshotFinalizersAndCheckandDeleteGroupSnapshotContent: set DeletionTimeStamp on group snapshot content [%s].", groupSnapshotContent.Name) + klog.V(5).Infof("processGroupSnapshotWithDeletionTimestamp[%s]: set DeletionTimeStamp on group snapshot content [%s].", utils.GroupSnapshotKey(groupSnapshot), groupSnapshotContent.Name) err := ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshotContents().Delete(context.TODO(), groupSnapshotContent.Name, metav1.DeleteOptions{}) if err != nil { ctrl.eventRecorder.Event(groupSnapshot, v1.EventTypeWarning, "GroupSnapshotContentObjectDeleteError", "Failed to delete group snapshot content API object") @@ -1161,7 +1153,20 @@ func (ctrl *csiSnapshotCommonController) checkandRemoveGroupSnapshotFinalizersAn } } - klog.V(5).Infof("checkandRemoveGroupSnapshotFinalizersAndCheckandDeleteGroupSnapshotContent: Remove Finalizer for VolumeGroupSnapshot[%s]", utils.GroupSnapshotKey(groupSnapshot)) + klog.V(5).Infof("processGroupSnapshotWithDeletionTimestamp[%s]: Delete individual snapshots that are part of the group snapshot", utils.GroupSnapshotKey(groupSnapshot)) + + // Delete the individual snapshots part of the group snapshot + for _, snapshot := range groupSnapshot.Status.VolumeSnapshotRefList { + err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Delete(context.TODO(), snapshot.Name, metav1.DeleteOptions{}) + if err != nil { + msg := fmt.Sprintf("failed to delete snapshot API object %s/%s part of group snapshot %s: %v", snapshot.Namespace, snapshot.Name, utils.GroupSnapshotKey(groupSnapshot), err) + klog.Error(msg) + ctrl.eventRecorder.Event(groupSnapshot, v1.EventTypeWarning, "SnapshotDeleteError", msg) + return fmt.Errorf(msg) + } + } + + klog.V(5).Infof("processGroupSnapshotWithDeletionTimestamp[%s] : Remove Finalizer for VolumeGroupSnapshot", utils.GroupSnapshotKey(groupSnapshot)) // remove VolumeSnapshotBoundFinalizer on the VolumeGroupSnapshot object: // a. If there is no group snapshot content found, remove the finalizer. // b. If the group snapshot content is being deleted, i.e., with deleteGroupSnapshotContent == true, diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index 2b1d051e9..d509fb296 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/pkg/common-controller/snapshot_controller.go @@ -286,6 +286,21 @@ func (ctrl *csiSnapshotCommonController) processSnapshotWithDeletionTimestamp(sn content = nil } + // Block deletion if this snapshot belongs to a group snapshot. + if snapshot.Status != nil && snapshot.Status.VolumeGroupSnapshotName != nil { + groupSnapshot, err := ctrl.groupSnapshotLister.VolumeGroupSnapshots(snapshot.Namespace).Get(*snapshot.Status.VolumeGroupSnapshotName) + if err == nil { + msg := fmt.Sprintf("deletion of the individual volume snapshot %s is not allowed as it belongs to group snapshot %s. Deleting the group snapshot will trigger the deletion of all the individual volume snapshots that are part of the group.", utils.SnapshotKey(snapshot), utils.GroupSnapshotKey(groupSnapshot)) + klog.Error(msg) + ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotDeleteError", msg) + return fmt.Errorf(msg) + } + if !apierrs.IsNotFound(err) { + klog.Errorf("failed to delete snapshot %s: %v", utils.SnapshotKey(snapshot), err) + return err + } + } + klog.V(5).Infof("processSnapshotWithDeletionTimestamp[%s]: delete snapshot content and remove finalizer from snapshot if needed", utils.SnapshotKey(snapshot)) return ctrl.checkandRemoveSnapshotFinalizersAndCheckandDeleteContent(snapshot, content, deleteContent) @@ -1139,6 +1154,27 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotStatus(snapshot *crdv1.Vo volumeSnapshotErr = content.Status.Error.DeepCopy() } + var groupSnapshotName string + if content.Status != nil && content.Status.VolumeGroupSnapshotHandle != nil { + // If this snapshot belongs to a group snapshot, find the group snapshot + // name from the group snapshot content + groupSnapshotContentList, err := ctrl.groupSnapshotContentLister.List(labels.Everything()) + if err != nil { + return nil, err + } + found := false + for _, groupSnapshotContent := range groupSnapshotContentList { + if groupSnapshotContent.Status != nil && groupSnapshotContent.Status.VolumeGroupSnapshotHandle != nil && *groupSnapshotContent.Status.VolumeGroupSnapshotHandle == *content.Status.VolumeGroupSnapshotHandle { + groupSnapshotName = groupSnapshotContent.Spec.VolumeGroupSnapshotRef.Name + found = true + break + } + } + if !found { + return nil, fmt.Errorf("updateSnapshotStatus: cannot find the group snapshot for VolumeSnapshot [%s], will not update snapshot status", utils.SnapshotKey(snapshot)) + } + } + klog.V(5).Infof("updateSnapshotStatus: updating VolumeSnapshot [%+v] based on VolumeSnapshotContentStatus [%+v]", snapshot, content.Status) snapshotObj, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Get(context.TODO(), snapshot.Name, metav1.GetOptions{}) @@ -1162,6 +1198,9 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotStatus(snapshot *crdv1.Vo if volumeSnapshotErr != nil { newStatus.Error = volumeSnapshotErr } + if groupSnapshotName != "" { + newStatus.VolumeGroupSnapshotName = &groupSnapshotName + } updated = true } else { newStatus = snapshotObj.Status.DeepCopy() @@ -1188,6 +1227,10 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotStatus(snapshot *crdv1.Vo newStatus.Error = volumeSnapshotErr updated = true } + if newStatus.VolumeGroupSnapshotName == nil && groupSnapshotName != "" { + newStatus.VolumeGroupSnapshotName = &groupSnapshotName + updated = true + } } if updated { diff --git a/pkg/sidecar-controller/content_create_test.go b/pkg/sidecar-controller/content_create_test.go index a58ac80df..c96c5df83 100644 --- a/pkg/sidecar-controller/content_create_test.go +++ b/pkg/sidecar-controller/content_create_test.go @@ -49,7 +49,7 @@ func TestSyncContent(t *testing.T) { readyToUse: true, }, }, - expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil}}, + expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil, ""}}, expectSuccess: true, errors: noerrors, test: testSyncContent, @@ -78,7 +78,7 @@ func TestSyncContent(t *testing.T) { size: defaultSize, }, }, - expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil}}, + expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil, ""}}, expectSuccess: true, errors: noerrors, test: testSyncContent, @@ -194,7 +194,7 @@ func TestSyncContent(t *testing.T) { readyToUse: true, }, }, - expectedListCalls: []listCall{{"sid1-6", map[string]string{}, true, time.Now(), 1, nil}}, + expectedListCalls: []listCall{{"sid1-6", map[string]string{}, true, time.Now(), 1, nil, ""}}, errors: noerrors, test: testSyncContent, }, diff --git a/pkg/sidecar-controller/csi_handler.go b/pkg/sidecar-controller/csi_handler.go index 711e7bac3..c0f89a771 100644 --- a/pkg/sidecar-controller/csi_handler.go +++ b/pkg/sidecar-controller/csi_handler.go @@ -34,7 +34,7 @@ import ( type Handler interface { CreateSnapshot(content *crdv1.VolumeSnapshotContent, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error) DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error - GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error) + GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, string, error) CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, volumeIDs []string, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, []*csi.Snapshot, time.Time, bool, error) GetGroupSnapshotStatus(content *crdv1alpha1.VolumeGroupSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, error) DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, SnapshotID []string, snapshotterCredentials map[string]string) error @@ -113,7 +113,7 @@ func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent, return nil } -func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error) { +func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, string, error) { ctx, cancel := context.WithTimeout(context.Background(), handler.timeout) defer cancel() @@ -124,15 +124,15 @@ func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotConten } else if content.Spec.Source.SnapshotHandle != nil { snapshotHandle = *content.Spec.Source.SnapshotHandle } else { - return false, time.Time{}, 0, fmt.Errorf("failed to list snapshot for content %s: snapshotHandle is missing", content.Name) + return false, time.Time{}, 0, "", fmt.Errorf("failed to list snapshot for content %s: snapshotHandle is missing", content.Name) } - csiSnapshotStatus, timestamp, size, err := handler.snapshotter.GetSnapshotStatus(ctx, snapshotHandle, snapshotterListCredentials) + csiSnapshotStatus, timestamp, size, groupSnapshotID, err := handler.snapshotter.GetSnapshotStatus(ctx, snapshotHandle, snapshotterListCredentials) if err != nil { - return false, time.Time{}, 0, fmt.Errorf("failed to list snapshot for content %s: %q", content.Name, err) + return false, time.Time{}, 0, "", fmt.Errorf("failed to list snapshot for content %s: %q", content.Name, err) } - return csiSnapshotStatus, timestamp, size, nil + return csiSnapshotStatus, timestamp, size, groupSnapshotID, nil } func makeSnapshotName(prefix, snapshotUID string, snapshotNameUUIDLength int) (string, error) { diff --git a/pkg/sidecar-controller/framework_test.go b/pkg/sidecar-controller/framework_test.go index 849a524bf..e69e38628 100644 --- a/pkg/sidecar-controller/framework_test.go +++ b/pkg/sidecar-controller/framework_test.go @@ -882,10 +882,11 @@ type listCall struct { snapshotID string secrets map[string]string // information to return - readyToUse bool - createTime time.Time - size int64 - err error + readyToUse bool + createTime time.Time + size int64 + err error + groupSnapshotID string } type deleteCall struct { @@ -982,10 +983,10 @@ func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string, return call.err } -func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error) { +func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, string, error) { if f.listCallCounter >= len(f.listCalls) { f.t.Errorf("Unexpected CSI list Snapshot call: snapshotID=%s, index: %d, calls: %+v", snapshotID, f.createCallCounter, f.createCalls) - return false, time.Time{}, 0, fmt.Errorf("unexpected call") + return false, time.Time{}, 0, "", fmt.Errorf("unexpected call") } call := f.listCalls[f.listCallCounter] f.listCallCounter++ @@ -1002,10 +1003,10 @@ func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID stri } if err != nil { - return false, time.Time{}, 0, fmt.Errorf("unexpected call") + return false, time.Time{}, 0, "", fmt.Errorf("unexpected call") } - return call.readyToUse, call.createTime, call.size, call.err + return call.readyToUse, call.createTime, call.size, call.groupSnapshotID, call.err } func newSnapshotError(message string) *crdv1.VolumeSnapshotError { diff --git a/pkg/sidecar-controller/groupsnapshot_helper.go b/pkg/sidecar-controller/groupsnapshot_helper.go index 0be2880ee..f4a0cc4e8 100644 --- a/pkg/sidecar-controller/groupsnapshot_helper.go +++ b/pkg/sidecar-controller/groupsnapshot_helper.go @@ -471,7 +471,6 @@ func (ctrl *csiSnapshotSideCarController) createGroupSnapshotWrapper(groupSnapsh }, }, } - vsc, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Create(context.TODO(), volumeSnapshotContent, metav1.CreateOptions{}) if err != nil { return groupSnapshotContent, err diff --git a/pkg/sidecar-controller/snapshot_controller.go b/pkg/sidecar-controller/snapshot_controller.go index 02a93da05..799ff678c 100644 --- a/pkg/sidecar-controller/snapshot_controller.go +++ b/pkg/sidecar-controller/snapshot_controller.go @@ -22,13 +22,14 @@ import ( "strings" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" - "github.com/kubernetes-csi/external-snapshotter/v6/pkg/utils" codes "google.golang.org/grpc/codes" "google.golang.org/grpc/status" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" klog "k8s.io/klog/v2" + + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" + "github.com/kubernetes-csi/external-snapshotter/v6/pkg/utils" ) // Design: @@ -251,7 +252,7 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c var size int64 readyToUse := false var driverName string - var snapshotID string + var snapshotID, groupSnapshotID string var snapshotterListCredentials map[string]string if content.Spec.Source.SnapshotHandle != nil { @@ -278,7 +279,7 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c } } - readyToUse, creationTime, size, err = ctrl.handler.GetSnapshotStatus(content, snapshotterListCredentials) + readyToUse, creationTime, size, groupSnapshotID, err = ctrl.handler.GetSnapshotStatus(content, snapshotterListCredentials) if err != nil { klog.Errorf("checkandUpdateContentStatusOperation: failed to call get snapshot status to check whether snapshot is ready to use %q", err) return content, err @@ -286,13 +287,13 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c driverName = content.Spec.Driver snapshotID = *content.Spec.Source.SnapshotHandle - klog.V(5).Infof("checkandUpdateContentStatusOperation: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t", driverName, snapshotID, creationTime, size, readyToUse) + klog.V(5).Infof("checkandUpdateContentStatusOperation: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t, groupSnapshotID %s", driverName, snapshotID, creationTime, size, readyToUse, groupSnapshotID) if creationTime.IsZero() { creationTime = time.Now() } - updatedContent, err := ctrl.updateSnapshotContentStatus(content, snapshotID, readyToUse, creationTime.UnixNano(), size) + updatedContent, err := ctrl.updateSnapshotContentStatus(content, snapshotID, readyToUse, creationTime.UnixNano(), size, groupSnapshotID) if err != nil { return content, err } @@ -354,7 +355,7 @@ func (ctrl *csiSnapshotSideCarController) createSnapshotWrapper(content *crdv1.V creationTime = time.Now() } - newContent, err := ctrl.updateSnapshotContentStatus(content, snapshotID, readyToUse, creationTime.UnixNano(), size) + newContent, err := ctrl.updateSnapshotContentStatus(content, snapshotID, readyToUse, creationTime.UnixNano(), size, "") if err != nil { klog.Errorf("error updating status for volume snapshot content %s: %v.", content.Name, err) return content, fmt.Errorf("error updating status for volume snapshot content %s: %v", content.Name, err) @@ -429,8 +430,9 @@ func (ctrl *csiSnapshotSideCarController) updateSnapshotContentStatus( snapshotHandle string, readyToUse bool, createdAt int64, - size int64) (*crdv1.VolumeSnapshotContent, error) { - klog.V(5).Infof("updateSnapshotContentStatus: updating VolumeSnapshotContent [%s], snapshotHandle %s, readyToUse %v, createdAt %v, size %d", content.Name, snapshotHandle, readyToUse, createdAt, size) + size int64, + groupSnapshotID string) (*crdv1.VolumeSnapshotContent, error) { + klog.V(5).Infof("updateSnapshotContentStatus: updating VolumeSnapshotContent [%s], snapshotHandle %s, readyToUse %v, createdAt %v, size %d, groupSnapshotID %s", content.Name, snapshotHandle, readyToUse, createdAt, size, groupSnapshotID) contentObj, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Get(context.TODO(), content.Name, metav1.GetOptions{}) if err != nil { @@ -446,6 +448,9 @@ func (ctrl *csiSnapshotSideCarController) updateSnapshotContentStatus( CreationTime: &createdAt, RestoreSize: &size, } + if groupSnapshotID != "" { + newStatus.VolumeGroupSnapshotHandle = &groupSnapshotID + } updated = true } else { newStatus = contentObj.Status.DeepCopy() @@ -468,6 +473,10 @@ func (ctrl *csiSnapshotSideCarController) updateSnapshotContentStatus( newStatus.RestoreSize = &size updated = true } + if newStatus.VolumeGroupSnapshotHandle == nil && groupSnapshotID != "" { + newStatus.VolumeGroupSnapshotHandle = &groupSnapshotID + updated = true + } } if updated { diff --git a/pkg/sidecar-controller/snapshot_delete_test.go b/pkg/sidecar-controller/snapshot_delete_test.go index ecdf6af15..f8c0a69cf 100644 --- a/pkg/sidecar-controller/snapshot_delete_test.go +++ b/pkg/sidecar-controller/snapshot_delete_test.go @@ -171,7 +171,7 @@ func TestDeleteSync(t *testing.T) { readyToUse: true, }, }, - expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil}}, + expectedListCalls: []listCall{{"sid1-1", map[string]string{}, true, time.Now(), 1, nil, ""}}, expectedDeleteCalls: []deleteCall{{"sid1-1", nil, nil}}, expectSuccess: true, test: testSyncContent, @@ -194,7 +194,7 @@ func TestDeleteSync(t *testing.T) { readyToUse: true, }, }, - expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil}}, + expectedListCalls: []listCall{{"sid1-2", map[string]string{}, true, time.Now(), 1, nil, ""}}, expectedDeleteCalls: []deleteCall{{"sid1-2", nil, nil}}, expectSuccess: true, test: testSyncContent, @@ -218,7 +218,7 @@ func TestDeleteSync(t *testing.T) { }, expectedDeleteCalls: []deleteCall{{"sid1-3", nil, fmt.Errorf("mock csi driver delete error")}}, expectedEvents: []string{"Warning SnapshotDeleteError"}, - expectedListCalls: []listCall{{"sid1-3", map[string]string{}, true, time.Now(), 1, nil}}, + expectedListCalls: []listCall{{"sid1-3", map[string]string{}, true, time.Now(), 1, nil, ""}}, test: testSyncContent, }, { @@ -238,7 +238,7 @@ func TestDeleteSync(t *testing.T) { name: "1-5 - csi driver delete snapshot returns error, bound finalizer should remain", initialContents: newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "snap1-5-volumehandle", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1), expectedContents: newContentArrayWithDeletionTimestamp("content1-5", "sid1-5", "snap1-5", "sid1-5", validSecretClass, "", "snap1-5-volumehandle", deletionPolicy, nil, &defaultSize, true, &timeNowMetav1), - expectedListCalls: []listCall{{"sid1-5", map[string]string{}, true, time.Now(), 1000, nil}}, + expectedListCalls: []listCall{{"sid1-5", map[string]string{}, true, time.Now(), 1000, nil, ""}}, expectedDeleteCalls: []deleteCall{{"sid1-5", nil, errors.New("mock csi driver delete error")}}, expectedEvents: []string{"Warning SnapshotDeleteError"}, errors: noerrors, @@ -249,7 +249,7 @@ func TestDeleteSync(t *testing.T) { name: "1-6 - content is deleted before deleting", initialContents: newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", classGold, "sid1-6", "", deletionPolicy, nil, nil, true), expectedContents: nocontents, - expectedListCalls: []listCall{{"sid1-6", nil, false, time.Now(), 0, nil}}, + expectedListCalls: []listCall{{"sid1-6", nil, false, time.Now(), 0, nil, ""}}, expectedDeleteCalls: []deleteCall{{"sid1-6", map[string]string{"foo": "bar"}, nil}}, expectedEvents: noevents, errors: noerrors, @@ -265,7 +265,7 @@ func TestDeleteSync(t *testing.T) { initialContents: newContentArrayWithReadyToUse("content1-7", "", "snap1-7", "sid1-7", validSecretClass, "sid1-7", "", deletePolicy, nil, &defaultSize, &True, true), expectedContents: newContentArrayWithReadyToUse("content1-7", "", "snap1-7", "sid1-7", validSecretClass, "sid1-7", "", deletePolicy, nil, &defaultSize, &True, true), expectedEvents: noevents, - expectedListCalls: []listCall{{"sid1-7", map[string]string{}, true, time.Now(), 1000, nil}}, + expectedListCalls: []listCall{{"sid1-7", map[string]string{}, true, time.Now(), 1000, nil, ""}}, expectSuccess: true, initialSecrets: []*v1.Secret{secret()}, errors: noerrors, @@ -276,7 +276,7 @@ func TestDeleteSync(t *testing.T) { initialContents: newContentArrayWithReadyToUse("content1-8", "sid1-8", "none-existed-snapshot", "sid1-8", validSecretClass, "sid1-8", "", retainPolicy, nil, &defaultSize, &True, true), expectedContents: newContentArrayWithReadyToUse("content1-8", "sid1-8", "none-existed-snapshot", "sid1-8", validSecretClass, "sid1-8", "", retainPolicy, nil, &defaultSize, &True, true), expectedEvents: noevents, - expectedListCalls: []listCall{{"sid1-8", map[string]string{}, true, time.Now(), 0, nil}}, + expectedListCalls: []listCall{{"sid1-8", map[string]string{}, true, time.Now(), 0, nil, ""}}, expectSuccess: true, errors: noerrors, test: testSyncContent, @@ -286,7 +286,7 @@ func TestDeleteSync(t *testing.T) { initialContents: newContentArrayWithDeletionTimestamp("content1-9", "sid1-9", "snap1-9", "sid1-9", emptySecretClass, "", "snap1-9-volumehandle", deletePolicy, nil, &defaultSize, true, &nonFractionalTime), expectedContents: newContentArrayWithDeletionTimestamp("content1-9", "sid1-9", "snap1-9", "", emptySecretClass, "", "snap1-9-volumehandle", deletePolicy, nil, &defaultSize, false, &nonFractionalTime), expectedEvents: noevents, - expectedListCalls: []listCall{{"sid1-9", map[string]string{}, true, time.Now(), 0, nil}}, + expectedListCalls: []listCall{{"sid1-9", map[string]string{}, true, time.Now(), 0, nil, ""}}, expectSuccess: true, errors: noerrors, initialSecrets: []*v1.Secret{}, // secret does not exist @@ -298,7 +298,7 @@ func TestDeleteSync(t *testing.T) { initialContents: newContentArrayWithDeletionTimestamp("content1-10", "sid1-10", "snap1-10", "sid1-10", emptySecretClass, "", "snap1-10-volumehandle", retainPolicy, nil, &defaultSize, true, &nonFractionalTime), expectedContents: newContentArrayWithDeletionTimestamp("content1-10", "sid1-10", "snap1-10", "sid1-10", emptySecretClass, "", "snap1-10-volumehandle", retainPolicy, nil, &defaultSize, false, &nonFractionalTime), expectedEvents: noevents, - expectedListCalls: []listCall{{"sid1-10", map[string]string{}, true, time.Now(), 0, nil}}, + expectedListCalls: []listCall{{"sid1-10", map[string]string{}, true, time.Now(), 0, nil, ""}}, expectSuccess: true, errors: noerrors, initialSecrets: []*v1.Secret{}, @@ -319,7 +319,7 @@ func TestDeleteSync(t *testing.T) { initialContents: newContentArrayWithDeletionTimestamp("content1-12", "sid1-12", "snap1-12", "sid1-12", emptySecretClass, "sid1-12", "", retainPolicy, nil, &defaultSize, true, &nonFractionalTime), expectedContents: newContentArrayWithDeletionTimestamp("content1-12", "sid1-12", "snap1-12", "sid1-12", emptySecretClass, "sid1-12", "", retainPolicy, nil, &defaultSize, false, &nonFractionalTime), expectedEvents: noevents, - expectedListCalls: []listCall{{"sid1-12", map[string]string{}, true, time.Now(), 0, nil}}, + expectedListCalls: []listCall{{"sid1-12", map[string]string{}, true, time.Now(), 0, nil, ""}}, expectSuccess: true, errors: noerrors, initialSecrets: []*v1.Secret{}, diff --git a/pkg/snapshotter/snapshotter.go b/pkg/snapshotter/snapshotter.go index 2a85b35a6..05f6fa660 100644 --- a/pkg/snapshotter/snapshotter.go +++ b/pkg/snapshotter/snapshotter.go @@ -38,7 +38,7 @@ type Snapshotter interface { DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) (err error) // GetSnapshotStatus returns if a snapshot is ready to use, creation time, and restore size. - GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error) + GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, string, error) } type snapshot struct { @@ -109,7 +109,7 @@ func (s *snapshot) isListSnapshotsSupported(ctx context.Context) (bool, error) { return false, nil } -func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error) { +func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, int64, string, error) { klog.V(5).Infof("GetSnapshotStatus: %s", snapshotID) client := csi.NewControllerClient(s.conn) @@ -117,10 +117,10 @@ func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string, sna // If the driver does not support ListSnapshots, assume the snapshot ID is valid. listSnapshotsSupported, err := s.isListSnapshotsSupported(ctx) if err != nil { - return false, time.Time{}, 0, fmt.Errorf("failed to check if ListSnapshots is supported: %s", err.Error()) + return false, time.Time{}, 0, "", fmt.Errorf("failed to check if ListSnapshots is supported: %s", err.Error()) } if !listSnapshotsSupported { - return true, time.Time{}, 0, nil + return true, time.Time{}, 0, "", nil } req := csi.ListSnapshotsRequest{ SnapshotId: snapshotID, @@ -128,13 +128,13 @@ func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string, sna } rsp, err := client.ListSnapshots(ctx, &req) if err != nil { - return false, time.Time{}, 0, err + return false, time.Time{}, 0, "", err } if rsp.Entries == nil || len(rsp.Entries) == 0 { - return false, time.Time{}, 0, fmt.Errorf("can not find snapshot for snapshotID %s", snapshotID) + return false, time.Time{}, 0, "", fmt.Errorf("can not find snapshot for snapshotID %s", snapshotID) } creationTime := rsp.Entries[0].Snapshot.CreationTime.AsTime() - return rsp.Entries[0].Snapshot.ReadyToUse, creationTime, rsp.Entries[0].Snapshot.SizeBytes, nil + return rsp.Entries[0].Snapshot.ReadyToUse, creationTime, rsp.Entries[0].Snapshot.SizeBytes, rsp.Entries[0].Snapshot.GroupSnapshotId, nil } diff --git a/pkg/snapshotter/snapshotter_test.go b/pkg/snapshotter/snapshotter_test.go index 54a7f97b6..6f225a287 100644 --- a/pkg/snapshotter/snapshotter_test.go +++ b/pkg/snapshotter/snapshotter_test.go @@ -468,7 +468,7 @@ func TestGetSnapshotStatus(t *testing.T) { } s := NewSnapshotter(csiConn) - ready, createTime, size, err := s.GetSnapshotStatus(context.Background(), test.snapshotID, test.snapshotterListCredentials) + ready, createTime, size, _, err := s.GetSnapshotStatus(context.Background(), test.snapshotID, test.snapshotterListCredentials) if test.expectError && err == nil { t.Errorf("test %q: Expected error, got none", test.name) }