From 0d445e165677157acc8d48ca21b5f1fb336f5737 Mon Sep 17 00:00:00 2001 From: Ellis Tarn Date: Mon, 3 Jan 2022 15:19:16 -0800 Subject: [PATCH] Implemented tests --- .../persistentvolumeclaim/controller.go | 2 - pkg/controllers/selection/suite_test.go | 75 ++++++++++++++----- pkg/test/expectations/expectations.go | 3 + pkg/test/volumes.go | 35 +++++++-- 4 files changed, 87 insertions(+), 28 deletions(-) diff --git a/pkg/controllers/persistentvolumeclaim/controller.go b/pkg/controllers/persistentvolumeclaim/controller.go index ec881e5ca934..7694e9ddb732 100644 --- a/pkg/controllers/persistentvolumeclaim/controller.go +++ b/pkg/controllers/persistentvolumeclaim/controller.go @@ -77,14 +77,12 @@ func (c *Controller) Reconcile(ctx context.Context, req reconcile.Request) (reco return reconcile.Result{}, err } if pod == nil { - logging.FromContext(ctx).Debugf("Skipping bind, no pod found for persistent volume claim") return reconcile.Result{}, nil } if nodeName, ok := pvc.Annotations[SelectedNodeAnnotation]; ok && nodeName == pod.Spec.NodeName { return reconcile.Result{}, nil } if !c.isBindable(pod) { - logging.FromContext(ctx).Debugf("Skipping bind, pod %s/%s is not pending", pod.Namespace, pod.Name) return reconcile.Result{}, nil } pvc.Annotations = functional.UnionStringMaps(pvc.Annotations, map[string]string{SelectedNodeAnnotation: pod.Spec.NodeName}) diff --git a/pkg/controllers/selection/suite_test.go b/pkg/controllers/selection/suite_test.go index 56f963313bbc..c18c4dc4b930 100644 --- a/pkg/controllers/selection/suite_test.go +++ b/pkg/controllers/selection/suite_test.go @@ -28,6 +28,7 @@ import ( "github.com/aws/karpenter/pkg/test" v1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -76,27 +77,61 @@ var _ = AfterEach(func() { }) var _ = Describe("Volume Topology Requirements", func() { - Describe("PersistentVolume Affinity", func() { - It("should schedule to zones supported by the volume", func() { - ExpectCreated(ctx, env.Client, test.PersistentVolumeClaim()) - }) - It("should schedule anywhere if the volume has no requirements", func() { - ExpectCreated(ctx, env.Client, test.PersistentVolumeClaim()) - }) - It("should not schedule if the zones are incompatible", func() { - ExpectCreated(ctx, env.Client, test.PersistentVolumeClaim()) - }) + var storageClass *storagev1.StorageClass + BeforeEach(func() { + storageClass = test.StorageClass(test.StorageClassOptions{Zones: []string{"test-zone-2", "test-zone-3"}}) }) - Describe("StorageClass Requirements", func() { - It("should schedule to zones supported by the storageclass", func() { - ExpectCreated(ctx, env.Client, test.PersistentVolumeClaim()) - }) - It("should schedule anywhere if the storageclass has no requirements", func() { - ExpectCreated(ctx, env.Client, test.PersistentVolumeClaim()) - }) - It("should not schedule if the zones are incompatible", func() { - ExpectCreated(ctx, env.Client, test.PersistentVolumeClaim()) - }) + It("should not schedule if invalid pvc", func() { + ExpectCreated(ctx, env.Client) + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod(test.PodOptions{ + PersistentVolumeClaims: []string{"invalid"}, + }))[0] + ExpectNotScheduled(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}) + ExpectCreated(ctx, env.Client, storageClass, persistentVolumeClaim) + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod(test.PodOptions{ + PersistentVolumeClaims: []string{persistentVolumeClaim.Name}, + NodeRequirements: []v1.NodeSelectorRequirement{{ + Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1", "test-zone-3"}, + }}, + }))[0] + node := ExpectScheduled(ctx, env.Client, pod) + Expect(node.Labels).To(HaveKeyWithValue(v1.LabelTopologyZone, "test-zone-3")) + }) + It("should not schedule if storage class zones are incompatible", func() { + persistentVolumeClaim := test.PersistentVolumeClaim(test.PersistentVolumeClaimOptions{StorageClassName: &storageClass.Name}) + ExpectCreated(ctx, env.Client, storageClass, persistentVolumeClaim) + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod(test.PodOptions{ + PersistentVolumeClaims: []string{persistentVolumeClaim.Name}, + NodeRequirements: []v1.NodeSelectorRequirement{{ + Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1"}, + }}, + }))[0] + ExpectNotScheduled(ctx, env.Client, pod) + }) + It("should schedule to volume zones if volume already bound", func() { + persistentVolume := test.PersistentVolume(test.PersistentVolumeOptions{Zones: []string{"test-zone-3"}}) + persistentVolumeClaim := test.PersistentVolumeClaim(test.PersistentVolumeClaimOptions{VolumeName: persistentVolume.Name, StorageClassName: &storageClass.Name}) + ExpectCreated(ctx, env.Client, storageClass, persistentVolumeClaim, persistentVolume) + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod(test.PodOptions{ + PersistentVolumeClaims: []string{persistentVolumeClaim.Name}, + }))[0] + node := ExpectScheduled(ctx, env.Client, pod) + Expect(node.Labels).To(HaveKeyWithValue(v1.LabelTopologyZone, "test-zone-3")) + }) + It("should not schedule if volume zones are incompatible", func() { + persistentVolume := test.PersistentVolume(test.PersistentVolumeOptions{Zones: []string{"test-zone-3"}}) + persistentVolumeClaim := test.PersistentVolumeClaim(test.PersistentVolumeClaimOptions{VolumeName: persistentVolume.Name, StorageClassName: &storageClass.Name}) + ExpectCreated(ctx, env.Client, storageClass, persistentVolumeClaim, persistentVolume) + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod(test.PodOptions{ + PersistentVolumeClaims: []string{persistentVolumeClaim.Name}, + NodeRequirements: []v1.NodeSelectorRequirement{{ + Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1"}, + }}, + }))[0] + ExpectNotScheduled(ctx, env.Client, pod) }) }) diff --git a/pkg/test/expectations/expectations.go b/pkg/test/expectations/expectations.go index 8c71df3540d6..a743183c5fbe 100644 --- a/pkg/test/expectations/expectations.go +++ b/pkg/test/expectations/expectations.go @@ -26,6 +26,7 @@ import ( appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/api/policy/v1beta1" + storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "knative.dev/pkg/ptr" @@ -134,6 +135,8 @@ func ExpectCleanedUp(ctx context.Context, c client.Client) { &appsv1.DaemonSet{}, &v1beta1.PodDisruptionBudget{}, &v1.PersistentVolumeClaim{}, + &v1.PersistentVolume{}, + &storagev1.StorageClass{}, &v1alpha5.Provisioner{}, } { for _, namespace := range namespaces.Items { diff --git a/pkg/test/volumes.go b/pkg/test/volumes.go index d677c7fc2aa4..aef5b66eb2fc 100644 --- a/pkg/test/volumes.go +++ b/pkg/test/volumes.go @@ -26,10 +26,12 @@ import ( type PersistentVolumeOptions struct { metav1.ObjectMeta + Zones []string + StorageClassName string } func PersistentVolume(overrides ...PersistentVolumeOptions) *v1.PersistentVolume { - options := PersistentVolumeClaimOptions{} + options := PersistentVolumeOptions{} for _, opts := range overrides { if err := mergo.Merge(&options, opts, mergo.WithOverride); err != nil { panic(fmt.Sprintf("Failed to merge options: %s", err.Error())) @@ -37,12 +39,22 @@ func PersistentVolume(overrides ...PersistentVolumeOptions) *v1.PersistentVolume } return &v1.PersistentVolume{ ObjectMeta: ObjectMeta(metav1.ObjectMeta{}), - Spec: v1.PersistentVolumeSpec{}, + Spec: v1.PersistentVolumeSpec{ + PersistentVolumeSource: v1.PersistentVolumeSource{CSI: &v1.CSIPersistentVolumeSource{Driver: "test-driver", VolumeHandle: "test-handle"}}, + StorageClassName: options.StorageClassName, + AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, + Capacity: v1.ResourceList{v1.ResourceStorage: resource.MustParse("100Gi")}, + NodeAffinity: &v1.VolumeNodeAffinity{Required: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{{MatchExpressions: []v1.NodeSelectorRequirement{ + {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: options.Zones}, + }}}}}, + }, } } type PersistentVolumeClaimOptions struct { metav1.ObjectMeta + StorageClassName *string + VolumeName string } func PersistentVolumeClaim(overrides ...PersistentVolumeClaimOptions) *v1.PersistentVolumeClaim { @@ -55,24 +67,35 @@ func PersistentVolumeClaim(overrides ...PersistentVolumeClaimOptions) *v1.Persis return &v1.PersistentVolumeClaim{ ObjectMeta: ObjectMeta(options.ObjectMeta), Spec: v1.PersistentVolumeClaimSpec{ - AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, - Resources: v1.ResourceRequirements{Requests: v1.ResourceList{v1.ResourceStorage: resource.MustParse("1Gi")}}, + StorageClassName: options.StorageClassName, + VolumeName: options.VolumeName, + AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, + Resources: v1.ResourceRequirements{Requests: v1.ResourceList{v1.ResourceStorage: resource.MustParse("1Gi")}}, }, } } type StorageClassOptions struct { metav1.ObjectMeta + Zones []string } func StorageClass(overrides ...StorageClassOptions) *storagev1.StorageClass { - options := PersistentVolumeClaimOptions{} + options := StorageClassOptions{} for _, opts := range overrides { if err := mergo.Merge(&options, opts, mergo.WithOverride); err != nil { panic(fmt.Sprintf("Failed to merge options: %s", err.Error())) } } + + var allowedTopologies []v1.TopologySelectorTerm + if options.Zones != nil { + allowedTopologies = []v1.TopologySelectorTerm{{MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{{Key: v1.LabelTopologyZone, Values: options.Zones}}}} + } + return &storagev1.StorageClass{ - ObjectMeta: ObjectMeta(options.ObjectMeta), + ObjectMeta: ObjectMeta(options.ObjectMeta), + Provisioner: "test-provisioner", + AllowedTopologies: allowedTopologies, } }