Skip to content

Commit

Permalink
chore: Allow the karpenter.sh/nodepool node label selector and vali…
Browse files Browse the repository at this point in the history 
…date nodeSelector on pods (aws#628)
  • Loading branch information
@jonathan-innis
jonathan-innis authored Oct 23, 2023
1 parent e05c7be commit 28b4b11
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 10 deletions.
4 changes: 2 additions & 2 deletions pkg/apis/v1beta1/nodeclaim_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,14 @@ func (in *NodeClaimSpec) validateTaintsField(taints []v1.Taint, existing map[tai
// NodeClaim requirements only support well known labels.
func (in *NodeClaimSpec) validateRequirements() (errs *apis.FieldError) {
for i, requirement := range in.Requirements {
if err := in.validateRequirement(requirement); err != nil {
if err := ValidateRequirement(requirement); err != nil {
errs = errs.Also(apis.ErrInvalidArrayValue(err, "requirements", i))
}
}
return errs
}

func (in *NodeClaimSpec) validateRequirement(requirement v1.NodeSelectorRequirement) error { //nolint:gocyclo
func ValidateRequirement(requirement v1.NodeSelectorRequirement) error { //nolint:gocyclo
var errs error
if normalized, ok := NormalizedLabels[requirement.Key]; ok {
requirement.Key = normalized
Expand Down
44 changes: 42 additions & 2 deletions pkg/controllers/provisioning/nodepool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ var _ = Describe("NodePool/Provisioning", func() {
It("should provision nodes for pods with supported node selectors", func() {
nodePool := test.NodePool()
schedulable := []*v1.Pod{
// Constrained by provisioner
// Constrained by nodepool
test.UnschedulablePod(test.PodOptions{NodeSelector: map[string]string{v1beta1.NodePoolLabelKey: nodePool.Name}}),
// Constrained by zone
test.UnschedulablePod(test.PodOptions{NodeSelector: map[string]string{v1.LabelTopologyZone: "test-zone-1"}}),
Expand All @@ -70,7 +70,7 @@ var _ = Describe("NodePool/Provisioning", func() {
test.UnschedulablePod(test.PodOptions{NodeSelector: map[string]string{v1.LabelOSStable: string(v1.Linux)}}),
}
unschedulable := []*v1.Pod{
// Ignored, matches another provisioner
// Ignored, matches another nodepool
test.UnschedulablePod(test.PodOptions{NodeSelector: map[string]string{v1beta1.NodePoolLabelKey: "unknown"}}),
// Ignored, invalid zone
test.UnschedulablePod(test.PodOptions{NodeSelector: map[string]string{v1.LabelTopologyZone: "unknown"}}),
Expand All @@ -95,6 +95,46 @@ var _ = Describe("NodePool/Provisioning", func() {
ExpectNotScheduled(ctx, env.Client, pod)
}
})
It("should provision nodes for pods with supported node affinities", func() {
nodePool := test.NodePool()
schedulable := []*v1.Pod{
// Constrained by nodepool
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1beta1.NodePoolLabelKey, Operator: v1.NodeSelectorOpIn, Values: []string{nodePool.Name}}}}),
// Constrained by zone
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1"}}}}),
// Constrained by instanceType
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelInstanceTypeStable, Operator: v1.NodeSelectorOpIn, Values: []string{"default-instance-type"}}}}),
// Constrained by architecture
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelArchStable, Operator: v1.NodeSelectorOpIn, Values: []string{"arm64"}}}}),
// Constrained by operatingSystem
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux)}}}}),
}
unschedulable := []*v1.Pod{
// Ignored, matches another nodepool
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1beta1.NodePoolLabelKey, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, invalid zone
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, invalid instance type
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelInstanceTypeStable, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, invalid architecture
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelArchStable, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, invalid operating system
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, invalid capacity type
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1beta1.CapacityTypeLabelKey, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, label selector does not match
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: "foo", Operator: v1.NodeSelectorOpIn, Values: []string{"bar"}}}}),
}
ExpectApplied(ctx, env.Client, nodePool)
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, schedulable...)
for _, pod := range schedulable {
ExpectScheduled(ctx, env.Client, pod)
}
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, unschedulable...)
for _, pod := range unschedulable {
ExpectNotScheduled(ctx, env.Client, pod)
}
})
It("should provision nodes for accelerators", func() {
ExpectApplied(ctx, env.Client, test.NodePool())
pods := []*v1.Pod{
Expand Down
42 changes: 36 additions & 6 deletions pkg/controllers/provisioning/provisioner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,19 +445,27 @@ func (p *Provisioner) getDaemonSetPods(ctx context.Context) ([]*v1.Pod, error) {

func (p *Provisioner) Validate(ctx context.Context, pod *v1.Pod) error {
return multierr.Combine(
validateProvisionerNameCanExist(pod),
validateKarpenterManagedLabelCanExist(pod),
validateNodeSelector(pod),
validateAffinity(pod),
p.volumeTopology.ValidatePersistentVolumeClaims(ctx, pod),
)
}

// validateProvisionerNameCanExist provides a more clear error message in the event of scheduling a pod that specifically doesn't
// validateKarpenterManagedLabelCanExist provides a more clear error message in the event of scheduling a pod that specifically doesn't
// want to run on a Karpenter node (e.g. a Karpenter controller replica).
func validateProvisionerNameCanExist(p *v1.Pod) error {
func validateKarpenterManagedLabelCanExist(p *v1.Pod) error {
hasProvisionerNameLabel, hasNodePoolLabel := false, false
for _, req := range scheduling.NewPodRequirements(p) {
if req.Key == v1alpha5.ProvisionerNameLabelKey && req.Operator() == v1.NodeSelectorOpDoesNotExist {
return fmt.Errorf("configured to not run on a Karpenter provisioned node via %s %s requirement",
v1alpha5.ProvisionerNameLabelKey, v1.NodeSelectorOpDoesNotExist)
hasProvisionerNameLabel = true
}
if req.Key == v1beta1.NodePoolLabelKey && req.Operator() == v1.NodeSelectorOpDoesNotExist {
hasNodePoolLabel = true
}
if hasProvisionerNameLabel && hasNodePoolLabel {
return fmt.Errorf("configured to not run on a Karpenter provisioned node via %s %s and %s %s requirements",
v1alpha5.ProvisionerNameLabelKey, v1.NodeSelectorOpDoesNotExist, v1beta1.NodePoolLabelKey, v1.NodeSelectorOpDoesNotExist)
}
}
return nil
Expand All @@ -475,6 +483,24 @@ func (p *Provisioner) injectTopology(ctx context.Context, pods []*v1.Pod) []*v1.
return schedulablePods
}

func validateNodeSelector(p *v1.Pod) (errs error) {
terms := lo.MapToSlice(p.Spec.NodeSelector, func(k string, v string) v1.NodeSelectorTerm {
return v1.NodeSelectorTerm{
MatchExpressions: []v1.NodeSelectorRequirement{
{
Key: k,
Operator: v1.NodeSelectorOpIn,
Values: []string{v},
},
},
}
})
for _, term := range terms {
errs = multierr.Append(errs, validateNodeSelectorTerm(term))
}
return errs
}

func validateAffinity(p *v1.Pod) (errs error) {
if p.Spec.Affinity == nil {
return nil
Expand All @@ -498,7 +524,11 @@ func validateNodeSelectorTerm(term v1.NodeSelectorTerm) (errs error) {
}
if term.MatchExpressions != nil {
for _, requirement := range term.MatchExpressions {
errs = multierr.Append(errs, v1alpha5.ValidateRequirement(requirement))
alphaErr := v1alpha5.ValidateRequirement(requirement)
betaErr := v1beta1.ValidateRequirement(requirement)
if alphaErr != nil && betaErr != nil {
errs = multierr.Append(errs, betaErr)
}
}
}
return errs
Expand Down
40 changes: 40 additions & 0 deletions pkg/controllers/provisioning/provisioner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,46 @@ var _ = Describe("Provisioner/Provisioning", func() {
ExpectNotScheduled(ctx, env.Client, pod)
}
})
It("should provision nodes for pods with supported node affinities", func() {
provisioner := test.Provisioner()
schedulable := []*v1.Pod{
// Constrained by provisioner
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1alpha5.ProvisionerNameLabelKey, Operator: v1.NodeSelectorOpIn, Values: []string{provisioner.Name}}}}),
// Constrained by zone
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1"}}}}),
// Constrained by instanceType
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelInstanceTypeStable, Operator: v1.NodeSelectorOpIn, Values: []string{"default-instance-type"}}}}),
// Constrained by architecture
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelArchStable, Operator: v1.NodeSelectorOpIn, Values: []string{"arm64"}}}}),
// Constrained by operatingSystem
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux)}}}}),
}
unschedulable := []*v1.Pod{
// Ignored, matches another provisioner
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1alpha5.ProvisionerNameLabelKey, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, invalid zone
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, invalid instance type
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelInstanceTypeStable, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, invalid architecture
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelArchStable, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, invalid operating system
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, invalid capacity type
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{"unknown"}}}}),
// Ignored, label selector does not match
test.UnschedulablePod(test.PodOptions{NodeRequirements: []v1.NodeSelectorRequirement{{Key: "foo", Operator: v1.NodeSelectorOpIn, Values: []string{"bar"}}}}),
}
ExpectApplied(ctx, env.Client, provisioner)
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, schedulable...)
for _, pod := range schedulable {
ExpectScheduled(ctx, env.Client, pod)
}
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, unschedulable...)
for _, pod := range unschedulable {
ExpectNotScheduled(ctx, env.Client, pod)
}
})
It("should provision nodes for accelerators", func() {
ExpectApplied(ctx, env.Client, test.Provisioner())
pods := []*v1.Pod{
Expand Down

0 comments on commit 28b4b11

Please sign in to comment.