instancetype: Ignore unexpected existing CRs during restore

5cc62c0 failed to accommodate repeat
attempts to reconcile a VirtualMachineRestore that in turn lead to
multiple calls to restoreInstancetypeControllerRevision being made for
the same ControllerRevision.

This change handles this case by ignoring any existing
ControllerRevisions found during the restore, assuming that the existing
ControllerRevision was created by a previous call to
restoreInstancetypeControllerRevision.

Future changes will check the contents of this ControllerRevision
against that of the VirtualMachineSnapshot to ensure they match.

Signed-off-by: Lee Yarwood <lyarwood@redhat.com>

pkg/storage/snapshot/restore.go

@@ -658,7 +658,9 @@ func (t *vmRestoreTarget) restoreInstancetypeControllerRevision(vmSnapshotRevisi
 	restoredCR.Name = restoredCRName
 
 	restoredCR, err = t.controller.Client.AppsV1().ControllerRevisions(vm.Namespace).Create(context.Background(), restoredCR, metav1.CreateOptions{})
-	if err != nil {
+	// This might not be our first time through the reconcile loop so accommodate previous calls to restoreInstancetypeControllerRevision by ignoring unexpected existing CRs for now.
+	// TODO - Check the contents of the existing CR here against that of the snapshot CR
+	if err != nil && !errors.IsAlreadyExists(err) {
 		return nil, err
 	}
 

pkg/storage/snapshot/restore_test.go

@@ -990,9 +990,9 @@ var _ = Describe("Restore controller", func() {
 					}).Return(expectedUpdatedVM, nil)
 			}
 
-			expectUpdateVMRestoreUpdatingTargetSpec := func(vmRestore *snapshotv1.VirtualMachineRestore) {
+			expectUpdateVMRestoreUpdatingTargetSpec := func(vmRestore *snapshotv1.VirtualMachineRestore, resourceVersion string) {
 				expectedUpdatedRestore := vmRestore.DeepCopy()
-				expectedUpdatedRestore.ResourceVersion = "1"
+				expectedUpdatedRestore.ResourceVersion = resourceVersion
 				expectedUpdatedRestore.Status.Conditions = []snapshotv1.Condition{
 					newProgressingCondition(corev1.ConditionTrue, "Updating target spec"),
 					newReadyCondition(corev1.ConditionFalse, "Waiting for target update"),
@@ -1074,7 +1074,7 @@ var _ = Describe("Restore controller", func() {
 					expectCreateControllerRevisionAlreadyExists(k8sClient, getExpectedCR())
 					expectUpdateVMRestoreInProgress(originalVM)
 					expectUpdateVMRestored(originalVM)
-					expectUpdateVMRestoreUpdatingTargetSpec(restore)
+					expectUpdateVMRestoreUpdatingTargetSpec(restore, "1")
 
 					addVirtualMachineRestore(restore)
 					controller.processVMRestoreWorkItem()
@@ -1150,7 +1150,7 @@ var _ = Describe("Restore controller", func() {
 						newVM.Spec.Preference.RevisionName = expectedCreatedCR.Name
 					}
 					expectCreateVM(newVM)
-					expectUpdateVMRestoreUpdatingTargetSpec(restore)
+					expectUpdateVMRestoreUpdatingTargetSpec(restore, "1")
 
 					addVirtualMachineRestore(restore)
 					controller.processVMRestoreWorkItem()
@@ -1231,9 +1231,10 @@ var _ = Describe("Restore controller", func() {
 					addVirtualMachineRestore(restore)
 					controller.processVMRestoreWorkItem()
 
-					// This is bug #8890, the fact that the CR already exists shouldn't cause the reconcile to fail
+					// We have already created the ControllerRevision but that shouldn't stop the reconcile from progressing
 					expectCreateControllerRevisionAlreadyExists(k8sClient, expectedCreatedCR)
-					expectUpdateVMRestoreFailure(restore, "2", fmt.Sprintf(" \"%s\" already exists", expectedCreatedCR.Name))
+					expectCreateVM(newVM)
+					expectUpdateVMRestoreUpdatingTargetSpec(restore, "2")
 
 					addVirtualMachineRestore(restore)
 					controller.processVMRestoreWorkItem()