diff --git a/src/compute-plane-services/nvca/internal/util/k8sutil/affinity.go b/src/compute-plane-services/nvca/internal/util/k8sutil/affinity.go index ddf640736..a14fc34a9 100644 --- a/src/compute-plane-services/nvca/internal/util/k8sutil/affinity.go +++ b/src/compute-plane-services/nvca/internal/util/k8sutil/affinity.go @@ -77,18 +77,35 @@ func SetCPUWorkloadNodeAffinity(pts *corev1.PodSpec, instanceTypeLabelKey string RemoveInstanceTypeRequiredAffinity(pts.Affinity.NodeAffinity, instanceTypeLabelKey) - pts.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution = append( - pts.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution, - corev1.PreferredSchedulingTerm{ - Weight: 100, - Preference: corev1.NodeSelectorTerm{ - MatchExpressions: []corev1.NodeSelectorRequirement{{ - Key: instanceTypeLabelKey, - Operator: corev1.NodeSelectorOpDoesNotExist, - }}, - }, + preferred := &pts.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution + for _, term := range *preferred { + if hasCPUWorkloadSoftAntiAffinity(term, instanceTypeLabelKey) { + return + } + } + + *preferred = append(*preferred, corev1.PreferredSchedulingTerm{ + Weight: 100, + Preference: corev1.NodeSelectorTerm{ + MatchExpressions: []corev1.NodeSelectorRequirement{{ + Key: instanceTypeLabelKey, + Operator: corev1.NodeSelectorOpDoesNotExist, + }}, }, - ) + }) +} + +// hasCPUWorkloadSoftAntiAffinity reports whether preferred already includes a +// preference that soft-avoids nodes with instanceTypeLabelKey, even +// when that preference also has other MatchExpressions/MatchFields. +func hasCPUWorkloadSoftAntiAffinity(term corev1.PreferredSchedulingTerm, instanceTypeLabelKey string) bool { + for _, expr := range term.Preference.MatchExpressions { + if expr.Key == instanceTypeLabelKey && + expr.Operator == corev1.NodeSelectorOpDoesNotExist { + return true + } + } + return false } // RemoveInstanceTypeRequiredAffinity removes any RequiredDuringSchedulingIgnoredDuringExecution diff --git a/src/compute-plane-services/nvca/internal/util/k8sutil/affinity_test.go b/src/compute-plane-services/nvca/internal/util/k8sutil/affinity_test.go new file mode 100644 index 000000000..0796fc364 --- /dev/null +++ b/src/compute-plane-services/nvca/internal/util/k8sutil/affinity_test.go @@ -0,0 +1,84 @@ +/* +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package k8sutil + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" +) + +func TestSetCPUWorkloadNodeAffinity_DedupesMultiExpressionPreference(t *testing.T) { + const instanceTypeKey = "nvca.nvcf.nvidia.io/instance-type" + + podSpec := &corev1.PodSpec{ + Affinity: &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []corev1.PreferredSchedulingTerm{ + { + Weight: 100, + Preference: corev1.NodeSelectorTerm{ + MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: "topology.kubernetes.io/zone", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"us-west"}, + }, + { + Key: instanceTypeKey, + Operator: corev1.NodeSelectorOpDoesNotExist, + }, + { + Key: "kubernetes.io/arch", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"amd64"}, + }, + }, + MatchFields: []corev1.NodeSelectorRequirement{{ + Key: "metadata.name", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"node-a"}, + }}, + }, + }, + }, + }, + }, + } + + SetCPUWorkloadNodeAffinity(podSpec, instanceTypeKey) + SetCPUWorkloadNodeAffinity(podSpec, instanceTypeKey) + + preferred := podSpec.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution + require.Len(t, preferred, 1, "should not append a duplicate soft anti-affinity term") + assert.Equal(t, int32(100), preferred[0].Weight) + assert.Len(t, preferred[0].Preference.MatchExpressions, 3) + assert.Len(t, preferred[0].Preference.MatchFields, 1) + + found := false + for _, expr := range preferred[0].Preference.MatchExpressions { + if expr.Key == instanceTypeKey && + expr.Operator == corev1.NodeSelectorOpDoesNotExist && + len(expr.Values) == 0 { + found = true + } + } + assert.True(t, found) +} diff --git a/src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook.go b/src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook.go index 35795b793..ca20b3f27 100644 --- a/src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook.go +++ b/src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook.go @@ -327,8 +327,6 @@ func (w *miniserviceMutatingWebhook) mutate(ctx context.Context, obj client.Obje // Pod spec mutations must only be applied on creation events. if isCreate { - w.mutatePodSpec(&t.Spec, meta) - // NVLink DRA mutations for claims/scheduling. if w.fff.IsAttributeEnabled(featureflag.AttrNVLinkOptimized) { w.mutateNVLinkDRA(obj.GetNamespace(), t) @@ -337,6 +335,8 @@ func (w *miniserviceMutatingWebhook) mutate(ctx context.Context, obj client.Obje if _, _, err := w.sharedStorageMutator.mutate(ctx, obj, meta); err != nil { return fmt.Errorf("mutate shared storage: %w", err) } + + w.mutatePodSpec(&t.Spec, meta) } } @@ -386,6 +386,8 @@ func (w *miniserviceMutatingWebhook) mutatePodSpec(ps *corev1.PodSpec, meta nvca if meta.NodeAffinityKey != "" { if w.fff.IsFeatureFlagEnabled(featureflag.HelmAllowCPUNodes) && !k8sutil.PodSpecRequestsGPU(ps) { k8sutil.SetCPUWorkloadNodeAffinity(ps, meta.NodeAffinityKey) + } else { + k8sutil.SetInstanceTypeNodeAffinity(ps, meta.NodeAffinityKey, meta.NodeAffinityValue) } } diff --git a/src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook_test.go b/src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook_test.go index 5c4ff38dd..f76f2d906 100644 --- a/src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook_test.go +++ b/src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook_test.go @@ -21,6 +21,7 @@ import ( "context" "encoding/json" "net/http" + "sync/atomic" "testing" "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/core" @@ -32,6 +33,7 @@ import ( jsonpatch "gomodules.xyz/jsonpatch/v2" admissionv1 "k8s.io/api/admission/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" k8sfake "k8s.io/client-go/kubernetes/fake" @@ -40,6 +42,7 @@ import ( nvcfdra "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/dra" "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/featureflag" featureflagmock "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/featureflag/mock" + "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/nodefeatures/sharedcluster" cmnnvcastorage "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/storage" nvcatypes "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/types" ) @@ -547,13 +550,25 @@ func TestMiniserviceOperatorWebhook_PodSpecCreateThenUpdate_IsIdempotentOrderPre { MatchExpressions: []corev1.NodeSelectorRequirement{ {Key: "baz", Operator: corev1.NodeSelectorOpIn, Values: []string{"buf"}}, + { + Key: "nvca.nvcf.nvidia.io/instance-type", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"ON-PREM.GPU.A100"}, + }, }, }, { - MatchExpressions: []corev1.NodeSelectorRequirement{{ - Key: nvcfdra.GPUCliqueNodeLabel, - Operator: corev1.NodeSelectorOpExists, - }}, + MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: nvcfdra.GPUCliqueNodeLabel, + Operator: corev1.NodeSelectorOpExists, + }, + { + Key: "nvca.nvcf.nvidia.io/instance-type", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"ON-PREM.GPU.A100"}, + }, + }, }, }, }, @@ -1012,6 +1027,188 @@ func TestMiniserviceMutatePodSpec_ServiceAccountName(t *testing.T) { } } +func TestMiniserviceMutatePodSpec_NodeAffinityWithPodAffinityWebhook(t *testing.T) { + const ( + instanceTypeKey = "nvca.nvcf.nvidia.io/instance-type" + instanceTypeValue = "ON-PREM.GPU.A100" + ) + + meta := nvcatypes.MiniserviceMetadata{ + NodeAffinityKey: instanceTypeKey, + NodeAffinityValue: instanceTypeValue, + } + sharedClusterOn := &atomic.Bool{} + sharedClusterOn.Store(true) + podWebhook := &podAffinityWebhook{PodAffinityOptions: PodAffinityOptions{ + SharedClusterOn: sharedClusterOn, + UniformInstanceLabels: true, + }} + presetInstanceTypeAffinity := func() *corev1.Affinity { + return &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{{ + MatchExpressions: []corev1.NodeSelectorRequirement{{ + Key: instanceTypeKey, + Operator: corev1.NodeSelectorOpIn, + Values: []string{"ON-PREM.GPU.H100"}, + }}, + }}, + }, + }, + } + } + + tests := []struct { + name string + podSpec corev1.PodSpec + miniserviceFirst bool + wantInstanceType bool + wantCPUPreference bool + }{ + { + name: "GPU pod, miniservice webhook first", + podSpec: corev1.PodSpec{Containers: []corev1.Container{{ + Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{ + corev1.ResourceName("nvidia.com/gpu"): resource.MustParse("1"), + }}, + }}}, + miniserviceFirst: true, + wantInstanceType: true, + }, + { + name: "GPU pod, pod affinity webhook first", + podSpec: corev1.PodSpec{Containers: []corev1.Container{{ + Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{ + corev1.ResourceName("nvidia.com/gpu"): resource.MustParse("1"), + }}, + }}}, + wantInstanceType: true, + }, + { + name: "GPU pod with preset instance type affinity, miniservice webhook first", + podSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{ + corev1.ResourceName("nvidia.com/gpu"): resource.MustParse("1"), + }}, + }}, + Affinity: presetInstanceTypeAffinity(), + }, + miniserviceFirst: true, + wantInstanceType: true, + }, + { + name: "GPU pod with preset instance type affinity, pod affinity webhook first", + podSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{ + corev1.ResourceName("nvidia.com/gpu"): resource.MustParse("1"), + }}, + }}, + Affinity: presetInstanceTypeAffinity(), + }, + wantInstanceType: true, + }, + { + name: "CPU pod, miniservice webhook first", + podSpec: corev1.PodSpec{Containers: []corev1.Container{{Name: "app"}}}, + miniserviceFirst: true, + wantCPUPreference: true, + }, + { + name: "CPU pod, pod affinity webhook first", + podSpec: corev1.PodSpec{Containers: []corev1.Container{{Name: "app"}}}, + wantCPUPreference: true, + }, + { + name: "CPU pod with preset instance type affinity, miniservice webhook first", + podSpec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "app"}}, + Affinity: presetInstanceTypeAffinity(), + }, + miniserviceFirst: true, + wantCPUPreference: true, + }, + { + name: "CPU pod with preset instance type affinity, pod affinity webhook first", + podSpec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "app"}}, + Affinity: presetInstanceTypeAffinity(), + }, + wantCPUPreference: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wh := &miniserviceMutatingWebhook{fff: &featureflagmock.Fetcher{ + EnabledFFs: []*featureflag.FeatureFlag{featureflag.HelmAllowCPUNodes}, + }} + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "workload", Namespace: testNamespace}, + Spec: *tt.podSpec.DeepCopy(), + } + + applyWebhooks := func() { + if tt.miniserviceFirst { + wh.mutatePodSpec(&pod.Spec, meta) + require.NoError(t, podWebhook.Default(t.Context(), pod)) + } else { + require.NoError(t, podWebhook.Default(t.Context(), pod)) + wh.mutatePodSpec(&pod.Spec, meta) + } + } + + applyWebhooks() + afterFirstPass := pod.Spec.DeepCopy() + applyWebhooks() + assert.Equal(t, afterFirstPass, &pod.Spec, "webhook mutations must be idempotent") + + require.NotNil(t, pod.Spec.Affinity) + require.NotNil(t, pod.Spec.Affinity.NodeAffinity) + required := pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution + require.NotNil(t, required) + require.Len(t, required.NodeSelectorTerms, 1) + + var instanceTypeRequirements, sharedClusterRequirements int + for _, expr := range required.NodeSelectorTerms[0].MatchExpressions { + switch expr.Key { + case instanceTypeKey: + instanceTypeRequirements++ + assert.Equal(t, corev1.NodeSelectorOpIn, expr.Operator) + assert.Equal(t, []string{instanceTypeValue}, expr.Values) + case sharedcluster.ScheduleLabelKey: + sharedClusterRequirements++ + assert.Equal(t, corev1.NodeSelectorOpIn, expr.Operator) + assert.Equal(t, []string{"true"}, expr.Values) + } + } + if tt.wantInstanceType { + assert.Equal(t, 1, instanceTypeRequirements) + } else { + assert.Zero(t, instanceTypeRequirements) + } + assert.Equal(t, 1, sharedClusterRequirements) + + var cpuPreferences int + for _, term := range pod.Spec.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution { + for _, expr := range term.Preference.MatchExpressions { + if term.Weight == 100 && expr.Key == instanceTypeKey && + expr.Operator == corev1.NodeSelectorOpDoesNotExist { + cpuPreferences++ + } + } + } + if tt.wantCPUPreference { + assert.Equal(t, 1, cpuPreferences) + } else { + assert.Zero(t, cpuPreferences) + } + }) + } +} + // ─── mutatePodSpec BYOObservability tests ───────────────────────────────────── func TestMiniserviceMutatePodSpec_BYOObservability(t *testing.T) {