diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go b/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go index def28682f0b..8594d133f33 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go @@ -22,11 +22,11 @@ import ( v1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/rand" corelisters "k8s.io/client-go/listers/core/v1" storagelisters "k8s.io/client-go/listers/storage/v1" + ephemeral "k8s.io/component-helpers/storage/ephemeral" storagehelpers "k8s.io/component-helpers/storage/volume" csitrans "k8s.io/csi-translation-lib" "k8s.io/klog/v2" @@ -152,7 +152,7 @@ func (pl *CSILimits) filterAttachableVolumes( for _, vol := range pod.Spec.Volumes { // CSI volumes can only be used through a PVC. pvcName := "" - ephemeral := false + isEphemeral := false switch { case vol.PersistentVolumeClaim != nil: pvcName = vol.PersistentVolumeClaim.ClaimName @@ -167,8 +167,8 @@ func (pl *CSILimits) filterAttachableVolumes( // just with a computed name and certain ownership. // That is checked below once the pvc object is // retrieved. - pvcName = pod.Name + "-" + vol.Name - ephemeral = true + pvcName = ephemeral.VolumeClaimName(pod, &vol) + isEphemeral = true default: continue } @@ -193,8 +193,10 @@ func (pl *CSILimits) filterAttachableVolumes( } // The PVC for an ephemeral volume must be owned by the pod. - if ephemeral && !metav1.IsControlledBy(pvc, pod) { - return fmt.Errorf("PVC %s/%s is not owned by pod", pod.Namespace, pvcName) + if isEphemeral { + if err := ephemeral.VolumeIsForPod(pod, pvc); err != nil { + return err + } } driverName, volumeHandle := pl.getCSIDriverInfo(csiNode, pvc) diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go b/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go index f94808d2a86..6dc35cf537a 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go @@ -568,7 +568,7 @@ func TestCSILimits(t *testing.T) { extraClaims: []v1.PersistentVolumeClaim{*conflictingClaim}, driverNames: []string{ebsCSIDriverName}, test: "ephemeral volume not owned", - wantStatus: framework.NewStatus(framework.Error, "PVC test/abc-xyz is not owned by pod"), + wantStatus: framework.NewStatus(framework.Error, "PVC test/abc-xyz was not created for pod test/abc (pod is not owner)"), }, { newPod: ephemeralVolumePod, diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi.go b/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi.go index 78e8458b794..ffbf1fd37e2 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi.go @@ -26,13 +26,13 @@ import ( v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/informers" corelisters "k8s.io/client-go/listers/core/v1" storagelisters "k8s.io/client-go/listers/storage/v1" + "k8s.io/component-helpers/storage/ephemeral" csilibplugins "k8s.io/csi-translation-lib/plugins" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/scheduler/framework" @@ -288,7 +288,7 @@ func (pl *nonCSILimits) filterVolumes(pod *v1.Pod, newPod bool, filteredVolumes } pvcName := "" - ephemeral := false + isEphemeral := false switch { case vol.PersistentVolumeClaim != nil: pvcName = vol.PersistentVolumeClaim.ClaimName @@ -303,8 +303,8 @@ func (pl *nonCSILimits) filterVolumes(pod *v1.Pod, newPod bool, filteredVolumes // just with a computed name and certain ownership. // That is checked below once the pvc object is // retrieved. - pvcName = pod.Name + "-" + vol.Name - ephemeral = true + pvcName = ephemeral.VolumeClaimName(pod, vol) + isEphemeral = true default: continue } @@ -332,8 +332,10 @@ func (pl *nonCSILimits) filterVolumes(pod *v1.Pod, newPod bool, filteredVolumes } // The PVC for an ephemeral volume must be owned by the pod. - if ephemeral && !metav1.IsControlledBy(pvc, pod) { - return fmt.Errorf("PVC %s/%s is not owned by pod", pod.Namespace, pvcName) + if isEphemeral { + if err := ephemeral.VolumeIsForPod(pod, pvc); err != nil { + return err + } } pvName := pvc.Spec.VolumeName diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi_test.go b/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi_test.go index a4ebd9113bc..2cdef2c0eee 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi_test.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi_test.go @@ -102,7 +102,7 @@ func TestEphemeralLimits(t *testing.T) { ephemeralEnabled: true, extraClaims: []v1.PersistentVolumeClaim{*conflictingClaim}, test: "volume not owned", - wantStatus: framework.NewStatus(framework.Error, "PVC test/abc-xyz is not owned by pod"), + wantStatus: framework.NewStatus(framework.Error, "PVC test/abc-xyz was not created for pod test/abc (pod is not owner)"), }, { newPod: ephemeralVolumePod, diff --git a/pkg/scheduler/framework/plugins/volumebinding/binder.go b/pkg/scheduler/framework/plugins/volumebinding/binder.go index ca931b6e4cf..c28e2a6a369 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/binder.go +++ b/pkg/scheduler/framework/plugins/volumebinding/binder.go @@ -40,6 +40,7 @@ import ( corelisters "k8s.io/client-go/listers/core/v1" storagelisters "k8s.io/client-go/listers/storage/v1" storagelistersv1beta1 "k8s.io/client-go/listers/storage/v1beta1" + "k8s.io/component-helpers/storage/ephemeral" storagehelpers "k8s.io/component-helpers/storage/volume" csitrans "k8s.io/csi-translation-lib" csiplugins "k8s.io/csi-translation-lib/plugins" @@ -686,7 +687,7 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim func (b *volumeBinder) isVolumeBound(pod *v1.Pod, vol *v1.Volume) (bound bool, pvc *v1.PersistentVolumeClaim, err error) { pvcName := "" - ephemeral := false + isEphemeral := false switch { case vol.PersistentVolumeClaim != nil: pvcName = vol.PersistentVolumeClaim.ClaimName @@ -699,16 +700,18 @@ func (b *volumeBinder) isVolumeBound(pod *v1.Pod, vol *v1.Volume) (bound bool, p } // Generic ephemeral inline volumes also use a PVC, // just with a computed name, and... - pvcName = pod.Name + "-" + vol.Name - ephemeral = true + pvcName = ephemeral.VolumeClaimName(pod, vol) + isEphemeral = true default: return true, nil, nil } bound, pvc, err = b.isPVCBound(pod.Namespace, pvcName) // ... the PVC must be owned by the pod. - if ephemeral && err == nil && pvc != nil && !metav1.IsControlledBy(pvc, pod) { - return false, nil, fmt.Errorf("PVC %s/%s is not owned by pod", pod.Namespace, pvcName) + if isEphemeral && err == nil && pvc != nil { + if err := ephemeral.VolumeIsForPod(pod, pvc); err != nil { + return false, nil, err + } } return } diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go index dfda81a0e50..b939c9e9677 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go @@ -25,9 +25,9 @@ import ( v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" corelisters "k8s.io/client-go/listers/core/v1" + "k8s.io/component-helpers/storage/ephemeral" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/scheduler/apis/config" "k8s.io/kubernetes/pkg/scheduler/apis/config/validation" @@ -127,13 +127,13 @@ func (pl *VolumeBinding) podHasPVCs(pod *v1.Pod) (bool, error) { hasPVC := false for _, vol := range pod.Spec.Volumes { var pvcName string - ephemeral := false + isEphemeral := false switch { case vol.PersistentVolumeClaim != nil: pvcName = vol.PersistentVolumeClaim.ClaimName case vol.Ephemeral != nil && pl.GenericEphemeralVolumeFeatureEnabled: - pvcName = pod.Name + "-" + vol.Name - ephemeral = true + pvcName = ephemeral.VolumeClaimName(pod, &vol) + isEphemeral = true default: // Volume is not using a PVC, ignore continue @@ -144,7 +144,7 @@ func (pl *VolumeBinding) podHasPVCs(pod *v1.Pod) (bool, error) { // The error usually has already enough context ("persistentvolumeclaim "myclaim" not found"), // but we can do better for generic ephemeral inline volumes where that situation // is normal directly after creating a pod. - if ephemeral && apierrors.IsNotFound(err) { + if isEphemeral && apierrors.IsNotFound(err) { err = fmt.Errorf("waiting for ephemeral volume controller to create the persistentvolumeclaim %q", pvcName) } return hasPVC, err @@ -158,8 +158,10 @@ func (pl *VolumeBinding) podHasPVCs(pod *v1.Pod) (bool, error) { return hasPVC, fmt.Errorf("persistentvolumeclaim %q is being deleted", pvc.Name) } - if ephemeral && !metav1.IsControlledBy(pvc, pod) { - return hasPVC, fmt.Errorf("persistentvolumeclaim %q was not created for the pod", pvc.Name) + if isEphemeral { + if err := ephemeral.VolumeIsForPod(pod, pvc); err != nil { + return hasPVC, err + } } } return hasPVC, nil