Skip to content

chore: ignore pods that have unbound pvc with volumeBindingMode immediate #2415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 7, 2025
Merged

chore: ignore pods that have unbound pvc with volumeBindingMode immediate #2415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e7270e2
chore: ignore pods that have unbound pvc with volumeBindingMode immed…
jigisha620 Jul 31, 2025
3f5525a
chore: Validate metrics for pod
jigisha620 Aug 4, 2025
6cd5d5b
chore: Fix spelling
jigisha620 Aug 4, 2025
3ab358b
chore: Make storageClass validation inline
jigisha620 Aug 6, 2025
ec607b5
chore: Address nits
jigisha620 Aug 6, 2025
494c254
chore: Update to use KRef for k8s object
jigisha620 Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 47 additions & 18 deletions pkg/controllers/provisioning/scheduling/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2798,9 +2798,10 @@ var _ = Context("Scheduling", func() {
ExpectReconcileSucceeded(ctx, nodeStateController, client.ObjectKeyFromObject(node))

sc := test.StorageClass(test.StorageClassOptions{
ObjectMeta: metav1.ObjectMeta{Name: "my-storage-class"},
Provisioner: lo.ToPtr(csiProvider),
Zones: []string{"test-zone-1"}})
ObjectMeta: metav1.ObjectMeta{Name: "my-storage-class"},
Provisioner: lo.ToPtr(csiProvider),
VolumeBindingMode: lo.ToPtr(storagev1.VolumeBindingWaitForFirstConsumer),
Zones: []string{"test-zone-1"}})
ExpectApplied(ctx, env.Client, sc)

var pods []*corev1.Pod
Expand Down Expand Up @@ -2922,8 +2923,9 @@ var _ = Context("Scheduling", func() {
ObjectMeta: metav1.ObjectMeta{
Name: "my-storage-class",
},
Provisioner: lo.ToPtr(csiProvider),
Zones: []string{"test-zone-1"}})
Provisioner: lo.ToPtr(csiProvider),
VolumeBindingMode: lo.ToPtr(storagev1.VolumeBindingWaitForFirstConsumer),
Zones: []string{"test-zone-1"}})
// Create another default storage class that shouldn't be used and has no associated limits
sc2 := test.StorageClass(test.StorageClassOptions{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -2932,8 +2934,9 @@ var _ = Context("Scheduling", func() {
isDefaultStorageClassAnnotation: "true",
},
},
Provisioner: lo.ToPtr("other-provider"),
Zones: []string{"test-zone-1"}})
Provisioner: lo.ToPtr("other-provider"),
VolumeBindingMode: lo.ToPtr(storagev1.VolumeBindingWaitForFirstConsumer),
Zones: []string{"test-zone-1"}})

initialPod := test.UnschedulablePod(test.PodOptions{})
// Pod has an ephemeral volume claim that has a specified storage class, so it should use the one specified
Expand Down Expand Up @@ -3037,8 +3040,9 @@ var _ = Context("Scheduling", func() {
isDefaultStorageClassAnnotation: "true",
},
},
Provisioner: lo.ToPtr(csiProvider),
Zones: []string{"test-zone-1"}})
Provisioner: lo.ToPtr(csiProvider),
VolumeBindingMode: lo.ToPtr(storagev1.VolumeBindingWaitForFirstConsumer),
Zones: []string{"test-zone-1"}})

initialPod := test.UnschedulablePod(test.PodOptions{})
// Pod has an ephemeral volume claim that has NO storage class, so it should use the default one
Expand Down Expand Up @@ -3135,17 +3139,19 @@ var _ = Context("Scheduling", func() {
isDefaultStorageClassAnnotation: "true",
},
},
Provisioner: lo.ToPtr("other-provider"),
Zones: []string{"test-zone-1"}})
Provisioner: lo.ToPtr("other-provider"),
VolumeBindingMode: lo.ToPtr(storagev1.VolumeBindingWaitForFirstConsumer),
Zones: []string{"test-zone-1"}})
sc2 := test.StorageClass(test.StorageClassOptions{
ObjectMeta: metav1.ObjectMeta{
Name: "newer-default-storage-class",
Annotations: map[string]string{
isDefaultStorageClassAnnotation: "true",
},
},
Provisioner: lo.ToPtr(csiProvider),
Zones: []string{"test-zone-1"}})
Provisioner: lo.ToPtr(csiProvider),
VolumeBindingMode: lo.ToPtr(storagev1.VolumeBindingWaitForFirstConsumer),
Zones: []string{"test-zone-1"}})

