diff --git a/pkg/controllers/allocation/scheduling/preferences.go b/pkg/controllers/allocation/scheduling/preferences.go index ed9da1f7c894..d3802ace98eb 100644 --- a/pkg/controllers/allocation/scheduling/preferences.go +++ b/pkg/controllers/allocation/scheduling/preferences.go @@ -83,8 +83,8 @@ func (p *Preferences) removePreferredNodeAffinityTerm(ctx context.Context, pod * terms := pod.Spec.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution // Remove the terms if there are any (terms are an OR semantic) if len(terms) > 0 { - // Sort ascending by weight to remove lightest preferences first - sort.SliceStable(terms, func(i, j int) bool { return terms[i].Weight < terms[j].Weight }) + // Sort descending by weight to remove heaviest preferences to try lighter ones + sort.SliceStable(terms, func(i, j int) bool { return terms[i].Weight > terms[j].Weight }) pod.Spec.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution = terms[1:] return ptr.String(fmt.Sprintf("spec.affinity.nodeAffinity.preferredDuringSchedulingIgnoredDuringExecution[0]=%s", pretty.Concise(terms[0]))) } diff --git a/pkg/controllers/allocation/scheduling/suite_test.go b/pkg/controllers/allocation/scheduling/suite_test.go index fe0c8e073122..a92bc5ea4f2a 100644 --- a/pkg/controllers/allocation/scheduling/suite_test.go +++ b/pkg/controllers/allocation/scheduling/suite_test.go @@ -402,6 +402,9 @@ var _ = Describe("Preferential Fallback", func() { {MatchExpressions: []v1.NodeSelectorRequirement{ {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1"}}, }}, + {MatchExpressions: []v1.NodeSelectorRequirement{ + {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-2"}}, // OR operator, never get to this one + }}, }}}} ExpectCreated(env.Client, provisioner) ExpectCreatedWithStatus(env.Client, pod) @@ -416,7 +419,8 @@ var _ = Describe("Preferential Fallback", func() { // Success ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(provisioner)) pod = ExpectPodExists(env.Client, pod.Name, pod.Namespace) - ExpectNodeExists(env.Client, pod.Spec.NodeName) + node := ExpectNodeExists(env.Client, pod.Spec.NodeName) + Expect(node.Labels).To(HaveKeyWithValue(v1.LabelTopologyZone, "test-zone-1")) }) }) Context("Preferred", func() { @@ -449,6 +453,38 @@ var _ = Describe("Preferential Fallback", func() { pod = ExpectPodExists(env.Client, pod.Name, pod.Namespace) ExpectNodeExists(env.Client, pod.Spec.NodeName) }) + It("should relax to use lighter weights", func() { + provisioner.Spec.Zones = []string{"test-zone-1", "test-zone-2"} + pod := test.UnschedulablePod() + pod.Spec.Affinity = &v1.Affinity{NodeAffinity: &v1.NodeAffinity{PreferredDuringSchedulingIgnoredDuringExecution: []v1.PreferredSchedulingTerm{ + { + Weight: 100, Preference: v1.NodeSelectorTerm{MatchExpressions: []v1.NodeSelectorRequirement{ + {Key: v1.LabelInstanceTypeStable, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-3"}}, + }}, + }, + { + Weight: 50, Preference: v1.NodeSelectorTerm{MatchExpressions: []v1.NodeSelectorRequirement{ + {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-2"}}, + }}, + }, + { + Weight: 1, Preference: v1.NodeSelectorTerm{MatchExpressions: []v1.NodeSelectorRequirement{ // OR operator, never get to this one + {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test-zone-1"}}, + }}, + }, + }}} + ExpectCreated(env.Client, provisioner) + ExpectCreatedWithStatus(env.Client, pod) + // Remove heaviest term + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(provisioner)) + pod = ExpectPodExists(env.Client, pod.Name, pod.Namespace) + Expect(pod.Spec.NodeName).To(BeEmpty()) + // Success + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(provisioner)) + pod = ExpectPodExists(env.Client, pod.Name, pod.Namespace) + node := ExpectNodeExists(env.Client, pod.Spec.NodeName) + Expect(node.Labels).To(HaveKeyWithValue(v1.LabelTopologyZone, "test-zone-2")) + }) }) }) diff --git a/pkg/scheduling/nodeaffinity.go b/pkg/scheduling/nodeaffinity.go index 019f966d9d31..6e2f404d9de7 100644 --- a/pkg/scheduling/nodeaffinity.go +++ b/pkg/scheduling/nodeaffinity.go @@ -15,6 +15,8 @@ limitations under the License. package scheduling import ( + "sort" + "github.com/awslabs/karpenter/pkg/utils/functional" v1 "k8s.io/api/core/v1" ) @@ -31,14 +33,15 @@ func NodeAffinityFor(pods ...*v1.Pod) (nodeAffinity NodeAffinity) { if pod.Spec.Affinity == nil || pod.Spec.Affinity.NodeAffinity == nil { continue } - // Preferences are treated as requirements. An outer loop will iteratively unconstrain them if unsatisfiable - for _, term := range pod.Spec.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution { - nodeAffinity = append(nodeAffinity, term.Preference.MatchExpressions...) + // Select heaviest preference and treat as a requirement. An outer loop will iteratively unconstrain them if unsatisfiable. + if preferred := pod.Spec.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution; len(preferred) > 0 { + sort.Slice(preferred, (func(i int, j int) bool { return preferred[i].Weight > preferred[j].Weight })) + nodeAffinity = append(nodeAffinity, preferred[0].Preference.MatchExpressions...) } - if pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil { - for _, term := range pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms { - nodeAffinity = append(nodeAffinity, term.MatchExpressions...) - } + // Select first requirement. An outer loop will iteratively remove OR requirements if unsatisfiable + if pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil && + len(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms) > 0 { + nodeAffinity = append(nodeAffinity, pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions...) } } return nodeAffinity diff --git a/pkg/utils/env/env.go b/pkg/utils/env/env.go index da822c3dc84c..dd06d26059a6 100644 --- a/pkg/utils/env/env.go +++ b/pkg/utils/env/env.go @@ -15,7 +15,6 @@ limitations under the License. package env import ( - "fmt" "os" "strconv" ) @@ -35,12 +34,10 @@ func WithDefaultString(key string, def string) string { func WithDefaultInt(key string, def int) int { val, ok := os.LookupEnv(key) if !ok { - fmt.Printf("COULDN'T FIND KEY %s", key) return def } i, err := strconv.Atoi(val) if err != nil { - fmt.Printf("CONVERSION FAILED FOR %s w/ value %s", key, val) return def } return i