diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go index 0eadcd9a2..9360c8537 100644 --- a/pkg/common-controller/framework_test.go +++ b/pkg/common-controller/framework_test.go @@ -683,7 +683,7 @@ func (r *snapshotReactor) modifyContentEvent(content *crdv1.VolumeSnapshotConten } } -// addSnapshotEvent simulates that a snapshot has been deleted in etcd and the +// addSnapshotEvent simulates that a snapshot has been created in etcd and the // controller receives 'snapshot added' event. func (r *snapshotReactor) addSnapshotEvent(snapshot *crdv1.VolumeSnapshot) { r.lock.Lock() @@ -1193,7 +1193,6 @@ func runSyncTests(t *testing.T, tests []controllerTest, snapshotClasses []*crdv1 snapshotscheme.AddToScheme(scheme.Scheme) for _, test := range tests { klog.V(4).Infof("starting test %q", test.name) - // Initialize the controller kubeClient := &kubefake.Clientset{} client := &fake.Clientset{} diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index eaaf1235e..89f44ecab 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/pkg/common-controller/snapshot_controller.go @@ -166,80 +166,23 @@ func (ctrl *csiSnapshotCommonController) syncSnapshot(snapshot *crdv1.VolumeSnap ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "ErrorPVCFinalizer", "Error check and remove PVC Finalizer for VolumeSnapshot") } - // Proceed with snapshot deletion only if snapshot is not in the middled of being - // created from a PVC with a finalizer. This is to ensure that the PVC finalizer - // can be removed even if a delete snapshot request is received before create - // snapshot has completed. - if snapshot.ObjectMeta.DeletionTimestamp != nil && !ctrl.isPVCwithFinalizerInUseByCurrentSnapshot(snapshot) { - err := ctrl.processSnapshotWithDeletionTimestamp(snapshot) - if err != nil { - return err - } - } else { - klog.V(5).Infof("syncSnapshot[%s]: check if we should add finalizers on snapshot", utils.SnapshotKey(snapshot)) - if err := ctrl.checkandAddSnapshotFinalizers(snapshot); err != nil { - klog.Errorf("error check and add Snapshot finalizers for snapshot [%s]: %v", snapshot.Name, errFinalizer) - ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotCheckandUpdateFailed", fmt.Sprintf("Failed to check and update snapshot: %s", err.Error())) - return err - } - // Need to build or update snapshot.Status in following cases: - // 1) snapshot.Status is nil - // 2) snapshot.Status.ReadyToUse is false - // 3) snapshot.Status.BoundVolumeSnapshotContentName is not set - if !utils.IsSnapshotReady(snapshot) || !utils.IsBoundVolumeSnapshotContentNameSet(snapshot) { - return ctrl.syncUnreadySnapshot(snapshot) - } - return ctrl.syncReadySnapshot(snapshot) - } - return nil -} - -// Check if PVC has a finalizer and if it is being used by the current snapshot as source. -func (ctrl *csiSnapshotCommonController) isPVCwithFinalizerInUseByCurrentSnapshot(snapshot *crdv1.VolumeSnapshot) bool { - // Get snapshot source which is a PVC - pvc, err := ctrl.getClaimFromVolumeSnapshot(snapshot) - if err != nil { - klog.Infof("cannot get claim from snapshot [%s]: [%v] Claim may be deleted already.", snapshot.Name, err) - return false - } - - // Check if there is a Finalizer on PVC. If not, return false - if !slice.ContainsString(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer, nil) { - return false - } - - if !utils.IsSnapshotReady(snapshot) { - klog.V(2).Infof("PVC %s/%s is being used by snapshot %s/%s as source", pvc.Namespace, pvc.Name, snapshot.Namespace, snapshot.Name) - return true - } - return false -} - -// checkContentAndBoundStatus is a helper function that checks the following: -// - It checks if content exists and returns the content object if it exists and nil otherwise. -// - It checks the deletionPolicy and returns true if policy is Delete and false otherwise. -// - It checks if snapshot and content are bound and returns true if bound and false otherwise. -func (ctrl *csiSnapshotCommonController) checkContentAndBoundStatus(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotContent, bool, bool, error) { - // If content is deleted already, remove SnapshotBound finalizer - content, err := ctrl.getContentFromStore(snapshot) - if err != nil || content == nil { - return nil, false, false, err + if snapshot.ObjectMeta.DeletionTimestamp != nil { + return ctrl.processSnapshotWithDeletionTimestamp(snapshot) } - deleteContent := false - // It is possible for getContentFromStore to return nil, nil - if content.Spec.DeletionPolicy == crdv1.VolumeSnapshotContentDelete { - klog.V(5).Infof("processFinalizersAndCheckandDeleteContent: Content [%s] deletion policy [%s] is delete.", content.Name, content.Spec.DeletionPolicy) - deleteContent = true + klog.V(5).Infof("syncSnapshot[%s]: check if we should add finalizers on snapshot", utils.SnapshotKey(snapshot)) + if err := ctrl.checkandAddSnapshotFinalizers(snapshot); err != nil { + klog.Errorf("error check and add Snapshot finalizers for snapshot [%s]: %v", snapshot.Name, errFinalizer) + ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotFinalizerError", fmt.Sprintf("Failed to check and update snapshot: %s", err.Error())) + return err } - - snapshotBound := false - // Check if the snapshot content is bound to the snapshot - if utils.IsSnapshotBound(snapshot, content) { - klog.Infof("syncSnapshot: VolumeSnapshot %s is bound to volumeSnapshotContent [%s]", snapshot.Name, content.Name) - snapshotBound = true - + // Need to build or update snapshot.Status in following cases: + // 1) snapshot.Status is nil + // 2) snapshot.Status.ReadyToUse is false + // 3) snapshot.Status.BoundVolumeSnapshotContentName is not set + if !utils.IsSnapshotReady(snapshot) || !utils.IsBoundVolumeSnapshotContentNameSet(snapshot) { + return ctrl.syncUnreadySnapshot(snapshot) } - return content, deleteContent, snapshotBound, nil + return ctrl.syncReadySnapshot(snapshot) } // processSnapshotWithDeletionTimestamp processes finalizers and deletes the content when appropriate. It has the following steps: @@ -248,84 +191,129 @@ func (ctrl *csiSnapshotCommonController) checkContentAndBoundStatus(snapshot *cr func (ctrl *csiSnapshotCommonController) processSnapshotWithDeletionTimestamp(snapshot *crdv1.VolumeSnapshot) error { klog.V(5).Infof("processSnapshotWithDeletionTimestamp VolumeSnapshot[%s]: %s", utils.SnapshotKey(snapshot), utils.GetSnapshotStatusForLogging(snapshot)) - // If content is really not found in the cache store, err is nil - content, deleteContent, _, err := ctrl.checkContentAndBoundStatus(snapshot) - if err != nil { - return err + var contentName string + if snapshot.Status != nil && snapshot.Status.BoundVolumeSnapshotContentName != nil { + contentName = *snapshot.Status.BoundVolumeSnapshotContentName } - - klog.V(5).Infof("processSnapshotWithDeletionTimestamp[%s]: delete snapshot content and remove finalizer from snapshot if needed", utils.SnapshotKey(snapshot)) - err = ctrl.checkandRemoveSnapshotFinalizersAndCheckandDeleteContent(snapshot, content, deleteContent) + // for a dynamically created snapshot, it's possible that a content has been created + // however the Status of the snapshot has not been updated yet, i.e., failed right + // after content creation. In this case, use the fixed naming scheme to get the content + // name and search + if contentName == "" && snapshot.Spec.Source.PersistentVolumeClaimName != nil { + contentName = utils.GetDynamicSnapshotContentNameForSnapshot(snapshot) + } + // find a content from cache store, note that it's complete legit that no + // content has been found from content cache store + content, err := ctrl.getContentFromStore(contentName) if err != nil { return err } + // check whether the content points back to the passed in snapshot, note that + // binding should always be bi-directional to trigger the deletion on content + // or adding any annotation to the content + var deleteContent bool = false + if content != nil && utils.IsVolumeSnapshotRefSet(snapshot, content) { + // content points back to snapshot, whether or not to delete a content now + // depends on the deletion policy of it. + deleteContent = (content.Spec.DeletionPolicy == crdv1.VolumeSnapshotContentDelete) + } else { + // the content is nil or points to a different snapshot, reset content to nil + // such that there is no operation done on the found content in + // checkandRemoveSnapshotFinalizersAndCheckandDeleteContent + content = nil + } - return nil + 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) } // checkandRemoveSnapshotFinalizersAndCheckandDeleteContent deletes the content and removes snapshot finalizers (VolumeSnapshotAsSourceFinalizer and VolumeSnapshotBoundFinalizer) if needed func (ctrl *csiSnapshotCommonController) checkandRemoveSnapshotFinalizersAndCheckandDeleteContent(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent, deleteContent bool) error { klog.V(5).Infof("checkandRemoveSnapshotFinalizersAndCheckandDeleteContent VolumeSnapshot[%s]: %s", utils.SnapshotKey(snapshot), utils.GetSnapshotStatusForLogging(snapshot)) - // Check is snapshot deletionTimestamp is set and any finalizer is on - if utils.IsSnapshotDeletionCandidate(snapshot) { - // Volume snapshot should be deleted. Check if it's used - // and remove finalizer if it's not. - // Check if a volume is being created from snapshot. - inUse := false - if content != nil { - inUse = ctrl.isVolumeBeingCreatedFromSnapshot(snapshot) - } - - klog.V(5).Infof("checkandRemoveSnapshotFinalizersAndCheckandDeleteContent[%s]: set DeletionTimeStamp on content.", utils.SnapshotKey(snapshot)) + if !utils.IsSnapshotDeletionCandidate(snapshot) { + return nil + } - // If content exists, set DeletionTimeStamp on the content; - // content won't be deleted immediately due to the finalizer - if content != nil && deleteContent && !inUse { - err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Delete(content.Name, &metav1.DeleteOptions{}) - if err != nil { - ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotContentObjectDeleteError", "Failed to delete snapshot content API object") - return fmt.Errorf("failed to delete VolumeSnapshotContent %s from API server: %q", content.Name, err) + // check if the snapshot is being used for restore a PVC, if yes, do nothing + // and wait until PVC restoration finishes + if content != nil && ctrl.isVolumeBeingCreatedFromSnapshot(snapshot) { + klog.V(4).Infof("checkandRemoveSnapshotFinalizersAndCheckandDeleteContent[%s]: snapshot is being used to restore a PVC", utils.SnapshotKey(snapshot)) + ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotDeletePending", "Snapshot is being used to restore a PVC") + // TODO(@xiangqian): should requeue this? + return nil + } - } + // regardless of the deletion policy, set the VolumeSnapshotBeingDeleted on + // content object, this is to allow sidecar controller to conduct a delete + // operation whenever the content has deletion timestamp set. + if content != nil { + klog.V(5).Infof("checkandRemoveSnapshotFinalizersAndCheckandDeleteContent[%s]: Set VolumeSnapshotBeingDeleted annotation on the content [%s]", utils.SnapshotKey(snapshot), content.Name) + if err := ctrl.setAnnVolumeSnapshotBeingDeleted(content); err != nil { + klog.V(4).Infof("checkandRemoveSnapshotFinalizersAndCheckandDeleteContent[%s]: failed to set VolumeSnapshotBeingDeleted annotation on the content [%s]", utils.SnapshotKey(snapshot), content.Name) + return err } + } - if !inUse { - klog.V(5).Infof("checkandRemoveSnapshotFinalizersAndCheckandDeleteContent: Set VolumeSnapshotBeingDeleted annotation on the content") - ctrl.setAnnVolumeSnapshotBeingDeleted(content) - - klog.V(5).Infof("checkandRemoveSnapshotFinalizersAndCheckandDeleteContent: Remove Finalizer for VolumeSnapshot[%s]", utils.SnapshotKey(snapshot)) - doesContentExist := false - if content != nil { - doesContentExist = true - } - return ctrl.removeSnapshotFinalizer(snapshot, !inUse, !doesContentExist) + // VolumeSnapshot should be deleted. Check and remove finalizers + klog.V(5).Infof("checkandRemoveSnapshotFinalizersAndCheckandDeleteContent[%s]: set DeletionTimeStamp on content.", utils.SnapshotKey(snapshot)) + // If content exists and has a deletion policy of Delete, set DeletionTimeStamp on the content; + // content won't be deleted immediately due to the VolumeSnapshotContentFinalizer + if content != nil && deleteContent { + err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Delete(content.Name, &metav1.DeleteOptions{}) + if err != nil { + ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotContentObjectDeleteError", "Failed to delete snapshot content API object") + return fmt.Errorf("failed to delete VolumeSnapshotContent %s from API server: %q", content.Name, err) } } - return nil + + klog.V(5).Infof("checkandRemoveSnapshotFinalizersAndCheckandDeleteContent: Remove Finalizer for VolumeSnapshot[%s]", utils.SnapshotKey(snapshot)) + // remove finalizers on the VolumeSnapshot object, there are two finalizers: + // 1. VolumeSnapshotAsSourceFinalizer, once reached here, the snapshot is not + // in use to restore PVC, and the finalizer will be removed directly. + // 2. VolumeSnapshotBoundFinalizer: + // a. If there is no content found, remove the finalizer. + // b. If the content is being deleted, i.e., with deleteContent == true, + // keep this finalizer until the content object is removed from API server + // by sidecar controller. + // c. If deletion will not cascade to the content, remove the finalizer on + // the snapshot such that it can be removed from API server. + removeBoundFinalizer := !(content != nil && deleteContent) + return ctrl.removeSnapshotFinalizer(snapshot, true, removeBoundFinalizer) } // checkandAddSnapshotFinalizers checks and adds snapshot finailzers when needed func (ctrl *csiSnapshotCommonController) checkandAddSnapshotFinalizers(snapshot *crdv1.VolumeSnapshot) error { - _, deleteContent, snapshotBound, err := ctrl.checkContentAndBoundStatus(snapshot) + // get the content for this Snapshot + var ( + content *crdv1.VolumeSnapshotContent = nil + err error = nil + ) + if snapshot.Spec.Source.VolumeSnapshotContentName != nil { + content, err = ctrl.getPreprovisionedContentFromStore(snapshot) + } + if snapshot.Spec.Source.PersistentVolumeClaimName != nil { + content, err = ctrl.getDynamicallyProvisionedContentFromStore(snapshot) + } if err != nil { return err } - addSourceFinalizer := false + // NOTE: Source finalizer will be added to snapshot if DeletionTimeStamp is nil + // and it is not set yet. This is because the logic to check whether a PVC is being + // created from the snapshot is expensive so we only go through it when we need + // to remove this finalizer and make sure it is removed when it is not needed any more. + addSourceFinalizer := utils.NeedToAddSnapshotAsSourceFinalizer(snapshot) + + // note that content could be nil, in this case bound finalizer is not needed addBoundFinalizer := false - // NOTE: Source finalizer will be added to snapshot if - // DeletionTimeStamp is nil and it is not set yet. - // This is because the logic to check whether a PVC is being - // created from the snapshot is expensive so we only go thru - // it when we need to remove this finalizer and make sure - // it is removed when it is not needed any more. - if utils.NeedToAddSnapshotAsSourceFinalizer(snapshot) { - addSourceFinalizer = true - } - if utils.NeedToAddSnapshotBoundFinalizer(snapshot) && snapshotBound && deleteContent { - // Add bound finalizer if snapshot is bound to content and deletion policy is delete - addBoundFinalizer = true + if content != nil { + // A bound finalizer is needed ONLY when all following conditions are satisfied: + // 1. the VolumeSnapshot is bound to some content + // 2. the VolumeSnapshot does not have deletion timestamp set + // 3. the matching content has a deletion policy to be Delete + // Note that if a matching content is found, it must points back to the snapshot + addBoundFinalizer = utils.NeedToAddSnapshotBoundFinalizer(snapshot) && (content.Spec.DeletionPolicy == crdv1.VolumeSnapshotContentDelete) } if addSourceFinalizer || addBoundFinalizer { // Snapshot is not being deleted -> it should have the finalizer. @@ -341,32 +329,23 @@ func (ctrl *csiSnapshotCommonController) syncReadySnapshot(snapshot *crdv1.Volum if !utils.IsBoundVolumeSnapshotContentNameSet(snapshot) { return fmt.Errorf("snapshot %s is not bound to a content.", utils.SnapshotKey(snapshot)) } - obj, found, err := ctrl.contentStore.GetByKey(*snapshot.Status.BoundVolumeSnapshotContentName) + content, err := ctrl.getContentFromStore(*snapshot.Status.BoundVolumeSnapshotContentName) if err != nil { - return err - } - if !found { - if err = ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentMissing", "VolumeSnapshotContent is missing"); err != nil { - return err - } - return nil - } else { - content, ok := obj.(*crdv1.VolumeSnapshotContent) - if !ok { - return fmt.Errorf("Cannot convert object from snapshot content store to VolumeSnapshotContent %q!?: %#v", *snapshot.Status.BoundVolumeSnapshotContentName, obj) - } - - klog.V(5).Infof("syncReadySnapshot[%s]: VolumeSnapshotContent %q found", utils.SnapshotKey(snapshot), content.Name) - if !utils.IsVolumeSnapshotRefSet(snapshot, content) { - // snapshot is bound but content is not bound to snapshot correctly - if err = ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotMisbound", "VolumeSnapshotContent is not bound to the VolumeSnapshot correctly"); err != nil { - return err - } - return nil - } - // Snapshot is correctly bound. return nil } + if content == nil { + // this meant there is no matching content in cache found + // update status of the snapshot and return + return ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentMissing", "VolumeSnapshotContent is missing") + } + klog.V(5).Infof("syncReadySnapshot[%s]: VolumeSnapshotContent %q found", utils.SnapshotKey(snapshot), content.Name) + // check binding from content side to make sure the binding is still valid + if !utils.IsVolumeSnapshotRefSet(snapshot, content) { + // snapshot is bound but content is not pointing to the snapshot + return ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotMisbound", "VolumeSnapshotContent is not bound to the VolumeSnapshot correctly") + } + // everything is verified, return + return nil } // syncUnreadySnapshot is the main controller method to decide what to do with a snapshot which is not set to ready. @@ -376,10 +355,17 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol // Pre-provisioned snapshot if snapshot.Spec.Source.VolumeSnapshotContentName != nil { - content, err := ctrl.findContentfromStore(snapshot) + content, err := ctrl.getPreprovisionedContentFromStore(snapshot) if err != nil { return err } + // if no content found yet, update status and return + if content == nil { + // can not find the desired VolumeSnapshotContent from cache store + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentMissing", "VolumeSnapshotContent is missing") + klog.V(4).Infof("sync snapshot[%s]: snapshot content %q requested but not found, will try again", utils.SnapshotKey(snapshot), *snapshot.Spec.Source.VolumeSnapshotContentName) + return fmt.Errorf("snapshot %s requests an non-existing content %s", utils.SnapshotKey(snapshot), *snapshot.Spec.Source.VolumeSnapshotContentName) + } // Set VolumeSnapshotRef UID newContent, err := ctrl.checkandBindSnapshotContent(snapshot, content) if err != nil { @@ -404,91 +390,160 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err)) return err } - return nil - } else { // snapshot.Spec.Source.VolumeSnapshotContentName == nil - dynamically creating snapshot - klog.V(5).Infof("before getMatchSnapshotContent for snapshot %s", uniqueSnapshotName) - var contentObj *crdv1.VolumeSnapshotContent - contentObj, _ = ctrl.getContentFromStore(snapshot) - // Ignore err from getContentFromStore. If content not found - // in the cache store by the content name, try to search it from - // the ctrl.contentStore.List() - if contentObj == nil { - contentObj = ctrl.getMatchSnapshotContent(snapshot) - } + } + // snapshot.Spec.Source.VolumeSnapshotContentName == nil - dynamically creating snapshot + klog.V(5).Infof("getDynamicallyProvisionedContentFromStore for snapshot %s", uniqueSnapshotName) + contentObj, err := ctrl.getDynamicallyProvisionedContentFromStore(snapshot) + if err != nil { + return err + } - if contentObj != nil { - klog.V(5).Infof("Found VolumeSnapshotContent object %s for snapshot %s", contentObj.Name, uniqueSnapshotName) - if contentObj.Spec.Source.SnapshotHandle != nil { - ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotHandleSet", fmt.Sprintf("Snapshot handle should not be set in content %s for dynamic provisioning", uniqueSnapshotName)) - return fmt.Errorf("snapshotHandle should not be set in the content for dynamic provisioning for snapshot %s", uniqueSnapshotName) - } - newSnapshot, err := ctrl.bindandUpdateVolumeSnapshot(contentObj, snapshot) - if err != nil { + if contentObj != nil { + klog.V(5).Infof("Found VolumeSnapshotContent object %s for snapshot %s", contentObj.Name, uniqueSnapshotName) + if contentObj.Spec.Source.SnapshotHandle != nil { + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotHandleSet", fmt.Sprintf("Snapshot handle should not be set in content %s for dynamic provisioning", uniqueSnapshotName)) + return fmt.Errorf("snapshotHandle should not be set in the content for dynamic provisioning for snapshot %s", uniqueSnapshotName) + } + newSnapshot, err := ctrl.bindandUpdateVolumeSnapshot(contentObj, snapshot) + if err != nil { + return err + } + klog.V(5).Infof("bindandUpdateVolumeSnapshot %v", newSnapshot) + return nil + } else if snapshot.Status == nil || snapshot.Status.Error == nil || isControllerUpdateFailError(snapshot.Status.Error) { + if snapshot.Spec.Source.PersistentVolumeClaimName == nil { + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotPVCSourceMissing", fmt.Sprintf("PVC source for snapshot %s is missing", uniqueSnapshotName)) + return fmt.Errorf("expected PVC source for snapshot %s but got nil", uniqueSnapshotName) + } else { + var err error + var content *crdv1.VolumeSnapshotContent + if content, err = ctrl.createSnapshotContent(snapshot); err != nil { + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentCreationFailed", fmt.Sprintf("Failed to create snapshot content with error %v", err)) return err } - klog.V(5).Infof("bindandUpdateVolumeSnapshot %v", newSnapshot) - return nil - } else if snapshot.Status == nil || snapshot.Status.Error == nil || isControllerUpdateFailError(snapshot.Status.Error) { - if snapshot.Spec.Source.PersistentVolumeClaimName == nil { - ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotPVCSourceMissing", fmt.Sprintf("PVC source for snapshot %s is missing", uniqueSnapshotName)) - return fmt.Errorf("expected PVC source for snapshot %s but got nil", uniqueSnapshotName) - } else { - var err error - var content *crdv1.VolumeSnapshotContent - if content, err = ctrl.createSnapshotContent(snapshot); err != nil { - ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentCreationFailed", fmt.Sprintf("Failed to create snapshot content with error %v", err)) - return err - } - // Update snapshot status with BoundVolumeSnapshotContentName - for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ { - klog.V(5).Infof("syncUnreadySnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshot)) - _, err = ctrl.updateSnapshotStatus(snapshot, content) - if err == nil { - break - } - klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err) - time.Sleep(ctrl.createSnapshotContentInterval) + // Update snapshot status with BoundVolumeSnapshotContentName + for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ { + klog.V(5).Infof("syncUnreadySnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshot)) + _, err = ctrl.updateSnapshotStatus(snapshot, content) + if err == nil { + break } + klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err) + time.Sleep(ctrl.createSnapshotContentInterval) + } - if err != nil { - // update snapshot status failed - ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err)) - return err - } + if err != nil { + // update snapshot status failed + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err)) + return err } } - return nil } + return nil } -// findContentfromStore finds content from content cache store -func (ctrl *csiSnapshotCommonController) findContentfromStore(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotContent, error) { - var contentName string - uniqueSnapshotName := utils.SnapshotKey(snapshot) - if snapshot.Spec.Source.VolumeSnapshotContentName != nil { - contentName = *snapshot.Spec.Source.VolumeSnapshotContentName - } else if snapshot.Status != nil && snapshot.Status.BoundVolumeSnapshotContentName != nil { - contentName = *snapshot.Status.BoundVolumeSnapshotContentName - } +// getPreprovisionedContentFromStore tries to find a pre-provisioned content object +// from content cache store for the passed in VolumeSnapshot. +// Note that this function assumes the passed in VolumeSnapshot is a pre-provisioned +// one, i.e., snapshot.Spec.Source.VolumeSnapshotContentName != nil. +// If no matching content is found, it returns (nil, nil). +// If it found a content which is not a pre-provisioned one, it updates the status +// of the snapshot with an event and returns an error. +// If it found a content which does not point to the passed in VolumeSnapshot, it +// updates the status of the snapshot with an event and returns an error. +// Otherwise, the found content will be returned. +func (ctrl *csiSnapshotCommonController) getPreprovisionedContentFromStore(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotContent, error) { + contentName := *snapshot.Spec.Source.VolumeSnapshotContentName if contentName == "" { - return nil, fmt.Errorf("content name not found for snapshot %s", uniqueSnapshotName) + return nil, fmt.Errorf("empty VolumeSnapshotContentName for snapshot %s", utils.SnapshotKey(snapshot)) + } + content, err := ctrl.getContentFromStore(contentName) + if err != nil { + return nil, err } + if content == nil { + // can not find the desired VolumeSnapshotContent from cache store + return nil, nil + } + // check whether the content is a pre-provisioned VolumeSnapshotContent + if content.Spec.Source.SnapshotHandle == nil { + // found a content which represents a dynamically provisioned snapshot + // update the snapshot and return an error + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentMismatch", "VolumeSnapshotContent is dynamically provisioned while expecting a pre-provisioned one") + klog.V(4).Infof("sync snapshot[%s]: snapshot content %q is dynamically provisioned while expecting a pre-provisioned one", utils.SnapshotKey(snapshot), contentName) + return nil, fmt.Errorf("snapshot %s expects a pre-provisioned VolumeSnapshotContent %s but gets a dynamically provisioned one", utils.SnapshotKey(snapshot), contentName) + } + // verify the content points back to the snapshot + ref := content.Spec.VolumeSnapshotRef + if ref.Name != snapshot.Name || ref.Namespace != snapshot.Namespace || (ref.UID != "" && ref.UID != snapshot.UID) { + klog.V(4).Infof("sync snapshot[%s]: VolumeSnapshotContent %s is bound to another snapshot %v", utils.SnapshotKey(snapshot), contentName, ref) + msg := fmt.Sprintf("VolumeSnapshotContent [%s] is bound to a different snapshot", contentName) + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentMisbound", msg) + return nil, fmt.Errorf(msg) + } + return content, nil +} - contentObj, found, err := ctrl.contentStore.GetByKey(contentName) +// getDynamicallyProvisionedContentFromStore tries to find a dynamically created +// content object for the passed in VolumeSnapshot from the content store. +// Note that this function assumes the passed in VolumeSnapshot is a dynamic +// one which requests creating a snapshot from a PVC. +// i.e., with snapshot.Spec.Source.PersistentVolumeClaimName != nil +// If no matching VolumeSnapshotContent exists in the content cache store, it +// returns (nil, nil) +// If a content is found but it's not dynamically provisioned, the passed in +// snapshot status will be updated with an error along with an event, +// and an error will be returned. +// If a content is found but it does not point to the passed in VolumeSnapshot, +// the passed in snapshot will be updated with an error along with an event, +// and an error will be returned. +func (ctrl *csiSnapshotCommonController) getDynamicallyProvisionedContentFromStore(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotContent, error) { + contentName := utils.GetDynamicSnapshotContentNameForSnapshot(snapshot) + content, err := ctrl.getContentFromStore(contentName) if err != nil { return nil, err } - if !found { - // snapshot is bound to a non-existing content. - ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentMissing", "VolumeSnapshotContent is missing") - klog.V(4).Infof("synchronizing unready snapshot[%s]: snapshotcontent %q requested and not found, will try again next time", uniqueSnapshotName, contentName) - return nil, fmt.Errorf("snapshot %s is bound to a non-existing content %s", uniqueSnapshotName, contentName) + if content == nil { + // no matching content with the desired name has been found in cache + return nil, nil } - content, ok := contentObj.(*crdv1.VolumeSnapshotContent) + // check whether the content represents a dynamically provisioned snapshot + if content.Spec.Source.VolumeHandle == nil { + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentMismatch", "VolumeSnapshotContent is pre-provisioned while expecting a dynamically provisioned one") + klog.V(4).Infof("sync snapshot[%s]: snapshot content %s is pre-provisioned while expecting a dynamically provisioned one", utils.SnapshotKey(snapshot), contentName) + return nil, fmt.Errorf("snapshot %s expects a dynamically provisioned VolumeSnapshotContent %s but gets a pre-provisioned one", utils.SnapshotKey(snapshot), contentName) + } + // check whether the content points back to the passed in VolumeSnapshot + ref := content.Spec.VolumeSnapshotRef + if ref.Name != snapshot.Name || ref.Namespace != snapshot.Namespace || ref.UID != snapshot.UID { + klog.V(4).Infof("sync snapshot[%s]: VolumeSnapshotContent %s is bound to another snapshot %v", utils.SnapshotKey(snapshot), contentName, ref) + msg := fmt.Sprintf("VolumeSnapshotContent [%s] is bound to a different snapshot", contentName) + ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentMisbound", msg) + return nil, fmt.Errorf(msg) + } + return content, nil +} + +// getContentFromStore tries to find a VolumeSnapshotContent from content cache +// store by name. +// Note that if no VolumeSnapshotContent exists in the cache store and no error +// encountered, it returns(nil, nil) +func (ctrl *csiSnapshotCommonController) getContentFromStore(contentName string) (*crdv1.VolumeSnapshotContent, error) { + obj, exist, err := ctrl.contentStore.GetByKey(contentName) + if err != nil { + // should never reach here based on implementation at: + // https://github.com/kubernetes/client-go/blob/master/tools/cache/store.go#L226 + return nil, err + } + if !exist { + // not able to find a matching content + return nil, nil + } + content, ok := obj.(*crdv1.VolumeSnapshotContent) if !ok { - return nil, fmt.Errorf("expected volume snapshot content, got %+v", contentObj) + return nil, fmt.Errorf("expected VolumeSnapshotContent, got %+v", obj) } return content, nil } @@ -603,7 +658,7 @@ func (ctrl *csiSnapshotCommonController) getCreateSnapshotInput(snapshot *crdv1. } // Create VolumeSnapshotContent name - contentName := utils.GetSnapshotContentNameForSnapshot(snapshot) + contentName := utils.GetDynamicSnapshotContentNameForSnapshot(snapshot) // Resolve snapshotting secret credentials. snapshotterSecretRef, err := utils.GetSecretReference(utils.SnapshotterSecretParams, class.Parameters, contentName, snapshot) @@ -614,33 +669,6 @@ func (ctrl *csiSnapshotCommonController) getCreateSnapshotInput(snapshot *crdv1. return class, volume, contentName, snapshotterSecretRef, nil } -// getMatchSnapshotContent looks up VolumeSnapshotContent for a VolumeSnapshot named snapshotName -func (ctrl *csiSnapshotCommonController) getMatchSnapshotContent(snapshot *crdv1.VolumeSnapshot) *crdv1.VolumeSnapshotContent { - var snapshotContentObj *crdv1.VolumeSnapshotContent - var found bool - - objs := ctrl.contentStore.List() - for _, obj := range objs { - content := obj.(*crdv1.VolumeSnapshotContent) - if content.Spec.VolumeSnapshotRef.Name == snapshot.Name && - content.Spec.VolumeSnapshotRef.Namespace == snapshot.Namespace && - content.Spec.VolumeSnapshotRef.UID == snapshot.UID && - content.Spec.VolumeSnapshotClassName != nil && snapshot.Spec.VolumeSnapshotClassName != nil && - *(content.Spec.VolumeSnapshotClassName) == *(snapshot.Spec.VolumeSnapshotClassName) { - found = true - snapshotContentObj = content - break - } - } - - if !found { - klog.V(4).Infof("No VolumeSnapshotContent for VolumeSnapshot %s found", utils.SnapshotKey(snapshot)) - return nil - } - - return snapshotContentObj -} - func (ctrl *csiSnapshotCommonController) storeSnapshotUpdate(snapshot interface{}) (bool, error) { return utils.StoreObjectUpdate(ctrl.snapshotStore, snapshot, "snapshot") } @@ -850,7 +878,10 @@ func (ctrl *csiSnapshotCommonController) checkandRemovePVCFinalizer(snapshot *cr return nil } -// The function checks whether the volumeSnapshotRef in snapshot content matches the given snapshot. If match, it binds the content with the snapshot. This is for static binding where user has specified snapshot name but not UID of the snapshot in content.Spec.VolumeSnapshotRef. +// The function checks whether the volumeSnapshotRef in the snapshot content matches +// the given snapshot. If match, it binds the content with the snapshot. This is for +// static binding where user has specified snapshot name but not UID of the snapshot +// in content.Spec.VolumeSnapshotRef. func (ctrl *csiSnapshotCommonController) checkandBindSnapshotContent(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshotContent, error) { if content.Spec.VolumeSnapshotRef.Name != snapshot.Name { return nil, fmt.Errorf("Could not bind snapshot %s and content %s, the VolumeSnapshotRef does not match", snapshot.Name, content.Name) @@ -1251,33 +1282,6 @@ func (ctrl *csiSnapshotCommonController) removeSnapshotFinalizer(snapshot *crdv1 return nil } -// getContentFromStore finds content from the cache store. -// If getContentFromStore returns (nil, nil), it means content not found -// and it may have already been deleted. -func (ctrl *csiSnapshotCommonController) getContentFromStore(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotContent, error) { - var contentName string - if snapshot.Status != nil && snapshot.Status.BoundVolumeSnapshotContentName != nil { - contentName = *snapshot.Status.BoundVolumeSnapshotContentName - } else { - contentName = utils.GetSnapshotContentNameForSnapshot(snapshot) - } - obj, found, err := ctrl.contentStore.GetByKey(contentName) - if err != nil { - return nil, err - } - // Not in the content cache store, no error - if !found { - return nil, nil - } - // Found in content cache store - content, ok := obj.(*crdv1.VolumeSnapshotContent) - if !ok { - return content, fmt.Errorf("Cannot convert object from snapshot content store to VolumeSnapshotContent %q!?: %#v", contentName, obj) - } - // Found in content cache store and convert object is successful - return content, nil -} - // getSnapshotFromStore finds snapshot from the cache store. // If getSnapshotFromStore returns (nil, nil), it means snapshot not found // and it may have already been deleted. diff --git a/pkg/common-controller/snapshot_delete_test.go b/pkg/common-controller/snapshot_delete_test.go index 35427874e..5d5375424 100644 --- a/pkg/common-controller/snapshot_delete_test.go +++ b/pkg/common-controller/snapshot_delete_test.go @@ -169,11 +169,11 @@ func TestDeleteSync(t *testing.T) { test: testSyncContent, }, { - name: "3-1 - content will be deleted if snapshot deletion timestamp is set", - initialContents: newContentArray("content3-1", "", "snap3-1", "sid3-1", validSecretClass, "", "", deletePolicy, nil, nil, true), + name: "3-1 - (dynamic) content will be deleted if snapshot deletion timestamp is set", + initialContents: newContentArray("snapcontent-snapuid3-1", "snapuid3-1", "snap3-1", "sid3-1", validSecretClass, "", "volume3-1", deletePolicy, nil, nil, true), expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "content3-1", &True, nil, nil, nil, false, true, &timeNowMetav1), - expectedSnapshots: withSnapshotFinalizers(newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "content3-1", &True, nil, nil, nil, false, false, &timeNowMetav1), + initialSnapshots: newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "snapcontent-snapuid3-1", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: withSnapshotFinalizers(newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "snapcontent-snapuid3-1", &False, nil, nil, nil, false, false, &timeNowMetav1), utils.VolumeSnapshotBoundFinalizer, ), initialClaims: newClaimArray("claim3-1", "pvc-uid3-1", "1Gi", "volume3-1", v1.ClaimBound, &classEmpty), @@ -183,11 +183,14 @@ func TestDeleteSync(t *testing.T) { test: testSyncSnapshot, }, { - name: "3-2 - content will not be deleted if deletion API call fails", - initialContents: newContentArray("content3-2", "", "snap3-2", "sid3-2", validSecretClass, "", "", deletePolicy, nil, nil, true), - expectedContents: newContentArray("content3-2", "", "snap3-2", "sid3-2", validSecretClass, "", "", deletePolicy, nil, nil, true), - initialSnapshots: newSnapshotArray("snap3-2", "snapuid3-2", "claim3-2", "", validSecretClass, "content3-2", &True, nil, nil, nil, false, true, &timeNowMetav1), - expectedSnapshots: newSnapshotArray("snap3-2", "snapuid3-2", "claim3-2", "", validSecretClass, "content3-2", &True, nil, nil, nil, false, true, &timeNowMetav1), + name: "3-2 - (dynamic) content will not be deleted if deletion API call fails", + initialContents: newContentArray("snapcontent-snapuid3-2", "snapuid3-2", "snap3-2", "sid3-2", validSecretClass, "", "volume3-2", deletePolicy, nil, nil, true), + expectedContents: withContentAnnotations(newContentArray("snapcontent-snapuid3-2", "snapuid3-2", "snap3-2", "sid3-2", validSecretClass, "", "volume3-2", deletePolicy, nil, nil, true), + map[string]string{ + "snapshot.storage.kubernetes.io/volumesnapshot-being-deleted": "yes", + }), + initialSnapshots: newSnapshotArray("snap3-2", "snapuid3-2", "claim3-2", "", validSecretClass, "snapcontent-snapuid3-2", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-2", "snapuid3-2", "claim3-2", "", validSecretClass, "snapcontent-snapuid3-2", &False, nil, nil, nil, false, true, &timeNowMetav1), initialClaims: newClaimArray("claim3-2", "pvc-uid3-2", "1Gi", "volume3-2", v1.ClaimBound, &classEmpty), expectedEvents: []string{"Warning SnapshotContentObjectDeleteError"}, initialSecrets: []*v1.Secret{secret()}, @@ -200,22 +203,135 @@ func TestDeleteSync(t *testing.T) { test: testSyncSnapshotError, }, { - name: "3-3 - content will not be deleted if retainPolicy is set", - initialContents: newContentArray("content3-3", "", "snap3-3", "sid3-3", validSecretClass, "", "", retainPolicy, nil, nil, true), - expectedContents: withContentAnnotations(newContentArray("content3-3", "", "snap3-3", "sid3-3", validSecretClass, "", "", retainPolicy, nil, nil, true), + name: "3-3 - (dynamic) content will not be deleted if retainPolicy is set, snapshot should have its finalizer removed", + initialContents: newContentArray("snapcontent-snapuid3-3", "snapuid3-3", "snap3-3", "sid3-3", validSecretClass, "", "volume3-3", retainPolicy, nil, nil, true), + expectedContents: withContentAnnotations(newContentArray("snapcontent-snapuid3-3", "snapuid3-3", "snap3-3", "sid3-3", validSecretClass, "", "volume3-3", retainPolicy, nil, nil, true), map[string]string{ "snapshot.storage.kubernetes.io/volumesnapshot-being-deleted": "yes", }), - initialSnapshots: newSnapshotArray("snap3-3", "snapuid3-3", "claim3-3", "", validSecretClass, "content3-3", &True, nil, nil, nil, false, true, &timeNowMetav1), - expectedSnapshots: withSnapshotFinalizers(newSnapshotArray("snap3-3", "snapuid3-3", "claim3-3", "", validSecretClass, "content3-3", &True, nil, nil, nil, false, false, &timeNowMetav1), + initialSnapshots: newSnapshotArray("snap3-3", "snapuid3-3", "claim3-3", "", validSecretClass, "snapcontent-snapuid3-3", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-3", "snapuid3-3", "claim3-3", "", validSecretClass, "snapcontent-snapuid3-3", &False, nil, nil, nil, false, false, &timeNowMetav1), + initialClaims: newClaimArray("claim3-3", "pvc-uid3-3", "1Gi", "volume3-3", v1.ClaimBound, &classEmpty), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "3-4 - (dynamic) snapshot should have its finalizer removed if no content has been found", + initialContents: nocontents, + expectedContents: nocontents, + initialSnapshots: newSnapshotArray("snap3-4", "snapuid3-4", "claim3-4", "", validSecretClass, "", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-4", "snapuid3-4", "claim3-4", "", validSecretClass, "", &False, nil, nil, nil, false, false, &timeNowMetav1), + initialClaims: newClaimArray("claim3-4", "pvc-uid3-4", "1Gi", "volume3-4", v1.ClaimBound, &classEmpty), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "3-5 - (dynamic) snapshot should have its finalizer removed if a content is found but points to a different snapshot - uid mismatch", + initialContents: newContentArray("snapcontent-snapuid3-5", "snapuid3-5-x", "snap3-5", "sid3-5", validSecretClass, "", "volume3-5", deletePolicy, nil, nil, true), + expectedContents: newContentArray("snapcontent-snapuid3-5", "snapuid3-5-x", "snap3-5", "sid3-5", validSecretClass, "", "volume3-5", deletePolicy, nil, nil, true), + initialSnapshots: newSnapshotArray("snap3-5", "snapuid3-5", "claim3-5", "", validSecretClass, "snapcontent-snapuid3-5", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-5", "snapuid3-5", "claim3-5", "", validSecretClass, "snapcontent-snapuid3-5", &False, nil, nil, nil, false, false, &timeNowMetav1), + initialClaims: newClaimArray("claim3-5", "pvc-uid3-5", "1Gi", "volume3-5", v1.ClaimBound, &classEmpty), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "3-6 - (dynamic) snapshot should have its finalizer removed if a content is found but points to a different snapshot - name mismatch", + initialContents: newContentArray("snapcontent-snapuid3-6", "snapuid3-6", "snap3-6-x", "sid3-6", validSecretClass, "", "volume3-6", deletePolicy, nil, nil, true), + expectedContents: newContentArray("snapcontent-snapuid3-6", "snapuid3-6", "snap3-6-x", "sid3-6", validSecretClass, "", "volume3-6", deletePolicy, nil, nil, true), + initialSnapshots: newSnapshotArray("snap3-6", "snapuid3-6", "claim3-6", "", validSecretClass, "snapcontent-snapuid3-6", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-6", "snapuid3-6", "claim3-6", "", validSecretClass, "snapcontent-snapuid3-6", &False, nil, nil, nil, false, false, &timeNowMetav1), + initialClaims: newClaimArray("claim3-6", "pvc-uid3-6", "1Gi", "volume3-6", v1.ClaimBound, &classEmpty), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "3-7 - (static) content will be deleted if snapshot deletion timestamp is set, snapshot should have its finalizers removed", + initialContents: newContentArray("content-3-7", "snapuid3-7", "snap3-7", "sid3-7", validSecretClass, "sid3-7", "", deletePolicy, nil, nil, true), + expectedContents: nocontents, + initialSnapshots: newSnapshotArray("snap3-7", "snapuid3-7", "", "content-3-7", validSecretClass, "content-3-7", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: withSnapshotFinalizers(newSnapshotArray("snap3-7", "snapuid3-7", "", "content-3-7", validSecretClass, "content-3-7", &False, nil, nil, nil, false, false, &timeNowMetav1), utils.VolumeSnapshotBoundFinalizer, ), - initialClaims: newClaimArray("claim3-3", "pvc-uid3-3", "1Gi", "volume3-3", v1.ClaimBound, &classEmpty), expectedEvents: noevents, initialSecrets: []*v1.Secret{secret()}, errors: noerrors, test: testSyncSnapshot, }, + { + name: "3-8 - (static) content will not be deleted if deletion API call fails, snapshot should have its finalizers remained", + initialContents: newContentArray("content-3-8", "snapuid3-8", "snap3-8", "sid3-8", validSecretClass, "sid3-8", "", deletePolicy, nil, nil, true), + expectedContents: withContentAnnotations(newContentArray("content-3-8", "snapuid3-8", "snap3-8", "sid3-8", validSecretClass, "sid3-8", "", deletePolicy, nil, nil, true), + map[string]string{ + "snapshot.storage.kubernetes.io/volumesnapshot-being-deleted": "yes", + }), + initialSnapshots: newSnapshotArray("snap3-8", "snapuid3-8", "", "content-3-8", validSecretClass, "content-3-8", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-8", "snapuid3-8", "", "content-3-8", validSecretClass, "content-3-8", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedEvents: []string{"Warning SnapshotContentObjectDeleteError"}, + initialSecrets: []*v1.Secret{secret()}, + errors: []reactorError{ + // Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshotContents().Delete call. + // All other calls will succeed. + {"delete", "volumesnapshotcontents", errors.New("mock delete error")}, + }, + expectSuccess: false, + test: testSyncSnapshotError, + }, + { + name: "3-9 - (static) content will not be deleted if retainPolicy is set, snapshot should have its finalizer removed", + initialContents: newContentArray("content-3-9", "snapuid3-9", "snap3-9", "sid3-9", validSecretClass, "sid3-9", "", retainPolicy, nil, nil, true), + expectedContents: withContentAnnotations(newContentArray("content-3-9", "snapuid3-9", "snap3-9", "sid3-9", validSecretClass, "sid3-9", "", retainPolicy, nil, nil, true), + map[string]string{ + "snapshot.storage.kubernetes.io/volumesnapshot-being-deleted": "yes", + }), + initialSnapshots: newSnapshotArray("snap3-9", "snapuid3-9", "", "content-3-9", validSecretClass, "content-3-9", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-9", "snapuid3-9", "", "content-3-9", validSecretClass, "content-3-9", &False, nil, nil, nil, false, false, &timeNowMetav1), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "3-10 - (static) snapshot should have its finalizer removed if no content has been found", + initialContents: nocontents, + expectedContents: nocontents, + initialSnapshots: newSnapshotArray("snap3-10", "snapuid3-10", "", "content-3-10", validSecretClass, "", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-10", "snapuid3-10", "", "content-3-10", validSecretClass, "", &False, nil, nil, nil, false, false, &timeNowMetav1), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "3-11 - (static) snapshot should have its finalizer removed if a content is found but points to a different snapshot - uid mismatch", + initialContents: newContentArray("content-3-11", "snapuid3-11-x", "snap3-11", "sid3-11", validSecretClass, "sid3-11", "", deletePolicy, nil, nil, true), + expectedContents: newContentArray("content-3-11", "snapuid3-11-x", "snap3-11", "sid3-11", validSecretClass, "sid3-11", "", deletePolicy, nil, nil, true), + initialSnapshots: newSnapshotArray("snap3-11", "snapuid3-11", "", "content-3-11", validSecretClass, "content-3-11", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-11", "snapuid3-11", "", "content-3-11", validSecretClass, "content-3-11", &False, nil, nil, nil, false, false, &timeNowMetav1), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "3-12 - (static) snapshot should have its finalizer removed if a content is found but points to a different snapshot - name mismatch", + initialContents: newContentArray("content-3-12", "snapuid3-12", "snap3-12-x", "sid3-12", validSecretClass, "sid3-12", "", deletePolicy, nil, nil, true), + expectedContents: newContentArray("content-3-12", "snapuid3-12", "snap3-12-x", "sid3-12", validSecretClass, "sid3-12", "", deletePolicy, nil, nil, true), + initialSnapshots: newSnapshotArray("snap3-12", "snapuid3-12", "", "content-3-12", validSecretClass, "content-3-12", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-12", "snapuid3-12", "", "content-3-12", validSecretClass, "content-3-12", &False, nil, nil, nil, false, false, &timeNowMetav1), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, } runSyncTests(t, tests, snapshotClasses) } diff --git a/pkg/common-controller/snapshot_update_test.go b/pkg/common-controller/snapshot_update_test.go index f0f9550c0..f57230446 100644 --- a/pkg/common-controller/snapshot_update_test.go +++ b/pkg/common-controller/snapshot_update_test.go @@ -49,7 +49,7 @@ func TestSync(t *testing.T) { tests := []controllerTest{ { // snapshot is bound to a non-existing content - name: "2-1 - snapshot is bound to a non-existing content", + name: "2-1 - (dynamic) snapshot is bound to a non-existing content", initialContents: nocontents, expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap2-1", "snapuid2-1", "claim2-1", "", validSecretClass, "content2-1", &True, nil, nil, nil, false, true, nil), @@ -59,22 +59,21 @@ func TestSync(t *testing.T) { test: testSyncSnapshot, }, { - name: "2-2 - snapshot points to a content but content does not point to snapshot(VolumeSnapshotRef does not match)", - initialContents: newContentArray("content2-2", "snapuid2-2-x", "snap2-2", "sid2-2", validSecretClass, "", "", deletionPolicy, nil, nil, false), - expectedContents: newContentArray("content2-2", "snapuid2-2-x", "snap2-2", "sid2-2", validSecretClass, "", "", deletionPolicy, nil, nil, false), + name: "2-2 - (static) snapshot points to a content but content does not point to snapshot(VolumeSnapshotRef does not match)", + initialContents: newContentArray("content2-2", "snapuid2-2-x", "snap2-2", "sid2-2", validSecretClass, "sid2-2", "", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("content2-2", "snapuid2-2-x", "snap2-2", "sid2-2", validSecretClass, "sid2-2", "", deletionPolicy, nil, nil, false), initialSnapshots: newSnapshotArray("snap2-2", "snapuid2-2", "", "content2-2", validSecretClass, "content2-2", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap2-2", "snapuid2-2", "", "content2-2", validSecretClass, "content2-2", &False, nil, nil, newVolumeError("Snapshot failed to bind VolumeSnapshotContent, Could not bind snapshot snap2-2 and content content2-2, the VolumeSnapshotRef does not match"), false, true, nil), - initialClaims: newClaimArray("claim2-2", "pvc-uid2-2", "1Gi", "volume2-2", v1.ClaimBound, &classEmpty), - expectedEvents: []string{"Warning SnapshotBindFailed"}, + expectedSnapshots: newSnapshotArray("snap2-2", "snapuid2-2", "", "content2-2", validSecretClass, "content2-2", &False, nil, nil, newVolumeError("VolumeSnapshotContent [content2-2] is bound to a different snapshot"), false, true, nil), + expectedEvents: []string{"Warning SnapshotContentMisbound"}, errors: noerrors, test: testSyncSnapshotError, }, { - name: "2-3 - success bind snapshot and content but not ready, no status changed", - initialContents: newContentArray("content2-3", "snapuid2-3", "snap2-3", "sid2-3", validSecretClass, "", "", deletionPolicy, nil, nil, false), - expectedContents: newContentArrayWithReadyToUse("content2-3", "snapuid2-3", "snap2-3", "sid2-3", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &True, false), - initialSnapshots: newSnapshotArray("snap2-3", "snapuid2-3", "claim2-3", "", validSecretClass, "content2-3", &False, metaTimeNow, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap2-3", "snapuid2-3", "claim2-3", "", validSecretClass, "content2-3", &True, metaTimeNow, nil, nil, false, true, nil), + name: "2-3 - (dynamic) success bind snapshot and content but not ready, no status changed", + initialContents: newContentArray("snapcontent-snapuid2-3", "snapuid2-3", "snap2-3", "sid2-3", validSecretClass, "", "pv-handle2-3", deletionPolicy, nil, nil, false), + expectedContents: newContentArrayWithReadyToUse("snapcontent-snapuid2-3", "snapuid2-3", "snap2-3", "sid2-3", validSecretClass, "", "pv-handle2-3", deletionPolicy, &timeNowStamp, nil, &True, false), + initialSnapshots: newSnapshotArray("snap2-3", "snapuid2-3", "claim2-3", "", validSecretClass, "", &False, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap2-3", "snapuid2-3", "claim2-3", "", validSecretClass, "snapcontent-snapuid2-3", &True, metaTimeNow, nil, nil, false, true, nil), initialClaims: newClaimArray("claim2-3", "pvc-uid2-3", "1Gi", "volume2-3", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume2-3", "pv-uid2-3", "pv-handle2-3", "1Gi", "pvc-uid2-3", "claim2-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, @@ -83,20 +82,20 @@ func TestSync(t *testing.T) { }, { // nothing changed - name: "2-4 - noop", - initialContents: newContentArray("content2-4", "snapuid2-4", "snap2-4", "sid2-4", validSecretClass, "", "", deletionPolicy, nil, nil, false), - expectedContents: newContentArray("content2-4", "snapuid2-4", "snap2-4", "sid2-4", validSecretClass, "", "", deletionPolicy, nil, nil, false), - initialSnapshots: newSnapshotArray("snap2-4", "snapuid2-4", "claim2-4", "", validSecretClass, "content2-4", &True, metaTimeNow, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap2-4", "snapuid2-4", "claim2-4", "", validSecretClass, "content2-4", &True, metaTimeNow, nil, nil, false, true, nil), + name: "2-4 - (static) noop", + initialContents: newContentArray("content2-4", "snapuid2-4", "snap2-4", "sid2-4", validSecretClass, "sid2-4", "", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("content2-4", "snapuid2-4", "snap2-4", "sid2-4", validSecretClass, "sid2-4", "", deletionPolicy, nil, nil, false), + initialSnapshots: newSnapshotArray("snap2-4", "snapuid2-4", "", "content2-4", validSecretClass, "content2-4", &True, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap2-4", "snapuid2-4", "", "content2-4", validSecretClass, "content2-4", &True, metaTimeNow, nil, nil, false, true, nil), errors: noerrors, test: testSyncSnapshot, }, { - name: "2-5 - snapshot and content bound, status ready false -> true", - initialContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), - expectedContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), - initialSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "content2-5", &False, metaTimeNow, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "content2-5", &False, metaTimeNow, nil, nil, false, true, nil), + name: "2-5 - (dynamic) snapshot and content bound, status ready false -> true", + initialContents: newContentArrayWithReadyToUse("snapcontent-snapuid2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "pv-handle2-5", deletionPolicy, &timeNowStamp, nil, &False, false), + expectedContents: newContentArrayWithReadyToUse("snapcontent-snapuid2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "pv-handle2-5", deletionPolicy, &timeNowStamp, nil, &False, false), + initialSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "snapcontent-snapuid2-5", &False, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "snapcontent-snapuid2-5", &False, metaTimeNow, nil, nil, false, true, nil), initialClaims: newClaimArray("claim2-5", "pvc-uid2-5", "1Gi", "volume2-5", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume2-5", "pv-uid2-5", "pv-handle2-5", "1Gi", "pvc-uid2-5", "claim2-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, @@ -104,9 +103,9 @@ func TestSync(t *testing.T) { test: testSyncSnapshot, }, { - name: "2-6 - snapshot bound to prebound content correctly, status ready false -> true, ref.UID '' -> 'snapuid2-6'", - initialContents: newContentArrayWithReadyToUse("content2-6", "snapuid2-6", "snap2-6", "sid2-6", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), - expectedContents: newContentArrayWithReadyToUse("content2-6", "snapuid2-6", "snap2-6", "sid2-6", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), + name: "2-6 - (static) snapshot bound to content correctly, status ready false -> true, ref.UID '' -> 'snapuid2-6'", + initialContents: newContentArrayWithReadyToUse("content2-6", "", "snap2-6", "sid2-6", validSecretClass, "sid2-6", "", deletionPolicy, &timeNowStamp, nil, &False, false), + expectedContents: newContentArrayWithReadyToUse("content2-6", "snapuid2-6", "snap2-6", "sid2-6", validSecretClass, "sid2-6", "", deletionPolicy, &timeNowStamp, nil, &False, false), initialSnapshots: newSnapshotArray("snap2-6", "snapuid2-6", "", "content2-6", validSecretClass, "content2-6", &False, metaTimeNow, nil, nil, false, true, nil), expectedSnapshots: newSnapshotArray("snap2-6", "snapuid2-6", "", "content2-6", validSecretClass, "content2-6", &False, metaTimeNow, nil, nil, false, true, nil), errors: noerrors, @@ -118,7 +117,7 @@ func TestSync(t *testing.T) { expectedContents: newContentArrayWithReadyToUse("content2-8", "snapuid2-8", "snap2-8", "sid2-8", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), initialSnapshots: newSnapshotArray("snap2-8", "snapuid2-8", "claim2-8", "", validSecretClass, "content2-8", &False, metaTimeNow, nil, nil, false, false, nil), expectedSnapshots: newSnapshotArray("snap2-8", "snapuid2-8", "claim2-8", "", validSecretClass, "content2-8", &False, metaTimeNow, nil, nil, false, false, nil), - expectedEvents: []string{"Warning SnapshotCheckandUpdateFailed"}, + expectedEvents: []string{"Warning SnapshotFinalizerError"}, initialClaims: newClaimArray("claim2-8", "pvc-uid2-8", "1Gi", "volume2-8", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume2-8", "pv-uid2-8", "pv-handle2-8", "1Gi", "pvc-uid2-8", "claim2-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, @@ -142,44 +141,30 @@ func TestSync(t *testing.T) { }, test: testSyncSnapshot, }, { - name: "7-1 - fail to create snapshot with non-existing snapshot class", - initialContents: nocontents, - expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-1: \"volumesnapshotclass.snapshot.storage.k8s.io \\\"non-existing\\\" not found\""), false, true, nil), - initialClaims: newClaimArray("claim7-1", "pvc-uid7-1", "1Gi", "volume7-1", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume7-1", "pv-uid7-1", "pv-handle7-1", "1Gi", "pvc-uid7-1", "claim7-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - expectedEvents: []string{"Warning SnapshotContentCreationFailed"}, + name: "2-10 - (static) do not bind content donest not point to the snapshot", + initialContents: newContentArray("content2-10", "snapuid2-10-x", "snap2-10", "sid2-10", validSecretClass, "sid2-10", "", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("content2-10", "snapuid2-10-x", "snap2-10", "sid2-10", validSecretClass, "sid2-10", "", deletionPolicy, nil, nil, false), + initialSnapshots: newSnapshotArray("snap2-10", "snapuid2-10", "", "content2-10", validSecretClass, "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap2-10", "snapuid2-10", "", "content2-10", validSecretClass, "", &False, nil, nil, newVolumeError("VolumeSnapshotContent [content2-10] is bound to a different snapshot"), false, true, nil), + expectedEvents: []string{"Warning SnapshotContentMisbound"}, errors: noerrors, - expectSuccess: false, test: testSyncSnapshot, }, { - name: "2-10 - do not bind when snapshot and content not match", - initialContents: newContentArray("content2-10", "snapuid2-10-x", "snap2-10", "sid2-10", validSecretClass, "", "", deletionPolicy, nil, nil, false), - expectedContents: newContentArray("content2-10", "snapuid2-10-x", "snap2-10", "sid2-10", validSecretClass, "", "", deletionPolicy, nil, nil, false), - initialSnapshots: newSnapshotArray("snap2-10", "snapuid2-10", "claim2-10", "", validSecretClass, "", &False, nil, nil, newVolumeError("mock driver error"), false, true, nil), - expectedSnapshots: newSnapshotArray("snap2-10", "snapuid2-10", "claim2-10", "", validSecretClass, "", &False, nil, nil, newVolumeError("mock driver error"), false, true, nil), - errors: noerrors, - test: testSyncSnapshot, - }, - { - name: "2-11 - successful bind snapshot content with volume snapshot classname", - initialContents: withContentSpecSnapshotClassName(newContentArray("content2-11", "snapuid2-11", "snap2-11", "sid2-11", validSecretClass, "", "", deletionPolicy, nil, nil, false), nil), - expectedContents: newContentArray("content2-11", "snapuid2-11", "snap2-11", "sid2-11", validSecretClass, "", "", deletionPolicy, nil, nil, false), + name: "2-11 - (static) successful bind snapshot content with content classname updated", + initialContents: withContentSpecSnapshotClassName(newContentArray("content2-11", "snapuid2-11", "snap2-11", "sid2-11", validSecretClass, "sid2-11", "", deletionPolicy, nil, nil, false), nil), + expectedContents: newContentArray("content2-11", "snapuid2-11", "snap2-11", "sid2-11", validSecretClass, "sid2-11", "", deletionPolicy, nil, nil, false), initialSnapshots: newSnapshotArray("snap2-11", "snapuid2-11", "", "content2-11", validSecretClass, "content2-11", &False, nil, nil, nil, false, true, nil), expectedSnapshots: newSnapshotArray("snap2-11", "snapuid2-11", "", "content2-11", validSecretClass, "content2-11", &True, nil, nil, nil, false, true, nil), - initialClaims: newClaimArray("claim2-11", "pvc-uid2-11", "1Gi", "volume2-11", v1.ClaimBound, &classEmpty), errors: noerrors, test: testSyncSnapshot, }, { - name: "2-12 - fail bind snapshot content with volume snapshot classname due to API call failed", - initialContents: withContentSpecSnapshotClassName(newContentArray("content2-12", "snapuid2-12", "snap2-12", "sid2-12", validSecretClass, "", "", deletionPolicy, nil, nil, false), nil), - expectedContents: withContentSpecSnapshotClassName(newContentArray("content2-12", "snapuid2-12", "snap2-12", "sid2-12", validSecretClass, "", "", deletionPolicy, nil, nil, false), nil), + name: "2-12 - (static) fail bind snapshot content with volume snapshot classname due to API call failed", + initialContents: withContentSpecSnapshotClassName(newContentArray("content2-12", "snapuid2-12", "snap2-12", "sid2-12", validSecretClass, "sid2-12", "", deletionPolicy, nil, nil, false), nil), + expectedContents: withContentSpecSnapshotClassName(newContentArray("content2-12", "snapuid2-12", "snap2-12", "sid2-12", validSecretClass, "sid2-12", "", deletionPolicy, nil, nil, false), nil), initialSnapshots: newSnapshotArray("snap2-12", "snapuid2-12", "", "content2-12", validSecretClass, "content2-12", &False, nil, nil, nil, false, true, nil), expectedSnapshots: newSnapshotArray("snap2-12", "snapuid2-12", "", "content2-12", validSecretClass, "content2-12", &False, nil, nil, newVolumeError("Snapshot failed to bind VolumeSnapshotContent, mock update error"), false, true, nil), - initialClaims: newClaimArray("claim2-12", "pvc-uid2-12", "1Gi", "volume2-12", v1.ClaimBound, &classEmpty), errors: []reactorError{ // Inject error to the forth client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. {"update", "volumesnapshotcontents", errors.New("mock update error")}, @@ -187,83 +172,136 @@ func TestSync(t *testing.T) { test: testSyncSnapshot, }, { - name: "3-1 - ready snapshot lost reference to VolumeSnapshotContent", + name: "2-13 - (dynamic) snapshot expects a dynamically provisioned content but found one which is pre-proviioned, bind should fail", + initialContents: newContentArray("snapcontent-snapuid2-13", "snapuid2-13", "snap2-13", "sid2-13", validSecretClass, "sid2-13", "", deletionPolicy, nil, nil, false), + expectedContents: newContentArrayWithReadyToUse("snapcontent-snapuid2-13", "snapuid2-13", "snap2-13", "sid2-13", validSecretClass, "sid2-13", "", deletionPolicy, &timeNowStamp, nil, &True, false), + initialSnapshots: newSnapshotArray("snap2-13", "snapuid2-13", "claim2-13", "", validSecretClass, "", &False, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap2-13", "snapuid2-13", "claim2-13", "", validSecretClass, "", &False, metaTimeNow, nil, newVolumeError("VolumeSnapshotContent is pre-provisioned while expecting a dynamically provisioned one"), false, true, nil), + initialClaims: newClaimArray("claim2-13", "pvc-uid2-13", "1Gi", "volume2-13", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume2-13", "pv-uid2-13", "pv-handle2-13", "1Gi", "pvc-uid2-13", "claim2-13", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + expectedEvents: []string{"Warning SnapshotContentMismatch"}, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, + { + // nothing changed + name: "2-14 - (dynamic) noop", + initialContents: newContentArray("snapcontent-snapuid2-14", "snapuid2-14", "snap2-14", "sid2-14", validSecretClass, "", "pv-handle-2-14", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("snapcontent-snapuid2-14", "snapuid2-14", "snap2-14", "sid2-14", validSecretClass, "", "pv-handle-2-14", deletionPolicy, nil, nil, false), + initialSnapshots: newSnapshotArray("snap2-14", "snapuid2-14", "claim2-14", "", validSecretClass, "snapcontent-snapuid2-14", &True, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap2-14", "snapuid2-14", "claim2-14", "", validSecretClass, "snapcontent-snapuid2-14", &True, metaTimeNow, nil, nil, false, true, nil), + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "3-1 - (dynamic) ready snapshot lost reference to VolumeSnapshotContent", initialContents: nocontents, expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "content3-1", &True, metaTimeNow, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "content3-1", &False, metaTimeNow, nil, newVolumeError("VolumeSnapshotContent is missing"), false, true, nil), + initialSnapshots: newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "snapcontent-snapuid3-1", &True, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap3-1", "snapuid3-1", "claim3-1", "", validSecretClass, "snapcontent-snapuid3-1", &False, metaTimeNow, nil, newVolumeError("VolumeSnapshotContent is missing"), false, true, nil), errors: noerrors, expectedEvents: []string{"Warning SnapshotContentMissing"}, test: testSyncSnapshot, }, { - name: "3-2 - ready snapshot bound to none-exist content", + name: "3-2 - (static) ready snapshot bound to none-exist content", initialContents: nocontents, expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap3-2", "snapuid3-2", "claim3-2", "", validSecretClass, "content3-2", &True, metaTimeNow, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap3-2", "snapuid3-2", "claim3-2", "", validSecretClass, "content3-2", &False, metaTimeNow, nil, newVolumeError("VolumeSnapshotContent is missing"), false, true, nil), + initialSnapshots: newSnapshotArray("snap3-2", "snapuid3-2", "", "content3-2", validSecretClass, "content3-2", &True, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap3-2", "snapuid3-2", "", "content3-2", validSecretClass, "content3-2", &False, metaTimeNow, nil, newVolumeError("VolumeSnapshotContent is missing"), false, true, nil), errors: noerrors, expectedEvents: []string{"Warning SnapshotContentMissing"}, test: testSyncSnapshot, }, { - name: "3-3 - ready snapshot(everything is well, do nothing)", - initialContents: newContentArray("content3-3", "snapuid3-3", "snap3-3", "sid3-3", validSecretClass, "", "", deletionPolicy, nil, nil, false), - expectedContents: newContentArray("content3-3", "snapuid3-3", "snap3-3", "sid3-3", validSecretClass, "", "", deletionPolicy, nil, nil, false), - initialSnapshots: newSnapshotArray("snap3-3", "snapuid3-3", "claim3-3", "", validSecretClass, "content3-3", &True, metaTimeNow, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap3-3", "snapuid3-3", "claim3-3", "", validSecretClass, "content3-3", &True, metaTimeNow, nil, nil, false, true, nil), + name: "3-3 - (static) ready snapshot(everything is well, do nothing)", + initialContents: newContentArray("content3-3", "snapuid3-3", "snap3-3", "sid3-3", validSecretClass, "sid3-3", "", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("content3-3", "snapuid3-3", "snap3-3", "sid3-3", validSecretClass, "sid3-3", "", deletionPolicy, nil, nil, false), + initialSnapshots: newSnapshotArray("snap3-3", "snapuid3-3", "", "content3-3", validSecretClass, "content3-3", &True, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap3-3", "snapuid3-3", "", "content3-3", validSecretClass, "content3-3", &True, metaTimeNow, nil, nil, false, true, nil), errors: noerrors, test: testSyncSnapshot, }, { - name: "3-4 - ready snapshot misbound to VolumeSnapshotContent", - initialContents: newContentArray("content3-4", "snapuid3-4-x", "snap3-4", "sid3-4", validSecretClass, "", "", deletionPolicy, nil, nil, false), - expectedContents: newContentArray("content3-4", "snapuid3-4-x", "snap3-4", "sid3-4", validSecretClass, "", "", deletionPolicy, nil, nil, false), - initialSnapshots: newSnapshotArray("snap3-4", "snapuid3-4", "claim3-4", "", validSecretClass, "content3-4", &True, metaTimeNow, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap3-4", "snapuid3-4", "claim3-4", "", validSecretClass, "content3-4", &False, metaTimeNow, nil, newVolumeError("VolumeSnapshotContent is not bound to the VolumeSnapshot correctly"), false, true, nil), + name: "3-4 - (static) ready snapshot misbound to VolumeSnapshotContent", + initialContents: newContentArray("content3-4", "snapuid3-4-x", "snap3-4", "sid3-4", validSecretClass, "sid3-4", "", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("content3-4", "snapuid3-4-x", "snap3-4", "sid3-4", validSecretClass, "sid3-4", "", deletionPolicy, nil, nil, false), + initialSnapshots: newSnapshotArray("snap3-4", "snapuid3-4", "", "content3-4", validSecretClass, "content3-4", &True, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap3-4", "snapuid3-4", "", "content3-4", validSecretClass, "content3-4", &False, metaTimeNow, nil, newVolumeError("VolumeSnapshotContent [content3-4] is bound to a different snapshot"), false, true, nil), + expectedEvents: []string{"Warning SnapshotContentMisbound"}, errors: noerrors, test: testSyncSnapshot, }, { - name: "4-1 - content bound to snapshot, snapshot status missing and rebuilt", - initialContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, nil, &size, &True, false), - expectedContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, nil, &size, &True, false), - initialSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "", &False, nil, nil, nil, true, true, nil), - expectedSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "content2-5", &True, nil, getSize(1), nil, false, true, nil), - initialClaims: newClaimArray("claim2-5", "pvc-uid2-5", "1Gi", "volume2-5", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume2-5", "pv-uid2-5", "pv-handle2-5", "1Gi", "pvc-uid2-5", "claim2-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + name: "3-5 - (dynamic) ready snapshot(everything is well, do nothing)", + initialContents: newContentArray("snapcontent-snapuid3-5", "snapuid3-5", "snap3-5", "sid3-5", validSecretClass, "", "volume-handle-3-5", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("snapcontent-snapuid3-5", "snapuid3-5", "snap3-5", "sid3-5", validSecretClass, "", "volume-handle-3-5", deletionPolicy, nil, nil, false), + initialSnapshots: newSnapshotArray("snap3-5", "snapuid3-5", "claim3-5", "", validSecretClass, "snapcontent-snapuid3-5", &True, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap3-5", "snapuid3-5", "claim3-5", "", validSecretClass, "snapcontent-snapuid3-5", &True, metaTimeNow, nil, nil, false, true, nil), + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "3-6 - (dynamic) ready snapshot misbound to VolumeSnapshotContent", + initialContents: newContentArray("snapcontent-snapuid3-6", "snapuid3-6-x", "snap3-6", "sid3-6", validSecretClass, "", "volume-handle-3-6", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("snapcontent-snapuid3-6", "snapuid3-6-x", "snap3-6", "sid3-6", validSecretClass, "", "volume-handle-3-6", deletionPolicy, nil, nil, false), + initialSnapshots: newSnapshotArray("snap3-6", "snapuid3-6", "claim3-6", "", validSecretClass, "snapcontent-snapuid3-6", &True, metaTimeNow, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap3-6", "snapuid3-6", "claim3-6", "", validSecretClass, "snapcontent-snapuid3-6", &False, metaTimeNow, nil, newVolumeError("VolumeSnapshotContent [snapcontent-snapuid3-6] is bound to a different snapshot"), false, true, nil), + expectedEvents: []string{"Warning SnapshotContentMisbound"}, + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "4-1 - (dynamic) content bound to snapshot, snapshot status missing and rebuilt", + initialContents: newContentArrayWithReadyToUse("snapcontent-snapuid4-1", "snapuid4-1", "snap4-1", "sid4-1", validSecretClass, "", "pv-handle4-1", deletionPolicy, nil, &size, &True, false), + expectedContents: newContentArrayWithReadyToUse("snapcontent-snapuid4-1", "snapuid4-1", "snap4-1", "sid4-1", validSecretClass, "", "pv-handle4-1", deletionPolicy, nil, &size, &True, false), + initialSnapshots: newSnapshotArray("snap4-1", "snapuid4-1", "claim4-1", "", validSecretClass, "", &False, nil, nil, nil, true, true, nil), + expectedSnapshots: newSnapshotArray("snap4-1", "snapuid4-1", "claim4-1", "", validSecretClass, "snapcontent-snapuid4-1", &True, nil, getSize(1), nil, false, true, nil), + initialClaims: newClaimArray("claim4-1", "pvc-uid4-1", "1Gi", "volume4-1", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume4-1", "pv-uid4-1", "pv-handle4-1", "1Gi", "pvc-uid4-1", "claim4-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, errors: noerrors, test: testSyncSnapshot, }, { - name: "4-2 - snapshot and content bound, ReadyToUse in snapshot status missing and rebuilt", - initialContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, nil, nil, &True, false), - expectedContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, nil, nil, &True, false), - initialSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "content2-5", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "content2-5", &True, nil, nil, nil, false, true, nil), - initialClaims: newClaimArray("claim2-5", "pvc-uid2-5", "1Gi", "volume2-5", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume2-5", "pv-uid2-5", "pv-handle2-5", "1Gi", "pvc-uid2-5", "claim2-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + name: "4-2 - (dynamic) snapshot and content bound, ReadyToUse in snapshot status missing and rebuilt", + initialContents: newContentArrayWithReadyToUse("snapcontent-snapuid4-2", "snapuid4-2", "snap4-2", "sid4-2", validSecretClass, "", "pv-handle4-2", deletionPolicy, nil, nil, &True, false), + expectedContents: newContentArrayWithReadyToUse("snapcontent-snapuid4-2", "snapuid4-2", "snap4-2", "sid4-2", validSecretClass, "", "pv-handle4-2", deletionPolicy, nil, nil, &True, false), + initialSnapshots: newSnapshotArray("snap4-2", "snapuid4-2", "claim4-2", "", validSecretClass, "snapcontent-snapuid4-2", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap4-2", "snapuid4-2", "claim4-2", "", validSecretClass, "snapcontent-snapuid4-2", &True, nil, nil, nil, false, true, nil), + initialClaims: newClaimArray("claim4-2", "pvc-uid4-2", "1Gi", "volume4-2", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume4-2", "pv-uid4-2", "pv-handle4-2", "1Gi", "pvc-uid4-2", "claim4-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, errors: noerrors, test: testSyncSnapshot, }, { - name: "4-3 - content bound to snapshot, fields in snapshot status missing and rebuilt", - initialContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, nil, &size, &True, false), - expectedContents: newContentArrayWithReadyToUse("content2-5", "snapuid2-5", "snap2-5", "sid2-5", validSecretClass, "", "", deletionPolicy, nil, &size, &True, false), - initialSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap2-5", "snapuid2-5", "claim2-5", "", validSecretClass, "content2-5", &True, nil, getSize(1), nil, false, true, nil), - initialClaims: newClaimArray("claim2-5", "pvc-uid2-5", "1Gi", "volume2-5", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume2-5", "pv-uid2-5", "pv-handle2-5", "1Gi", "pvc-uid2-5", "claim2-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + name: "4-3 - (dynamic) content bound to snapshot, fields in snapshot status missing and rebuilt", + initialContents: newContentArrayWithReadyToUse("snapcontent-snapuid4-3", "snapuid4-3", "snap4-3", "sid4-3", validSecretClass, "", "pv-handle4-3", deletionPolicy, nil, &size, &True, false), + expectedContents: newContentArrayWithReadyToUse("snapcontent-snapuid4-3", "snapuid4-3", "snap4-3", "sid4-3", validSecretClass, "", "pv-handle4-3", deletionPolicy, nil, &size, &True, false), + initialSnapshots: newSnapshotArray("snap4-3", "snapuid4-3", "claim4-3", "", validSecretClass, "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap4-3", "snapuid4-3", "claim4-3", "", validSecretClass, "snapcontent-snapuid4-3", &True, nil, getSize(1), nil, false, true, nil), + initialClaims: newClaimArray("claim4-3", "pvc-uid4-3", "1Gi", "volume4-3", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume4-3", "pv-uid4-3", "pv-handle4-3", "1Gi", "pvc-uid4-3", "claim4-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "4-4 - (dynamic) content bound to snapshot, fields in snapshot status missing and rebuilt", + initialContents: newContentArrayWithReadyToUse("content4-4", "snapuid4-4", "snap4-4", "sid4-4", validSecretClass, "sid4-4", "", deletionPolicy, nil, &size, &True, false), + expectedContents: newContentArrayWithReadyToUse("content4-4", "snapuid4-4", "snap4-4", "sid4-4", validSecretClass, "sid4-4", "", deletionPolicy, nil, &size, &True, false), + initialSnapshots: newSnapshotArray("snap4-4", "snapuid4-4", "", "content4-4", validSecretClass, "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap4-4", "snapuid4-4", "", "content4-4", validSecretClass, "content4-4", &True, nil, getSize(1), nil, false, true, nil), initialSecrets: []*v1.Secret{secret()}, errors: noerrors, test: testSyncSnapshot, }, { name: "5-1 - content missing finalizer is updated to have finalizer", - initialContents: newContentArray("content5-1", "snapuid5-1", "snap5-1", "sid5-1", validSecretClass, "", "", deletionPolicy, nil, nil, false), - expectedContents: newContentArray("content5-1", "snapuid5-1", "snap5-1", "sid5-1", validSecretClass, "", "", deletionPolicy, nil, nil, true), + initialContents: newContentArray("content5-1", "snapuid5-1", "snap5-1", "sid5-1", validSecretClass, "", "pv-handle5-1", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("content5-1", "snapuid5-1", "snap5-1", "sid5-1", validSecretClass, "", "pv-handle5-1", deletionPolicy, nil, nil, true), initialClaims: newClaimArray("claim5-1", "pvc-uid5-1", "1Gi", "volume5-1", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume5-1", "pv-uid5-1", "pv-handle5-1", "1Gi", "pvc-uid5-1", "claim5-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, @@ -272,8 +310,8 @@ func TestSync(t *testing.T) { }, { name: "5-2 - content missing finalizer update attempt fails because of failed API call", - initialContents: newContentArray("content5-2", "snapuid5-2", "snap5-2", "sid5-2", validSecretClass, "", "", deletionPolicy, nil, nil, false), - expectedContents: newContentArray("content5-2", "snapuid5-2", "snap5-2", "sid5-2", validSecretClass, "", "", deletionPolicy, nil, nil, false), + initialContents: newContentArray("content5-2", "snapuid5-2", "snap5-2", "sid5-2", validSecretClass, "", "pv-handle5-2", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("content5-2", "snapuid5-2", "snap5-2", "sid5-2", validSecretClass, "", "pv-handle5-2", deletionPolicy, nil, nil, false), initialClaims: newClaimArray("claim5-2", "pvc-uid5-2", "1Gi", "volume5-2", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume5-2", "pv-uid5-2", "pv-handle5-2", "1Gi", "pvc-uid5-2", "claim5-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, @@ -285,11 +323,11 @@ func TestSync(t *testing.T) { test: testSyncContentError, }, { - name: "5-3 - snapshot deletion candidate marked for deletion", - initialSnapshots: newSnapshotArray("snap5-3", "snapuid5-3", "claim5-3", "", validSecretClass, "content5-3", &False, nil, nil, nil, false, true, &timeNowMetav1), - expectedSnapshots: newSnapshotArray("snap5-3", "snapuid5-3", "claim5-3", "", validSecretClass, "content5-3", &False, nil, nil, nil, false, true, &timeNowMetav1), - initialContents: newContentArray("content5-3", "snapuid5-3", "snap5-3", "sid5-3", validSecretClass, "", "", deletionPolicy, nil, nil, true), - expectedContents: withContentAnnotations(newContentArray("content5-3", "snapuid5-3", "snap5-3", "sid5-3", validSecretClass, "", "", deletionPolicy, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), + name: "5-3 - (dynamic) snapshot deletion candidate marked for deletion", + initialSnapshots: newSnapshotArray("snap5-3", "snapuid5-3", "claim5-3", "", validSecretClass, "snapcontent-snapuid5-3", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap5-3", "snapuid5-3", "claim5-3", "", validSecretClass, "snapcontent-snapuid5-3", &False, nil, nil, nil, false, true, &timeNowMetav1), + initialContents: newContentArray("snapcontent-snapuid5-3", "snapuid5-3", "snap5-3", "sid5-3", validSecretClass, "", "pv-handle5-3", deletionPolicy, nil, nil, true), + expectedContents: withContentAnnotations(newContentArray("snapcontent-snapuid5-3", "snapuid5-3", "snap5-3", "sid5-3", validSecretClass, "", "pv-handle5-3", deletionPolicy, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), initialClaims: newClaimArray("claim5-3", "pvc-uid5-3", "1Gi", "volume5-3", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume5-3", "pv-uid5-3", "pv-handle5-3", "1Gi", "pvc-uid5-3", "claim5-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, @@ -297,12 +335,12 @@ func TestSync(t *testing.T) { test: testSyncContent, }, { - name: "5-4 - snapshot deletion candidate fail to mark for deletion due to failed API call", - initialSnapshots: newSnapshotArray("snap5-4", "snapuid5-4", "claim5-4", "", validSecretClass, "content5-4", &False, nil, nil, nil, false, true, &timeNowMetav1), - expectedSnapshots: newSnapshotArray("snap5-4", "snapuid5-4", "claim5-4", "", validSecretClass, "content5-4", &False, nil, nil, nil, false, true, &timeNowMetav1), - initialContents: newContentArray("content5-4", "snapuid5-4", "snap5-4", "sid5-4", validSecretClass, "", "", deletionPolicy, nil, nil, true), + name: "5-4 - (dynamic) snapshot deletion candidate fail to mark for deletion due to failed API call", + initialSnapshots: newSnapshotArray("snap5-4", "snapuid5-4", "claim5-4", "", validSecretClass, "snapcontent-snapuid5-4", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap5-4", "snapuid5-4", "claim5-4", "", validSecretClass, "snapcontent-snapuid5-4", &False, nil, nil, nil, false, true, &timeNowMetav1), + initialContents: newContentArray("snapcontent-snapuid5-4", "snapuid5-4", "snap5-4", "sid5-4", validSecretClass, "", "pv-handle5-4", deletionPolicy, nil, nil, true), // result of the test framework - annotation is still set in memory, but update call fails. - expectedContents: withContentAnnotations(newContentArray("content5-4", "snapuid5-4", "snap5-4", "sid5-4", validSecretClass, "", "", deletionPolicy, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), + expectedContents: withContentAnnotations(newContentArray("snapcontent-snapuid5-4", "snapuid5-4", "snap5-4", "sid5-4", validSecretClass, "", "pv-handle5-4", deletionPolicy, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), initialClaims: newClaimArray("claim5-4", "pvc-uid5-4", "1Gi", "volume5-4", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume5-4", "pv-uid5-4", "pv-handle5-4", "1Gi", "pvc-uid5-4", "claim5-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, @@ -314,18 +352,64 @@ func TestSync(t *testing.T) { test: testSyncContentError, }, { - name: "5-5 - snapshot deletion candidate marked for deletion by syncSnapshot", - initialSnapshots: newSnapshotArray("snap5-5", "snapuid5-5", "claim5-5", "", validSecretClass, "content5-5", &True, nil, nil, nil, false, true, &timeNowMetav1), - expectedSnapshots: withSnapshotFinalizers(newSnapshotArray("snap5-5", "snapuid5-5", "claim5-5", "", validSecretClass, "content5-5", &True, nil, nil, nil, false, false, &timeNowMetav1), - utils.VolumeSnapshotBoundFinalizer, - ), - initialContents: newContentArray("content5-5", "snapuid5-5", "snap5-5", "sid5-5", validSecretClass, "", "", crdv1.VolumeSnapshotContentRetain, nil, nil, true), - expectedContents: withContentAnnotations(newContentArray("content5-5", "snapuid5-5", "snap5-5", "sid5-5", validSecretClass, "", "", crdv1.VolumeSnapshotContentRetain, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), - initialClaims: newClaimArray("claim5-5", "pvc-uid5-5", "1Gi", "volume5-5", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume5-5", "pv-uid5-5", "pv-handle5-5", "1Gi", "pvc-uid5-5", "claim5-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + name: "5-5 - (dynamic) snapshot deletion candidate marked for deletion by syncSnapshot", + initialSnapshots: newSnapshotArray("snap5-5", "snapuid5-5", "claim5-5", "", validSecretClass, "snapcontent-snapuid5-5", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap5-5", "snapuid5-5", "claim5-5", "", validSecretClass, "snapcontent-snapuid5-5", &False, nil, nil, nil, false, false, &timeNowMetav1), + initialContents: newContentArray("snapcontent-snapuid5-5", "snapuid5-5", "snap5-5", "sid5-5", validSecretClass, "", "pv-handle5-5", crdv1.VolumeSnapshotContentRetain, nil, nil, true), + expectedContents: withContentAnnotations(newContentArray("snapcontent-snapuid5-5", "snapuid5-5", "snap5-5", "sid5-5", validSecretClass, "", "pv-handle5-5", crdv1.VolumeSnapshotContentRetain, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), + initialClaims: newClaimArray("claim5-5", "pvc-uid5-5", "1Gi", "volume5-5", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume5-5", "pv-uid5-5", "pv-handle5-5", "1Gi", "pvc-uid5-5", "claim5-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + initialSecrets: []*v1.Secret{secret()}, + expectSuccess: true, + test: testSyncSnapshot, + }, + { + name: "5-6 - (static) snapshot deletion candidate marked for deletion", + initialSnapshots: newSnapshotArray("snap5-6", "snapuid5-6", "", "content5-6", validSecretClass, "content5-6", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap5-6", "snapuid5-6", "", "content5-6", validSecretClass, "content5-6", &False, nil, nil, nil, false, true, &timeNowMetav1), + initialContents: newContentArray("content5-6", "snapuid5-6", "snap5-6", "sid5-6", validSecretClass, "sid5-6", "", deletionPolicy, nil, nil, true), + expectedContents: withContentAnnotations(newContentArray("content5-6", "snapuid5-6", "snap5-6", "sid5-6", validSecretClass, "sid5-6", "", deletionPolicy, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), + initialSecrets: []*v1.Secret{secret()}, + expectSuccess: true, + test: testSyncContent, + }, + { + name: "5-7 - (static) snapshot deletion candidate fail to mark for deletion due to failed API call", + initialSnapshots: newSnapshotArray("snap5-7", "snapuid5-7", "", "content5-7", validSecretClass, "content5-7", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap5-7", "snapuid5-7", "", "content5-7", validSecretClass, "content5-7", &False, nil, nil, nil, false, true, &timeNowMetav1), + initialContents: newContentArray("content5-7", "snapuid5-7", "snap5-7", "sid5-7", validSecretClass, "sid5-7", "", deletionPolicy, nil, nil, true), + // result of the test framework - annotation is still set in memory, but update call fails. + expectedContents: withContentAnnotations(newContentArray("content5-7", "snapuid5-7", "snap5-7", "sid5-7", validSecretClass, "sid5-7", "", deletionPolicy, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), initialSecrets: []*v1.Secret{secret()}, - expectSuccess: true, - test: testSyncSnapshot, + errors: []reactorError{ + // Inject error to the forth client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. + {"update", "volumesnapshotcontents", errors.New("mock update error")}, + }, + expectSuccess: false, + test: testSyncContentError, + }, + { + name: "5-8 - (dynamic) snapshot deletion candidate marked for deletion by syncSnapshot", + initialSnapshots: newSnapshotArray("snap5-8", "snapuid5-8", "", "content5-8", validSecretClass, "content5-8", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap5-8", "snapuid5-8", "", "content5-8", validSecretClass, "content5-8", &False, nil, nil, nil, false, false, &timeNowMetav1), + initialContents: newContentArray("content5-8", "snapuid5-8", "snap5-8", "sid5-8", validSecretClass, "sid5-8", "", crdv1.VolumeSnapshotContentRetain, nil, nil, true), + expectedContents: withContentAnnotations(newContentArray("content5-8", "snapuid5-8", "snap5-8", "sid5-8", validSecretClass, "sid5-8", "", crdv1.VolumeSnapshotContentRetain, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), + initialSecrets: []*v1.Secret{secret()}, + expectSuccess: true, + test: testSyncSnapshot, + }, + { + name: "7-1 - fail to create snapshot with non-existing snapshot class", + initialContents: nocontents, + expectedContents: nocontents, + initialSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-1: \"failed to retrieve snapshot class non-existing from the informer: \\\"volumesnapshotclass.snapshot.storage.k8s.io \\\\\\\"non-existing\\\\\\\" not found\\\"\""), false, true, nil), + initialClaims: newClaimArray("claim7-1", "pvc-uid7-1", "1Gi", "volume7-1", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume7-1", "pv-uid7-1", "pv-handle7-1", "1Gi", "pvc-uid7-1", "claim7-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + expectedEvents: []string{"Warning SnapshotContentCreationFailed"}, + errors: noerrors, + expectSuccess: false, + test: testSyncSnapshot, }, } diff --git a/pkg/utils/util.go b/pkg/utils/util.go index 99fcdc9aa..249f8cb70 100644 --- a/pkg/utils/util.go +++ b/pkg/utils/util.go @@ -173,15 +173,9 @@ func StoreObjectUpdate(store cache.Store, obj interface{}, className string) (bo return true, nil } -// GetSnapshotContentNameForSnapshot returns SnapshotContent.Name for the create VolumeSnapshotContent. -// The name must be unique. -func GetSnapshotContentNameForSnapshot(snapshot *crdv1.VolumeSnapshot) string { - // If VolumeSnapshot object has SnapshotContentName, use it directly. - // This might be the case for static provisioning. - if snapshot.Spec.Source.VolumeSnapshotContentName != nil { - return *snapshot.Spec.Source.VolumeSnapshotContentName - } - // Construct SnapshotContentName for dynamic provisioning. +// GetDynamicSnapshotContentNameForSnapshot returns a unique content name for the +// passed in VolumeSnapshot to dynamically provision a snapshot. +func GetDynamicSnapshotContentNameForSnapshot(snapshot *crdv1.VolumeSnapshot) string { return "snapcontent-" + string(snapshot.UID) } @@ -408,14 +402,6 @@ func GetSnapshotStatusForLogging(snapshot *crdv1.VolumeSnapshot) string { return fmt.Sprintf("bound to: %q, Completed: %v", snapshotContentName, ready) } -// IsSnapshotBound returns true/false if snapshot is bound -func IsSnapshotBound(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) bool { - if IsVolumeSnapshotRefSet(snapshot, content) && IsBoundVolumeSnapshotContentNameSet(snapshot) { - return true - } - return false -} - func IsVolumeSnapshotRefSet(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) bool { if content.Spec.VolumeSnapshotRef.Name == snapshot.Name && content.Spec.VolumeSnapshotRef.Namespace == snapshot.Namespace &&