ExpectApplied(ctx, env.Client, sc)
// Wait a few seconds to apply the second storage class to get a newer creationTimestamp
Expand Down Expand Up @@ -3243,7 +3249,6 @@ var _ = Context("Scheduling", func() {
})
It("should not launch nodes for pod with storageClass that uses an unsupported provisioner", func() {
scheduling.UnsupportedProvisioners = lo.Assign(scheduling.UnsupportedProvisioners, sets.New("other-provider"))
// Launch an initial pod onto a node and register the CSI Node with a volume count limit of 1
Copy link
Contributor Author

@jigisha620 jigisha620 Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropping this comment because it's not relevant here.

sc := test.StorageClass(test.StorageClassOptions{
ObjectMeta: metav1.ObjectMeta{
Name: "default-storage-class",
Expand Down Expand Up @@ -3293,6 +3298,28 @@ var _ = Context("Scheduling", func() {
// no nodes should be created as the storage class is using an unsupported provisioner
Expect(nodeList.Items).To(HaveLen(0))
})
It("should not launch nodes for pod with unbound volume for volumeBindingMode immediate", func() {
sc := test.StorageClass(test.StorageClassOptions{
ObjectMeta: metav1.ObjectMeta{
Name: "default-storage-class",
},
Provisioner: lo.ToPtr("other-provider"),
VolumeBindingMode: lo.ToPtr(storagev1.VolumeBindingImmediate),
Zones: []string{"test-zone-1"}},
)
pvc := test.PersistentVolumeClaim(test.PersistentVolumeClaimOptions{
ObjectMeta: metav1.ObjectMeta{
Name: "tmp-ephemeral",
},
StorageClassName: lo.ToPtr(sc.Name),
})
pod := test.UnschedulablePod(test.PodOptions{
PersistentVolumeClaims: []string{pvc.Name},
})
ExpectApplied(ctx, env.Client, nodePool, sc, pvc, pod)
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod)
ExpectNotScheduled(ctx, env.Client, pod)
})
DescribeTable(
"should launch nodes for pods with ephemeral volume without a storage class when the PVC is bound",
func(storageClassName string) {
Expand Down Expand Up @@ -3392,8 +3419,9 @@ var _ = Context("Scheduling", func() {
isDefaultStorageClassAnnotation: "true",
},
},
Provisioner: lo.ToPtr(plugins.AWSEBSInTreePluginName),
Zones: []string{"test-zone-1"}})
Provisioner: lo.ToPtr(plugins.AWSEBSInTreePluginName),
VolumeBindingMode: lo.ToPtr(storagev1.VolumeBindingWaitForFirstConsumer),
Zones: []string{"test-zone-1"}})
pvc := test.PersistentVolumeClaim(test.PersistentVolumeClaimOptions{
StorageClassName: lo.ToPtr(sc.Name),
})
Expand Down Expand Up @@ -3450,8 +3478,9 @@ var _ = Context("Scheduling", func() {
isDefaultStorageClassAnnotation: "true",
},
},
Provisioner: lo.ToPtr(plugins.AWSEBSInTreePluginName),
Zones: []string{"test-zone-1"}})
Provisioner: lo.ToPtr(plugins.AWSEBSInTreePluginName),
VolumeBindingMode: lo.ToPtr(storagev1.VolumeBindingWaitForFirstConsumer),
Zones: []string{"test-zone-1"}})

initialPod := test.UnschedulablePod(test.PodOptions{})
// Pod has an ephemeral volume claim that references the in-tree storage provider
Expand Down
23 changes: 10 additions & 13 deletions pkg/controllers/provisioning/scheduling/volumetopology.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,16 @@ func (v *VolumeTopology) ValidatePersistentVolumeClaims(ctx context.Context, pod
if storageClassName == "" {
return serrors.Wrap(fmt.Errorf("unbound pvc must define a storage class"), "PersistentVolumeClaim", klog.KRef("", pvc.Name))
}
if err := v.validateStorageClass(ctx, storageClassName); err != nil {
return serrors.Wrap(fmt.Errorf("failed to validate storage class, %w", err), "PersistentVolumeClaim", klog.KRef("", pvc.Name), "StorageClass", klog.KRef("", storageClassName))
storageClass := &storagev1.StorageClass{}
if err := v.kubeClient.Get(ctx, types.NamespacedName{Name: storageClassName}, storageClass); err != nil {
return serrors.Wrap(fmt.Errorf("failed to validate pvc, failed to get storage class, %w", err), "PersistentVolumeClaim", klog.KObj(pvc), "StorageClass", klog.KRef("", storageClassName))
}
if UnsupportedProvisioners.Has(storageClass.Provisioner) {
return serrors.Wrap(fmt.Errorf("failed to validate pvc, provisioner is not supported"), "PersistentVolumeClaim", klog.KObj(pvc), "StorageClass", klog.KObj(storageClass), "Provisioner", storageClass.Provisioner)
}
// Ignore pods than have unbound pvc for voluemBindingMode immediate
if storageClass.VolumeBindingMode != nil && *storageClass.VolumeBindingMode == storagev1.VolumeBindingImmediate {
return serrors.Wrap(fmt.Errorf("failed to validate pvc, pvc with immediate volume binding mode must be bound"), "PersistentVolumeClaim", klog.KObj(pvc), "StorageClass", klog.KObj(storageClass))
}
}
return nil
Expand All @@ -195,14 +203,3 @@ func (v *VolumeTopology) validateVolume(ctx context.Context, volumeName string)
}
return nil
}

func (v *VolumeTopology) validateStorageClass(ctx context.Context, storageClassName string) error {
storageClass := &storagev1.StorageClass{}
if err := v.kubeClient.Get(ctx, types.NamespacedName{Name: storageClassName}, storageClass); err != nil {
return err
}
if UnsupportedProvisioners.Has(storageClass.Provisioner) {
return serrors.Wrap(fmt.Errorf("storageClass provisioner is not supported"), "StorageClass", klog.KRef("", storageClass.Provisioner))
}
return nil
}
5 changes: 4 additions & 1 deletion pkg/controllers/provisioning/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1741,7 +1741,10 @@ var _ = Describe("Provisioning", func() {
Context("Volume Topology Requirements", func() {
var storageClass *storagev1.StorageClass
BeforeEach(func() {
storageClass = test.StorageClass(test.StorageClassOptions{Zones: []string{"test-zone-2", "test-zone-3"}})
storageClass = test.StorageClass(test.StorageClassOptions{
Zones: []string{"test-zone-2", "test-zone-3"},
VolumeBindingMode: lo.ToPtr(storagev1.VolumeBindingWaitForFirstConsumer),
})
})
It("should not schedule if invalid pvc", func() {
ExpectApplied(ctx, env.Client, test.NodePool())
Expand Down
Loading