diff --git a/pkg/controller/volume/scheduling/scheduler_binder.go b/pkg/controller/volume/scheduling/scheduler_binder.go index 0c9c9516148..7a6c202c4dd 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder.go +++ b/pkg/controller/volume/scheduling/scheduler_binder.go @@ -124,6 +124,9 @@ type SchedulerVolumeBinder interface { // This function is called serially. AssumePodVolumes(assumedPod *v1.Pod, nodeName string) (allFullyBound bool, err error) + // RevertAssumedPodVolumes will revert assumed PV and PVC cache. + RevertAssumedPodVolumes(assumedPod *v1.Pod, nodeName string) + // BindPodVolumes will: // 1. Initiate the volume binding by making the API call to prebind the PV // to its matching PVC. @@ -381,6 +384,12 @@ func (b *volumeBinder) AssumePodVolumes(assumedPod *v1.Pod, nodeName string) (al return } +// RevertAssumedPodVolumes will revert assumed PV and PVC cache. +func (b *volumeBinder) RevertAssumedPodVolumes(assumedPod *v1.Pod, nodeName string) { + b.revertAssumedPVs(b.podBindingCache.GetBindings(assumedPod, nodeName)) + b.revertAssumedPVCs(b.podBindingCache.GetProvisionedPVCs(assumedPod, nodeName)) +} + // BindPodVolumes gets the cached bindings and PVCs to provision in podBindingCache, // makes the API update for those PVs/PVCs, and waits for the PVCs to be completely bound // by the PV controller. diff --git a/pkg/controller/volume/scheduling/scheduler_binder_fake.go b/pkg/controller/volume/scheduling/scheduler_binder_fake.go index 5ebe0a93502..eb5fe5c77a1 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder_fake.go +++ b/pkg/controller/volume/scheduling/scheduler_binder_fake.go @@ -58,6 +58,9 @@ func (b *FakeVolumeBinder) AssumePodVolumes(assumedPod *v1.Pod, nodeName string) return b.config.AllBound, b.config.AssumeErr } +// RevertAssumedPodVolumes implements SchedulerVolumeBinder.RevertAssumedPodVolumes +func (b *FakeVolumeBinder) RevertAssumedPodVolumes(assumedPod *v1.Pod, nodeName string) {} + // BindPodVolumes implements SchedulerVolumeBinder.BindPodVolumes. func (b *FakeVolumeBinder) BindPodVolumes(assumedPod *v1.Pod) error { b.BindCalled = true diff --git a/pkg/controller/volume/scheduling/scheduler_binder_test.go b/pkg/controller/volume/scheduling/scheduler_binder_test.go index fdfd8abaa03..00332ca9d8a 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder_test.go +++ b/pkg/controller/volume/scheduling/scheduler_binder_test.go @@ -465,13 +465,14 @@ func (env *testEnv) validateAssume(t *testing.T, pod *v1.Pod, bindings []*bindin } } -func (env *testEnv) validateFailedAssume(t *testing.T, pod *v1.Pod, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { +func (env *testEnv) validateCacheRestored(t *testing.T, pod *v1.Pod, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { // All PVs have been unmodified in cache pvCache := env.internalBinder.pvCache for _, b := range bindings { pv, _ := pvCache.GetPV(b.pv.Name) + apiPV, _ := pvCache.GetAPIPV(b.pv.Name) // PV could be nil if it's missing from cache - if pv != nil && pv != b.pv { + if pv != nil && pv != apiPV { t.Errorf("PV %q was modified in cache", b.pv.Name) } } @@ -1237,7 +1238,7 @@ func TestAssumePodVolumes(t *testing.T) { scenario.expectedProvisionings = scenario.provisionedPVCs } if scenario.shouldFail { - testEnv.validateFailedAssume(t, pod, scenario.expectedBindings, scenario.expectedProvisionings) + testEnv.validateCacheRestored(t, pod, scenario.bindings, scenario.provisionedPVCs) } else { testEnv.validateAssume(t, pod, scenario.expectedBindings, scenario.expectedProvisionings) } @@ -1249,6 +1250,34 @@ func TestAssumePodVolumes(t *testing.T) { } } +func TestRevertAssumedPodVolumes(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + podPVCs := []*v1.PersistentVolumeClaim{unboundPVC, provisionedPVC} + bindings := []*bindingInfo{makeBinding(unboundPVC, pvNode1a)} + pvs := []*v1.PersistentVolume{pvNode1a} + provisionedPVCs := []*v1.PersistentVolumeClaim{provisionedPVC} + expectedBindings := []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)} + expectedProvisionings := []*v1.PersistentVolumeClaim{selectedNodePVC} + + // Setup + testEnv := newTestBinder(t, ctx.Done()) + testEnv.initClaims(podPVCs, podPVCs) + pod := makePod(podPVCs) + testEnv.initPodCache(pod, "node1", bindings, provisionedPVCs) + testEnv.initVolumes(pvs, pvs) + + allbound, err := testEnv.binder.AssumePodVolumes(pod, "node1") + if allbound || err != nil { + t.Errorf("No volumes are assumed") + } + testEnv.validateAssume(t, pod, expectedBindings, expectedProvisionings) + + testEnv.binder.RevertAssumedPodVolumes(pod, "node1") + testEnv.validateCacheRestored(t, pod, bindings, provisionedPVCs) +} + func TestBindAPIUpdate(t *testing.T) { type scenarioType struct { // Inputs diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go index c8a8f5e1656..d5409865a76 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go @@ -205,9 +205,10 @@ func (pl *VolumeBinding) PreBind(ctx context.Context, cs *framework.CycleState, return nil } -// Unreserve clears pod binding state. -// TODO(#90962) Revert assumed PV/PVC cache +// Unreserve clears assumed PV and PVC cache and pod binding state. +// It's idempotent, and does nothing if no cache or binding state found for the given pod. func (pl *VolumeBinding) Unreserve(ctx context.Context, cs *framework.CycleState, pod *v1.Pod, nodeName string) { + pl.Binder.RevertAssumedPodVolumes(pod, nodeName) pl.Binder.DeletePodBindings(pod) return }