Skip to content

Commit

Permalink
Merge pull request #11582 from kubevirt-bot/cherry-pick-11095-to-rele…
Browse files Browse the repository at this point in the history 
…ase-1.2

[release-1.2] Update vmsnapshot error phase in case of volumesnapshot errors which prevent the snapshot from being ready
  • Loading branch information
@kubevirt-bot
kubevirt-bot committed Mar 31, 2024
2 parents 7bfcb03 + 2ef27b0 commit 2b90c7b
Show file tree
Hide file tree
Showing 3 changed files with 225 additions and 30 deletions.
68 changes: 39 additions & 29 deletions pkg/storage/snapshot/snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ func vmSnapshotSucceeded(vmSnapshot *snapshotv1.VirtualMachineSnapshot) bool {
}

func vmSnapshotProgressing(vmSnapshot *snapshotv1.VirtualMachineSnapshot) bool {
return vmSnapshotError(vmSnapshot) == nil && !VmSnapshotReady(vmSnapshot) &&
!vmSnapshotFailed(vmSnapshot) && !vmSnapshotSucceeded(vmSnapshot)
return !VmSnapshotReady(vmSnapshot) && !vmSnapshotFailed(vmSnapshot) && !vmSnapshotSucceeded(vmSnapshot)
}

func deleteContentPolicy(vmSnapshot *snapshotv1.VirtualMachineSnapshot) bool {
Expand Down Expand Up @@ -131,7 +130,7 @@ func vmSnapshotDeadlineExceeded(vmSnapshot *snapshotv1.VirtualMachineSnapshot) b
if vmSnapshotFailed(vmSnapshot) {
return true
}
if vmSnapshot.Status == nil || vmSnapshot.Status.Phase != snapshotv1.InProgress {
if !vmSnapshotProgressing(vmSnapshot) {
return false
}
return timeUntilDeadline(vmSnapshot) < 0
Expand Down Expand Up @@ -292,7 +291,7 @@ func (ctrl *VMSnapshotController) updateVMSnapshotContent(content *snapshotv1.Vi

}

currentlyCreated := vmSnapshotContentCreated(content)
contentCreated := vmSnapshotContentCreated(content)
currentlyError := (content.Status != nil && content.Status.Error != nil) || vmSnapshotError(vmSnapshot) != nil

for _, volumeBackup := range content.Spec.VolumeBackups {
Expand All @@ -308,8 +307,8 @@ func (ctrl *VMSnapshotController) updateVMSnapshotContent(content *snapshotv1.Vi
}

if volumeSnapshot == nil {
// check if snapshot was deleted
if currentlyCreated {
// check if content was created and snapshot was deleted
if contentCreated {
log.Log.Warningf("VolumeSnapshot %s no longer exists", vsName)
ctrl.Recorder.Eventf(
content,
Expand Down Expand Up @@ -406,10 +405,13 @@ func (ctrl *VMSnapshotController) updateVMSnapshotContent(content *snapshotv1.Vi
if vss.CreationTime == nil {
created = false
}

if vss.ReadyToUse == nil || !*vss.ReadyToUse {
ready = false
}
if vss.Error != nil && vss.Error.Message != nil {
errorMessage = fmt.Sprintf("VolumeSnapshot %s error: %s", vss.VolumeSnapshotName, *vss.Error.Message)
break
}
}
}

Expand All @@ -422,7 +424,7 @@ func (ctrl *VMSnapshotController) updateVMSnapshotContent(content *snapshotv1.Vi
}
}

if errorMessage != "" {
if errorMessage != "" && !ready {
contentCpy.Status.Error = &snapshotv1.Error{
Time: currentTime(),
Message: &errorMessage,
Expand Down Expand Up @@ -706,6 +708,9 @@ func (ctrl *VMSnapshotController) updateSnapshotStatus(vmSnapshot *snapshotv1.Vi
vmSnapshotCpy.Status.Phase = snapshotv1.Failed
updateSnapshotCondition(vmSnapshotCpy, newProgressingCondition(corev1.ConditionFalse, vmSnapshotDeadlineExceededError))
updateSnapshotCondition(vmSnapshotCpy, newFailureCondition(corev1.ConditionTrue, vmSnapshotDeadlineExceededError))
} else if vmSnapshotError(vmSnapshotCpy) != nil {
updateSnapshotCondition(vmSnapshotCpy, newProgressingCondition(corev1.ConditionFalse, "In error state"))
updateSnapshotCondition(vmSnapshotCpy, newReadyCondition(corev1.ConditionFalse, "Error"))
} else if vmSnapshotProgressing(vmSnapshotCpy) {
vmSnapshotCpy.Status.Phase = snapshotv1.InProgress
if source != nil {
Expand All @@ -715,27 +720,10 @@ func (ctrl *VMSnapshotController) updateSnapshotStatus(vmSnapshot *snapshotv1.Vi
updateSnapshotCondition(vmSnapshotCpy, newProgressingCondition(corev1.ConditionFalse, "Source not locked"))
}

online, err := source.Online()
indications, err := updateVMSnapshotIndications(source)
if err != nil {
return err
}

indications := []snapshotv1.Indication{}
if online {
indications = append(indications, snapshotv1.VMSnapshotOnlineSnapshotIndication)

ga, err := source.GuestAgent()
if err != nil {
return err
}

if ga {
indications = append(indications, snapshotv1.VMSnapshotGuestAgentIndication)

} else {
indications = append(indications, snapshotv1.VMSnapshotNoGuestAgentIndication)
}
}
vmSnapshotCpy.Status.Indications = indications
} else {
updateSnapshotCondition(vmSnapshotCpy, newProgressingCondition(corev1.ConditionFalse, "Source does not exist"))
Expand All @@ -746,9 +734,6 @@ func (ctrl *VMSnapshotController) updateSnapshotStatus(vmSnapshot *snapshotv1.Vi
updateSnapshotCondition(vmSnapshotCpy, newProgressingCondition(corev1.ConditionFalse, "VM snapshot is deleting"))
updateSnapshotCondition(vmSnapshotCpy, newReadyCondition(corev1.ConditionFalse, "VM snapshot is deleting"))
}
} else if vmSnapshotError(vmSnapshotCpy) != nil {
updateSnapshotCondition(vmSnapshotCpy, newProgressingCondition(corev1.ConditionFalse, "In error state"))
updateSnapshotCondition(vmSnapshotCpy, newReadyCondition(corev1.ConditionFalse, "Error"))
} else if VmSnapshotReady(vmSnapshotCpy) {
vmSnapshotCpy.Status.Phase = snapshotv1.Succeeded
updateSnapshotCondition(vmSnapshotCpy, newProgressingCondition(corev1.ConditionFalse, "Operation complete"))
Expand All @@ -769,6 +754,31 @@ func (ctrl *VMSnapshotController) updateSnapshotStatus(vmSnapshot *snapshotv1.Vi
return nil
}

func updateVMSnapshotIndications(source snapshotSource) ([]snapshotv1.Indication, error) {
indications := []snapshotv1.Indication{}
online, err := source.Online()
if err != nil {
return indications, err
}

if online {
indications = append(indications, snapshotv1.VMSnapshotOnlineSnapshotIndication)

ga, err := source.GuestAgent()
if err != nil {
return indications, err
}

if ga {
indications = append(indications, snapshotv1.VMSnapshotGuestAgentIndication)

} else {
indications = append(indications, snapshotv1.VMSnapshotNoGuestAgentIndication)
}
}
return indications, nil
}

func updateSnapshotSnapshotableVolumes(snapshot *snapshotv1.VirtualMachineSnapshot, content *snapshotv1.VirtualMachineSnapshotContent) {
if content == nil {
return
Expand Down
144 changes: 143 additions & 1 deletion pkg/storage/snapshot/snapshot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,20 @@ var _ = Describe("Snapshot controlleer", func() {
return createVirtualMachineSnapshotContent(vmSnapshot, vm, pvcs)
}

createErrorVMSnapshotContent := func() *snapshotv1.VirtualMachineSnapshotContent {
content := createVMSnapshotContent()
content.Status = &snapshotv1.VirtualMachineSnapshotContentStatus{
ReadyToUse: &f,
CreationTime: timeFunc(),
}
errorMessage := "VolumeSnapshot vol1 error: error"
content.Status.Error = &snapshotv1.Error{
Time: timeFunc(),
Message: &errorMessage,
}
return content
}

createReadyVMSnapshotContent := func() *snapshotv1.VirtualMachineSnapshotContent {
content := createVMSnapshotContent()
content.Status = &snapshotv1.VirtualMachineSnapshotContentStatus{
Expand All @@ -170,6 +184,21 @@ var _ = Describe("Snapshot controlleer", func() {
return content
}

createVMSnapshotErrored := func() *snapshotv1.VirtualMachineSnapshot {
vms := createVMSnapshotInProgress()
content := createErrorVMSnapshotContent()
vms.Status.VirtualMachineSnapshotContentName = &content.Name
vms.Status.CreationTime = timeFunc()
vms.Status.ReadyToUse = &f
vms.Status.Indications = nil
vms.Status.Conditions = []snapshotv1.Condition{
newProgressingCondition(corev1.ConditionFalse, "In error state"),
newReadyCondition(corev1.ConditionFalse, "Error"),
}
vms.Status.Error = content.Status.Error
return vms
}

createStorageClass := func() *storagev1.StorageClass {
return &storagev1.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1111,6 +1140,111 @@ var _ = Describe("Snapshot controlleer", func() {
controller.processVMSnapshotWorkItem()
})

It("should update VirtualMachineSnapshot error when VirtualMachineSnapshotContent error", func() {
vmSnapshot := createVMSnapshotInProgress()
vm := createLockedVM()
vmSnapshotContent := createErrorVMSnapshotContent()

vmSnapshotContentSource.Add(vmSnapshotContent)
vmSource.Add(vm)
addVirtualMachineSnapshot(vmSnapshot)

updatedSnapshot := vmSnapshot.DeepCopy()
updatedSnapshot.Status.VirtualMachineSnapshotContentName = &vmSnapshotContent.Name
updatedSnapshot.Status.CreationTime = timeFunc()
updatedSnapshot.Status.ReadyToUse = &f
updatedSnapshot.Status.Indications = nil
updatedSnapshot.Status.Conditions = []snapshotv1.Condition{
newProgressingCondition(corev1.ConditionFalse, "In error state"),
newReadyCondition(corev1.ConditionFalse, "Error"),
}
updatedSnapshot.Status.Error = vmSnapshotContent.Status.Error

expectVMSnapshotUpdate(vmSnapshotClient, updatedSnapshot)

controller.processVMSnapshotWorkItem()
})

It("should update VirtualMachineSnapshot deadline exceeded after error phase", func() {
vmSnapshot := createVMSnapshotErrored()
negativeDeadline, _ := time.ParseDuration("-1m")
vmSnapshot.Spec.FailureDeadline = &metav1.Duration{Duration: negativeDeadline}
vm := createLockedVM()
vmSnapshotContent := createErrorVMSnapshotContent()

vmSnapshotContentSource.Add(vmSnapshotContent)
vmSource.Add(vm)
addVirtualMachineSnapshot(vmSnapshot)

updatedSnapshot := vmSnapshot.DeepCopy()
updatedSnapshot.Status.Phase = snapshotv1.Failed
updatedSnapshot.Status.Conditions = append(updatedSnapshot.Status.Conditions,
newFailureCondition(corev1.ConditionTrue, vmSnapshotDeadlineExceededError))

expectVMSnapshotContentDelete(vmSnapshotClient, vmSnapshotContent.Name)
expectVMSnapshotUpdate(vmSnapshotClient, updatedSnapshot)

controller.processVMSnapshotWorkItem()
})

It("should remove error if vmsnapshotcontent not in error anymore", func() {
vmSnapshot := createVMSnapshotErrored()
vm := createLockedVM()
vmSnapshotContent := createVMSnapshotContent()

vmSnapshotContentSource.Add(vmSnapshotContent)
vmSource.Add(vm)
addVirtualMachineSnapshot(vmSnapshot)

updatedSnapshot := vmSnapshot.DeepCopy()
updatedSnapshot.Status.Phase = snapshotv1.InProgress
updatedSnapshot.Status.Conditions = []snapshotv1.Condition{
newProgressingCondition(corev1.ConditionTrue, "Source locked and operation in progress"),
newReadyCondition(corev1.ConditionFalse, "Not ready"),
}
updatedSnapshot.Status.Indications = append(updatedSnapshot.Status.Indications, snapshotv1.VMSnapshotGuestAgentIndication)

expectVMSnapshotUpdate(vmSnapshotClient, updatedSnapshot)

controller.processVMSnapshotWorkItem()
})

It("shouldn't timeout if VirtualMachineSnapshot succeeded", func() {
vmSnapshot := createVMSnapshotSuccess()
negativeDeadline, _ := time.ParseDuration("-1m")
vmSnapshot.Spec.FailureDeadline = &metav1.Duration{Duration: negativeDeadline}
vm := createVM()

vmSource.Add(vm)
addVirtualMachineSnapshot(vmSnapshot)

controller.processVMSnapshotWorkItem()
})

It("should timeout if VirtualMachineSnapshot passed failure deadline", func() {
vmSnapshot := createVMSnapshotInProgress()
negativeDeadline, _ := time.ParseDuration("-1m")
vmSnapshot.Spec.FailureDeadline = &metav1.Duration{Duration: negativeDeadline}
vm := createLockedVM()
vmSnapshotContent := createVMSnapshotContent()

vmSnapshotContentSource.Add(vmSnapshotContent)
vmSource.Add(vm)
addVirtualMachineSnapshot(vmSnapshot)

updatedSnapshot := vmSnapshot.DeepCopy()
updatedSnapshot.Status.Phase = snapshotv1.Failed
updatedSnapshot.Status.Conditions = []snapshotv1.Condition{
newProgressingCondition(corev1.ConditionFalse, vmSnapshotDeadlineExceededError),
newFailureCondition(corev1.ConditionTrue, vmSnapshotDeadlineExceededError),
}

expectVMSnapshotContentDelete(vmSnapshotClient, vmSnapshotContent.Name)
expectVMSnapshotUpdate(vmSnapshotClient, updatedSnapshot)

controller.processVMSnapshotWorkItem()
})

It("should create VolumeSnapshot", func() {
vm := createLockedVM()
storageClass := createStorageClass()
Expand Down Expand Up @@ -1353,7 +1487,6 @@ var _ = Describe("Snapshot controlleer", func() {

vmSnapshotSource.Add(vmSnapshot)
vmSnapshotContentSource.Add(vmSnapshotContent)
expectVMSnapshotContentUpdate(vmSnapshotClient, updatedContent)

volumeSnapshots := createVolumeSnapshots(vmSnapshotContent)
for i := range volumeSnapshots {
Expand All @@ -1373,7 +1506,16 @@ var _ = Describe("Snapshot controlleer", func() {
Error: translateError(volumeSnapshots[i].Status.Error),
}
updatedContent.Status.VolumeSnapshotStatus = append(updatedContent.Status.VolumeSnapshotStatus, vss)

if i == 0 && !rtu {
errorMessage := fmt.Sprintf("VolumeSnapshot %s error: %s", vss.VolumeSnapshotName, *vss.Error.Message)
updatedContent.Status.Error = &snapshotv1.Error{
Time: timeFunc(),
Message: &errorMessage,
}
}
}
expectVMSnapshotContentUpdate(vmSnapshotClient, updatedContent)

controller.processVMSnapshotContentWorkItem()
},
Expand Down
43 changes: 43 additions & 0 deletions tests/storage/snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1313,6 +1313,49 @@ var _ = SIGDescribe("VirtualMachineSnapshot Tests", func() {

Expect(snapshot.Status.CreationTime).To(BeNil())
})

It("vmsnapshot should update error if vmsnapshotcontent is unready to use and error", func() {
vm = tests.StartVMAndExpectRunning(virtClient, vm)
// Delete DV and wait pvc get deletionTimestamp
// when pvc is deleting snapshot is not possible
volumeName := vm.Spec.DataVolumeTemplates[0].Name
By("Deleting Data volume")
err = virtClient.CdiClient().CdiV1beta1().DataVolumes(vm.Namespace).Delete(context.Background(), volumeName, metav1.DeleteOptions{})
Expect(err).ToNot(HaveOccurred())
Eventually(func() *metav1.Time {
pvc, err := virtClient.CoreV1().PersistentVolumeClaims(vm.Namespace).Get(context.Background(), volumeName, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
return pvc.DeletionTimestamp
}, 30*time.Second, time.Second).ShouldNot(BeNil())

By("Creating VMSnapshot")
snapshot = newSnapshot()

_, err = virtClient.VirtualMachineSnapshot(snapshot.Namespace).Create(context.Background(), snapshot, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())

Eventually(func() *snapshotv1.VirtualMachineSnapshotStatus {
snapshot, err = virtClient.VirtualMachineSnapshot(vm.Namespace).Get(context.Background(), snapshot.Name, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
return snapshot.Status
}, time.Minute, 2*time.Second).Should(gstruct.PointTo(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
"Conditions": ContainElements(
gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
"Type": Equal(snapshotv1.ConditionReady),
"Status": Equal(corev1.ConditionFalse),
"Reason": Equal("Not ready")}),
gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
"Type": Equal(snapshotv1.ConditionProgressing),
"Status": Equal(corev1.ConditionFalse),
"Reason": Equal("In error state")}),
),
"Phase": Equal(snapshotv1.InProgress),
"Error": gstruct.PointTo(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
"Message": gstruct.PointTo(ContainSubstring("Failed to create snapshot content with error")),
})),
"CreationTime": BeNil(),
})))
})
})

Context("[Serial]With more complicated VM with/out GC of succeeded DV", Serial, func() {
Expand Down

0 comments on commit 2b90c7b

Please sign in to comment.