Merge pull request #57 from replicatedhq/laverya/wait-for-pvcs-to-be-…

…deleted-before-reusing-names

wait for PVCs to be deleted before reusing the PVC's name

pkg/migrate/migrate.go

@@ -907,12 +907,24 @@ func swapPVs(ctx context.Context, w *log.Logger, clientset k8sclient.Interface,
 	if err != nil {
 		return fmt.Errorf("failed to delete original PVC %s in %s: %w", pvcName, ns, err)
 	}
-	w.Printf("Deleting migrated-to PVC %s in %s to release the PV\n", pvcName, ns)
+	w.Printf("Deleting migrated-to PVC %s in %s to release the PV\n", newPvcName(pvcName), ns)
 	err = clientset.CoreV1().PersistentVolumeClaims(ns).Delete(ctx, newPvcName(pvcName), metav1.DeleteOptions{})
 	if err != nil {
 		return fmt.Errorf("failed to delete migrated-to PVC %s in %s: %w", newPvcName(pvcName), ns, err)
 	}
 
+	// wait for the deleted PVCs to actually no longer exist
+	w.Printf("Waiting for original PVC %s in %s to finish deleting\n", pvcName, ns)
+	err = waitForDeletion(ctx, clientset, pvcName, ns)
+	if err != nil {
+		return fmt.Errorf("failed to ensure deletion of original PVC %s in %s: %w", pvcName, ns, err)
+	}
+	w.Printf("Waiting for migrated-to PVC %s in %s to finish deleting\n", newPvcName(pvcName), ns)
+	err = waitForDeletion(ctx, clientset, newPvcName(pvcName), ns)
+	if err != nil {
+		return fmt.Errorf("failed to ensure deletion of migrated-to PVC %s in %s: %w", newPvcName(pvcName), ns, err)
+	}
+
 	// remove claimrefs from original and migrated-to PVs
 	w.Printf("Removing claimref from original PV %s\n", originalPVC.Spec.VolumeName)
 	err = mutatePV(ctx, w, clientset, originalPVC.Spec.VolumeName, func(volume *corev1.PersistentVolume) *corev1.PersistentVolume {
@@ -987,6 +999,26 @@ func swapPVs(ctx context.Context, w *log.Logger, clientset k8sclient.Interface,
 	return nil
 }
 
+// waitForDeletion waits for the provided pvcName to not be found, and returns early if any error besides 'not found' is given
+func waitForDeletion(ctx context.Context, clientset k8sclient.Interface, pvcName, ns string) error {
+	for true {
+		_, err := clientset.CoreV1().PersistentVolumeClaims(ns).Get(ctx, pvcName, metav1.GetOptions{})
+		if k8serrors.IsNotFound(err) {
+			break
+		} else if err != nil {
+			return err
+		}
+
+		select {
+		case <-time.After(time.Second / 20):
+			continue
+		case <-ctx.Done():
+			return fmt.Errorf("context ended waiting for PVC %s in %s to be deleted", pvcName, ns)
+		}
+	}
+	return nil
+}
+
 // reset the reclaim policy of the specified PV.
 // If 'reclaim' is non-nil, that value will be used, otherwise the value will be taken from the annotation.
 // If 'reclaim' is not specified and the annotation does not exist, the reclaim policy will not be updated.

pkg/migrate/migrate_test.go

@@ -2162,3 +2162,51 @@ func Test_swapDefaults(t *testing.T) {
 		})
 	}
 }
+
+func Test_waitForDeletion(t *testing.T) {
+	tests := []struct {
+		name           string
+		backgroundFunc func(context.Context, *log.Logger, k8sclient.Interface)
+	}{
+		{
+			name: "wait 0.5s",
+			backgroundFunc: func(ctx context.Context, logger *log.Logger, k k8sclient.Interface) {
+				// wait a period of time before deleting the PVC
+				time.Sleep(time.Second / 2)
+				err := k.CoreV1().PersistentVolumeClaims("test").Delete(ctx, "test", metav1.DeleteOptions{})
+				if err != nil {
+					logger.Printf("got error deleting pvc test in test: %s", err.Error())
+				}
+				logger.Printf("deleted PVC")
+				return
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := require.New(t)
+			testCtx, cancelfunc := context.WithTimeout(context.Background(), time.Minute) // if your test takes more than 1m, there are issues
+			defer cancelfunc()
+			clientset := fake.NewSimpleClientset(
+				[]runtime.Object{
+					&corev1.PersistentVolumeClaim{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "test",
+							Namespace: "test",
+						},
+					},
+				}...,
+			)
+			testlog := log.New(testWriter{t: t}, "", 0)
+			if tt.backgroundFunc != nil {
+				go tt.backgroundFunc(testCtx, testlog, clientset)
+			}
+			err := waitForDeletion(testCtx, clientset, "test", "test")
+			req.NoError(err)
+			actualPVC, err := clientset.CoreV1().PersistentVolumeClaims("test").Get(testCtx, "test", metav1.GetOptions{})
+			req.Errorf(err, "the PVC 'test' in 'test' should not have been found after waiting for its deletion")
+			var nilPVC *corev1.PersistentVolumeClaim
+			req.Equal(nilPVC, actualPVC)
+		})
+	}
+}