mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-25 12:43:23 +00:00
Don't count unrelated volumes in scheduler predicate
This commit is contained in:
Fabio Bertinatto
parent
e5cec2edc8
commit
ee7b48b7c5
@ -23,12 +23,13 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"k8s.io/api/core/v1"
|
||||
v1 "k8s.io/api/core/v1"
|
||||
"k8s.io/api/storage/v1beta1"
|
||||
"k8s.io/apimachinery/pkg/api/resource"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
utilfeature "k8s.io/apiserver/pkg/util/feature"
|
||||
featuregatetesting "k8s.io/component-base/featuregate/testing"
|
||||
csilibplugins "k8s.io/csi-translation-lib/plugins"
|
||||
"k8s.io/kubernetes/pkg/features"
|
||||
schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
|
||||
volumeutil "k8s.io/kubernetes/pkg/volume/util"
|
||||
@ -102,6 +103,32 @@ func TestVolumeCountConflicts(t *testing.T) {
|
||||
},
|
||||
},
|
||||
}
|
||||
unboundPVCwithInvalidSCPod := &v1.Pod{
|
||||
Spec: v1.PodSpec{
|
||||
Volumes: []v1.Volume{
|
||||
{
|
||||
VolumeSource: v1.VolumeSource{
|
||||
PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
|
||||
ClaimName: "unboundPVCwithInvalidSCPod",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
unboundPVCwithDefaultSCPod := &v1.Pod{
|
||||
Spec: v1.PodSpec{
|
||||
Volumes: []v1.Volume{
|
||||
{
|
||||
VolumeSource: v1.VolumeSource{
|
||||
PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
|
||||
ClaimName: "unboundPVCwithDefaultSCPod",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
splitVolsPod := &v1.Pod{
|
||||
Spec: v1.PodSpec{
|
||||
Volumes: []v1.Volume{
|
||||
@ -361,26 +388,43 @@ func TestVolumeCountConflicts(t *testing.T) {
|
||||
newPod: onePVCPod(EBSVolumeFilterType),
|
||||
existingPods: []*v1.Pod{oneVolPod, deletedPVCPod},
|
||||
filterName: EBSVolumeFilterType,
|
||||
maxVols: 2,
|
||||
maxVols: 1,
|
||||
fits: false,
|
||||
test: "pod with missing PVC is counted towards the PV limit",
|
||||
test: "missing PVC is not counted towards the PV limit",
|
||||
},
|
||||
{
|
||||
newPod: onePVCPod(EBSVolumeFilterType),
|
||||
existingPods: []*v1.Pod{oneVolPod, deletedPVCPod},
|
||||
filterName: EBSVolumeFilterType,
|
||||
maxVols: 3,
|
||||
maxVols: 2,
|
||||
fits: true,
|
||||
test: "pod with missing PVC is counted towards the PV limit",
|
||||
test: "missing PVC is not counted towards the PV limit",
|
||||
},
|
||||
{
|
||||
newPod: onePVCPod(EBSVolumeFilterType),
|
||||
existingPods: []*v1.Pod{oneVolPod, twoDeletedPVCPod},
|
||||
filterName: EBSVolumeFilterType,
|
||||
maxVols: 3,
|
||||
fits: false,
|
||||
test: "pod with missing two PVCs is counted towards the PV limit twice",
|
||||
maxVols: 2,
|
||||
fits: true,
|
||||
test: "two missing PVCs are not counted towards the PV limit twice",
|
||||
},
|
||||
{
|
||||
newPod: unboundPVCwithInvalidSCPod,
|
||||
existingPods: []*v1.Pod{oneVolPod},
|
||||
filterName: EBSVolumeFilterType,
|
||||
maxVols: 1,
|
||||
fits: true,
|
||||
test: "unbound PVC with invalid SC is not counted towards the PV limit",
|
||||
},
|
||||
{
|
||||
newPod: unboundPVCwithDefaultSCPod,
|
||||
existingPods: []*v1.Pod{oneVolPod},
|
||||
filterName: EBSVolumeFilterType,
|
||||
maxVols: 1,
|
||||
fits: true,
|
||||
test: "unbound PVC from different provisioner is not counted towards the PV limit",
|
||||
},
|
||||
|
||||
{
|
||||
newPod: onePVCPod(EBSVolumeFilterType),
|
||||
existingPods: []*v1.Pod{oneVolPod, deletedPVPod},
|
||||
@ -791,7 +835,11 @@ func TestVolumeCountConflicts(t *testing.T) {
|
||||
// running attachable predicate tests without feature gate and no limit present on nodes
|
||||
for _, test := range tests {
|
||||
os.Setenv(KubeMaxPDVols, strconv.Itoa(test.maxVols))
|
||||
pred := NewMaxPDVolumeCountPredicate(test.filterName, getFakePVInfo(test.filterName), getFakePVCInfo(test.filterName))
|
||||
pred := NewMaxPDVolumeCountPredicate(test.filterName,
|
||||
getFakeStorageClassInfo(test.filterName),
|
||||
getFakePVInfo(test.filterName),
|
||||
getFakePVCInfo(test.filterName))
|
||||
|
||||
fits, reasons, err := pred(test.newPod, GetPredicateMetadata(test.newPod, nil), schedulernodeinfo.NewNodeInfo(test.existingPods...))
|
||||
if err != nil {
|
||||
t.Errorf("[%s]%s: unexpected error: %v", test.filterName, test.test, err)
|
||||
@ -809,7 +857,10 @@ func TestVolumeCountConflicts(t *testing.T) {
|
||||
// running attachable predicate tests with feature gate and limit present on nodes
|
||||
for _, test := range tests {
|
||||
node := getNodeWithPodAndVolumeLimits("node", test.existingPods, int64(test.maxVols), test.filterName)
|
||||
pred := NewMaxPDVolumeCountPredicate(test.filterName, getFakePVInfo(test.filterName), getFakePVCInfo(test.filterName))
|
||||
pred := NewMaxPDVolumeCountPredicate(test.filterName,
|
||||
getFakeStorageClassInfo(test.filterName),
|
||||
getFakePVInfo(test.filterName),
|
||||
getFakePVCInfo(test.filterName))
|
||||
fits, reasons, err := pred(test.newPod, GetPredicateMetadata(test.newPod, nil), node)
|
||||
if err != nil {
|
||||
t.Errorf("Using allocatable [%s]%s: unexpected error: %v", test.filterName, test.test, err)
|
||||
@ -823,6 +874,32 @@ func TestVolumeCountConflicts(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func getFakeStorageClassInfo(sc string) FakeStorageClassInfo {
|
||||
var provisioner string
|
||||
switch sc {
|
||||
case EBSVolumeFilterType:
|
||||
provisioner = csilibplugins.AWSEBSInTreePluginName
|
||||
case GCEPDVolumeFilterType:
|
||||
provisioner = csilibplugins.GCEPDInTreePluginName
|
||||
case AzureDiskVolumeFilterType:
|
||||
provisioner = csilibplugins.AzureDiskInTreePluginName
|
||||
case CinderVolumeFilterType:
|
||||
provisioner = csilibplugins.CinderInTreePluginName
|
||||
default:
|
||||
return FakeStorageClassInfo{}
|
||||
}
|
||||
return FakeStorageClassInfo{
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: sc},
|
||||
Provisioner: provisioner,
|
||||
},
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "standard-sc"},
|
||||
Provisioner: "standard-sc",
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func getFakePVInfo(filterName string) FakePersistentVolumeInfo {
|
||||
return FakePersistentVolumeInfo{
|
||||
{
|
||||
@ -846,27 +923,59 @@ func getFakePVCInfo(filterName string) FakePersistentVolumeClaimInfo {
|
||||
return FakePersistentVolumeClaimInfo{
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "some" + filterName + "Vol"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "some" + filterName + "Vol"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{
|
||||
VolumeName: "some" + filterName + "Vol",
|
||||
StorageClassName: &filterName,
|
||||
},
|
||||
},
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "someNon" + filterName + "Vol"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "someNon" + filterName + "Vol"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{
|
||||
VolumeName: "someNon" + filterName + "Vol",
|
||||
StorageClassName: &filterName,
|
||||
},
|
||||
},
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "pvcWithDeletedPV"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "pvcWithDeletedPV"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{
|
||||
VolumeName: "pvcWithDeletedPV",
|
||||
StorageClassName: &filterName,
|
||||
},
|
||||
},
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "anotherPVCWithDeletedPV"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{VolumeName: "anotherPVCWithDeletedPV"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{
|
||||
VolumeName: "anotherPVCWithDeletedPV",
|
||||
StorageClassName: &filterName,
|
||||
},
|
||||
},
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "unboundPVC"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{VolumeName: ""},
|
||||
Spec: v1.PersistentVolumeClaimSpec{
|
||||
VolumeName: "",
|
||||
StorageClassName: &filterName,
|
||||
},
|
||||
},
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "anotherUnboundPVC"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{VolumeName: ""},
|
||||
Spec: v1.PersistentVolumeClaimSpec{
|
||||
VolumeName: "",
|
||||
StorageClassName: &filterName,
|
||||
},
|
||||
},
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "unboundPVCwithDefaultSCPod"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{
|
||||
VolumeName: "",
|
||||
StorageClassName: utilpointer.StringPtr("standard-sc"),
|
||||
},
|
||||
},
|
||||
{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "unboundPVCwithInvalidSCPod"},
|
||||
Spec: v1.PersistentVolumeClaimSpec{
|
||||
VolumeName: "",
|
||||
StorageClassName: utilpointer.StringPtr("invalid-sc"),
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@ -287,6 +287,7 @@ type MaxPDVolumeCountChecker struct {
|
||||
maxVolumeFunc func(node *v1.Node) int
|
||||
pvInfo PersistentVolumeInfo
|
||||
pvcInfo PersistentVolumeClaimInfo
|
||||
scInfo StorageClassInfo
|
||||
|
||||
// The string below is generated randomly during the struct's initialization.
|
||||
// It is used to prefix volumeID generated inside the predicate() method to
|
||||
@ -299,6 +300,8 @@ type VolumeFilter struct {
|
||||
// Filter normal volumes
|
||||
FilterVolume func(vol *v1.Volume) (id string, relevant bool)
|
||||
FilterPersistentVolume func(pv *v1.PersistentVolume) (id string, relevant bool)
|
||||
// MatchProvisioner evaluates if the StorageClass provisioner matches the running predicate
|
||||
MatchProvisioner func(sc *storagev1.StorageClass) (relevant bool)
|
||||
// IsMigrated returns a boolean specifying whether the plugin is migrated to a CSI driver
|
||||
IsMigrated func(csiNode *storagev1beta1.CSINode) bool
|
||||
}
|
||||
@ -314,7 +317,7 @@ type VolumeFilter struct {
|
||||
// types, counts the number of unique volumes, and rejects the new pod if it would place the total count over
|
||||
// the maximum.
|
||||
func NewMaxPDVolumeCountPredicate(
|
||||
filterName string, pvInfo PersistentVolumeInfo, pvcInfo PersistentVolumeClaimInfo) FitPredicate {
|
||||
filterName string, scInfo StorageClassInfo, pvInfo PersistentVolumeInfo, pvcInfo PersistentVolumeClaimInfo) FitPredicate {
|
||||
var filter VolumeFilter
|
||||
var volumeLimitKey v1.ResourceName
|
||||
|
||||
@ -344,6 +347,7 @@ func NewMaxPDVolumeCountPredicate(
|
||||
maxVolumeFunc: getMaxVolumeFunc(filterName),
|
||||
pvInfo: pvInfo,
|
||||
pvcInfo: pvcInfo,
|
||||
scInfo: scInfo,
|
||||
randomVolumeIDPrefix: rand.String(32),
|
||||
}
|
||||
|
||||
@ -428,19 +432,23 @@ func (c *MaxPDVolumeCountChecker) filterVolumes(volumes []v1.Volume, namespace s
|
||||
if pvName == "" {
|
||||
// PVC is not bound. It was either deleted and created again or
|
||||
// it was forcefully unbound by admin. The pod can still use the
|
||||
// original PV where it was bound to -> log the error and count
|
||||
// the PV towards the PV limit
|
||||
klog.V(4).Infof("PVC %s/%s is not bound, assuming PVC matches predicate when counting limits", namespace, pvcName)
|
||||
filteredVolumes[pvID] = true
|
||||
// original PV where it was bound to, so we count the volume if
|
||||
// it belongs to the running predicate.
|
||||
if c.matchProvisioner(pvc) {
|
||||
klog.V(4).Infof("PVC %s/%s is not bound, assuming PVC matches predicate when counting limits", namespace, pvcName)
|
||||
filteredVolumes[pvID] = true
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
pv, err := c.pvInfo.GetPersistentVolumeInfo(pvName)
|
||||
if err != nil || pv == nil {
|
||||
// if the PV is not found, log the error
|
||||
// and count the PV towards the PV limit
|
||||
klog.V(4).Infof("Unable to look up PV info for %s/%s/%s, assuming PV matches predicate when counting limits: %v", namespace, pvcName, pvName, err)
|
||||
filteredVolumes[pvID] = true
|
||||
// If the PV is invalid and PVC belongs to the running predicate,
|
||||
// log the error and count the PV towards the PV limit.
|
||||
if c.matchProvisioner(pvc) {
|
||||
klog.V(4).Infof("Unable to look up PV info for %s/%s/%s, assuming PV matches predicate when counting limits: %v", namespace, pvcName, pvName, err)
|
||||
filteredVolumes[pvID] = true
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
@ -453,6 +461,20 @@ func (c *MaxPDVolumeCountChecker) filterVolumes(volumes []v1.Volume, namespace s
|
||||
return nil
|
||||
}
|
||||
|
||||
// matchProvisioner helps identify if the given PVC belongs to the running predicate.
|
||||
func (c *MaxPDVolumeCountChecker) matchProvisioner(pvc *v1.PersistentVolumeClaim) bool {
|
||||
if pvc.Spec.StorageClassName == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
storageClass, err := c.scInfo.GetStorageClassInfo(*pvc.Spec.StorageClassName)
|
||||
if err != nil || storageClass == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
return c.filter.MatchProvisioner(storageClass)
|
||||
}
|
||||
|
||||
func (c *MaxPDVolumeCountChecker) predicate(pod *v1.Pod, meta PredicateMetadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) {
|
||||
// If a pod doesn't have any volume attached to it, the predicate will always be true.
|
||||
// Thus we make a fast path for it, to avoid unnecessary computations in this case.
|
||||
@ -530,6 +552,13 @@ var EBSVolumeFilter = VolumeFilter{
|
||||
return "", false
|
||||
},
|
||||
|
||||
MatchProvisioner: func(sc *storagev1.StorageClass) (relevant bool) {
|
||||
if sc.Provisioner == csilibplugins.AWSEBSInTreePluginName {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
},
|
||||
|
||||
IsMigrated: func(csiNode *storagev1beta1.CSINode) bool {
|
||||
return isCSIMigrationOn(csiNode, csilibplugins.AWSEBSInTreePluginName)
|
||||
},
|
||||
@ -551,6 +580,13 @@ var GCEPDVolumeFilter = VolumeFilter{
|
||||
return "", false
|
||||
},
|
||||
|
||||
MatchProvisioner: func(sc *storagev1.StorageClass) (relevant bool) {
|
||||
if sc.Provisioner == csilibplugins.GCEPDInTreePluginName {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
},
|
||||
|
||||
IsMigrated: func(csiNode *storagev1beta1.CSINode) bool {
|
||||
return isCSIMigrationOn(csiNode, csilibplugins.GCEPDInTreePluginName)
|
||||
},
|
||||
@ -572,6 +608,13 @@ var AzureDiskVolumeFilter = VolumeFilter{
|
||||
return "", false
|
||||
},
|
||||
|
||||
MatchProvisioner: func(sc *storagev1.StorageClass) (relevant bool) {
|
||||
if sc.Provisioner == csilibplugins.AzureDiskInTreePluginName {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
},
|
||||
|
||||
IsMigrated: func(csiNode *storagev1beta1.CSINode) bool {
|
||||
return isCSIMigrationOn(csiNode, csilibplugins.AzureDiskInTreePluginName)
|
||||
},
|
||||
@ -594,6 +637,13 @@ var CinderVolumeFilter = VolumeFilter{
|
||||
return "", false
|
||||
},
|
||||
|
||||
MatchProvisioner: func(sc *storagev1.StorageClass) (relevant bool) {
|
||||
if sc.Provisioner == csilibplugins.CinderInTreePluginName {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
},
|
||||
|
||||
IsMigrated: func(csiNode *storagev1beta1.CSINode) bool {
|
||||
return isCSIMigrationOn(csiNode, csilibplugins.CinderInTreePluginName)
|
||||
},
|
||||
|
||||
@ -61,21 +61,21 @@ func init() {
|
||||
factory.RegisterFitPredicateFactory(
|
||||
predicates.MaxEBSVolumeCountPred,
|
||||
func(args factory.PluginFactoryArgs) predicates.FitPredicate {
|
||||
return predicates.NewMaxPDVolumeCountPredicate(predicates.EBSVolumeFilterType, args.PVInfo, args.PVCInfo)
|
||||
return predicates.NewMaxPDVolumeCountPredicate(predicates.EBSVolumeFilterType, args.StorageClassInfo, args.PVInfo, args.PVCInfo)
|
||||
},
|
||||
)
|
||||
// Fit is determined by whether or not there would be too many GCE PD volumes attached to the node
|
||||
factory.RegisterFitPredicateFactory(
|
||||
predicates.MaxGCEPDVolumeCountPred,
|
||||
func(args factory.PluginFactoryArgs) predicates.FitPredicate {
|
||||
return predicates.NewMaxPDVolumeCountPredicate(predicates.GCEPDVolumeFilterType, args.PVInfo, args.PVCInfo)
|
||||
return predicates.NewMaxPDVolumeCountPredicate(predicates.GCEPDVolumeFilterType, args.StorageClassInfo, args.PVInfo, args.PVCInfo)
|
||||
},
|
||||
)
|
||||
// Fit is determined by whether or not there would be too many Azure Disk volumes attached to the node
|
||||
factory.RegisterFitPredicateFactory(
|
||||
predicates.MaxAzureDiskVolumeCountPred,
|
||||
func(args factory.PluginFactoryArgs) predicates.FitPredicate {
|
||||
return predicates.NewMaxPDVolumeCountPredicate(predicates.AzureDiskVolumeFilterType, args.PVInfo, args.PVCInfo)
|
||||
return predicates.NewMaxPDVolumeCountPredicate(predicates.AzureDiskVolumeFilterType, args.StorageClassInfo, args.PVInfo, args.PVCInfo)
|
||||
},
|
||||
)
|
||||
factory.RegisterFitPredicateFactory(
|
||||
@ -87,7 +87,7 @@ func init() {
|
||||
factory.RegisterFitPredicateFactory(
|
||||
predicates.MaxCinderVolumeCountPred,
|
||||
func(args factory.PluginFactoryArgs) predicates.FitPredicate {
|
||||
return predicates.NewMaxPDVolumeCountPredicate(predicates.CinderVolumeFilterType, args.PVInfo, args.PVCInfo)
|
||||
return predicates.NewMaxPDVolumeCountPredicate(predicates.CinderVolumeFilterType, args.StorageClassInfo, args.PVInfo, args.PVCInfo)
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user