Fix: a bug where karpenter.sh/provisioner-name was not selectable by …

…pods (#2328)
aws · Aug 20, 2022 · 2b32cc4 · 2b32cc4
1 parent f9e6c43
commit 2b32cc4
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 9 deletions.
diff --git a/pkg/apis/provisioning/v1alpha5/labels.go b/pkg/apis/provisioning/v1alpha5/labels.go
@@ -57,6 +57,7 @@ var (
 	// Karpenter is aware of these labels, and they can be used to further narrow down
 	// the range of the corresponding values by either provisioner or pods.
 	WellKnownLabels = sets.NewString(
+		ProvisionerNameLabelKey,
 		v1.LabelTopologyZone,
 		v1.LabelTopologyRegion,
 		v1.LabelInstanceTypeStable,

diff --git a/pkg/apis/provisioning/v1alpha5/provisioner_validation.go b/pkg/apis/provisioning/v1alpha5/provisioner_validation.go
@@ -89,6 +89,9 @@ func (s *ProvisionerSpec) Validate(ctx context.Context) (errs *apis.FieldError)
 
 func (s *ProvisionerSpec) validateLabels() (errs *apis.FieldError) {
 	for key, value := range s.Labels {
+		if key == ProvisionerNameLabelKey {
+			errs = errs.Also(apis.ErrInvalidKeyName(key, "labels", "restricted"))
+		}
 		for _, err := range validation.IsQualifiedName(key) {
 			errs = errs.Also(apis.ErrInvalidKeyName(key, "labels", err))
 		}
@@ -153,6 +156,9 @@ func (s *ProvisionerSpec) validateTaintsField(taints []v1.Taint, existing map[ta
 // Provisioner requirements only support well known labels.
 func (s *ProvisionerSpec) validateRequirements() (errs *apis.FieldError) {
 	for i, requirement := range s.Requirements {
+		if requirement.Key == ProvisionerNameLabelKey {
+			errs = errs.Also(apis.ErrInvalidArrayValue(fmt.Sprintf("%s is restricted", requirement.Key), "requirements", i))
+		}
 		if err := ValidateRequirement(requirement); err != nil {
 			errs = errs.Also(apis.ErrInvalidArrayValue(err, "requirements", i))
 		}

diff --git a/pkg/apis/provisioning/v1alpha5/suite_test.go b/pkg/apis/provisioning/v1alpha5/suite_test.go
@@ -29,6 +29,7 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 var ctx context.Context
@@ -108,6 +109,10 @@ var _ = Describe("Validation", func() {
 			provisioner.Spec.Labels = map[string]string{"foo": randomdata.SillyName()}
 			Expect(provisioner.Validate(ctx)).To(Succeed())
 		})
+		It("should fail for the provisioner name label", func() {
+			provisioner.Spec.Labels = map[string]string{ProvisionerNameLabelKey: randomdata.SillyName()}
+			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
+		})
 		It("should fail for invalid label keys", func() {
 			provisioner.Spec.Labels = map[string]string{"spaces are not allowed": randomdata.SillyName()}
 			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
@@ -186,7 +191,13 @@ var _ = Describe("Validation", func() {
 			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
 		})
 	})
-	Context("Validation", func() {
+	Context("Requirements", func() {
+		It("should fail for the provisioner name label", func() {
+			provisioner.Spec.Requirements = []v1.NodeSelectorRequirement{
+				{Key: ProvisionerNameLabelKey, Operator: v1.NodeSelectorOpIn, Values: []string{randomdata.SillyName()}},
+			}
+			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
+		})
 		It("should allow supported ops", func() {
 			provisioner.Spec.Requirements = []v1.NodeSelectorRequirement{
 				{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}},
@@ -222,7 +233,7 @@ var _ = Describe("Validation", func() {
 			}
 		})
 		It("should allow well known label exceptions", func() {
-			for label := range WellKnownLabels {
+			for label := range WellKnownLabels.Difference(sets.NewString(ProvisionerNameLabelKey)) {
 				provisioner.Spec.Requirements = []v1.NodeSelectorRequirement{
 					{Key: label, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}},
 				}

diff --git a/test/suites/integration/scheduling_test.go b/test/suites/integration/scheduling_test.go
@@ -22,14 +22,16 @@ var _ = Describe("Scheduling", func() {
 			SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": env.ClusterName},
 			SubnetSelector:        map[string]string{"karpenter.sh/discovery": env.ClusterName},
 		}})
+		provisioner := test.Provisioner(test.ProvisionerOptions{ProviderRef: &v1alpha5.ProviderRef{Name: provider.Name}})
 		nodeSelector := map[string]string{
 			// Well Known
-			v1.LabelTopologyRegion:     env.Region,
-			v1.LabelTopologyZone:       fmt.Sprintf("%sa", env.Region),
-			v1.LabelInstanceTypeStable: "g4dn.8xlarge",
-			v1.LabelOSStable:           "linux",
-			v1.LabelArchStable:         "amd64",
-			v1alpha5.LabelCapacityType: "on-demand",
+			v1alpha5.ProvisionerNameLabelKey: provisioner.Name,
+			v1.LabelTopologyRegion:           env.Region,
+			v1.LabelTopologyZone:             fmt.Sprintf("%sa", env.Region),
+			v1.LabelInstanceTypeStable:       "g4dn.8xlarge",
+			v1.LabelOSStable:                 "linux",
+			v1.LabelArchStable:               "amd64",
+			v1alpha5.LabelCapacityType:       "on-demand",
 			// Well Known to AWS
 			awsv1alpha1.LabelInstanceHypervisor:      "nitro",
 			awsv1alpha1.LabelInstanceCategory:        "g",
@@ -54,7 +56,6 @@ var _ = Describe("Scheduling", func() {
 		requirements := lo.MapToSlice(nodeSelector, func(key string, value string) v1.NodeSelectorRequirement {
 			return v1.NodeSelectorRequirement{Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}}
 		})
-		provisioner := test.Provisioner(test.ProvisionerOptions{ProviderRef: &v1alpha5.ProviderRef{Name: provider.Name}})
 		deployment := test.Deployment(test.DeploymentOptions{Replicas: 1, PodOptions: test.PodOptions{
 			NodeSelector:     nodeSelector,
 			NodePreferences:  requirements,