Merge pull request #980 from johbo/unmounted-rwo-volume

Unmounted RWO PVCs without NodeAffinity on the PV should be backed up as well
k8up-io · Jun 17, 2024 · debabf3 · debabf3
2 parents 1da3f1f + fa178ec
commit debabf3
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 4 deletions.
diff --git a/operator/backupcontroller/controller_integration_test.go b/operator/backupcontroller/controller_integration_test.go
@@ -14,6 +14,7 @@ import (
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
@@ -457,6 +458,42 @@ func (ts *BackupTestSuite) Test_GivenBackupAndUnmountedRWOPVCOnTwoNodes_ExpectBa
 	ts.assertJobSpecs(&job2, nodeNamePv2, []corev1.Volume{volumePvc2}, nil, []string{})
 }
 
+func (ts *BackupTestSuite) Test_GivenBackupAndUnmountedRWOPVC_ExpectBackup() {
+	pvc1 := ts.newPvc("test-pvc-without-node-affinity", corev1.ReadWriteOnce)
+	pv1 := &corev1.PersistentVolume{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-pvc-without-node-affinity",
+		},
+		Spec: corev1.PersistentVolumeSpec{
+			AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
+			Capacity: map[corev1.ResourceName]resource.Quantity{
+				corev1.ResourceStorage: resource.MustParse("1Mi"),
+			},
+			PersistentVolumeSource: corev1.PersistentVolumeSource{
+				HostPath: &corev1.HostPathVolumeSource{Path: "/tmp/integration-tests"},
+			},
+		},
+	}
+	volumePvc1 := corev1.Volume{
+		Name: "test-pvc-without-node-affinity",
+		VolumeSource: corev1.VolumeSource{
+			PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
+				ClaimName: pvc1.Name,
+			},
+		},
+	}
+	ts.EnsureResources(ts.BackupResource, pvc1, pv1)
+
+	pvc1.Status.Phase = corev1.ClaimBound
+	ts.UpdateStatus(pvc1)
+
+	ts.whenReconciling(ts.BackupResource)
+	job := ts.expectABackupJob()
+
+	ts.Assert().Nil(job.Spec.Template.Spec.NodeSelector)
+	ts.Assert().Equal(volumePvc1.VolumeSource.PersistentVolumeClaim.ClaimName, job.Spec.Template.Spec.Volumes[0].VolumeSource.PersistentVolumeClaim.ClaimName)
+}
+
 func (ts *BackupTestSuite) Test_GivenBackupAndRWOPVCWithFinishedPod_ExpectFinishedPodToBeIgnored() {
 	pvc1 := ts.newPvc("test-pvc1", corev1.ReadWriteOnce)
 	nodeNamePod1 := "worker"

diff --git a/operator/backupcontroller/executor.go b/operator/backupcontroller/executor.go
@@ -91,15 +91,15 @@ func (b *BackupExecutor) listAndFilterPVCs(ctx context.Context, annotation strin
 
 	for _, pvc := range claimlist.Items {
 		if pvc.Status.Phase != corev1.ClaimBound {
-			log.Info("PVC is not bound", "pvc", pvc.GetName())
+			log.Info("PVC is not bound, skipping PVC", "pvc", pvc.GetName())
 			continue
 		}
 
 		backupAnnotation, hasBackupAnnotation := pvc.GetAnnotations()[annotation]
 
 		isRWO := containsAccessMode(pvc.Spec.AccessModes, corev1.ReadWriteOnce)
 		if !containsAccessMode(pvc.Spec.AccessModes, corev1.ReadWriteMany) && !isRWO && !hasBackupAnnotation {
-			log.Info("PVC is neither RWX nor RWO and has no backup annotation", "pvc", pvc.GetName())
+			log.Info("PVC is neither RWX nor RWO and has no backup annotation, skipping PVC", "pvc", pvc.GetName())
 			continue
 		}
 
@@ -143,8 +143,7 @@ func (b *BackupExecutor) listAndFilterPVCs(ctx context.Context, annotation strin
 
 			bi.node = findNode(pv, pvc)
 			if bi.node == "" {
-				log.Info("RWO PVC not bound and no PV node affinity set, skipping", "pvc", pvc.GetName(), "affinity", pv.Spec.NodeAffinity)
-				continue
+				log.Info("RWO PVC not bound and no PV node affinity set, adding", "pvc", pvc.GetName(), "affinity", pv.Spec.NodeAffinity)
 			}
 			log.V(1).Info("node found in PV or PVC", "pvc", pvc.GetName(), "node", bi.node)
 		} else {