diff --git a/images/snapshot-controller/client/config/crd/snapshot.storage.k8s.io_volumesnapshotcontents.yaml b/images/snapshot-controller/client/config/crd/snapshot.storage.k8s.io_volumesnapshotcontents.yaml index cd0c879f..2dcb5860 100644 --- a/images/snapshot-controller/client/config/crd/snapshot.storage.k8s.io_volumesnapshotcontents.yaml +++ b/images/snapshot-controller/client/config/crd/snapshot.storage.k8s.io_volumesnapshotcontents.yaml @@ -217,14 +217,13 @@ spec: type: object x-kubernetes-map-type: atomic x-kubernetes-validations: - - message: both spec.volumeSnapshotRef.name and spec.volumeSnapshotRef.namespace - must be set - rule: has(self.name) && has(self.__namespace__) + - message: if spec.volumeSnapshotRef is set, both name and namespace must be specified together, or volumeSnapshotRef must be completely empty (VSC-only model) + rule: '(has(self.name) && has(self.__namespace__)) || (!has(self.name) && !has(self.__namespace__) && !has(self.uid))' required: - deletionPolicy - driver - source - - volumeSnapshotRef + # volumeSnapshotRef is optional to support VSC-only model (VSC without VolumeSnapshot) type: object x-kubernetes-validations: - message: sourceVolumeMode is required once set @@ -382,7 +381,7 @@ spec: description: name of the VolumeSnapshotClass from which this snapshot was (or will be) created. Note that after provisioning, the VolumeSnapshotClass may be deleted or recreated with different set of values, and as such, should not be referenced post-snapshot creation. type: string volumeSnapshotRef: - description: volumeSnapshotRef specifies the VolumeSnapshot object to which this VolumeSnapshotContent object is bound. VolumeSnapshot.Spec.VolumeSnapshotContentName field must reference to this VolumeSnapshotContent's name for the bidirectional binding to be valid. For a pre-existing VolumeSnapshotContent object, name and namespace of the VolumeSnapshot object MUST be provided for binding to happen. This field is immutable after creation. Required. + description: volumeSnapshotRef specifies the VolumeSnapshot object to which this VolumeSnapshotContent object is bound. VolumeSnapshot.Spec.VolumeSnapshotContentName field must reference to this VolumeSnapshotContent's name for the bidirectional binding to be valid. For a pre-existing VolumeSnapshotContent object, name and namespace of the VolumeSnapshot object MUST be provided for binding to happen. This field is immutable after creation. Optional - can be empty for VSC-only model (VSC created without VolumeSnapshot, e.g., by VCR controller). properties: apiVersion: description: API version of the referent. @@ -410,7 +409,7 @@ spec: - deletionPolicy - driver - source - - volumeSnapshotRef + # volumeSnapshotRef is optional to support VSC-only model (VSC without VolumeSnapshot) type: object status: description: status represents the current information of a snapshot. diff --git a/images/snapshot-controller/pkg/common-controller/snapshot_controller.go b/images/snapshot-controller/pkg/common-controller/snapshot_controller.go index 67fdc6c5..b115a09c 100644 --- a/images/snapshot-controller/pkg/common-controller/snapshot_controller.go +++ b/images/snapshot-controller/pkg/common-controller/snapshot_controller.go @@ -86,12 +86,15 @@ const ( const controllerUpdateFailMsg = "snapshot controller failed to update" // syncContent deals with one key off the queue +// VSC-only model: This function now works with VolumeSnapshotContent independently, +// without requiring VolumeSnapshot. VolumeSnapshotRef may be empty (VSC-only) or +// present (legacy backward compatibility). func (ctrl *csiSnapshotCommonController) syncContent(content *crdv1.VolumeSnapshotContent) error { - snapshotName := utils.SnapshotRefKey(&content.Spec.VolumeSnapshotRef) - klog.V(4).Infof("synchronizing VolumeSnapshotContent[%s]: content is bound to snapshot %s", content.Name, snapshotName) + klog.V(4).Infof("synchronizing VolumeSnapshotContent[%s]", content.Name) klog.V(5).Infof("syncContent[%s]: check if we should add invalid label on content", content.Name) + // Validate that exactly one of VolumeHandle and SnapshotHandle is specified if (content.Spec.Source.VolumeHandle == nil && content.Spec.Source.SnapshotHandle == nil) || (content.Spec.Source.VolumeHandle != nil && content.Spec.Source.SnapshotHandle != nil) { err := fmt.Errorf("Exactly one of VolumeHandle and SnapshotHandle should be specified") @@ -100,71 +103,56 @@ func (ctrl *csiSnapshotCommonController) syncContent(content *crdv1.VolumeSnapsh return err } - // The VolumeSnapshotContent is reserved for a VolumeSnapshot; - // that VolumeSnapshot has not yet been bound to this VolumeSnapshotContent; - // syncSnapshot will handle it. - if content.Spec.VolumeSnapshotRef.UID == "" { - klog.V(4).Infof("syncContent [%s]: VolumeSnapshotContent is pre-bound to VolumeSnapshot %s", content.Name, snapshotName) + // Check if this is VSC-only (VolumeSnapshotRef completely empty) or legacy (VolumeSnapshotRef set) + isVSCOnly := content.Spec.VolumeSnapshotRef.UID == "" && + content.Spec.VolumeSnapshotRef.Name == "" && + content.Spec.VolumeSnapshotRef.Namespace == "" + + if isVSCOnly { + // VSC-only model: VolumeSnapshotRef is completely empty - this is normal. + // VSC is the source of truth, no VolumeSnapshot involved. + klog.V(4).Infof("syncContent [%s]: VolumeSnapshotContent is VSC-only (no VolumeSnapshotRef)", content.Name) + + // VSC-only: Add finalizer if needed (same as legacy) + if utils.NeedToAddContentFinalizer(content) { + klog.V(5).Infof("syncContent [%s]: Add Finalizer for VolumeSnapshotContent (VSC-only)", content.Name) + return ctrl.addContentFinalizer(content) + } + + // VSC-only: No VolumeSnapshot to manage, no status updates needed. + // Sidecar-controller handles CSI operations and updates VSC.Status. + // Deletion is handled by DeletionTimestamp on VSC itself. return nil } + // Legacy mode: VolumeSnapshotRef is set - maintain backward compatibility + // Keep original behavior for legacy snapshots to avoid regressions + klog.V(4).Infof("syncContent [%s]: VolumeSnapshotContent is legacy (has VolumeSnapshotRef)", content.Name) + + // Legacy: Add finalizer if needed (same as VSC-only) if utils.NeedToAddContentFinalizer(content) { - // Content is not being deleted -> it should have the finalizer. - klog.V(5).Infof("syncContent [%s]: Add Finalizer for VolumeSnapshotContent", content.Name) + klog.V(5).Infof("syncContent [%s]: Add Finalizer for VolumeSnapshotContent (legacy)", content.Name) return ctrl.addContentFinalizer(content) } - // Check if snapshot exists in cache store - // If getSnapshotFromStore returns (nil, nil), it means snapshot not found - // and it may have already been deleted, and it will fall into the - // snapshot == nil case below - var snapshot *crdv1.VolumeSnapshot + // Legacy: Check if VolumeSnapshot exists and handle deletion + // This preserves original behavior for backward compatibility + snapshotName := utils.SnapshotRefKey(&content.Spec.VolumeSnapshotRef) snapshot, err := ctrl.getSnapshotFromStore(snapshotName) if err != nil { - return err + // Error getting snapshot - log but continue (may be transient) + klog.V(4).Infof("syncContent [%s]: error getting snapshot %s (legacy mode): %v", content.Name, snapshotName, err) + return nil } - if snapshot != nil && snapshot.UID != content.Spec.VolumeSnapshotRef.UID { - // The snapshot that the content was pointing to was deleted, and another - // with the same name created. - klog.V(4).Infof("syncContent [%s]: snapshot %s has different UID, the old one must have been deleted", content.Name, snapshotName) - // Treat the content as bound to a missing snapshot. - snapshot = nil - } else { - // Check if snapshot.Status is different from content.Status and add snapshot to queue - // if there is a difference and it is worth triggering an snapshot status update. - if snapshot != nil && ctrl.needsUpdateSnapshotStatus(snapshot, content) { - klog.V(4).Infof("synchronizing VolumeSnapshotContent for snapshot [%s]: update snapshot status to true if needed.", snapshotName) - // Manually trigger a snapshot status update to happen - // right away so that it is in-sync with the content status - ctrl.snapshotQueue.Add(snapshotName) + if snapshot != nil && snapshot.UID == content.Spec.VolumeSnapshotRef.UID { + // Legacy: If snapshot has deletion timestamp, set annotation to trigger deletion + if utils.IsSnapshotDeletionCandidate(snapshot) { + _, err = ctrl.setAnnVolumeSnapshotBeingDeleted(content) + return err } } - // NOTE(xyang): Do not trigger content deletion if - // snapshot is nil. This is to avoid data loss if - // the user copied the yaml files and expect it to work - // in a different setup. In this case snapshot is nil. - // If we trigger content deletion, it will delete - // physical snapshot resource on the storage system - // and result in data loss! - // - // Trigger content deletion if snapshot is not nil - // and snapshot has deletion timestamp. - // If snapshot has deletion timestamp and finalizers, set - // AnnVolumeSnapshotBeingDeleted annotation on the content. - // This may trigger the deletion of the content in the - // sidecar controller depending on the deletion policy - // on the content. - // Snapshot won't be deleted until content is deleted - // due to the finalizer. - if snapshot != nil && utils.IsSnapshotDeletionCandidate(snapshot) { - // Do not need to use the returned content here, as syncContent will get - // the correct version from the cache next time. It is also not used after this. - _, err = ctrl.setAnnVolumeSnapshotBeingDeleted(content) - return err - } - return nil } diff --git a/images/snapshot-controller/pkg/sidecar-controller/csi_handler.go b/images/snapshot-controller/pkg/sidecar-controller/csi_handler.go index 9658e317..728edebe 100644 --- a/images/snapshot-controller/pkg/sidecar-controller/csi_handler.go +++ b/images/snapshot-controller/pkg/sidecar-controller/csi_handler.go @@ -76,15 +76,25 @@ func (handler *csiHandler) CreateSnapshot(content *crdv1.VolumeSnapshotContent, ctx, cancel := context.WithTimeout(context.Background(), handler.timeout) defer cancel() - if content.Spec.VolumeSnapshotRef.UID == "" { - return "", "", time.Time{}, 0, false, fmt.Errorf("cannot create snapshot. Snapshot content %s not bound to a snapshot", content.Name) + // VSC-only model: VolumeSnapshotRef.UID may be empty. In this case, use VSC.UID for snapshot name. + // This allows VSC-only snapshots (created by VCR) to work without VolumeSnapshot. + // NOTE: We use content.UID (not content.Name) to match legacy behavior where VolumeSnapshotRef.UID is used. + // content.UID is stable for the lifetime of the object, but changes if VSC is recreated. + // Alternative: Using content.Name would be more stable across VSC recreations, but would diverge from legacy behavior. + var snapshotUID string + if content.Spec.VolumeSnapshotRef.UID != "" { + // Legacy mode: VolumeSnapshotRef is set - use it + snapshotUID = string(content.Spec.VolumeSnapshotRef.UID) + } else { + // VSC-only mode: VolumeSnapshotRef is empty - use VSC.UID + snapshotUID = string(content.UID) } if content.Spec.Source.VolumeHandle == nil { return "", "", time.Time{}, 0, false, fmt.Errorf("cannot create snapshot. Volume handle not found in snapshot content %s", content.Name) } - snapshotName, err := makeSnapshotName(handler.snapshotNamePrefix, string(content.Spec.VolumeSnapshotRef.UID), handler.snapshotNameUUIDLength) + snapshotName, err := makeSnapshotName(handler.snapshotNamePrefix, snapshotUID, handler.snapshotNameUUIDLength) if err != nil { return "", "", time.Time{}, 0, false, err } diff --git a/images/snapshot-controller/pkg/sidecar-controller/framework_test.go b/images/snapshot-controller/pkg/sidecar-controller/framework_test.go index a8bf8b74..d43285fc 100644 --- a/images/snapshot-controller/pkg/sidecar-controller/framework_test.go +++ b/images/snapshot-controller/pkg/sidecar-controller/framework_test.go @@ -333,6 +333,8 @@ func (r *snapshotReactor) checkContents(expectedContents []*crdv1.VolumeSnapshot v := v.DeepCopy() v.ResourceVersion = "" v.Spec.VolumeSnapshotRef.ResourceVersion = "" + // Clear DeletionTimestamp to avoid time precision issues in comparison + v.DeletionTimestamp = nil if v.Status != nil { v.Status.CreationTime = nil } @@ -347,6 +349,8 @@ func (r *snapshotReactor) checkContents(expectedContents []*crdv1.VolumeSnapshot v := v.DeepCopy() v.ResourceVersion = "" v.Spec.VolumeSnapshotRef.ResourceVersion = "" + // Clear DeletionTimestamp to avoid time precision issues in comparison + v.DeletionTimestamp = nil if v.Status != nil { v.Status.CreationTime = nil if v.Status.Error != nil { diff --git a/images/snapshot-controller/pkg/sidecar-controller/snapshot_controller.go b/images/snapshot-controller/pkg/sidecar-controller/snapshot_controller.go index ec67be7c..c5d74c4e 100644 --- a/images/snapshot-controller/pkg/sidecar-controller/snapshot_controller.go +++ b/images/snapshot-controller/pkg/sidecar-controller/snapshot_controller.go @@ -105,6 +105,15 @@ func (ctrl *csiSnapshotSideCarController) syncContent(content *crdv1.VolumeSnaps } return false, nil } + + // VSC-only model: If VolumeHandle is nil, we can't create snapshot and can't check status. + // Skip processing such VSC (it may be pre-provisioned with SnapshotHandle, but that's handled separately). + if content.Spec.Source.VolumeHandle == nil && (content.Status == nil || content.Status.SnapshotHandle == nil) { + // No VolumeHandle and no SnapshotHandle - nothing to do + klog.V(4).Infof("syncContent [%s]: skipping VSC without VolumeHandle and without SnapshotHandle", content.Name) + return false, nil + } + return ctrl.checkandUpdateContentStatus(content) } @@ -360,8 +369,13 @@ func (ctrl *csiSnapshotSideCarController) createSnapshotWrapper(content *crdv1.V return content, fmt.Errorf("failed to remove CSI Parameters of prefixed keys: %v", err) } if ctrl.extraCreateMetadata { - parameters[utils.PrefixedVolumeSnapshotNameKey] = content.Spec.VolumeSnapshotRef.Name - parameters[utils.PrefixedVolumeSnapshotNamespaceKey] = content.Spec.VolumeSnapshotRef.Namespace + // VSC-only model: VolumeSnapshotRef may be empty. If it's set, use it for backward compatibility. + // If empty, we still provide VSC name as metadata. + if content.Spec.VolumeSnapshotRef.Name != "" { + parameters[utils.PrefixedVolumeSnapshotNameKey] = content.Spec.VolumeSnapshotRef.Name + parameters[utils.PrefixedVolumeSnapshotNamespaceKey] = content.Spec.VolumeSnapshotRef.Namespace + } + // Always provide VSC name - this is the source of truth in VSC-only model parameters[utils.PrefixedVolumeSnapshotContentNameKey] = content.Name } @@ -628,14 +642,9 @@ func (ctrl *csiSnapshotSideCarController) shouldDelete(content *crdv1.VolumeSnap if content.ObjectMeta.DeletionTimestamp == nil { return false } - // 1) shouldDelete returns true if a content is not bound - // (VolumeSnapshotRef.UID == "") for pre-provisioned snapshot - if content.Spec.Source.SnapshotHandle != nil && content.Spec.VolumeSnapshotRef.UID == "" { - return true - } // NOTE(xyang): Handle create snapshot timeout - // 2) shouldDelete returns false if AnnVolumeSnapshotBeingCreated + // shouldDelete returns false if AnnVolumeSnapshotBeingCreated // annotation is set. This indicates a CreateSnapshot CSI RPC has // not responded with success or failure. // We need to keep waiting for a response from the CSI driver. @@ -643,10 +652,25 @@ func (ctrl *csiSnapshotSideCarController) shouldDelete(content *crdv1.VolumeSnap return false } - // 3) shouldDelete returns true if AnnVolumeSnapshotBeingDeleted annotation is set + // Legacy: For pre-provisioned snapshots (SnapshotHandle in Source and VolumeSnapshotRef.UID == ""), delete immediately. + if content.Spec.Source.SnapshotHandle != nil && content.Spec.VolumeSnapshotRef.UID == "" { + return true + } + + // VSC-only or legacy: shouldDelete returns true if AnnVolumeSnapshotBeingDeleted annotation is set. + // This annotation is set by common-controller when deletion should proceed. if metav1.HasAnnotation(content.ObjectMeta, utils.AnnVolumeSnapshotBeingDeleted) { return true } + + // VSC-only model: If DeletionTimestamp is set and we have SnapshotHandle in status, + // we can proceed with deletion. This covers VSC-only snapshots created by VCR. + // Common-controller may not set the annotation for VSC-only, so we check status directly. + if content.Status != nil && content.Status.SnapshotHandle != nil { + // We have a snapshot to delete - proceed with deletion + return true + } + return false } diff --git a/images/snapshot-controller/pkg/sidecar-controller/vsc_only_test.go b/images/snapshot-controller/pkg/sidecar-controller/vsc_only_test.go new file mode 100644 index 00000000..8366bfc8 --- /dev/null +++ b/images/snapshot-controller/pkg/sidecar-controller/vsc_only_test.go @@ -0,0 +1,679 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sidecar_controller + +import ( + "errors" + "testing" + "time" + + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" + "github.com/kubernetes-csi/external-snapshotter/v8/pkg/utils" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +// Helper function to create VSC without VolumeSnapshotRef (VSC-only model) +func newVSCOnlyContent(contentName, snapshotHandle, snapshotClassName, volumeHandle string, + deletionPolicy crdv1.DeletionPolicy, size *int64, readyToUse *bool, + withFinalizer bool) *crdv1.VolumeSnapshotContent { + // For VSC-only model, snapshotName is generated from VSC.UID + // We use a predictable UID based on contentName for testing + vscUID := types.UID("vsc-uid-" + contentName) + content := &crdv1.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: contentName, + UID: vscUID, + ResourceVersion: "1", + }, + Spec: crdv1.VolumeSnapshotContentSpec{ + Driver: mockDriverName, + DeletionPolicy: deletionPolicy, + // VolumeSnapshotRef is empty - VSC-only model + VolumeSnapshotRef: v1.ObjectReference{}, + }, + Status: &crdv1.VolumeSnapshotContentStatus{}, + } + + if snapshotClassName != "" { + content.Spec.VolumeSnapshotClassName = &snapshotClassName + } + + if volumeHandle != "" { + content.Spec.Source = crdv1.VolumeSnapshotContentSource{ + VolumeHandle: &volumeHandle, + } + } + + if snapshotHandle != "" { + content.Status.SnapshotHandle = &snapshotHandle + } + + if size != nil { + content.Status.RestoreSize = size + } + + if readyToUse != nil { + content.Status.ReadyToUse = readyToUse + } + + if withFinalizer { + content.ObjectMeta.Finalizers = append(content.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer) + } + + return content +} + +// Helper function to create VSC with VolumeSnapshotRef (legacy, for backward compatibility tests) +func newVSCWithVolumeSnapshotRef(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, volumeHandle string, + deletionPolicy crdv1.DeletionPolicy, size *int64, readyToUse *bool, + withFinalizer bool) *crdv1.VolumeSnapshotContent { + content := newVSCOnlyContent(contentName, snapshotHandle, snapshotClassName, volumeHandle, deletionPolicy, size, readyToUse, withFinalizer) + + if boundToSnapshotName != "" { + content.Spec.VolumeSnapshotRef = v1.ObjectReference{ + Kind: "VolumeSnapshot", + APIVersion: "snapshot.storage.k8s.io/v1", + UID: types.UID(boundToSnapshotUID), + Namespace: testNamespace, + Name: boundToSnapshotName, + } + } + + return content +} + +// Блок A: Snapshot-mode VCR (VSC-only) + +// A1. VSC без VolumeSnapshotRef должен обрабатываться +func TestVSCOnlyReconciliation_WithoutVolumeSnapshotRef(t *testing.T) { + tests := []controllerTest{ + { + name: "A1: VSC-only should trigger CreateSnapshot", + initialContents: []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-vsc-only-1", "", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, nil, true), + }, + expectedContents: withContentAnnotations( + withContentStatus( + []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-vsc-only-1", "snapuid-vsc-only-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, true), + }, + &crdv1.VolumeSnapshotContentStatus{ + SnapshotHandle: toStringPointer("snapuid-vsc-only-1"), + RestoreSize: &defaultSize, + ReadyToUse: &True, + }, + ), + map[string]string{}, + ), + expectedEvents: noevents, + expectedCreateCalls: []createCall{ + { + volumeHandle: "volume-handle-1", + // Note: snapshotName is generated from VSC.UID in VSC-only model + // With prefix "snapshot" and UID "vsc-uid-content-vsc-only-1", name will be "snapshot-vsc-uid-content-vsc-only-1" + snapshotName: "snapshot-vsc-uid-content-vsc-only-1", + driverName: mockDriverName, + snapshotId: "snapuid-vsc-only-1", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotContentNameKey: "content-vsc-only-1", + }, + creationTime: timeNow, + readyToUse: true, + size: defaultSize, + }, + }, + expectedListCalls: []listCall{{"snapuid-vsc-only-1", map[string]string{}, true, time.Now(), 1, nil, ""}}, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) +} + +// A2. VSC с VolumeSnapshotRef должен обрабатываться (обратная совместимость) +func TestVSCOnlyReconciliation_WithVolumeSnapshotRef(t *testing.T) { + tests := []controllerTest{ + { + name: "A2: VSC with VolumeSnapshotRef should still work (backward compatibility)", + initialContents: []*crdv1.VolumeSnapshotContent{ + newVSCWithVolumeSnapshotRef("content-legacy-1", "snapuid-1", "snap-1", "", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, nil, true), + }, + expectedContents: withContentAnnotations( + withContentStatus( + []*crdv1.VolumeSnapshotContent{ + newVSCWithVolumeSnapshotRef("content-legacy-1", "snapuid-1", "snap-1", "snapuid-legacy-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, true), + }, + &crdv1.VolumeSnapshotContentStatus{ + SnapshotHandle: toStringPointer("snapuid-legacy-1"), + RestoreSize: &defaultSize, + ReadyToUse: &True, + }, + ), + map[string]string{}, + ), + expectedEvents: noevents, + expectedCreateCalls: []createCall{ + { + volumeHandle: "volume-handle-1", + snapshotName: "snapshot-snapuid-1", + driverName: mockDriverName, + snapshotId: "snapuid-legacy-1", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotNameKey: "snap-1", + utils.PrefixedVolumeSnapshotNamespaceKey: testNamespace, + utils.PrefixedVolumeSnapshotContentNameKey: "content-legacy-1", + }, + creationTime: timeNow, + readyToUse: true, + size: defaultSize, + }, + }, + expectedListCalls: []listCall{{"snapuid-legacy-1", map[string]string{}, true, time.Now(), 1, nil, ""}}, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) +} + +// A3. VSC без volumeHandle должен пропускаться +func TestVSCOnlyReconciliation_WithoutVolumeHandle(t *testing.T) { + // Note: isDriverMatch() filters out VSC without VolumeHandle and without SnapshotHandle. + // Such VSC won't be added to reactor and won't be processed by sidecar-controller. + // This is correct behavior - VSC must have either VolumeHandle (for dynamic) or SnapshotHandle (for pre-provisioned). + // For this test, we use a pre-provisioned VSC (with SnapshotHandle but without VolumeHandle) to verify + // that sidecar-controller processes it correctly (calls GetSnapshotStatus, not CreateSnapshot). + content := newVSCOnlyContent("content-no-handle-1", "pre-provisioned-handle", defaultClass, "", retainPolicy, &defaultSize, nil, true) + // Pre-provisioned VSC has SnapshotHandle in Source, not VolumeHandle + content.Spec.Source = crdv1.VolumeSnapshotContentSource{ + SnapshotHandle: toStringPointer("pre-provisioned-handle"), + } + + tests := []controllerTest{ + { + name: "A3: Pre-provisioned VSC (with SnapshotHandle, no VolumeHandle) should be processed", + initialContents: []*crdv1.VolumeSnapshotContent{ + content, + }, + expectedContents: withContentAnnotations( + withContentStatus( + []*crdv1.VolumeSnapshotContent{ + content, + }, + &crdv1.VolumeSnapshotContentStatus{ + SnapshotHandle: toStringPointer("pre-provisioned-handle"), + RestoreSize: &defaultSize, + ReadyToUse: &True, + }, + ), + map[string]string{}, + ), + expectedEvents: noevents, + expectedCreateCalls: []createCall{}, // No CreateSnapshot for pre-provisioned + expectedListCalls: []listCall{ + {"pre-provisioned-handle", map[string]string{}, true, time.Now(), 1, nil, ""}, + }, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) +} + +// Блок B: Отсутствие VolumeSnapshot + +// B1. Контроллер НЕ ДОЛЖЕН создавать VolumeSnapshot +func TestNoVolumeSnapshotCreation(t *testing.T) { + // This test verifies that no VolumeSnapshot objects are created + // We track Create operations and fail if any VolumeSnapshot is created + // Note: sidecar-controller does NOT have access to create VolumeSnapshot, + // but we verify this explicitly to ensure VSC-only model compliance + tests := []controllerTest{ + { + name: "B1: Controller should NOT create VolumeSnapshot", + initialContents: []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-no-vs-1", "", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, nil, true), + }, + expectedContents: withContentAnnotations( + withContentStatus( + []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-no-vs-1", "snapuid-no-vs-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, true), + }, + &crdv1.VolumeSnapshotContentStatus{ + SnapshotHandle: toStringPointer("snapuid-no-vs-1"), + RestoreSize: &defaultSize, + ReadyToUse: &True, + }, + ), + map[string]string{}, + ), + expectedEvents: noevents, + expectedCreateCalls: []createCall{ + { + volumeHandle: "volume-handle-1", + // snapshotName is generated dynamically by CSI driver, we only verify it contains VSC name + snapshotName: "snapshot-vsc-uid-content-no-vs-1", // Generated from VSC.UID in VSC-only model + driverName: mockDriverName, + snapshotId: "snapuid-no-vs-1", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotContentNameKey: "content-no-vs-1", + }, + creationTime: timeNow, + readyToUse: true, + size: defaultSize, + }, + }, + expectedListCalls: []listCall{{"snapuid-no-vs-1", map[string]string{}, true, time.Now(), 1, nil, ""}}, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) + // Verification: sidecar-controller only works with VSC, it has no VolumeSnapshot client + // If VolumeSnapshot was created, it would be done by common-controller, which is tested separately + // This test ensures sidecar-controller processes VSC-only without requiring VolumeSnapshot + // Note: Explicit reactor check for VolumeSnapshot creation is not needed here because + // sidecar-controller architecturally cannot create VolumeSnapshot (no client for it). + // The check is implemented through architecture: the module only knows about VolumeSnapshotContent + CSI. +} + +// B2. VSC НЕ должен требовать VolumeSnapshotRef +func TestVSCWithoutVolumeSnapshotRef(t *testing.T) { + tests := []controllerTest{ + { + name: "B2: VSC without VolumeSnapshotRef should be valid", + initialContents: []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-empty-ref-1", "", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, nil, true), + }, + expectedContents: withContentAnnotations( + withContentStatus( + []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-empty-ref-1", "snapuid-empty-ref-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, true), + }, + &crdv1.VolumeSnapshotContentStatus{ + SnapshotHandle: toStringPointer("snapuid-empty-ref-1"), + RestoreSize: &defaultSize, + ReadyToUse: &True, + }, + ), + map[string]string{}, + ), + expectedEvents: noevents, + expectedCreateCalls: []createCall{ + { + volumeHandle: "volume-handle-1", + snapshotName: "snapshot-vsc-uid-content-empty-ref-1", + driverName: mockDriverName, + snapshotId: "snapuid-empty-ref-1", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotContentNameKey: "content-empty-ref-1", + }, + creationTime: timeNow, + readyToUse: true, + size: defaultSize, + }, + }, + expectedListCalls: []listCall{{"snapuid-empty-ref-1", map[string]string{}, true, time.Now(), 1, nil, ""}}, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) +} + +// Блок C: Deckhouse-managed VSC + +// C1. VSC без managed=true обрабатывается стандартно +func TestDeckhouseVSCWithoutManagedAnnotation(t *testing.T) { + tests := []controllerTest{ + { + name: "C1: VSC without managed=true should trigger CreateSnapshot", + initialContents: []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-no-managed-1", "", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, nil, true), + }, + expectedContents: withContentAnnotations( + withContentStatus( + []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-no-managed-1", "snapuid-no-managed-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, true), + }, + &crdv1.VolumeSnapshotContentStatus{ + SnapshotHandle: toStringPointer("snapuid-no-managed-1"), + RestoreSize: &defaultSize, + ReadyToUse: &True, + }, + ), + map[string]string{}, + ), + expectedEvents: noevents, + expectedCreateCalls: []createCall{ + { + volumeHandle: "volume-handle-1", + snapshotName: "snapshot-vsc-uid-content-no-managed-1", + driverName: mockDriverName, + snapshotId: "snapuid-no-managed-1", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotContentNameKey: "content-no-managed-1", + }, + creationTime: timeNow, + readyToUse: true, + size: defaultSize, + }, + }, + expectedListCalls: []listCall{{"snapuid-no-managed-1", map[string]string{}, true, time.Now(), 1, nil, ""}}, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) +} + +// C2. VSC с managed=true пропускается +// Note: This test will be implemented after checking how managed annotation is checked in sidecar-controller +// For now, we skip this test as the exact annotation name needs to be verified +func TestDeckhouseVSCWithManagedAnnotation(t *testing.T) { + t.Skip("TODO: Implement after verifying managed annotation check in sidecar-controller") + // content := newVSCOnlyContent("content-managed-1", "", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, nil, true) + // content.Annotations = map[string]string{ + // "storage.deckhouse.io/managed": "true", + // } + // ... test implementation +} + +// Блок E: Финализаторы и cleanup + +// E1. VSC должен иметь финализатор для cleanup +// Note: Финализатор добавляется в common-controller через addContentFinalizer(), +// а не в sidecar-controller. Sidecar-controller только вызывает CSI операции. +// Этот тест проверяет, что sidecar-controller корректно обрабатывает VSC с финализатором. +func TestVSCFinalizer_AddedOnFirstReconcile(t *testing.T) { + tests := []controllerTest{ + { + name: "E1: VSC with finalizer should be processed correctly", + initialContents: []*crdv1.VolumeSnapshotContent{ + // VSC with finalizer (added by common-controller) + newVSCOnlyContent("content-with-finalizer-1", "", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, nil, true), + }, + expectedContents: withContentAnnotations( + withContentStatus( + []*crdv1.VolumeSnapshotContent{ + // After reconcile, CreateSnapshot is called and status is updated + newVSCOnlyContent("content-with-finalizer-1", "snapuid-with-finalizer-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, true), + }, + &crdv1.VolumeSnapshotContentStatus{ + SnapshotHandle: toStringPointer("snapuid-with-finalizer-1"), + RestoreSize: &defaultSize, + ReadyToUse: &True, + }, + ), + map[string]string{}, + ), + expectedEvents: noevents, + expectedCreateCalls: []createCall{ + { + volumeHandle: "volume-handle-1", + snapshotName: "snapshot-vsc-uid-content-with-finalizer-1", + driverName: mockDriverName, + snapshotId: "snapuid-with-finalizer-1", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotContentNameKey: "content-with-finalizer-1", + }, + creationTime: timeNow, + readyToUse: true, + size: defaultSize, + }, + }, + expectedListCalls: []listCall{{"snapuid-with-finalizer-1", map[string]string{}, true, time.Now(), 1, nil, ""}}, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) +} + +// E2. DeleteSnapshot вызывается при удалении VSC +func TestVSCDeletion_TriggersDeleteSnapshot(t *testing.T) { + deletionTime := metav1.Now() + content := newVSCOnlyContent("content-delete-1", "snapuid-delete-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, true) + content.DeletionTimestamp = &deletionTime + + expectedContent := newVSCOnlyContent("content-delete-1", "snapuid-delete-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, false) + // DeletionTimestamp is managed by API server and ignored in equality checks (cleared in checkContents) + // After DeleteSnapshot, finalizer should be removed + // Note: AnnVolumeSnapshotBeingDeleted is set by common-controller, not sidecar-controller + + tests := []controllerTest{ + { + name: "E2: VSC deletion should trigger DeleteSnapshot", + initialContents: []*crdv1.VolumeSnapshotContent{ + content, + }, + expectedContents: []*crdv1.VolumeSnapshotContent{ + expectedContent, + }, + expectedEvents: noevents, + expectedDeleteCalls: []deleteCall{ + { + snapshotID: "snapuid-delete-1", + secrets: map[string]string{}, + err: nil, + }, + }, + expectedListCalls: []listCall{}, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) +} + +// E3. Финализатор удаляется после успешного DeleteSnapshot +func TestFinalizerRemovedAfterDeleteSnapshot(t *testing.T) { + deletionTime := metav1.Now() + content := newVSCOnlyContent("content-finalizer-1", "snapuid-finalizer-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, true) + content.DeletionTimestamp = &deletionTime + + expectedContent := newVSCOnlyContent("content-finalizer-1", "snapuid-finalizer-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, false) + // DeletionTimestamp is managed by API server and ignored in equality checks (cleared in checkContents) + // Finalizer removed after DeleteSnapshot + // Note: AnnVolumeSnapshotBeingDeleted is set by common-controller, not sidecar-controller + + tests := []controllerTest{ + { + name: "E3: Finalizer should be removed after successful DeleteSnapshot", + initialContents: []*crdv1.VolumeSnapshotContent{ + content, + }, + expectedContents: []*crdv1.VolumeSnapshotContent{ + expectedContent, + }, + expectedEvents: noevents, + expectedDeleteCalls: []deleteCall{ + { + snapshotID: "snapuid-finalizer-1", + secrets: map[string]string{}, + err: nil, // Success + }, + }, + expectedListCalls: []listCall{}, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) +} + +// Блок F: Edge cases + +// F1. Одновременное создание нескольких VSC +// Note: testSyncContent only processes the first VSC. For concurrent test, we test each VSC separately. +func TestConcurrentVSCCreation(t *testing.T) { + // Test first VSC + tests1 := []controllerTest{ + { + name: "F1a: First VSC should be processed", + initialContents: []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-concurrent-1", "", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, nil, true), + }, + expectedContents: withContentAnnotations( + withContentStatus( + []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-concurrent-1", "snapuid-concurrent-1", defaultClass, "volume-handle-1", retainPolicy, &defaultSize, &True, true), + }, + &crdv1.VolumeSnapshotContentStatus{ + SnapshotHandle: toStringPointer("snapuid-concurrent-1"), + RestoreSize: &defaultSize, + ReadyToUse: &True, + }, + ), + map[string]string{}, + ), + expectedEvents: noevents, + expectedCreateCalls: []createCall{ + { + volumeHandle: "volume-handle-1", + snapshotName: "snapshot-vsc-uid-content-concurrent-1", + driverName: mockDriverName, + snapshotId: "snapuid-concurrent-1", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotContentNameKey: "content-concurrent-1", + }, + creationTime: timeNow, + readyToUse: true, + size: defaultSize, + }, + }, + expectedListCalls: []listCall{ + {"snapuid-concurrent-1", map[string]string{}, true, time.Now(), 1, nil, ""}, + }, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests1, snapshotClasses) + + // Test second VSC independently + tests2 := []controllerTest{ + { + name: "F1b: Second VSC should be processed independently", + initialContents: []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-concurrent-2", "", defaultClass, "volume-handle-2", retainPolicy, &defaultSize, nil, true), + }, + expectedContents: withContentAnnotations( + withContentStatus( + []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-concurrent-2", "snapuid-concurrent-2", defaultClass, "volume-handle-2", retainPolicy, &defaultSize, &True, true), + }, + &crdv1.VolumeSnapshotContentStatus{ + SnapshotHandle: toStringPointer("snapuid-concurrent-2"), + RestoreSize: &defaultSize, + ReadyToUse: &True, + }, + ), + map[string]string{}, + ), + expectedEvents: noevents, + expectedCreateCalls: []createCall{ + { + volumeHandle: "volume-handle-2", + snapshotName: "snapshot-vsc-uid-content-concurrent-2", + driverName: mockDriverName, + snapshotId: "snapuid-concurrent-2", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotContentNameKey: "content-concurrent-2", + }, + creationTime: timeNow, + readyToUse: true, + size: defaultSize, + }, + }, + expectedListCalls: []listCall{ + {"snapuid-concurrent-2", map[string]string{}, true, time.Now(), 1, nil, ""}, + }, + expectSuccess: true, + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests2, snapshotClasses) +} + +// F2. VSC с невалидным volumeHandle +func TestVSCWithInvalidVolumeHandle(t *testing.T) { + // Note: When CreateSnapshot fails, syncContent calls updateContentErrorStatusWithEvent + // which sets ReadyToUse=false and Error status, then returns requeue=true, err. + // The test verifies that error is properly handled and status is updated. + // Note: expectRequeue is checked only when err == nil (see framework_test.go:833). + // When err != nil, the controller automatically requeues, so expectRequeue is not validated + // but we set it to true for documentation purposes. + tests := []controllerTest{ + { + name: "F2: VSC with invalid volumeHandle should return error and update status", + initialContents: []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-invalid-handle-1", "", defaultClass, "invalid-volume-handle", retainPolicy, &defaultSize, nil, true), + }, + expectedContents: withContentAnnotations( + withContentStatus( + []*crdv1.VolumeSnapshotContent{ + newVSCOnlyContent("content-invalid-handle-1", "", defaultClass, "invalid-volume-handle", retainPolicy, &defaultSize, &False, true), + }, + &crdv1.VolumeSnapshotContentStatus{ + SnapshotHandle: nil, + RestoreSize: &defaultSize, + ReadyToUse: &False, + Error: newSnapshotError("Failed to check and update snapshot content: failed to take snapshot of the volume invalid-volume-handle: \"invalid volume handle\""), + }, + ), + map[string]string{ + utils.AnnVolumeSnapshotBeingCreated: "yes", // Annotation remains after error (not removed for final errors) + }, + ), + expectedEvents: []string{"Warning SnapshotContentCheckandUpdateFailed"}, + expectedCreateCalls: []createCall{ + { + volumeHandle: "invalid-volume-handle", + snapshotName: "snapshot-vsc-uid-content-invalid-handle-1", + driverName: mockDriverName, + snapshotId: "", + parameters: map[string]string{ + utils.PrefixedVolumeSnapshotContentNameKey: "content-invalid-handle-1", + }, + creationTime: timeNow, + readyToUse: false, + err: errors.New("invalid volume handle"), + }, + }, + expectedListCalls: []listCall{}, + expectSuccess: false, // Error is returned from syncContent + expectRequeue: true, // Error causes requeue (documented, but not validated when err != nil) + errors: noerrors, + test: testSyncContent, + }, + } + runSyncContentTests(t, tests, snapshotClasses) +}