diff --git a/cmd/kube-scheduler/app/server_test.go b/cmd/kube-scheduler/app/server_test.go index 30b07905d1b..c3bab8481a1 100644 --- a/cmd/kube-scheduler/app/server_test.go +++ b/cmd/kube-scheduler/app/server_test.go @@ -202,7 +202,6 @@ profiles: "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, } testcases := []struct { @@ -237,7 +236,6 @@ profiles: "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, }, }, @@ -255,7 +253,6 @@ profiles: "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, }, }, @@ -338,7 +335,6 @@ profiles: "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, }, }, diff --git a/pkg/controller/volume/scheduling/BUILD b/pkg/controller/volume/scheduling/BUILD index 887891cbd75..abb112b39a1 100644 --- a/pkg/controller/volume/scheduling/BUILD +++ b/pkg/controller/volume/scheduling/BUILD @@ -5,7 +5,6 @@ go_library( srcs = [ "scheduler_assume_cache.go", "scheduler_binder.go", - "scheduler_binder_cache.go", "scheduler_binder_fake.go", ], importpath = "k8s.io/kubernetes/pkg/controller/volume/scheduling", @@ -18,6 +17,7 @@ go_library( "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/storage/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", @@ -28,6 +28,7 @@ go_library( "//staging/src/k8s.io/client-go/informers/core/v1:go_default_library", "//staging/src/k8s.io/client-go/informers/storage/v1:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", + "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", "//staging/src/k8s.io/client-go/listers/storage/v1:go_default_library", "//staging/src/k8s.io/client-go/tools/cache:go_default_library", "//staging/src/k8s.io/csi-translation-lib:go_default_library", @@ -40,7 +41,6 @@ go_test( name = "go_default_test", srcs = [ "scheduler_assume_cache_test.go", - "scheduler_binder_cache_test.go", "scheduler_binder_test.go", ], embed = [":go_default_library"], diff --git a/pkg/controller/volume/scheduling/scheduler_binder.go b/pkg/controller/volume/scheduling/scheduler_binder.go index 7a6c202c4dd..6dbab627f98 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder.go +++ b/pkg/controller/volume/scheduling/scheduler_binder.go @@ -25,6 +25,7 @@ import ( v1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/sets" @@ -34,6 +35,7 @@ import ( coreinformers "k8s.io/client-go/informers/core/v1" storageinformers "k8s.io/client-go/informers/storage/v1" clientset "k8s.io/client-go/kubernetes" + corelisters "k8s.io/client-go/listers/core/v1" storagelisters "k8s.io/client-go/listers/storage/v1" csitrans "k8s.io/csi-translation-lib" csiplugins "k8s.io/csi-translation-lib/plugins" @@ -63,6 +65,24 @@ const ( ErrReasonNodeConflict ConflictReason = "node(s) had volume node affinity conflict" ) +// BindingInfo holds a binding between PV and PVC. +type BindingInfo struct { + // PVC that needs to be bound + pvc *v1.PersistentVolumeClaim + + // Proposed PV to bind to this PVC + pv *v1.PersistentVolume +} + +// PodVolumes holds pod's volumes information used in volume scheduling. +type PodVolumes struct { + // StaticBindings are binding decisions for PVCs which can be bound to + // pre-provisioned static PVs. + StaticBindings []*BindingInfo + // DynamicProvisions are PVCs that require dynamic provisioning + DynamicProvisions []*v1.PersistentVolumeClaim +} + // InTreeToCSITranslator contains methods required to check migratable status // and perform translations from InTree PV's to CSI type InTreeToCSITranslator interface { @@ -102,7 +122,8 @@ type SchedulerVolumeBinder interface { // and unbound with immediate binding (including prebound) GetPodVolumes(pod *v1.Pod) (boundClaims, unboundClaimsDelayBinding, unboundClaimsImmediate []*v1.PersistentVolumeClaim, err error) - // FindPodVolumes checks if all of a Pod's PVCs can be satisfied by the node. + // FindPodVolumes checks if all of a Pod's PVCs can be satisfied by the + // node and returns pod's volumes information. // // If a PVC is bound, it checks if the PV's NodeAffinity matches the Node. // Otherwise, it tries to find an available PV to bind to the PVC. @@ -110,8 +131,8 @@ type SchedulerVolumeBinder interface { // It returns an error when something went wrong or a list of reasons why the node is // (currently) not usable for the pod. // - // This function is called by the volume binding scheduler predicate and can be called in parallel - FindPodVolumes(pod *v1.Pod, boundClaims, claimsToBind []*v1.PersistentVolumeClaim, node *v1.Node) (reasons ConflictReasons, err error) + // This function is called by the scheduler VolumeBinding plugin and can be called in parallel + FindPodVolumes(pod *v1.Pod, boundClaims, claimsToBind []*v1.PersistentVolumeClaim, node *v1.Node) (podVolumes *PodVolumes, reasons ConflictReasons, err error) // AssumePodVolumes will: // 1. Take the PV matches for unbound PVCs and update the PV cache assuming @@ -122,10 +143,10 @@ type SchedulerVolumeBinder interface { // It returns true if all volumes are fully bound // // This function is called serially. - AssumePodVolumes(assumedPod *v1.Pod, nodeName string) (allFullyBound bool, err error) + AssumePodVolumes(assumedPod *v1.Pod, nodeName string, podVolumes *PodVolumes) (allFullyBound bool, err error) // RevertAssumedPodVolumes will revert assumed PV and PVC cache. - RevertAssumedPodVolumes(assumedPod *v1.Pod, nodeName string) + RevertAssumedPodVolumes(podVolumes *PodVolumes) // BindPodVolumes will: // 1. Initiate the volume binding by making the API call to prebind the PV @@ -135,28 +156,19 @@ type SchedulerVolumeBinder interface { // 3. Wait for PVCs to be completely bound by the PV controller // // This function can be called in parallel. - BindPodVolumes(assumedPod *v1.Pod) error - - // GetBindingsCache returns the cache used (if any) to store volume binding decisions. - GetBindingsCache() PodBindingCache - - // DeletePodBindings will delete pod's bindingDecisions in podBindingCache. - DeletePodBindings(pod *v1.Pod) + BindPodVolumes(assumedPod *v1.Pod, podVolumes *PodVolumes) error } type volumeBinder struct { kubeClient clientset.Interface classLister storagelisters.StorageClassLister + podLister corelisters.PodLister nodeInformer coreinformers.NodeInformer csiNodeInformer storageinformers.CSINodeInformer pvcCache PVCAssumeCache pvCache PVAssumeCache - // Stores binding decisions that were made in FindPodVolumes for use in AssumePodVolumes. - // AssumePodVolumes modifies the bindings again for use in BindPodVolumes. - podBindingCache PodBindingCache - // Amount of time to wait for the bind operation to succeed bindTimeout time.Duration @@ -166,44 +178,31 @@ type volumeBinder struct { // NewVolumeBinder sets up all the caches needed for the scheduler to make volume binding decisions. func NewVolumeBinder( kubeClient clientset.Interface, + podInformer coreinformers.PodInformer, nodeInformer coreinformers.NodeInformer, csiNodeInformer storageinformers.CSINodeInformer, pvcInformer coreinformers.PersistentVolumeClaimInformer, pvInformer coreinformers.PersistentVolumeInformer, storageClassInformer storageinformers.StorageClassInformer, bindTimeout time.Duration) SchedulerVolumeBinder { - - b := &volumeBinder{ + return &volumeBinder{ kubeClient: kubeClient, + podLister: podInformer.Lister(), classLister: storageClassInformer.Lister(), nodeInformer: nodeInformer, csiNodeInformer: csiNodeInformer, pvcCache: NewPVCAssumeCache(pvcInformer.Informer()), pvCache: NewPVAssumeCache(pvInformer.Informer()), - podBindingCache: NewPodBindingCache(), bindTimeout: bindTimeout, translator: csitrans.New(), } - - return b } -func (b *volumeBinder) GetBindingsCache() PodBindingCache { - return b.podBindingCache -} - -// DeletePodBindings will delete pod's bindingDecisions in podBindingCache. -func (b *volumeBinder) DeletePodBindings(pod *v1.Pod) { - cache := b.podBindingCache - if pod != nil { - cache.DeleteBindings(pod) - } -} - -// FindPodVolumes caches the matching PVs and PVCs to provision per node in podBindingCache. -// This method intentionally takes in a *v1.Node object instead of using volumebinder.nodeInformer. -// That's necessary because some operations will need to pass in to the predicate fake node objects. -func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, boundClaims, claimsToBind []*v1.PersistentVolumeClaim, node *v1.Node) (reasons ConflictReasons, err error) { +// FindPodVolumes finds the matching PVs for PVCs and nodes to provision PVs +// for the given pod and node. If the node does not fit, confilict reasons are +// returned. +func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, boundClaims, claimsToBind []*v1.PersistentVolumeClaim, node *v1.Node) (podVolumes *PodVolumes, reasons ConflictReasons, err error) { + podVolumes = &PodVolumes{} podName := getPodName(pod) // Warning: Below log needs high verbosity as it can be printed several times (#60933). @@ -235,16 +234,10 @@ func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, boundClaims, claimsToBind []* }() var ( - matchedBindings []*bindingInfo + matchedBindings []*BindingInfo provisionedClaims []*v1.PersistentVolumeClaim ) defer func() { - // We recreate bindings for each new schedule loop. - if len(matchedBindings) == 0 && len(provisionedClaims) == 0 { - // Clear cache if no claims to bind or provision for this node. - b.podBindingCache.ClearBindings(pod, node.Name) - return - } // Although we do not distinguish nil from empty in this function, for // easier testing, we normalize empty to nil. if len(matchedBindings) == 0 { @@ -253,15 +246,15 @@ func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, boundClaims, claimsToBind []* if len(provisionedClaims) == 0 { provisionedClaims = nil } - // Mark cache with all matched and provisioned claims for this node - b.podBindingCache.UpdateBindings(pod, node.Name, matchedBindings, provisionedClaims) + podVolumes.StaticBindings = matchedBindings + podVolumes.DynamicProvisions = provisionedClaims }() // Check PV node affinity on bound volumes if len(boundClaims) > 0 { boundVolumesSatisfied, err = b.checkBoundClaims(boundClaims, node, podName) if err != nil { - return nil, err + return } } @@ -291,7 +284,7 @@ func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, boundClaims, claimsToBind []* var unboundClaims []*v1.PersistentVolumeClaim unboundVolumesSatisfied, matchedBindings, unboundClaims, err = b.findMatchingVolumes(pod, claimsToFindMatching, node) if err != nil { - return nil, err + return } claimsToProvision = append(claimsToProvision, unboundClaims...) } @@ -300,7 +293,7 @@ func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, boundClaims, claimsToBind []* if len(claimsToProvision) > 0 { unboundVolumesSatisfied, provisionedClaims, err = b.checkVolumeProvisions(pod, claimsToProvision, node) if err != nil { - return nil, err + return } } } @@ -308,12 +301,12 @@ func (b *volumeBinder) FindPodVolumes(pod *v1.Pod, boundClaims, claimsToBind []* return } -// AssumePodVolumes will take the cached matching PVs and PVCs to provision -// in podBindingCache for the chosen node, and: +// AssumePodVolumes will take the matching PVs and PVCs to provision in pod's +// volume information for the chosen node, and: // 1. Update the pvCache with the new prebound PV. // 2. Update the pvcCache with the new PVCs with annotations set -// 3. Update podBindingCache again with cached API updates for PVs and PVCs. -func (b *volumeBinder) AssumePodVolumes(assumedPod *v1.Pod, nodeName string) (allFullyBound bool, err error) { +// 3. Update PodVolumes again with cached API updates for PVs and PVCs. +func (b *volumeBinder) AssumePodVolumes(assumedPod *v1.Pod, nodeName string, podVolumes *PodVolumes) (allFullyBound bool, err error) { podName := getPodName(assumedPod) klog.V(4).Infof("AssumePodVolumes for pod %q, node %q", podName, nodeName) @@ -330,12 +323,9 @@ func (b *volumeBinder) AssumePodVolumes(assumedPod *v1.Pod, nodeName string) (al return true, nil } - claimsToBind := b.podBindingCache.GetBindings(assumedPod, nodeName) - claimsToProvision := b.podBindingCache.GetProvisionedPVCs(assumedPod, nodeName) - // Assume PV - newBindings := []*bindingInfo{} - for _, binding := range claimsToBind { + newBindings := []*BindingInfo{} + for _, binding := range podVolumes.StaticBindings { newPV, dirty, err := pvutil.GetBindVolumeToClaim(binding.pv, binding.pvc) klog.V(5).Infof("AssumePodVolumes: GetBindVolumeToClaim for pod %q, PV %q, PVC %q. newPV %p, dirty %v, err: %v", podName, @@ -356,12 +346,12 @@ func (b *volumeBinder) AssumePodVolumes(assumedPod *v1.Pod, nodeName string) (al return false, err } } - newBindings = append(newBindings, &bindingInfo{pv: newPV, pvc: binding.pvc}) + newBindings = append(newBindings, &BindingInfo{pv: newPV, pvc: binding.pvc}) } // Assume PVCs newProvisionedPVCs := []*v1.PersistentVolumeClaim{} - for _, claim := range claimsToProvision { + for _, claim := range podVolumes.DynamicProvisions { // The claims from method args can be pointing to watcher cache. We must not // modify these, therefore create a copy. claimClone := claim.DeepCopy() @@ -376,24 +366,21 @@ func (b *volumeBinder) AssumePodVolumes(assumedPod *v1.Pod, nodeName string) (al newProvisionedPVCs = append(newProvisionedPVCs, claimClone) } - // Update cache with the assumed pvcs and pvs - // Even if length is zero, update the cache with an empty slice to indicate that no - // operations are needed - b.podBindingCache.UpdateBindings(assumedPod, nodeName, newBindings, newProvisionedPVCs) - + podVolumes.StaticBindings = newBindings + podVolumes.DynamicProvisions = newProvisionedPVCs 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)) +func (b *volumeBinder) RevertAssumedPodVolumes(podVolumes *PodVolumes) { + b.revertAssumedPVs(podVolumes.StaticBindings) + b.revertAssumedPVCs(podVolumes.DynamicProvisions) } -// BindPodVolumes gets the cached bindings and PVCs to provision in podBindingCache, +// BindPodVolumes gets the cached bindings and PVCs to provision in pod's volumes information, // makes the API update for those PVs/PVCs, and waits for the PVCs to be completely bound // by the PV controller. -func (b *volumeBinder) BindPodVolumes(assumedPod *v1.Pod) (err error) { +func (b *volumeBinder) BindPodVolumes(assumedPod *v1.Pod, podVolumes *PodVolumes) (err error) { podName := getPodName(assumedPod) klog.V(4).Infof("BindPodVolumes for pod %q, node %q", podName, assumedPod.Spec.NodeName) @@ -405,8 +392,8 @@ func (b *volumeBinder) BindPodVolumes(assumedPod *v1.Pod) (err error) { } }() - bindings := b.podBindingCache.GetBindings(assumedPod, assumedPod.Spec.NodeName) - claimsToProvision := b.podBindingCache.GetProvisionedPVCs(assumedPod, assumedPod.Spec.NodeName) + bindings := podVolumes.StaticBindings + claimsToProvision := podVolumes.DynamicProvisions // Start API operations err = b.bindAPIUpdate(podName, bindings, claimsToProvision) @@ -432,9 +419,8 @@ func getPVCName(pvc *v1.PersistentVolumeClaim) string { return pvc.Namespace + "/" + pvc.Name } -// bindAPIUpdate gets the cached bindings and PVCs to provision in podBindingCache -// and makes the API update for those PVs/PVCs. -func (b *volumeBinder) bindAPIUpdate(podName string, bindings []*bindingInfo, claimsToProvision []*v1.PersistentVolumeClaim) error { +// bindAPIUpdate makes the API update for those PVs/PVCs. +func (b *volumeBinder) bindAPIUpdate(podName string, bindings []*BindingInfo, claimsToProvision []*v1.PersistentVolumeClaim) error { if bindings == nil { return fmt.Errorf("failed to get cached bindings for pod %q", podName) } @@ -456,7 +442,7 @@ func (b *volumeBinder) bindAPIUpdate(podName string, bindings []*bindingInfo, cl }() var ( - binding *bindingInfo + binding *BindingInfo i int claim *v1.PersistentVolumeClaim ) @@ -509,7 +495,7 @@ var ( // PV/PVC cache can be assumed again in main scheduler loop, we must check // latest state in API server which are shared with PV controller and // provisioners -func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*bindingInfo, claimsToProvision []*v1.PersistentVolumeClaim) (bool, error) { +func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claimsToProvision []*v1.PersistentVolumeClaim) (bool, error) { podName := getPodName(pod) if bindings == nil { return false, fmt.Errorf("failed to get cached bindings for pod %q", podName) @@ -531,13 +517,14 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*bindingInfo, claim // Check for any conditions that might require scheduling retry - // When pod is removed from scheduling queue because of deletion or any - // other reasons, binding operation should be cancelled. There is no need - // to check PV/PVC bindings any more. - // We check pod binding cache here which will be cleared when pod is - // removed from scheduling queue. - if b.podBindingCache.GetDecisions(pod) == nil { - return false, fmt.Errorf("pod %q does not exist any more", podName) + // When pod is deleted, binding operation should be cancelled. There is no + // need to check PV/PVC bindings any more. + _, err = b.podLister.Pods(pod.Namespace).Get(pod.Name) + if err != nil { + if apierrors.IsNotFound(err) { + return false, fmt.Errorf("pod %q does not exist any more", podName) + } + klog.Errorf("failed to get pod %s/%s from the lister: %v", pod.Namespace, pod.Name, err) } for _, binding := range bindings { @@ -756,7 +743,7 @@ func (b *volumeBinder) checkBoundClaims(claims []*v1.PersistentVolumeClaim, node // findMatchingVolumes tries to find matching volumes for given claims, // and return unbound claims for further provision. -func (b *volumeBinder) findMatchingVolumes(pod *v1.Pod, claimsToBind []*v1.PersistentVolumeClaim, node *v1.Node) (foundMatches bool, bindings []*bindingInfo, unboundClaims []*v1.PersistentVolumeClaim, err error) { +func (b *volumeBinder) findMatchingVolumes(pod *v1.Pod, claimsToBind []*v1.PersistentVolumeClaim, node *v1.Node) (foundMatches bool, bindings []*BindingInfo, unboundClaims []*v1.PersistentVolumeClaim, err error) { podName := getPodName(pod) // Sort all the claims by increasing size request to get the smallest fits sort.Sort(byPVCSize(claimsToBind)) @@ -785,7 +772,7 @@ func (b *volumeBinder) findMatchingVolumes(pod *v1.Pod, claimsToBind []*v1.Persi // matching PV needs to be excluded so we don't select it again chosenPVs[pv.Name] = pv - bindings = append(bindings, &bindingInfo{pv: pv, pvc: pvc}) + bindings = append(bindings, &BindingInfo{pv: pv, pvc: pvc}) klog.V(5).Infof("Found matching PV %q for PVC %q on node %q for pod %q", pv.Name, pvcName, node.Name, podName) } @@ -837,9 +824,9 @@ func (b *volumeBinder) checkVolumeProvisions(pod *v1.Pod, claimsToProvision []*v return true, provisionedClaims, nil } -func (b *volumeBinder) revertAssumedPVs(bindings []*bindingInfo) { - for _, bindingInfo := range bindings { - b.pvCache.Restore(bindingInfo.pv.Name) +func (b *volumeBinder) revertAssumedPVs(bindings []*BindingInfo) { + for _, BindingInfo := range bindings { + b.pvCache.Restore(BindingInfo.pv.Name) } } @@ -849,14 +836,6 @@ func (b *volumeBinder) revertAssumedPVCs(claims []*v1.PersistentVolumeClaim) { } } -type bindingInfo struct { - // Claim that needs to be bound - pvc *v1.PersistentVolumeClaim - - // Proposed PV to bind to this claim - pv *v1.PersistentVolume -} - type byPVCSize []*v1.PersistentVolumeClaim func (a byPVCSize) Len() int { diff --git a/pkg/controller/volume/scheduling/scheduler_binder_cache.go b/pkg/controller/volume/scheduling/scheduler_binder_cache.go deleted file mode 100644 index 5452e1f349c..00000000000 --- a/pkg/controller/volume/scheduling/scheduler_binder_cache.go +++ /dev/null @@ -1,167 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package scheduling - -import ( - "sync" - - v1 "k8s.io/api/core/v1" - "k8s.io/kubernetes/pkg/controller/volume/scheduling/metrics" -) - -// PodBindingCache stores PV binding decisions per pod per node. -// Pod entries are removed when the Pod is deleted or updated to -// no longer be schedulable. -type PodBindingCache interface { - // UpdateBindings will update the cache with the given bindings for the - // pod and node. - UpdateBindings(pod *v1.Pod, node string, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) - - // ClearBindings will clear the cached bindings for the given pod and node. - ClearBindings(pod *v1.Pod, node string) - - // GetBindings will return the cached bindings for the given pod and node. - // A nil return value means that the entry was not found. An empty slice - // means that no binding operations are needed. - GetBindings(pod *v1.Pod, node string) []*bindingInfo - - // A nil return value means that the entry was not found. An empty slice - // means that no provisioning operations are needed. - GetProvisionedPVCs(pod *v1.Pod, node string) []*v1.PersistentVolumeClaim - - // GetDecisions will return all cached decisions for the given pod. - GetDecisions(pod *v1.Pod) nodeDecisions - - // DeleteBindings will remove all cached bindings and provisionings for the given pod. - // TODO: separate the func if it is needed to delete bindings/provisionings individually - DeleteBindings(pod *v1.Pod) -} - -type podBindingCache struct { - // synchronizes bindingDecisions - rwMutex sync.RWMutex - - // Key = pod name - // Value = nodeDecisions - bindingDecisions map[string]nodeDecisions -} - -// Key = nodeName -// Value = bindings & provisioned PVCs of the node -type nodeDecisions map[string]nodeDecision - -// A decision includes bindingInfo and provisioned PVCs of the node -type nodeDecision struct { - bindings []*bindingInfo - provisionings []*v1.PersistentVolumeClaim -} - -// NewPodBindingCache creates a pod binding cache. -func NewPodBindingCache() PodBindingCache { - return &podBindingCache{bindingDecisions: map[string]nodeDecisions{}} -} - -func (c *podBindingCache) GetDecisions(pod *v1.Pod) nodeDecisions { - c.rwMutex.RLock() - defer c.rwMutex.RUnlock() - podName := getPodName(pod) - decisions, ok := c.bindingDecisions[podName] - if !ok { - return nil - } - return decisions -} - -func (c *podBindingCache) DeleteBindings(pod *v1.Pod) { - c.rwMutex.Lock() - defer c.rwMutex.Unlock() - - podName := getPodName(pod) - - if _, ok := c.bindingDecisions[podName]; ok { - delete(c.bindingDecisions, podName) - metrics.VolumeBindingRequestSchedulerBinderCache.WithLabelValues("delete").Inc() - } -} - -func (c *podBindingCache) UpdateBindings(pod *v1.Pod, node string, bindings []*bindingInfo, pvcs []*v1.PersistentVolumeClaim) { - c.rwMutex.Lock() - defer c.rwMutex.Unlock() - - podName := getPodName(pod) - decisions, ok := c.bindingDecisions[podName] - if !ok { - decisions = nodeDecisions{} - c.bindingDecisions[podName] = decisions - } - decision, ok := decisions[node] - if !ok { - decision = nodeDecision{ - bindings: bindings, - provisionings: pvcs, - } - metrics.VolumeBindingRequestSchedulerBinderCache.WithLabelValues("add").Inc() - } else { - decision.bindings = bindings - decision.provisionings = pvcs - } - decisions[node] = decision -} - -func (c *podBindingCache) GetBindings(pod *v1.Pod, node string) []*bindingInfo { - c.rwMutex.RLock() - defer c.rwMutex.RUnlock() - - podName := getPodName(pod) - decisions, ok := c.bindingDecisions[podName] - if !ok { - return nil - } - decision, ok := decisions[node] - if !ok { - return nil - } - return decision.bindings -} - -func (c *podBindingCache) GetProvisionedPVCs(pod *v1.Pod, node string) []*v1.PersistentVolumeClaim { - c.rwMutex.RLock() - defer c.rwMutex.RUnlock() - - podName := getPodName(pod) - decisions, ok := c.bindingDecisions[podName] - if !ok { - return nil - } - decision, ok := decisions[node] - if !ok { - return nil - } - return decision.provisionings -} - -func (c *podBindingCache) ClearBindings(pod *v1.Pod, node string) { - c.rwMutex.Lock() - defer c.rwMutex.Unlock() - - podName := getPodName(pod) - decisions, ok := c.bindingDecisions[podName] - if !ok { - return - } - delete(decisions, node) -} diff --git a/pkg/controller/volume/scheduling/scheduler_binder_cache_test.go b/pkg/controller/volume/scheduling/scheduler_binder_cache_test.go deleted file mode 100644 index 54eb05678d4..00000000000 --- a/pkg/controller/volume/scheduling/scheduler_binder_cache_test.go +++ /dev/null @@ -1,150 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package scheduling - -import ( - "reflect" - "testing" - - "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func TestUpdateGetBindings(t *testing.T) { - scenarios := map[string]struct { - updateBindings []*bindingInfo - updateProvisionings []*v1.PersistentVolumeClaim - updatePod string - updateNode string - - getBindings []*bindingInfo - getProvisionings []*v1.PersistentVolumeClaim - getPod string - getNode string - }{ - "no-pod": { - getPod: "pod1", - getNode: "node1", - }, - "no-node": { - updatePod: "pod1", - updateNode: "node1", - updateBindings: []*bindingInfo{}, - updateProvisionings: []*v1.PersistentVolumeClaim{}, - getPod: "pod1", - getNode: "node2", - }, - "binding-nil": { - updatePod: "pod1", - updateNode: "node1", - updateBindings: nil, - updateProvisionings: nil, - getPod: "pod1", - getNode: "node1", - }, - "binding-exists": { - updatePod: "pod1", - updateNode: "node1", - updateBindings: []*bindingInfo{{pvc: &v1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "pvc1"}}}}, - updateProvisionings: []*v1.PersistentVolumeClaim{{ObjectMeta: metav1.ObjectMeta{Name: "pvc2"}}}, - getPod: "pod1", - getNode: "node1", - getBindings: []*bindingInfo{{pvc: &v1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "pvc1"}}}}, - getProvisionings: []*v1.PersistentVolumeClaim{{ObjectMeta: metav1.ObjectMeta{Name: "pvc2"}}}, - }, - } - - for name, scenario := range scenarios { - cache := NewPodBindingCache() - - // Perform updates - updatePod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: scenario.updatePod, Namespace: "ns"}} - cache.UpdateBindings(updatePod, scenario.updateNode, scenario.updateBindings, scenario.updateProvisionings) - - // Verify updated bindings - bindings := cache.GetBindings(updatePod, scenario.updateNode) - if !reflect.DeepEqual(bindings, scenario.updateBindings) { - t.Errorf("Test %v failed: returned bindings after update different. Got %+v, expected %+v", name, bindings, scenario.updateBindings) - } - - // Verify updated provisionings - provisionings := cache.GetProvisionedPVCs(updatePod, scenario.updateNode) - if !reflect.DeepEqual(provisionings, scenario.updateProvisionings) { - t.Errorf("Test %v failed: returned provisionings after update different. Got %+v, expected %+v", name, provisionings, scenario.updateProvisionings) - } - - // Get bindings - getPod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: scenario.getPod, Namespace: "ns"}} - bindings = cache.GetBindings(getPod, scenario.getNode) - if !reflect.DeepEqual(bindings, scenario.getBindings) { - t.Errorf("Test %v failed: unexpected bindings returned. Got %+v, expected %+v", name, bindings, scenario.updateBindings) - } - - // Get provisionings - provisionings = cache.GetProvisionedPVCs(getPod, scenario.getNode) - if !reflect.DeepEqual(provisionings, scenario.getProvisionings) { - t.Errorf("Test %v failed: unexpected bindings returned. Got %+v, expected %+v", name, provisionings, scenario.getProvisionings) - } - } -} - -func TestDeleteBindings(t *testing.T) { - initialBindings := []*bindingInfo{{pvc: &v1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "pvc1"}}}} - initialProvisionings := []*v1.PersistentVolumeClaim{{ObjectMeta: metav1.ObjectMeta{Name: "pvc2"}}} - cache := NewPodBindingCache() - - pod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "pod1", Namespace: "ns"}} - - // Get nil bindings and provisionings - bindings := cache.GetBindings(pod, "node1") - if bindings != nil { - t.Errorf("Test failed: expected initial nil bindings, got %+v", bindings) - } - provisionings := cache.GetProvisionedPVCs(pod, "node1") - if provisionings != nil { - t.Errorf("Test failed: expected initial nil provisionings, got %+v", provisionings) - } - - // Delete nothing - cache.DeleteBindings(pod) - - // Perform updates - cache.UpdateBindings(pod, "node1", initialBindings, initialProvisionings) - - // Get bindings and provisionings - bindings = cache.GetBindings(pod, "node1") - if !reflect.DeepEqual(bindings, initialBindings) { - t.Errorf("Test failed: expected bindings %+v, got %+v", initialBindings, bindings) - } - provisionings = cache.GetProvisionedPVCs(pod, "node1") - if !reflect.DeepEqual(provisionings, initialProvisionings) { - t.Errorf("Test failed: expected provisionings %+v, got %+v", initialProvisionings, provisionings) - } - - // Delete - cache.DeleteBindings(pod) - - // Get bindings and provisionings - bindings = cache.GetBindings(pod, "node1") - if bindings != nil { - t.Errorf("Test failed: expected nil bindings, got %+v", bindings) - } - provisionings = cache.GetProvisionedPVCs(pod, "node1") - if provisionings != nil { - t.Errorf("Test failed: expected nil provisionings, got %+v", provisionings) - } -} diff --git a/pkg/controller/volume/scheduling/scheduler_binder_fake.go b/pkg/controller/volume/scheduling/scheduler_binder_fake.go index eb5fe5c77a1..00dbd46191f 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder_fake.go +++ b/pkg/controller/volume/scheduling/scheduler_binder_fake.go @@ -16,7 +16,7 @@ limitations under the License. package scheduling -import "k8s.io/api/core/v1" +import v1 "k8s.io/api/core/v1" // FakeVolumeBinderConfig holds configurations for fake volume binder. type FakeVolumeBinderConfig struct { @@ -48,29 +48,21 @@ func (b *FakeVolumeBinder) GetPodVolumes(pod *v1.Pod) (boundClaims, unboundClaim } // FindPodVolumes implements SchedulerVolumeBinder.FindPodVolumes. -func (b *FakeVolumeBinder) FindPodVolumes(pod *v1.Pod, _, _ []*v1.PersistentVolumeClaim, node *v1.Node) (reasons ConflictReasons, err error) { - return b.config.FindReasons, b.config.FindErr +func (b *FakeVolumeBinder) FindPodVolumes(pod *v1.Pod, _, _ []*v1.PersistentVolumeClaim, node *v1.Node) (podVolumes *PodVolumes, reasons ConflictReasons, err error) { + return nil, b.config.FindReasons, b.config.FindErr } // AssumePodVolumes implements SchedulerVolumeBinder.AssumePodVolumes. -func (b *FakeVolumeBinder) AssumePodVolumes(assumedPod *v1.Pod, nodeName string) (bool, error) { +func (b *FakeVolumeBinder) AssumePodVolumes(assumedPod *v1.Pod, nodeName string, podVolumes *PodVolumes) (bool, error) { b.AssumeCalled = true return b.config.AllBound, b.config.AssumeErr } // RevertAssumedPodVolumes implements SchedulerVolumeBinder.RevertAssumedPodVolumes -func (b *FakeVolumeBinder) RevertAssumedPodVolumes(assumedPod *v1.Pod, nodeName string) {} +func (b *FakeVolumeBinder) RevertAssumedPodVolumes(_ *PodVolumes) {} // BindPodVolumes implements SchedulerVolumeBinder.BindPodVolumes. -func (b *FakeVolumeBinder) BindPodVolumes(assumedPod *v1.Pod) error { +func (b *FakeVolumeBinder) BindPodVolumes(assumedPod *v1.Pod, podVolumes *PodVolumes) error { b.BindCalled = true return b.config.BindErr } - -// GetBindingsCache implements SchedulerVolumeBinder.GetBindingsCache. -func (b *FakeVolumeBinder) GetBindingsCache() PodBindingCache { - return nil -} - -// DeletePodBindings implements SchedulerVolumeBinder.DeletePodBindings. -func (b *FakeVolumeBinder) DeletePodBindings(pod *v1.Pod) {} diff --git a/pkg/controller/volume/scheduling/scheduler_binder_test.go b/pkg/controller/volume/scheduling/scheduler_binder_test.go index 00332ca9d8a..786bc0dbe4a 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder_test.go +++ b/pkg/controller/volume/scheduling/scheduler_binder_test.go @@ -123,6 +123,7 @@ type testEnv struct { reactor *pvtesting.VolumeReactor binder SchedulerVolumeBinder internalBinder *volumeBinder + internalPodInformer coreinformers.PodInformer internalNodeInformer coreinformers.NodeInformer internalCSINodeInformer storageinformers.CSINodeInformer internalPVCache *assumeCache @@ -144,12 +145,14 @@ func newTestBinder(t *testing.T, stopCh <-chan struct{}) *testEnv { }) informerFactory := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc()) + podInformer := informerFactory.Core().V1().Pods() nodeInformer := informerFactory.Core().V1().Nodes() csiNodeInformer := informerFactory.Storage().V1().CSINodes() pvcInformer := informerFactory.Core().V1().PersistentVolumeClaims() classInformer := informerFactory.Storage().V1().StorageClasses() binder := NewVolumeBinder( client, + podInformer, nodeInformer, csiNodeInformer, pvcInformer, @@ -246,6 +249,7 @@ func newTestBinder(t *testing.T, stopCh <-chan struct{}) *testEnv { reactor: reactor, binder: binder, internalBinder: internalBinder, + internalPodInformer: podInformer, internalNodeInformer: nodeInformer, internalCSINodeInformer: csiNodeInformer, internalPVCache: internalPVCache, @@ -358,7 +362,7 @@ func (env *testEnv) deleteClaims(pvcs []*v1.PersistentVolumeClaim) { } } -func (env *testEnv) assumeVolumes(t *testing.T, node string, pod *v1.Pod, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { +func (env *testEnv) assumeVolumes(t *testing.T, node string, pod *v1.Pod, bindings []*BindingInfo, provisionings []*v1.PersistentVolumeClaim) { pvCache := env.internalBinder.pvCache for _, binding := range bindings { if err := pvCache.Assume(binding.pv); err != nil { @@ -372,18 +376,17 @@ func (env *testEnv) assumeVolumes(t *testing.T, node string, pod *v1.Pod, bindin t.Fatalf("error: %v", err) } } - - env.internalBinder.podBindingCache.UpdateBindings(pod, node, bindings, provisionings) } -func (env *testEnv) initPodCache(pod *v1.Pod, node string, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { - cache := env.internalBinder.podBindingCache - cache.UpdateBindings(pod, node, bindings, provisionings) -} - -func (env *testEnv) validatePodCache(t *testing.T, node string, pod *v1.Pod, expectedBindings []*bindingInfo, expectedProvisionings []*v1.PersistentVolumeClaim) { - cache := env.internalBinder.podBindingCache - bindings := cache.GetBindings(pod, node) +func (env *testEnv) validatePodCache(t *testing.T, node string, pod *v1.Pod, podVolumes *PodVolumes, expectedBindings []*BindingInfo, expectedProvisionings []*v1.PersistentVolumeClaim) { + var ( + bindings []*BindingInfo + provisionedClaims []*v1.PersistentVolumeClaim + ) + if podVolumes != nil { + bindings = podVolumes.StaticBindings + provisionedClaims = podVolumes.DynamicProvisions + } if aLen, eLen := len(bindings), len(expectedBindings); aLen != eLen { t.Errorf("expected %v bindings, got %v", eLen, aLen) } else if expectedBindings == nil && bindings != nil { @@ -406,7 +409,6 @@ func (env *testEnv) validatePodCache(t *testing.T, node string, pod *v1.Pod, exp } } - provisionedClaims := cache.GetProvisionedPVCs(pod, node) if aLen, eLen := len(provisionedClaims), len(expectedProvisionings); aLen != eLen { t.Errorf("expected %v provisioned claims, got %v", eLen, aLen) } else if expectedProvisionings == nil && provisionedClaims != nil { @@ -424,12 +426,7 @@ func (env *testEnv) validatePodCache(t *testing.T, node string, pod *v1.Pod, exp } } -func (env *testEnv) getPodBindings(t *testing.T, node string, pod *v1.Pod) []*bindingInfo { - cache := env.internalBinder.podBindingCache - return cache.GetBindings(pod, node) -} - -func (env *testEnv) validateAssume(t *testing.T, pod *v1.Pod, bindings []*bindingInfo, provisionings []*v1.PersistentVolumeClaim) { +func (env *testEnv) validateAssume(t *testing.T, pod *v1.Pod, bindings []*BindingInfo, provisionings []*v1.PersistentVolumeClaim) { // Check pv cache pvCache := env.internalBinder.pvCache for _, b := range bindings { @@ -465,7 +462,7 @@ func (env *testEnv) validateAssume(t *testing.T, pod *v1.Pod, bindings []*bindin } } -func (env *testEnv) validateCacheRestored(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 { @@ -725,8 +722,8 @@ func makePodWithoutPVC() *v1.Pod { return pod } -func makeBinding(pvc *v1.PersistentVolumeClaim, pv *v1.PersistentVolume) *bindingInfo { - return &bindingInfo{pvc: pvc, pv: pv} +func makeBinding(pvc *v1.PersistentVolumeClaim, pv *v1.PersistentVolume) *BindingInfo { + return &BindingInfo{pvc: pvc, pv: pv} } func addProvisionAnn(pvc *v1.PersistentVolumeClaim) *v1.PersistentVolumeClaim { @@ -773,13 +770,13 @@ func checkReasons(t *testing.T, actual, expected ConflictReasons) { } // findPodVolumes gets and finds volumes for given pod and node -func findPodVolumes(binder SchedulerVolumeBinder, pod *v1.Pod, node *v1.Node) (ConflictReasons, error) { +func findPodVolumes(binder SchedulerVolumeBinder, pod *v1.Pod, node *v1.Node) (*PodVolumes, ConflictReasons, error) { boundClaims, claimsToBind, unboundClaimsImmediate, err := binder.GetPodVolumes(pod) if err != nil { - return nil, err + return nil, nil, err } if len(unboundClaimsImmediate) > 0 { - return nil, fmt.Errorf("pod has unbound immediate PersistentVolumeClaims") + return nil, nil, fmt.Errorf("pod has unbound immediate PersistentVolumeClaims") } return binder.FindPodVolumes(pod, boundClaims, claimsToBind, node) } @@ -795,7 +792,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { pod *v1.Pod // Expected podBindingCache fields - expectedBindings []*bindingInfo + expectedBindings []*BindingInfo // Expected return values reasons ConflictReasons @@ -829,7 +826,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { "unbound-pvc,pv-same-node": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, pvs: []*v1.PersistentVolume{pvNode2, pvNode1a, pvNode1b}, - expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, + expectedBindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a)}, }, "unbound-pvc,pv-different-node": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, @@ -839,23 +836,23 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { "two-unbound-pvcs": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, unboundPVC2}, pvs: []*v1.PersistentVolume{pvNode1a, pvNode1b}, - expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)}, + expectedBindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)}, }, "two-unbound-pvcs,order-by-size": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC2, unboundPVC}, pvs: []*v1.PersistentVolume{pvNode1a, pvNode1b}, - expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)}, + expectedBindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)}, }, "two-unbound-pvcs,partial-match": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, unboundPVC2}, pvs: []*v1.PersistentVolume{pvNode1a}, - expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, + expectedBindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a)}, reasons: ConflictReasons{ErrReasonBindConflict}, }, "one-bound,one-unbound": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, boundPVC}, pvs: []*v1.PersistentVolume{pvBound, pvNode1a}, - expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, + expectedBindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a)}, }, "one-bound,one-unbound,no-match": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, boundPVC}, @@ -920,7 +917,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { } // Execute - reasons, err := findPodVolumes(testEnv.binder, scenario.pod, testNode) + podVolumes, reasons, err := findPodVolumes(testEnv.binder, scenario.pod, testNode) // Validate if !scenario.shouldFail && err != nil { @@ -930,7 +927,7 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) { t.Error("returned success but expected error") } checkReasons(t, reasons, scenario.reasons) - testEnv.validatePodCache(t, testNode.Name, scenario.pod, scenario.expectedBindings, nil) + testEnv.validatePodCache(t, testNode.Name, scenario.pod, podVolumes, scenario.expectedBindings, nil) } for name, scenario := range scenarios { @@ -949,7 +946,7 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { pod *v1.Pod // Expected podBindingCache fields - expectedBindings []*bindingInfo + expectedBindings []*BindingInfo expectedProvisions []*v1.PersistentVolumeClaim // Expected return values @@ -964,7 +961,7 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { "two-unbound-pvcs,one-matched,one-provisioned": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, provisionedPVC}, pvs: []*v1.PersistentVolume{pvNode1a}, - expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, + expectedBindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a)}, expectedProvisions: []*v1.PersistentVolumeClaim{provisionedPVC}, }, "one-bound,one-provisioned": { @@ -1025,7 +1022,7 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { } // Execute - reasons, err := findPodVolumes(testEnv.binder, scenario.pod, testNode) + podVolumes, reasons, err := findPodVolumes(testEnv.binder, scenario.pod, testNode) // Validate if !scenario.shouldFail && err != nil { @@ -1035,7 +1032,7 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) { t.Error("returned success but expected error") } checkReasons(t, reasons, scenario.reasons) - testEnv.validatePodCache(t, testNode.Name, scenario.pod, scenario.expectedBindings, scenario.expectedProvisions) + testEnv.validatePodCache(t, testNode.Name, scenario.pod, podVolumes, scenario.expectedBindings, scenario.expectedProvisions) } for name, scenario := range scenarios { @@ -1125,7 +1122,7 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { } // Execute - reasons, err := findPodVolumes(testEnv.binder, scenario.pod, node) + _, reasons, err := findPodVolumes(testEnv.binder, scenario.pod, node) // Validate if !scenario.shouldFail && err != nil { @@ -1147,14 +1144,14 @@ func TestAssumePodVolumes(t *testing.T) { // Inputs podPVCs []*v1.PersistentVolumeClaim pvs []*v1.PersistentVolume - bindings []*bindingInfo + bindings []*BindingInfo provisionedPVCs []*v1.PersistentVolumeClaim // Expected return values shouldFail bool expectedAllBound bool - expectedBindings []*bindingInfo + expectedBindings []*BindingInfo expectedProvisionings []*v1.PersistentVolumeClaim } scenarios := map[string]scenarioType{ @@ -1165,42 +1162,42 @@ func TestAssumePodVolumes(t *testing.T) { }, "one-binding": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a)}, pvs: []*v1.PersistentVolume{pvNode1a}, - expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + expectedBindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, expectedProvisionings: []*v1.PersistentVolumeClaim{}, }, "two-bindings": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, unboundPVC2}, - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)}, pvs: []*v1.PersistentVolume{pvNode1a, pvNode1b}, - expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound), makeBinding(unboundPVC2, pvNode1bBound)}, + expectedBindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound), makeBinding(unboundPVC2, pvNode1bBound)}, expectedProvisionings: []*v1.PersistentVolumeClaim{}, }, "pv-already-bound": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, pvs: []*v1.PersistentVolume{pvNode1aBound}, - expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + expectedBindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, expectedProvisionings: []*v1.PersistentVolumeClaim{}, }, "tmpupdate-failed": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)}, pvs: []*v1.PersistentVolume{pvNode1a}, shouldFail: true, }, "one-binding, one-pvc-provisioned": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, provisionedPVC}, - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a)}, pvs: []*v1.PersistentVolume{pvNode1a}, provisionedPVCs: []*v1.PersistentVolumeClaim{provisionedPVC}, - expectedBindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + expectedBindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, expectedProvisionings: []*v1.PersistentVolumeClaim{selectedNodePVC}, }, "one-binding, one-provision-tmpupdate-failed": { podPVCs: []*v1.PersistentVolumeClaim{unboundPVC, provisionedPVCHigherVersion}, - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1a)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1a)}, pvs: []*v1.PersistentVolume{pvNode1a}, provisionedPVCs: []*v1.PersistentVolumeClaim{provisionedPVC2}, shouldFail: true, @@ -1215,11 +1212,14 @@ func TestAssumePodVolumes(t *testing.T) { testEnv := newTestBinder(t, ctx.Done()) testEnv.initClaims(scenario.podPVCs, scenario.podPVCs) pod := makePod(scenario.podPVCs) - testEnv.initPodCache(pod, "node1", scenario.bindings, scenario.provisionedPVCs) + podVolumes := &PodVolumes{ + StaticBindings: scenario.bindings, + DynamicProvisions: scenario.provisionedPVCs, + } testEnv.initVolumes(scenario.pvs, scenario.pvs) // Execute - allBound, err := testEnv.binder.AssumePodVolumes(pod, "node1") + allBound, err := testEnv.binder.AssumePodVolumes(pod, "node1", podVolumes) // Validate if !scenario.shouldFail && err != nil { @@ -1242,7 +1242,7 @@ func TestAssumePodVolumes(t *testing.T) { } else { testEnv.validateAssume(t, pod, scenario.expectedBindings, scenario.expectedProvisionings) } - testEnv.validatePodCache(t, pod.Spec.NodeName, pod, scenario.expectedBindings, scenario.expectedProvisionings) + testEnv.validatePodCache(t, pod.Spec.NodeName, pod, podVolumes, scenario.expectedBindings, scenario.expectedProvisionings) } for name, scenario := range scenarios { @@ -1255,33 +1255,36 @@ func TestRevertAssumedPodVolumes(t *testing.T) { defer cancel() podPVCs := []*v1.PersistentVolumeClaim{unboundPVC, provisionedPVC} - bindings := []*bindingInfo{makeBinding(unboundPVC, pvNode1a)} + bindings := []*BindingInfo{makeBinding(unboundPVC, pvNode1a)} pvs := []*v1.PersistentVolume{pvNode1a} provisionedPVCs := []*v1.PersistentVolumeClaim{provisionedPVC} - expectedBindings := []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)} + 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) + podVolumes := &PodVolumes{ + StaticBindings: bindings, + DynamicProvisions: provisionedPVCs, + } testEnv.initVolumes(pvs, pvs) - allbound, err := testEnv.binder.AssumePodVolumes(pod, "node1") + allbound, err := testEnv.binder.AssumePodVolumes(pod, "node1", podVolumes) if allbound || err != nil { t.Errorf("No volumes are assumed") } testEnv.validateAssume(t, pod, expectedBindings, expectedProvisionings) - testEnv.binder.RevertAssumedPodVolumes(pod, "node1") + testEnv.binder.RevertAssumedPodVolumes(podVolumes) testEnv.validateCacheRestored(t, pod, bindings, provisionedPVCs) } func TestBindAPIUpdate(t *testing.T) { type scenarioType struct { // Inputs - bindings []*bindingInfo + bindings []*BindingInfo cachedPVs []*v1.PersistentVolume // if nil, use cachedPVs apiPVs []*v1.PersistentVolume @@ -1310,33 +1313,33 @@ func TestBindAPIUpdate(t *testing.T) { shouldFail: true, }, "nothing-to-bind-provisionings-nil": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, shouldFail: true, }, "nothing-to-bind-empty": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, }, "one-binding": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, cachedPVs: []*v1.PersistentVolume{pvNode1a}, expectedPVs: []*v1.PersistentVolume{pvNode1aBound}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, }, "two-bindings": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound), makeBinding(unboundPVC2, pvNode1bBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound), makeBinding(unboundPVC2, pvNode1bBound)}, cachedPVs: []*v1.PersistentVolume{pvNode1a, pvNode1b}, expectedPVs: []*v1.PersistentVolume{pvNode1aBound, pvNode1bBound}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, }, "api-already-updated": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, cachedPVs: []*v1.PersistentVolume{pvNode1aBound}, expectedPVs: []*v1.PersistentVolume{pvNode1aBound}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, }, "api-update-failed": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound), makeBinding(unboundPVC2, pvNode1bBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound), makeBinding(unboundPVC2, pvNode1bBound)}, cachedPVs: []*v1.PersistentVolume{pvNode1a, pvNode1b}, apiPVs: []*v1.PersistentVolume{pvNode1a, pvNode1bBoundHigherVersion}, expectedPVs: []*v1.PersistentVolume{pvNode1aBound, pvNode1b}, @@ -1345,13 +1348,13 @@ func TestBindAPIUpdate(t *testing.T) { shouldFail: true, }, "one-provisioned-pvc": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, cachedPVCs: []*v1.PersistentVolumeClaim{provisionedPVC}, expectedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, }, "provision-api-update-failed": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC), addProvisionAnn(provisionedPVC2)}, cachedPVCs: []*v1.PersistentVolumeClaim{provisionedPVC, provisionedPVC2}, apiPVCs: []*v1.PersistentVolumeClaim{provisionedPVC, provisionedPVCHigherVersion}, @@ -1360,7 +1363,7 @@ func TestBindAPIUpdate(t *testing.T) { shouldFail: true, }, "binding-succeed, provision-api-update-failed": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, cachedPVs: []*v1.PersistentVolume{pvNode1a}, expectedPVs: []*v1.PersistentVolume{pvNode1aBound}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC), addProvisionAnn(provisionedPVC2)}, @@ -1420,7 +1423,7 @@ func TestCheckBindings(t *testing.T) { initPVs []*v1.PersistentVolume initPVCs []*v1.PersistentVolumeClaim - bindings []*bindingInfo + bindings []*BindingInfo provisionedPVCs []*v1.PersistentVolumeClaim // api updates before checking @@ -1444,41 +1447,41 @@ func TestCheckBindings(t *testing.T) { shouldFail: true, }, "nothing-to-bind-provisionings-nil": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, shouldFail: true, }, "nothing-to-bind": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, expectedBound: true, }, "binding-bound": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{pvNode1aBound}, initPVCs: []*v1.PersistentVolumeClaim{boundPVCNode1a}, expectedBound: true, }, "binding-prebound": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{pvNode1aBound}, initPVCs: []*v1.PersistentVolumeClaim{preboundPVCNode1a}, }, "binding-unbound": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{pvNode1aBound}, initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, }, "binding-pvc-not-exists": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{pvNode1aBound}, shouldFail: true, }, "binding-pv-not-exists": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{pvNode1aBound}, initPVCs: []*v1.PersistentVolumeClaim{boundPVCNode1a}, @@ -1486,7 +1489,7 @@ func TestCheckBindings(t *testing.T) { shouldFail: true, }, "binding-claimref-nil": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{pvNode1a}, initPVCs: []*v1.PersistentVolumeClaim{boundPVCNode1a}, @@ -1495,7 +1498,7 @@ func TestCheckBindings(t *testing.T) { shouldFail: true, }, "binding-claimref-uid-empty": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{pvNode1aBound}, initPVCs: []*v1.PersistentVolumeClaim{boundPVCNode1a}, @@ -1504,13 +1507,13 @@ func TestCheckBindings(t *testing.T) { shouldFail: true, }, "binding-one-bound,one-unbound": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound), makeBinding(unboundPVC2, pvNode1bBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound), makeBinding(unboundPVC2, pvNode1bBound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{pvNode1aBound, pvNode1bBound}, initPVCs: []*v1.PersistentVolumeClaim{boundPVCNode1a, unboundPVC2}, }, "provisioning-pvc-bound": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, initPVs: []*v1.PersistentVolume{pvBound}, initPVCs: []*v1.PersistentVolumeClaim{provisionedPVCBound}, @@ -1518,26 +1521,26 @@ func TestCheckBindings(t *testing.T) { expectedBound: true, }, "provisioning-pvc-unbound": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, initPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, }, "provisioning-pvc-not-exists": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, initPVCs: []*v1.PersistentVolumeClaim{provisionedPVC}, deletePVCs: true, shouldFail: true, }, "provisioning-pvc-annotations-nil": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, initPVCs: []*v1.PersistentVolumeClaim{provisionedPVC}, apiPVCs: []*v1.PersistentVolumeClaim{provisionedPVC}, shouldFail: true, }, "provisioning-pvc-selected-node-dropped": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, initPVCs: []*v1.PersistentVolumeClaim{provisionedPVC}, apiPVCs: []*v1.PersistentVolumeClaim{pvcSetEmptyAnnotations(provisionedPVC)}, @@ -1545,13 +1548,13 @@ func TestCheckBindings(t *testing.T) { }, "provisioning-pvc-selected-node-wrong-node": { initPVCs: []*v1.PersistentVolumeClaim{provisionedPVC}, - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, apiPVCs: []*v1.PersistentVolumeClaim{pvcSetSelectedNode(provisionedPVC, "wrong-node")}, shouldFail: true, }, "binding-bound-provisioning-unbound": { - bindings: []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, pvNode1aBound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, initPVs: []*v1.PersistentVolume{pvNode1aBound}, initPVCs: []*v1.PersistentVolumeClaim{boundPVCNode1a, addProvisionAnn(provisionedPVC)}, @@ -1559,7 +1562,7 @@ func TestCheckBindings(t *testing.T) { "tolerate-provisioning-pvc-bound-pv-not-found": { initPVs: []*v1.PersistentVolume{pvNode1a}, initPVCs: []*v1.PersistentVolumeClaim{provisionedPVC}, - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVC)}, apiPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provisionedPVCBound)}, deletePVs: true, @@ -1573,6 +1576,7 @@ func TestCheckBindings(t *testing.T) { // Setup pod := makePod(nil) testEnv := newTestBinder(t, ctx.Done()) + testEnv.internalPodInformer.Informer().GetIndexer().Add(pod) testEnv.initNodes([]*v1.Node{node1}) testEnv.initVolumes(scenario.initPVs, nil) testEnv.initClaims(scenario.initPVCs, nil) @@ -1618,7 +1622,7 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { initNodes []*v1.Node initCSINodes []*storagev1.CSINode - bindings []*bindingInfo + bindings []*BindingInfo provisionedPVCs []*v1.PersistentVolumeClaim // API updates before checking @@ -1632,7 +1636,7 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { } scenarios := map[string]scenarioType{ "provisioning-pvc-bound": { - bindings: []*bindingInfo{}, + bindings: []*BindingInfo{}, provisionedPVCs: []*v1.PersistentVolumeClaim{addProvisionAnn(provMigrationPVCBound)}, initPVs: []*v1.PersistentVolume{migrationPVBound}, initPVCs: []*v1.PersistentVolumeClaim{provMigrationPVCBound}, @@ -1642,7 +1646,7 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { expectedBound: true, }, "binding-node-pv-same-zone": { - bindings: []*bindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, @@ -1651,7 +1655,7 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { migrationEnabled: true, }, "binding-without-csinode": { - bindings: []*bindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, @@ -1660,7 +1664,7 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { migrationEnabled: true, }, "binding-non-migrated-plugin": { - bindings: []*bindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, @@ -1669,7 +1673,7 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { migrationEnabled: true, }, "binding-node-pv-in-different-zones": { - bindings: []*bindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, @@ -1679,7 +1683,7 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { shouldFail: true, }, "binding-node-pv-different-zones-migration-off": { - bindings: []*bindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, + bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, provisionedPVCs: []*v1.PersistentVolumeClaim{}, initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, @@ -1699,6 +1703,7 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { // Setup pod := makePod(nil) testEnv := newTestBinder(t, ctx.Done()) + testEnv.internalPodInformer.Informer().GetIndexer().Add(pod) testEnv.initNodes(scenario.initNodes) testEnv.initCSINodes(scenario.initCSINodes) testEnv.initVolumes(scenario.initPVs, nil) @@ -1741,7 +1746,7 @@ func TestBindPodVolumes(t *testing.T) { initPVCs []*v1.PersistentVolumeClaim // assume PV & PVC with these binding results - binding *bindingInfo + binding *BindingInfo claimToProvision *v1.PersistentVolumeClaim // API updates after assume before bind @@ -1819,19 +1824,7 @@ func TestBindPodVolumes(t *testing.T) { initPVs: []*v1.PersistentVolume{pvNode1a}, initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, delayFunc: func(t *testing.T, testEnv *testEnv, pod *v1.Pod, pvs []*v1.PersistentVolume, pvcs []*v1.PersistentVolumeClaim) { - bindingsCache := testEnv.binder.GetBindingsCache() - if bindingsCache == nil { - t.Fatalf("Failed to get bindings cache") - } - - // Delete the pod from the cache - bindingsCache.DeleteBindings(pod) - - // Check that it's deleted - bindings := bindingsCache.GetBindings(pod, "node1") - if bindings != nil { - t.Fatalf("Failed to delete bindings") - } + testEnv.client.CoreV1().Pods(pod.Namespace).Delete(context.Background(), pod.Name, metav1.DeleteOptions{}) }, shouldFail: true, }, @@ -1892,15 +1885,16 @@ func TestBindPodVolumes(t *testing.T) { // Setup pod := makePod(nil) testEnv := newTestBinder(t, ctx.Done()) + testEnv.internalPodInformer.Informer().GetIndexer().Add(pod) if scenario.nodes == nil { scenario.nodes = []*v1.Node{node1} } + bindings := []*BindingInfo{} + claimsToProvision := []*v1.PersistentVolumeClaim{} if !scenario.bindingsNil { - bindings := []*bindingInfo{} if scenario.binding != nil { - bindings = []*bindingInfo{scenario.binding} + bindings = []*BindingInfo{scenario.binding} } - claimsToProvision := []*v1.PersistentVolumeClaim{} if scenario.claimToProvision != nil { claimsToProvision = []*v1.PersistentVolumeClaim{scenario.claimToProvision} } @@ -1934,7 +1928,11 @@ func TestBindPodVolumes(t *testing.T) { } // Execute - err := testEnv.binder.BindPodVolumes(pod) + podVolumes := &PodVolumes{ + StaticBindings: bindings, + DynamicProvisions: claimsToProvision, + } + err := testEnv.binder.BindPodVolumes(pod, podVolumes) // Validate if !scenario.shouldFail && err != nil { @@ -1974,17 +1972,17 @@ func TestFindAssumeVolumes(t *testing.T) { // Execute // 1. Find matching PVs - reasons, err := findPodVolumes(testEnv.binder, pod, testNode) + podVolumes, reasons, err := findPodVolumes(testEnv.binder, pod, testNode) if err != nil { t.Errorf("Test failed: FindPodVolumes returned error: %v", err) } if len(reasons) > 0 { t.Errorf("Test failed: couldn't find PVs for all PVCs: %v", reasons) } - expectedBindings := testEnv.getPodBindings(t, testNode.Name, pod) + expectedBindings := podVolumes.StaticBindings // 2. Assume matches - allBound, err := testEnv.binder.AssumePodVolumes(pod, testNode.Name) + allBound, err := testEnv.binder.AssumePodVolumes(pod, testNode.Name, podVolumes) if err != nil { t.Errorf("Test failed: AssumePodVolumes returned error: %v", err) } @@ -1994,19 +1992,19 @@ func TestFindAssumeVolumes(t *testing.T) { testEnv.validateAssume(t, pod, expectedBindings, nil) // After assume, claimref should be set on pv - expectedBindings = testEnv.getPodBindings(t, testNode.Name, pod) + expectedBindings = podVolumes.StaticBindings // 3. Find matching PVs again // This should always return the original chosen pv // Run this many times in case sorting returns different orders for the two PVs. for i := 0; i < 50; i++ { - reasons, err := findPodVolumes(testEnv.binder, pod, testNode) + podVolumes, reasons, err := findPodVolumes(testEnv.binder, pod, testNode) if err != nil { t.Errorf("Test failed: FindPodVolumes returned error: %v", err) } if len(reasons) > 0 { t.Errorf("Test failed: couldn't find PVs for all PVCs: %v", reasons) } - testEnv.validatePodCache(t, testNode.Name, pod, expectedBindings, nil) + testEnv.validatePodCache(t, testNode.Name, pod, podVolumes, expectedBindings, nil) } } diff --git a/pkg/scheduler/algorithmprovider/registry.go b/pkg/scheduler/algorithmprovider/registry.go index 02ded546cad..1987804c804 100644 --- a/pkg/scheduler/algorithmprovider/registry.go +++ b/pkg/scheduler/algorithmprovider/registry.go @@ -157,11 +157,6 @@ func getDefaultConfig() *schedulerapi.Plugins { {Name: defaultbinder.Name}, }, }, - PostBind: &schedulerapi.PluginSet{ - Enabled: []schedulerapi.Plugin{ - {Name: volumebinding.Name}, - }, - }, } } diff --git a/pkg/scheduler/algorithmprovider/registry_test.go b/pkg/scheduler/algorithmprovider/registry_test.go index f8bc917ecb7..41f842cdcba 100644 --- a/pkg/scheduler/algorithmprovider/registry_test.go +++ b/pkg/scheduler/algorithmprovider/registry_test.go @@ -127,11 +127,6 @@ func TestClusterAutoscalerProvider(t *testing.T) { {Name: defaultbinder.Name}, }, }, - PostBind: &schedulerapi.PluginSet{ - Enabled: []schedulerapi.Plugin{ - {Name: volumebinding.Name}, - }, - }, } r := NewRegistry() @@ -229,11 +224,6 @@ func TestApplyFeatureGates(t *testing.T) { {Name: defaultbinder.Name}, }, }, - PostBind: &schedulerapi.PluginSet{ - Enabled: []schedulerapi.Plugin{ - {Name: volumebinding.Name}, - }, - }, }, }, { @@ -317,11 +307,6 @@ func TestApplyFeatureGates(t *testing.T) { {Name: defaultbinder.Name}, }, }, - PostBind: &schedulerapi.PluginSet{ - Enabled: []schedulerapi.Plugin{ - {Name: volumebinding.Name}, - }, - }, }, }, } diff --git a/pkg/scheduler/apis/config/testing/compatibility_test.go b/pkg/scheduler/apis/config/testing/compatibility_test.go index 636fa8ea57a..ce734f0a610 100644 --- a/pkg/scheduler/apis/config/testing/compatibility_test.go +++ b/pkg/scheduler/apis/config/testing/compatibility_test.go @@ -706,7 +706,6 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, wantExtenders: []config.Extender{{ URLPrefix: "/prefix", @@ -814,7 +813,6 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, wantExtenders: []config.Extender{{ URLPrefix: "/prefix", @@ -935,7 +933,6 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, wantExtenders: []config.Extender{{ URLPrefix: "/prefix", @@ -1058,7 +1055,6 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, wantExtenders: []config.Extender{{ URLPrefix: "/prefix", @@ -1181,7 +1177,6 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, wantExtenders: []config.Extender{{ URLPrefix: "/prefix", @@ -1308,7 +1303,6 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, wantExtenders: []config.Extender{{ URLPrefix: "/prefix", @@ -1438,7 +1432,6 @@ func TestAlgorithmProviderCompatibility(t *testing.T) { "ReservePlugin": {{Name: "VolumeBinding"}}, "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, } testcases := []struct { @@ -1510,7 +1503,6 @@ func TestAlgorithmProviderCompatibility(t *testing.T) { "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, "BindPlugin": {{Name: "DefaultBinder"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, }, } @@ -1602,7 +1594,6 @@ func TestPluginsConfigurationCompatibility(t *testing.T) { "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, "BindPlugin": {{Name: "DefaultBinder"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, } testcases := []struct { @@ -1953,7 +1944,6 @@ func TestPluginsConfigurationCompatibility(t *testing.T) { "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, "BindPlugin": {{Name: "DefaultBinder"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, wantPluginConfig: nil, }, diff --git a/pkg/scheduler/framework/plugins/legacy_registry.go b/pkg/scheduler/framework/plugins/legacy_registry.go index 4cca85d99ea..58839b5cc1f 100644 --- a/pkg/scheduler/framework/plugins/legacy_registry.go +++ b/pkg/scheduler/framework/plugins/legacy_registry.go @@ -274,7 +274,6 @@ func NewLegacyRegistry() *LegacyRegistry { plugins.Reserve = appendToPluginSet(plugins.Reserve, volumebinding.Name, nil) plugins.PreBind = appendToPluginSet(plugins.PreBind, volumebinding.Name, nil) plugins.Unreserve = appendToPluginSet(plugins.Unreserve, volumebinding.Name, nil) - plugins.PostBind = appendToPluginSet(plugins.PostBind, volumebinding.Name, nil) return }) registry.registerPredicateConfigProducer(NoDiskConflictPred, diff --git a/pkg/scheduler/framework/plugins/volumebinding/BUILD b/pkg/scheduler/framework/plugins/volumebinding/BUILD index 09593e452a7..9b8a012cdd1 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/BUILD +++ b/pkg/scheduler/framework/plugins/volumebinding/BUILD @@ -11,8 +11,6 @@ go_library( "//pkg/scheduler/framework/v1alpha1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", - "//staging/src/k8s.io/client-go/tools/cache:go_default_library", "//vendor/k8s.io/klog/v2:go_default_library", ], ) diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go index d5409865a76..8b001def107 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go @@ -24,8 +24,6 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/controller/volume/scheduling" "k8s.io/kubernetes/pkg/scheduler/apis/config" @@ -39,11 +37,18 @@ const ( stateKey framework.StateKey = Name ) +// the state is initialized in PreFilter phase. because we save the pointer in +// framework.CycleState, in the later phases we don't need to call Write method +// to update the value type stateData struct { skip bool // set true if pod does not have PVCs boundClaims []*v1.PersistentVolumeClaim claimsToBind []*v1.PersistentVolumeClaim allBound bool + // podVolumesByNode holds the pod's volume information found in the Filter + // phase for each node + // it's initialized in the PreFilter phase + podVolumesByNode map[string]*scheduling.PodVolumes } func (d *stateData) Clone() framework.StateData { @@ -52,12 +57,7 @@ func (d *stateData) Clone() framework.StateData { // VolumeBinding is a plugin that binds pod volumes in scheduling. // In the Filter phase, pod binding cache is created for the pod and used in -// Reserve and PreBind phases. Pod binding cache will be cleared at -// Unreserve and PostBind extension points. However, if pod fails before -// the Reserve phase and is deleted from the apiserver later, its pod binding -// cache cannot be cleared at plugin extension points. We register an -// event handler to clear pod binding cache when the pod is deleted to -// prevent memory leaking. +// Reserve and PreBind phases. type VolumeBinding struct { Binder scheduling.SchedulerVolumeBinder } @@ -67,7 +67,6 @@ var _ framework.FilterPlugin = &VolumeBinding{} var _ framework.ReservePlugin = &VolumeBinding{} var _ framework.PreBindPlugin = &VolumeBinding{} var _ framework.UnreservePlugin = &VolumeBinding{} -var _ framework.PostBindPlugin = &VolumeBinding{} // Name is the name of the plugin used in Registry and configurations. const Name = "VolumeBinding" @@ -90,7 +89,7 @@ func podHasPVCs(pod *v1.Pod) bool { // immediate PVCs bound. If not all immediate PVCs are bound, an // UnschedulableAndUnresolvable is returned. func (pl *VolumeBinding) PreFilter(ctx context.Context, state *framework.CycleState, pod *v1.Pod) *framework.Status { - // If pod does not request any PVC, we don't need to do anything. + // If pod does not reference any PVC, we don't need to do anything. if !podHasPVCs(pod) { state.Write(stateKey, &stateData{skip: true}) return nil @@ -107,7 +106,7 @@ func (pl *VolumeBinding) PreFilter(ctx context.Context, state *framework.CycleSt status.AppendReason("pod has unbound immediate PersistentVolumeClaims") return status } - state.Write(stateKey, &stateData{boundClaims: boundClaims, claimsToBind: claimsToBind}) + state.Write(stateKey, &stateData{boundClaims: boundClaims, claimsToBind: claimsToBind, podVolumesByNode: make(map[string]*scheduling.PodVolumes)}) return nil } @@ -155,7 +154,7 @@ func (pl *VolumeBinding) Filter(ctx context.Context, cs *framework.CycleState, p return nil } - reasons, err := pl.Binder.FindPodVolumes(pod, state.boundClaims, state.claimsToBind, node) + podVolumes, reasons, err := pl.Binder.FindPodVolumes(pod, state.boundClaims, state.claimsToBind, node) if err != nil { return framework.NewStatus(framework.Error, err.Error()) @@ -168,16 +167,31 @@ func (pl *VolumeBinding) Filter(ctx context.Context, cs *framework.CycleState, p } return status } + + cs.Lock() + state.podVolumesByNode[node.Name] = podVolumes + cs.Unlock() return nil } // Reserve reserves volumes of pod and saves binding status in cycle state. func (pl *VolumeBinding) Reserve(ctx context.Context, cs *framework.CycleState, pod *v1.Pod, nodeName string) *framework.Status { - allBound, err := pl.Binder.AssumePodVolumes(pod, nodeName) + state, err := getStateData(cs) if err != nil { return framework.NewStatus(framework.Error, err.Error()) } - cs.Write(stateKey, &stateData{allBound: allBound}) + // we don't need to hold the lock as only one node will be reserved for the given pod + podVolumes, ok := state.podVolumesByNode[nodeName] + if ok { + allBound, err := pl.Binder.AssumePodVolumes(pod, nodeName, podVolumes) + if err != nil { + return framework.NewStatus(framework.Error, err.Error()) + } + state.allBound = allBound + } else { + // may not exist if the pod does not reference any PVC + state.allBound = true + } return nil } @@ -195,8 +209,13 @@ func (pl *VolumeBinding) PreBind(ctx context.Context, cs *framework.CycleState, // no need to bind volumes return nil } + // we don't need to hold the lock as only one node will be pre-bound for the given pod + podVolumes, ok := s.podVolumesByNode[nodeName] + if !ok { + return framework.NewStatus(framework.Error, fmt.Sprintf("no pod volumes found for node %q", nodeName)) + } klog.V(5).Infof("Trying to bind volumes for pod \"%v/%v\"", pod.Namespace, pod.Name) - err = pl.Binder.BindPodVolumes(pod) + err = pl.Binder.BindPodVolumes(pod, podVolumes) if err != nil { klog.V(1).Infof("Failed to bind volumes for pod \"%v/%v\": %v", pod.Namespace, pod.Name, err) return framework.NewStatus(framework.Error, err.Error()) @@ -205,17 +224,19 @@ func (pl *VolumeBinding) PreBind(ctx context.Context, cs *framework.CycleState, return nil } -// 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. +// Unreserve clears assumed PV and PVC cache. +// It's idempotent, and does nothing if no cache 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 -} - -// PostBind is called after a pod is successfully bound. -func (pl *VolumeBinding) PostBind(ctx context.Context, cs *framework.CycleState, pod *v1.Pod, nodeName string) { - pl.Binder.DeletePodBindings(pod) + s, err := getStateData(cs) + if err != nil { + return + } + // we don't need to hold the lock as only one node may be unreserved + podVolumes, ok := s.podVolumesByNode[nodeName] + if !ok { + return + } + pl.Binder.RevertAssumedPodVolumes(podVolumes) return } @@ -228,37 +249,13 @@ func New(plArgs runtime.Object, fh framework.FrameworkHandle) (framework.Plugin, if err := validateArgs(args); err != nil { return nil, err } + podInformer := fh.SharedInformerFactory().Core().V1().Pods() nodeInformer := fh.SharedInformerFactory().Core().V1().Nodes() pvcInformer := fh.SharedInformerFactory().Core().V1().PersistentVolumeClaims() pvInformer := fh.SharedInformerFactory().Core().V1().PersistentVolumes() storageClassInformer := fh.SharedInformerFactory().Storage().V1().StorageClasses() csiNodeInformer := fh.SharedInformerFactory().Storage().V1().CSINodes() - binder := scheduling.NewVolumeBinder(fh.ClientSet(), nodeInformer, csiNodeInformer, pvcInformer, pvInformer, storageClassInformer, time.Duration(args.BindTimeoutSeconds)*time.Second) - // TODO(#90962) Because pod volume binding cache in SchedulerVolumeBinder is - // used only in current scheduling cycle, we can share it via - // framework.CycleState, then we don't need to register this event handler - // and Unreserve/PostBind extension points to clear pod volume binding - // cache. - fh.SharedInformerFactory().Core().V1().Pods().Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ - DeleteFunc: func(obj interface{}) { - var pod *v1.Pod - switch t := obj.(type) { - case *v1.Pod: - pod = obj.(*v1.Pod) - case cache.DeletedFinalStateUnknown: - var ok bool - pod, ok = t.Obj.(*v1.Pod) - if !ok { - utilruntime.HandleError(fmt.Errorf("unable to convert object %T to *v1.Pod", obj)) - return - } - default: - utilruntime.HandleError(fmt.Errorf("unable to handle object %T", obj)) - return - } - binder.DeletePodBindings(pod) - }, - }) + binder := scheduling.NewVolumeBinder(fh.ClientSet(), podInformer, nodeInformer, csiNodeInformer, pvcInformer, pvInformer, storageClassInformer, time.Duration(args.BindTimeoutSeconds)*time.Second) return &VolumeBinding{ Binder: binder, }, nil diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go index 0708332de07..bd711107a23 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding_test.go @@ -134,7 +134,8 @@ func TestVolumeBinding(t *testing.T) { boundClaims: []*v1.PersistentVolumeClaim{ makePVC("pvc-a", "pv-a", waitSC.Name), }, - claimsToBind: []*v1.PersistentVolumeClaim{}, + claimsToBind: []*v1.PersistentVolumeClaim{}, + podVolumesByNode: map[string]*scheduling.PodVolumes{}, }, }, { @@ -158,6 +159,7 @@ func TestVolumeBinding(t *testing.T) { claimsToBind: []*v1.PersistentVolumeClaim{ makePVC("pvc-a", "", waitSC.Name), }, + podVolumesByNode: map[string]*scheduling.PodVolumes{}, }, wantFilterStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, string(scheduling.ErrReasonBindConflict)), }, @@ -199,6 +201,7 @@ func TestVolumeBinding(t *testing.T) { claimsToBind: []*v1.PersistentVolumeClaim{ makePVC("pvc-b", "", waitSC.Name), }, + podVolumesByNode: map[string]*scheduling.PodVolumes{}, }, wantFilterStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, string(scheduling.ErrReasonNodeConflict), string(scheduling.ErrReasonBindConflict)), }, @@ -221,7 +224,8 @@ func TestVolumeBinding(t *testing.T) { boundClaims: []*v1.PersistentVolumeClaim{ makePVC("pvc-a", "pv-a", waitSC.Name), }, - claimsToBind: []*v1.PersistentVolumeClaim{}, + claimsToBind: []*v1.PersistentVolumeClaim{}, + podVolumesByNode: map[string]*scheduling.PodVolumes{}, }, wantFilterStatus: framework.NewStatus(framework.Error, `could not find v1.PersistentVolume "pv-a"`), }, @@ -229,7 +233,8 @@ func TestVolumeBinding(t *testing.T) { for _, item := range table { t.Run(item.name, func(t *testing.T) { - ctx := context.Background() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() client := fake.NewSimpleClientset() informerFactory := informers.NewSharedInformerFactory(client, 0) opts := []runtime.Option{ @@ -256,15 +261,11 @@ func TestVolumeBinding(t *testing.T) { if item.node != nil { client.CoreV1().Nodes().Create(ctx, item.node, metav1.CreateOptions{}) } - if len(item.pvcs) > 0 { - for _, pvc := range item.pvcs { - client.CoreV1().PersistentVolumeClaims(pvc.Namespace).Create(ctx, pvc, metav1.CreateOptions{}) - } + for _, pvc := range item.pvcs { + client.CoreV1().PersistentVolumeClaims(pvc.Namespace).Create(ctx, pvc, metav1.CreateOptions{}) } - if len(item.pvs) > 0 { - for _, pv := range item.pvs { - client.CoreV1().PersistentVolumes().Create(ctx, pv, metav1.CreateOptions{}) - } + for _, pv := range item.pvs { + client.CoreV1().PersistentVolumes().Create(ctx, pv, metav1.CreateOptions{}) } caches := informerFactory.WaitForCacheSync(ctx.Done()) for _, synced := range caches { diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index 54ddd06c027..02b7d649d50 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -826,7 +826,7 @@ func setupTestSchedulerWithVolumeBinding(volumeBinder scheduling.SchedulerVolume st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New), st.RegisterPluginAsExtensions(volumebinding.Name, func(plArgs runtime.Object, handle framework.FrameworkHandle) (framework.Plugin, error) { return &volumebinding.VolumeBinding{Binder: volumeBinder}, nil - }, "PreFilter", "Filter", "Reserve", "Unreserve", "PreBind", "PostBind"), + }, "PreFilter", "Filter", "Reserve", "Unreserve", "PreBind"), } s, bindingChan, errChan := setupTestScheduler(queuedPodStore, scache, informerFactory, broadcaster, fns...) informerFactory.Start(stop) diff --git a/test/integration/scheduler/scheduler_test.go b/test/integration/scheduler/scheduler_test.go index b2f101c4473..2fd0591d7ca 100644 --- a/test/integration/scheduler/scheduler_test.go +++ b/test/integration/scheduler/scheduler_test.go @@ -148,7 +148,6 @@ func TestSchedulerCreationFromConfigMap(t *testing.T) { "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, "BindPlugin": {{Name: "DefaultBinder"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, }, { @@ -243,7 +242,6 @@ kind: Policy "UnreservePlugin": {{Name: "VolumeBinding"}}, "PreBindPlugin": {{Name: "VolumeBinding"}}, "BindPlugin": {{Name: "DefaultBinder"}}, - "PostBindPlugin": {{Name: "VolumeBinding"}}, }, }, {