Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Nolan Brubaker <brubakern@vmware.com>
  • Loading branch information
nrb committed Jan 11, 2019
1 parent 3f447f2 commit 073f13d
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
11 changes: 7 additions & 4 deletions pkg/restore/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,19 +590,16 @@ func (ctx *context) waitForPVDeletion(name string, pvClient client.Dynamic) (boo
var shouldRestore bool
err := wait.PollImmediate(time.Second, ctx.resourceTerminatingTimeout, func() (bool, error) {
clusterPV, err := pvClient.Get(name, metav1.GetOptions{})

if apierrors.IsNotFound(err) {
pvLogger.Debug("PV not found, safe to restore")
// PV not found, can safely exit loop and proceed with restore.
shouldRestore = true
return true, nil
}

if err != nil {
return false, errors.Wrapf(err, "could not retrieve in-cluster copy of PV %s", name)

}

phase, err := collections.GetString(clusterPV.UnstructuredContent(), "status.phase")
if err != nil {
// Break the loop since we couldn't read the phase
Expand All @@ -615,6 +612,11 @@ func (ctx *context) waitForPVDeletion(name string, pvClient client.Dynamic) (boo
return false, nil
}

// Check for the namespace and PVC to see if anything that's referencing the PV is deleting.
// If either the namespace or PVC is in a deleting/terminating state, wait for them to finish before
// trying to restore the PV
// Not doing so may result in the underlying PV disappearing but not restoring due to timing issues,
// then the PVC getting restored and showing as lost.
namespace, err := collections.GetString(clusterPV.UnstructuredContent(), "spec.claimRef.namespace")
if err != nil {
return false, errors.Wrapf(err, "error looking up namespace name for in-cluster PV %s", name)
Expand All @@ -633,12 +635,12 @@ func (ctx *context) waitForPVDeletion(name string, pvClient client.Dynamic) (boo
}

pvc, err := pvcClient.Get(pvcName, metav1.GetOptions{})

if apierrors.IsNotFound(err) {
pvLogger.Debugf("PVC %s for PV not found, waiting", pvcName)
// PVC wasn't found, but the PV still exists, so continue to wait.
return false, nil
}

if err != nil {
return false, errors.Wrapf(err, "error getting claim %s for persistent volume", pvcName)
}
Expand All @@ -649,6 +651,7 @@ func (ctx *context) waitForPVDeletion(name string, pvClient client.Dynamic) (boo
return false, nil
}

// Check the namespace associated with the claimRef to see if it's deleting/terminating before proceeding
ns, err := ctx.namespaceClient.Get(namespace, metav1.GetOptions{})
if apierrors.IsNotFound(err) {
pvLogger.Debugf("namespace %s for PV not found, waiting", namespace)
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/kube/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"

arktest "github.com/heptio/ark/pkg/util/test"
"github.com/stretchr/testify/assert"
)

func TestNamespaceAndName(t *testing.T) {
Expand Down

0 comments on commit 073f13d

Please sign in to comment.