diff --git a/pkg/backup/backup_pv_action.go b/pkg/backup/backup_pv_action.go index 684e2bfa6..5c0637911 100644 --- a/pkg/backup/backup_pv_action.go +++ b/pkg/backup/backup_pv_action.go @@ -17,13 +17,14 @@ limitations under the License. package backup import ( + "github.com/pkg/errors" "github.com/sirupsen/logrus" + corev1api "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "github.com/heptio/ark/pkg/apis/ark/v1" "github.com/heptio/ark/pkg/kuberesource" - "github.com/heptio/ark/pkg/util/collections" ) // backupPVAction inspects a PersistentVolumeClaim for the PersistentVolume @@ -47,23 +48,18 @@ func (a *backupPVAction) AppliesTo() (ResourceSelector, error) { func (a *backupPVAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runtime.Unstructured, []ResourceIdentifier, error) { a.log.Info("Executing backupPVAction") - var additionalItems []ResourceIdentifier - - pvc := item.UnstructuredContent() - - volumeName, err := collections.GetString(pvc, "spec.volumeName") - // if there's no volume name, it's not an error, since it's possible - // for the PVC not be bound; don't return an additional PV item to - // back up. - if err != nil || volumeName == "" { - a.log.Info("No spec.volumeName found for PersistentVolumeClaim") - return nil, nil, nil + var pvc corev1api.PersistentVolumeClaim + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(item.UnstructuredContent(), &pvc); err != nil { + return nil, nil, errors.Wrap(err, "unable to convert unstructured item to persistent volume claim") } - additionalItems = append(additionalItems, ResourceIdentifier{ - GroupResource: kuberesource.PersistentVolumes, - Name: volumeName, - }) + if pvc.Status.Phase != corev1api.ClaimBound || pvc.Spec.VolumeName == "" { + return item, nil, nil + } - return item, additionalItems, nil + pv := ResourceIdentifier{ + GroupResource: kuberesource.PersistentVolumes, + Name: pvc.Spec.VolumeName, + } + return item, []ResourceIdentifier{pv}, nil } diff --git a/pkg/backup/backup_pv_action_test.go b/pkg/backup/backup_pv_action_test.go index 9c0c1ef36..bb33b0415 100644 --- a/pkg/backup/backup_pv_action_test.go +++ b/pkg/backup/backup_pv_action_test.go @@ -24,13 +24,15 @@ import ( arktest "github.com/heptio/ark/pkg/util/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1api "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) func TestBackupPVAction(t *testing.T) { pvc := &unstructured.Unstructured{ Object: map[string]interface{}{ - "spec": map[string]interface{}{}, + "spec": map[string]interface{}{}, + "status": map[string]interface{}{}, }, } @@ -51,11 +53,39 @@ func TestBackupPVAction(t *testing.T) { assert.NoError(t, err) assert.Len(t, additional, 0) - // non-empty spec.volumeName should result in - // no error and an additional item for the PV + // non-empty spec.volumeName when status.phase is empty + // should result in no error and no additional items pvc.Object["spec"].(map[string]interface{})["volumeName"] = "myVolume" _, additional, err = a.Execute(pvc, backup) require.NoError(t, err) + require.Len(t, additional, 0) + + // non-empty spec.volumeName when status.phase is 'Pending' + // should result in no error and no additional items + pvc.Object["status"].(map[string]interface{})["phase"] = corev1api.ClaimPending + _, additional, err = a.Execute(pvc, backup) + require.NoError(t, err) + require.Len(t, additional, 0) + + // non-empty spec.volumeName when status.phase is 'Lost' + // should result in no error and no additional items + pvc.Object["status"].(map[string]interface{})["phase"] = corev1api.ClaimLost + _, additional, err = a.Execute(pvc, backup) + require.NoError(t, err) + require.Len(t, additional, 0) + + // non-empty spec.volumeName when status.phase is 'Bound' + // should result in no error and one additional item for the PV + pvc.Object["status"].(map[string]interface{})["phase"] = corev1api.ClaimBound + _, additional, err = a.Execute(pvc, backup) + require.NoError(t, err) require.Len(t, additional, 1) assert.Equal(t, ResourceIdentifier{GroupResource: kuberesource.PersistentVolumes, Name: "myVolume"}, additional[0]) + + // empty spec.volumeName when status.phase is 'Bound' should + // result in no error and no additional items + pvc.Object["spec"].(map[string]interface{})["volumeName"] = "" + _, additional, err = a.Execute(pvc, backup) + assert.NoError(t, err) + assert.Len(t, additional, 0) }