Skip to content

Commit

Permalink
have to call UpdateStatus as well as Update otherwise status does not…
Browse files Browse the repository at this point in the history 
… get updated, duh

Signed-off-by: Michael Henriksen <[email protected]>
  • Loading branch information
@mhenriks
mhenriks committed Aug 17, 2020
1 parent ebde161 commit 77d5257
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 17 deletions.
42 changes: 31 additions & 11 deletions pkg/virt-controller/watch/snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (ctrl *SnapshotController) updateVMSnapshot(vmSnapshot *snapshotv1.VirtualM
}

// unlock the source if done/error
if !vmSnapshotProgressing(vmSnapshot) && source != nil && source.Locked() {
if !vmSnapshotProgressing(vmSnapshot) && source != nil {
if err = source.Unlock(); err != nil {
return err
}
Expand Down Expand Up @@ -664,7 +664,9 @@ func (ctrl *SnapshotController) getContent(vmSnapshot *snapshotv1.VirtualMachine
}

func (s *vmSnapshotSource) Locked() bool {
return s.vm.Status.SnapshotInProgress != nil && *s.vm.Status.SnapshotInProgress == s.snapshot.Name
return s.vm.Status.SnapshotInProgress != nil &&
*s.vm.Status.SnapshotInProgress == s.snapshot.Name &&
controller.HasFinalizer(s.vm, sourceFinalizer)
}

func (s *vmSnapshotSource) Lock() (bool, error) {
Expand All @@ -684,28 +686,46 @@ func (s *vmSnapshotSource) Lock() (bool, error) {

log.Log.Infof("Adding VM snapshot finalizer to %s", s.vm.Name)

var err error
vmCopy := s.vm.DeepCopy()
vmCopy.Status.SnapshotInProgress = &s.snapshot.Name
controller.AddFinalizer(vmCopy, sourceFinalizer)

_, err := s.client.VirtualMachine(vmCopy.Namespace).Update(vmCopy)
if err != nil {
return false, err
if vmCopy.Status.SnapshotInProgress == nil {
vmCopy.Status.SnapshotInProgress = &s.snapshot.Name
vmCopy, err = s.client.VirtualMachine(vmCopy.Namespace).UpdateStatus(vmCopy)
if err != nil {
return false, err
}
}

if !controller.HasFinalizer(vmCopy, sourceFinalizer) {
controller.AddFinalizer(vmCopy, sourceFinalizer)
_, err = s.client.VirtualMachine(vmCopy.Namespace).Update(vmCopy)
if err != nil {
return false, err
}
}

return true, nil
}

func (s *vmSnapshotSource) Unlock() error {
if !s.Locked() {
if s.vm.Status.SnapshotInProgress == nil || *s.vm.Status.SnapshotInProgress != s.snapshot.Name {
return nil
}

var err error
vmCopy := s.vm.DeepCopy()
vmCopy.Status.SnapshotInProgress = nil
controller.RemoveFinalizer(vmCopy, sourceFinalizer)

_, err := s.client.VirtualMachine(vmCopy.Namespace).Update(vmCopy)
if controller.HasFinalizer(vmCopy, sourceFinalizer) {
controller.RemoveFinalizer(vmCopy, sourceFinalizer)
vmCopy, err = s.client.VirtualMachine(vmCopy.Namespace).Update(vmCopy)
if err != nil {
return err
}
}

vmCopy.Status.SnapshotInProgress = nil
_, err = s.client.VirtualMachine(vmCopy.Namespace).UpdateStatus(vmCopy)
if err != nil {
return err
}
Expand Down
46 changes: 40 additions & 6 deletions pkg/virt-controller/watch/snapshot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,11 +527,27 @@ var _ = Describe("Snapshot controlleer", func() {
It("should unlock source VirtualMachine", func() {
vmSnapshot := createVMSnapshotSuccess()
vm := createLockedVM()
updatedVM := createVM()
updatedVM := vm.DeepCopy()
updatedVM.Finalizers = []string{}
updatedVM.ResourceVersion = "1"
vmSource.Add(vm)
vmInterface.EXPECT().Update(updatedVM).Return(updatedVM, nil)
statusUpdate := updatedVM.DeepCopy()
statusUpdate.Status.SnapshotInProgress = nil
vmInterface.EXPECT().UpdateStatus(statusUpdate).Return(statusUpdate, nil)
addVirtualMachineSnapshot(vmSnapshot)
controller.processVMSnapshotWorkItem()
})

It("should finish unlock source VirtualMachine", func() {
vmSnapshot := createVMSnapshotSuccess()
vm := createLockedVM()
vm.Finalizers = []string{}
statusUpdate := vm.DeepCopy()
statusUpdate.ResourceVersion = "1"
statusUpdate.Status.SnapshotInProgress = nil
vmSource.Add(vm)
vmInterface.EXPECT().UpdateStatus(statusUpdate).Return(statusUpdate, nil)
addVirtualMachineSnapshot(vmSnapshot)
controller.processVMSnapshotWorkItem()
})
Expand Down Expand Up @@ -597,10 +613,26 @@ var _ = Describe("Snapshot controlleer", func() {
It("should lock source", func() {
vmSnapshot := createVMSnapshotInProgress()
vm := createVM()
vmUpdate := vm.DeepCopy()
vmStatusUpdate := vm.DeepCopy()
vmStatusUpdate.ResourceVersion = "1"
vmStatusUpdate.Status.SnapshotInProgress = &vmSnapshotName
vmUpdate := vmStatusUpdate.DeepCopy()
vmUpdate.Finalizers = []string{"snapshot.kubevirt.io/snapshot-source-protection"}

vmSource.Add(vm)
vmInterface.EXPECT().UpdateStatus(vmStatusUpdate).Return(vmStatusUpdate, nil)
vmInterface.EXPECT().Update(vmUpdate).Return(vmUpdate, nil)
addVirtualMachineSnapshot(vmSnapshot)
controller.processVMSnapshotWorkItem()
})

It("should finish lock source", func() {
vmSnapshot := createVMSnapshotInProgress()
vm := createVM()
vm.Status.SnapshotInProgress = &vmSnapshotName
vmUpdate := vm.DeepCopy()
vmUpdate.ResourceVersion = "1"
vmUpdate.Status.SnapshotInProgress = &vmSnapshotName
vmUpdate.Finalizers = []string{"snapshot.kubevirt.io/snapshot-source-protection"}

vmSource.Add(vm)
vmInterface.EXPECT().Update(vmUpdate).Return(vmUpdate, nil)
Expand All @@ -611,12 +643,14 @@ var _ = Describe("Snapshot controlleer", func() {
It("should lock source when VM updated", func() {
vmSnapshot := createVMSnapshotInProgress()
vm := createVM()
vmUpdate := vm.DeepCopy()
vmStatusUpdate := vm.DeepCopy()
vmStatusUpdate.ResourceVersion = "1"
vmStatusUpdate.Status.SnapshotInProgress = &vmSnapshotName
vmUpdate := vmStatusUpdate.DeepCopy()
vmUpdate.Finalizers = []string{"snapshot.kubevirt.io/snapshot-source-protection"}
vmUpdate.ResourceVersion = "1"
vmUpdate.Status.SnapshotInProgress = &vmSnapshotName

vmSnapshotSource.Add(vmSnapshot)
vmInterface.EXPECT().UpdateStatus(vmStatusUpdate).Return(vmStatusUpdate, nil)
vmInterface.EXPECT().Update(vmUpdate).Return(vmUpdate, nil)
addVM(vm)
controller.processVMSnapshotWorkItem()
Expand Down

0 comments on commit 77d5257

Please sign in to comment.