Skip to content

Commit

Permalink
fix: validate volume names (#165)
Browse files Browse the repository at this point in the history
We need to validate volume names if specified to ensure they exist so we
don't launch a node for pods that won't schedule anyway.

Fixes aws/karpenter/#3219
  • Loading branch information
tzneal authored Jan 18, 2023
1 parent 12c3355 commit df9dab2
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 7 deletions.
11 changes: 11 additions & 0 deletions pkg/controllers/provisioning/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,17 @@ var _ = Describe("Volume Topology Requirements", func() {
ExpectNotScheduled(ctx, env.Client, invalidPod)
ExpectScheduled(ctx, env.Client, pod)
})
It("should schedule valid pods when a pod with an invalid pvc is encountered (volume name)", func() {
invalidVolumeName := "invalid-volume-name"
persistentVolumeClaim := test.PersistentVolumeClaim(test.PersistentVolumeClaimOptions{VolumeName: invalidVolumeName})
ExpectApplied(ctx, env.Client, test.Provisioner(), persistentVolumeClaim)
invalidPod := ExpectProvisioned(ctx, env.Client, cluster, recorder, provisioningController, prov, test.UnschedulablePod(test.PodOptions{
PersistentVolumeClaims: []string{persistentVolumeClaim.Name},
}))[0]
pod := ExpectProvisioned(ctx, env.Client, cluster, recorder, provisioningController, prov, test.UnschedulablePod(test.PodOptions{}))[0]
ExpectNotScheduled(ctx, env.Client, invalidPod)
ExpectScheduled(ctx, env.Client, pod)
})
It("should schedule to storage class zones if volume does not exist", func() {
persistentVolumeClaim := test.PersistentVolumeClaim(test.PersistentVolumeClaimOptions{StorageClassName: &storageClass.Name})
ExpectApplied(ctx, env.Client, test.Provisioner(), storageClass, persistentVolumeClaim)
Expand Down
37 changes: 30 additions & 7 deletions pkg/controllers/provisioning/volumetopology.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ func (v *VolumeTopology) getPersistentVolumeClaim(ctx context.Context, pod *v1.P
func (v *VolumeTopology) validatePersistentVolumeClaims(ctx context.Context, pod *v1.Pod) error {
for _, volume := range pod.Spec.Volumes {
var storageClassName *string
var volumeName string
if volume.PersistentVolumeClaim != nil {
// validate the PVC if it exists
pvc, err := v.getPersistentVolumeClaim(ctx, pod, volume)
Expand All @@ -154,19 +155,41 @@ func (v *VolumeTopology) validatePersistentVolumeClaims(ctx context.Context, pod
// may not have a PVC
if pvc == nil {
continue

}

storageClassName = pvc.Spec.StorageClassName
volumeName = pvc.Spec.VolumeName
} else if volume.Ephemeral != nil {
storageClassName = volume.Ephemeral.VolumeClaimTemplate.Spec.StorageClassName
}

// we have a storage class name, so ensure that it exists
if storageClassName != nil && ptr.StringValue(storageClassName) != "" {
storageClass := &storagev1.StorageClass{}
if err := v.kubeClient.Get(ctx, types.NamespacedName{Name: ptr.StringValue(storageClassName)}, storageClass); err != nil {
return err
}
if err := v.validateStorageClass(ctx, storageClassName); err != nil {
return err
}
if err := v.validateVolume(ctx, volumeName); err != nil {
return err
}
}
return nil
}

func (v *VolumeTopology) validateVolume(ctx context.Context, volumeName string) error {
// we have a volume name, so ensure that it exists
if volumeName != "" {
pv := &v1.PersistentVolume{}
if err := v.kubeClient.Get(ctx, types.NamespacedName{Name: volumeName}, pv); err != nil {
return err
}
}
return nil
}

func (v *VolumeTopology) validateStorageClass(ctx context.Context, storageClassName *string) error {
// we have a storage class name, so ensure that it exists
if ptr.StringValue(storageClassName) != "" {
storageClass := &storagev1.StorageClass{}
if err := v.kubeClient.Get(ctx, types.NamespacedName{Name: ptr.StringValue(storageClassName)}, storageClass); err != nil {
return err
}
}
return nil
Expand Down

0 comments on commit df9dab2

Please sign in to comment.