From 866d826ee6e8a4e49b34af057480709c5ae56e69 Mon Sep 17 00:00:00 2001 From: Jonathan Innis Date: Thu, 13 Apr 2023 12:14:12 -0700 Subject: [PATCH] Discover default storage class when none specified --- .../provisioning/scheduling/suite_test.go | 86 ++++++++++++++++++- pkg/scheduling/volumeusage.go | 29 +++++++ 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/pkg/controllers/provisioning/scheduling/suite_test.go b/pkg/controllers/provisioning/scheduling/suite_test.go index 84a9b767be..5be88cfb98 100644 --- a/pkg/controllers/provisioning/scheduling/suite_test.go +++ b/pkg/controllers/provisioning/scheduling/suite_test.go @@ -62,10 +62,11 @@ var env *test.Environment var fakeClock *clock.FakeClock var cluster *state.Cluster var cloudProvider *fake.CloudProvider -var machineStateController controller.Controller var nodeStateController controller.Controller var podStateController controller.Controller +const csiProvider = "fake.csi.provider" + func TestScheduling(t *testing.T) { ctx = TestContextWithLogger(t) RegisterFailHandler(Fail) @@ -81,7 +82,6 @@ var _ = BeforeSuite(func() { cloudProvider.InstanceTypes = instanceTypes fakeClock = clock.NewFakeClock(time.Now()) cluster = state.NewCluster(fakeClock, env.Client, cloudProvider) - machineStateController = informer.NewMachineController(env.Client, cluster) nodeStateController = informer.NewNodeController(env.Client, cluster) podStateController = informer.NewPodController(env.Client, cluster) prov = provisioning.NewProvisioner(env.Client, env.KubernetesInterface.CoreV1(), events.NewRecorder(&record.FakeRecorder{}), cloudProvider, cluster) @@ -2501,7 +2501,6 @@ var _ = Describe("VolumeUsage", func() { Expect(nodeList.Items).To(HaveLen(1)) }) It("should not fail for non-dynamic PVCs", func() { - const csiProvider = "fake.csi.provider" cloudProvider.InstanceTypes = []*cloudprovider.InstanceType{ fake.NewInstanceType( fake.InstanceTypeOptions{ @@ -2619,6 +2618,87 @@ var _ = Describe("VolumeUsage", func() { // 5 of the same PVC should all be schedulable on the same node Expect(nodeList.Items).To(HaveLen(1)) }) + It("should launch nodes for pods with ephemeral volume using a default storage class", func() { + // Launch an initial pod onto a node and register the CSI Node with a volume count limit of 1 + sc := test.StorageClass(test.StorageClassOptions{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-storage-class", + Annotations: map[string]string{ + pscheduling.IsDefaultStorageClassAnnotation: "true", + }, + }, + Provisioner: ptr.String(csiProvider), + 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 + initialPod.Spec.Volumes = append(initialPod.Spec.Volumes, v1.Volume{ + Name: "tmp-ephemeral", + VolumeSource: v1.VolumeSource{ + Ephemeral: &v1.EphemeralVolumeSource{ + VolumeClaimTemplate: &v1.PersistentVolumeClaimTemplate{ + Spec: v1.PersistentVolumeClaimSpec{ + AccessModes: []v1.PersistentVolumeAccessMode{ + v1.ReadWriteOnce, + }, + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("1Gi"), + }, + }, + }, + }, + }, + }, + }) + ExpectApplied(ctx, env.Client, provisioner, sc, initialPod) + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, initialPod) + node := ExpectScheduled(ctx, env.Client, initialPod) + csiNode := &storagev1.CSINode{ + ObjectMeta: metav1.ObjectMeta{ + Name: node.Name, + }, + Spec: storagev1.CSINodeSpec{ + Drivers: []storagev1.CSINodeDriver{ + { + Name: csiProvider, + NodeID: "fake-node-id", + Allocatable: &storagev1.VolumeNodeResources{ + Count: ptr.Int32(1), + }, + }, + }, + }, + } + ExpectApplied(ctx, env.Client, csiNode) + ExpectReconcileSucceeded(ctx, nodeStateController, client.ObjectKeyFromObject(node)) + + pod := test.UnschedulablePod(test.PodOptions{}) + // Pod has an ephemeral volume claim that has NO storage class, so it should use the default one + pod.Spec.Volumes = append(pod.Spec.Volumes, v1.Volume{ + Name: "tmp-ephemeral", + VolumeSource: v1.VolumeSource{ + Ephemeral: &v1.EphemeralVolumeSource{ + VolumeClaimTemplate: &v1.PersistentVolumeClaimTemplate{ + Spec: v1.PersistentVolumeClaimSpec{ + AccessModes: []v1.PersistentVolumeAccessMode{ + v1.ReadWriteOnce, + }, + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("1Gi"), + }, + }, + }, + }, + }, + }, + }) + ExpectApplied(ctx, env.Client, sc, provisioner, pod) + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) + node2 := ExpectScheduled(ctx, env.Client, pod) + Expect(node.Name).ToNot(Equal(node2.Name)) + }) It("should not launch nodes for pods with ephemeral volume using a non-existent storage classes", func() { ExpectApplied(ctx, env.Client, provisioner) pod := test.UnschedulablePod(test.PodOptions{}) diff --git a/pkg/scheduling/volumeusage.go b/pkg/scheduling/volumeusage.go index 766d5c8008..2b52e0cdf9 100644 --- a/pkg/scheduling/volumeusage.go +++ b/pkg/scheduling/volumeusage.go @@ -18,6 +18,7 @@ import ( "context" "fmt" + "github.com/samber/lo" csitranslation "k8s.io/csi-translation-lib" "knative.dev/pkg/logging" @@ -30,6 +31,10 @@ import ( "github.com/aws/karpenter-core/pkg/utils/pretty" ) +const ( + IsDefaultStorageClassAnnotation = "storageclass.kubernetes.io/is-default-class" +) + // translator is a CSI Translator that translates in-tree plugin names to their out-of-tree CSI driver names var translator = csitranslation.New() @@ -151,9 +156,14 @@ func (v *VolumeUsage) Validate(ctx context.Context, kubeClient client.Client, po return result, nil } +//nolint:gocyclo func (v *VolumeUsage) validate(ctx context.Context, kubeClient client.Client, pod *v1.Pod) (volumes, error) { ctx = logging.WithLogger(ctx, logging.FromContext(ctx).With("pod", pod.Name)) podPVCs := volumes{} + defaultStorageClassName, err := v.discoverDefaultStorageClassName(ctx, kubeClient) + if err != nil { + return nil, fmt.Errorf("discovering default storage class, %w", err) + } for _, volume := range pod.Spec.Volumes { ctx = logging.WithLogger(ctx, logging.FromContext(ctx).With("volume", volume.Name)) var pvcID string @@ -166,11 +176,17 @@ func (v *VolumeUsage) validate(ctx context.Context, kubeClient client.Client, po } pvcID = fmt.Sprintf("%s/%s", pod.Namespace, volume.PersistentVolumeClaim.ClaimName) storageClassName = pvc.Spec.StorageClassName + if storageClassName == nil || *storageClassName == "" { + storageClassName = defaultStorageClassName + } volumeName = pvc.Spec.VolumeName } else if volume.Ephemeral != nil { // generated name per https://kubernetes.io/docs/concepts/storage/ephemeral-volumes/#persistentvolumeclaim-naming pvcID = fmt.Sprintf("%s/%s-%s", pod.Namespace, pod.Name, volume.Name) storageClassName = volume.Ephemeral.VolumeClaimTemplate.Spec.StorageClassName + if storageClassName == nil || *storageClassName == "" { + storageClassName = defaultStorageClassName + } volumeName = volume.Ephemeral.VolumeClaimTemplate.Spec.VolumeName } else { continue @@ -187,6 +203,19 @@ func (v *VolumeUsage) validate(ctx context.Context, kubeClient client.Client, po return podPVCs, nil } +func (v *VolumeUsage) discoverDefaultStorageClassName(ctx context.Context, kubeClient client.Client) (*string, error) { + storageClassList := &storagev1.StorageClassList{} + if err := kubeClient.List(ctx, storageClassList); err != nil { + return nil, err + } + if sc, ok := lo.Find(storageClassList.Items, func(sc storagev1.StorageClass) bool { + return sc.Annotations[IsDefaultStorageClassAnnotation] == "true" + }); ok { + return lo.ToPtr(sc.Name), nil + } + return nil, nil +} + // resolveDriver resolves the storage driver name in the following order: // 1. If the PV associated with the pod volume is using CSI.driver in its spec, then use that name // 2. If the StorageClass associated with the PV has a Provisioner