fix: NUMA refit follow-ups from post-merge review

charts/hami/templates/device-plugin/daemonsetnvidia.yaml

@@ -42,6 +42,11 @@ spec:
       priorityClassName: system-node-critical
       hostPID: {{ .Values.devicePlugin.hostPID }}
       hostNetwork: {{ .Values.devicePlugin.hostNetwork | default false }}
+      {{- if .Values.devicePlugin.hostNetwork }}
+      # Cluster DNS is required for the default NUMA refit endpoint, which is
+      # a Service name.
+      dnsPolicy: ClusterFirstWithHostNet
+      {{- end }}
       {{- include "hami.devicePlugin.imagePullSecrets" . | nindent 6 }}
       {{- if .Values.devicePlugin.gpuOperatorToolkitReady.enabled }}
       initContainers:

charts/hami/values.yaml

@@ -367,7 +367,11 @@ devicePlugin:
     # extender configmap (tlsConfig.insecure). Set false to verify, and use
     # caFile to point at a CA bundle mounted into the device plugin.
     tlsInsecure: true
+    # Path to a CA bundle already present inside the device-plugin container.
     caFile: ""
+    # Name of a Secret (key ca.crt) to mount as the CA bundle instead; used
+    # when caFile is empty.
+    caSecret: ""
   # Pre-configured device memory in MB for GPUs that don't support memory query (e.g., unified memory architecture GPUs like NVIDIA GB10/DGX Spark).
   # Set to 0 to use auto-detection (default). For unified memory GPUs, set to the total GPU memory (e.g., 131072 for 128GB).
   # Can be overridden per-node via nodeConfiguration.config.

pkg/device-plugin/nvidiadevice/nvinternal/plugin/numa_refit_client.go

@@ -67,30 +67,37 @@ const (
 // numaRefitTLSConfig verifies the scheduler certificate by default, against
 // SchedulerCAFileEnvName when provided; SchedulerTLSInsecureEnvName is an
 // explicit operator opt-out for the self-signed webhook certificate.
-func numaRefitTLSConfig() *tls.Config {
+func numaRefitTLSConfig() (*tls.Config, error) {
 	config := &tls.Config{MinVersion: tls.VersionTLS12}
 	if caFile := os.Getenv(SchedulerCAFileEnvName); caFile != "" {
 		pem, err := os.ReadFile(caFile)
 		if err != nil {
-			klog.ErrorS(err, "cannot read scheduler CA bundle", "path", caFile)
-		} else if pool := x509.NewCertPool(); pool.AppendCertsFromPEM(pem) {
-			config.RootCAs = pool
-		} else {
-			klog.ErrorS(nil, "scheduler CA bundle contains no usable certificates", "path", caFile)
+			return nil, fmt.Errorf("cannot read scheduler CA bundle %q: %w", caFile, err)
 		}
+		pool := x509.NewCertPool()
+		if !pool.AppendCertsFromPEM(pem) {
+			return nil, fmt.Errorf("scheduler CA bundle %q contains no usable certificates", caFile)
+		}
+		config.RootCAs = pool
 	}
 	if insecure, err := strconv.ParseBool(os.Getenv(SchedulerTLSInsecureEnvName)); err == nil {
 		config.InsecureSkipVerify = insecure
 	}
-	return config
+	return config, nil
 }
 
-// numaRefitHTTPClient reaches the scheduler service.
-var numaRefitHTTPClient = &http.Client{
-	Timeout: numaRefitTimeout,
-	Transport: &http.Transport{
-		TLSClientConfig: numaRefitTLSConfig(),
-	},
+// numaRefitHTTPClient reaches the scheduler service. It is built per refit so
+// a rotated CA bundle is picked up without restarting the device plugin; the
+// refit is a rare path, taken only when an allocation mismatches.
+func numaRefitHTTPClient() (*http.Client, error) {
+	tlsConfig, err := numaRefitTLSConfig()
+	if err != nil {
+		return nil, err
+	}
+	return &http.Client{
+		Timeout:   numaRefitTimeout,
+		Transport: &http.Transport{TLSClientConfig: tlsConfig},
+	}, nil
 }
 
 // tryNumaRefit asks the scheduler to move this container's pending
@@ -110,13 +117,13 @@ func (plugin *NvidiaDevicePlugin) tryNumaRefit(ctx context.Context, pod *corev1.
 		return nil, nil
 	}
 
-	// When kubelet pins replicas via MustIncludeDeviceIDs, only their
-	// physical devices can satisfy the allocation, so restrict the refit to
-	// them; otherwise any available physical device is eligible.
+	// Kubelet builds AvailableDeviceIDs as a superset of MustIncludeDeviceIDs
+	// (available.Union(allocated) with mustInclude = allocated), so the
+	// available set is the candidate pool. Restricting it to MustInclude
+	// would shrink the pool below the requested device count; the pinned
+	// replicas are honored by selectPreferredDeviceIDsFromAnnotatedDevices,
+	// which seeds them first.
 	allowedUUIDs := allowedPhysicalDeviceIDs(req.AvailableDeviceIDs)
-	if len(req.MustIncludeDeviceIDs) > 0 {
-		allowedUUIDs = allowedPhysicalDeviceIDs(req.MustIncludeDeviceIDs)
-	}
 	newDevices, err := plugin.requestNumaRefit(ctx, pod, containerIndex, allowedUUIDs)
 	if err == nil {
 		replicas, selectErr := plugin.selectPreferredDeviceIDsFromAnnotatedDevices(req.AvailableDeviceIDs, req.MustIncludeDeviceIDs, newDevices, int(req.AllocationSize))
@@ -162,7 +169,11 @@ func (plugin *NvidiaDevicePlugin) requestNumaRefit(ctx context.Context, pod *cor
 	}
 	httpReq.Header.Set("Content-Type", "application/json")
 
-	httpResp, err := numaRefitHTTPClient.Do(httpReq)
+	httpClient, err := numaRefitHTTPClient()
+	if err != nil {
+		return nil, err
+	}
+	httpResp, err := httpClient.Do(httpReq)
 	if err != nil {
 		return nil, err
 	}

pkg/device-plugin/nvidiadevice/nvinternal/plugin/numa_refit_client_test.go

@@ -21,6 +21,8 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -202,14 +204,16 @@ func TestGetPreferredAllocationRefitDisabled(t *testing.T) {
 func TestNumaRefitTLSConfigVerifiesByDefault(t *testing.T) {
 	t.Setenv(SchedulerTLSInsecureEnvName, "")
 	t.Setenv(SchedulerCAFileEnvName, "")
-	config := numaRefitTLSConfig()
+	config, err := numaRefitTLSConfig()
+	require.NoError(t, err)
 	require.False(t, config.InsecureSkipVerify)
 	require.Nil(t, config.RootCAs)
 }
 
 func TestNumaRefitTLSConfigInsecureOptOut(t *testing.T) {
 	t.Setenv(SchedulerTLSInsecureEnvName, "true")
-	config := numaRefitTLSConfig()
+	config, err := numaRefitTLSConfig()
+	require.NoError(t, err)
 	require.True(t, config.InsecureSkipVerify)
 }
 
@@ -240,3 +244,48 @@ func TestGetPreferredAllocationRefitCommittedButUnmappable(t *testing.T) {
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "committed")
 }
+
+func TestNumaRefitTLSConfigRejectsUnusableCA(t *testing.T) {
+	t.Setenv(SchedulerTLSInsecureEnvName, "")
+
+	missing := filepath.Join(t.TempDir(), "absent.crt")
+	t.Setenv(SchedulerCAFileEnvName, missing)
+	_, err := numaRefitTLSConfig()
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "cannot read scheduler CA bundle")
+
+	garbage := filepath.Join(t.TempDir(), "garbage.crt")
+	require.NoError(t, os.WriteFile(garbage, []byte("not a certificate"), 0o600))
+	t.Setenv(SchedulerCAFileEnvName, garbage)
+	_, err = numaRefitTLSConfig()
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "no usable certificates")
+}
+
+// A refit must not send the allowed set shrunk to MustIncludeDeviceIDs:
+// kubelet builds AvailableDeviceIDs as a superset of it.
+func TestGetPreferredAllocationRefitUsesAvailableSuperset(t *testing.T) {
+	refitted := device.ContainerDevices{{UUID: numaTestGPUB, Type: nvidia.NvidiaGPUDevice, Usedmem: 20000, Usedcores: 30}}
+	server, lastRequest := numaRefitTestServer(t, device.NumaRefitResponse{
+		Succeeded:        true,
+		ContainerDevices: device.EncodeContainerDevices(refitted),
+	})
+	t.Setenv(SchedulerEndpointEnvName, server.URL)
+	t.Setenv(util.NodeNameEnvName, "node-a")
+	setupInRequestDevices(t)
+
+	pod := numaRefitTestPod("best-effort")
+	mockAllocateGlobals(t, pod)
+
+	plugin := &NvidiaDevicePlugin{}
+	_, err := plugin.GetPreferredAllocation(context.Background(), &kubeletdevicepluginv1beta1.PreferredAllocationRequest{
+		ContainerRequests: []*kubeletdevicepluginv1beta1.ContainerPreferredAllocationRequest{{
+			AvailableDeviceIDs:   []string{numaTestGPUB + "-0", numaTestGPUC + "-0"},
+			MustIncludeDeviceIDs: []string{numaTestGPUB + "-0"},
+			AllocationSize:       1,
+		}},
+	})
+
+	require.NoError(t, err)
+	require.ElementsMatch(t, []string{numaTestGPUB, numaTestGPUC}, lastRequest.AllowedDeviceUUIDs)
+}

pkg/device-plugin/nvidiadevice/nvinternal/plugin/server_numa_alignment_test.go

@@ -39,6 +39,7 @@ import (
 const (
 	numaTestGPUA = "GPU-aaaaaaaa-1111-2222-3333-444444444444"
 	numaTestGPUB = "GPU-bbbbbbbb-1111-2222-3333-444444444444"
+	numaTestGPUC = "GPU-cccccccc-1111-2222-3333-444444444444"
 )
 
 func TestSelectPreferredMismatchErrorIsTyped(t *testing.T) {

pkg/scheduler/numa_refit_handler.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"maps"
 	"strings"
+	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -48,14 +49,25 @@ var patchPodAnnotations = func(pod *corev1.Pod, annotations map[string]string) e
 	if err != nil {
 		return err
 	}
+	// The refit holds the allocation lock that Filter also takes, so this
+	// call must not block scheduling on an unreachable API server. The
+	// device plugin gives up after numaRefitTimeout anyway.
+	ctx, cancel := context.WithTimeout(context.Background(), refitPatchTimeout)
+	defer cancel()
 	_, err = client.GetClient().CoreV1().Pods(pod.Namespace).
-		Patch(context.Background(), pod.Name, k8stypes.MergePatchType, payload, metav1.PatchOptions{})
+		Patch(ctx, pod.Name, k8stypes.MergePatchType, payload, metav1.PatchOptions{})
 	return err
 }
 
 // maxAllowedDeviceUUIDs bounds the allowed set a refit request may carry.
 const maxAllowedDeviceUUIDs = 512
 
+// refitPatchTimeout bounds the annotation patch. It is deliberately no longer
+// than the device plugin's own refit budget: a patch that has not landed by
+// then cannot be used, and waiting longer would hold the allocation lock
+// against every concurrent Filter call.
+const refitPatchTimeout = 2 * time.Second
+
 // RefitNumaAllocation moves one container's device reservation onto a device
 // from the caller-supplied allowed set, re-running the pod's normal
 // policy-chain fit restricted to that set. The device plugin calls it (via
@@ -245,7 +257,14 @@ func (s *Scheduler) RefitNumaAllocation(req device.NumaRefitRequest) device.Numa
 	maps.Copy(patchedAnnotations, pod.Annotations)
 	maps.Copy(patchedAnnotations, annotations)
 	if rawDevices, decodeErr := device.DecodePodDevices(device.SupportDevices, patchedAnnotations); decodeErr == nil {
+		// Mirror whichever accounting shape the pod already has: once the
+		// init-container usage has been released, collapsing again would
+		// re-inflate the reservation back to the init peak, the same hazard
+		// PodManager.AddPod guards against on a re-add.
 		effective := device.CollapseInitContainerUsage(pod, rawDevices)
+		if pi.InitContainerResourceReleased {
+			effective = device.SteadyStateDeviceUsage(pod, rawDevices)
+		}
 		if _, ok := s.podManager.ReplacePodDevices(key, effective); ok {
 			s.quotaManager.AddUsage(pod, effective)
 		} else {

pkg/scheduler/numa_refit_handler_test.go

@@ -466,3 +466,62 @@ func TestRefitNumaAllocationHeterogeneousReservation(t *testing.T) {
 	assert.Assert(t, strings.Contains(response.FailureReason, "heterogeneous"), "reason: %s", response.FailureReason)
 	assert.Equal(t, *calls, 0)
 }
+
+func TestRefitNumaAllocationKeepsShrunkAccounting(t *testing.T) {
+	// A pod whose init-container usage was already released must not have its
+	// reservation re-inflated back to the init peak by a later refit.
+	nodes := newNodeManager()
+	nodes.addNode(refitNode, &device.NodeInfo{
+		ID: refitNode, Node: &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: refitNode}},
+		Devices: map[string][]device.DeviceInfo{nvidia.NvidiaGPUDevice: {
+			{ID: "GPU-a", Count: 10, Devmem: 40000, Devcore: 100, Numa: 1, Type: nvidia.NvidiaGPUDevice, Health: true},
+			{ID: "GPU-b", Count: 10, Devmem: 40000, Devcore: 100, Numa: 0, Type: nvidia.NvidiaGPUDevice, Health: true},
+		}},
+	})
+
+	initDevices := device.ContainerDevices{{UUID: "GPU-a", Type: nvidia.NvidiaGPUDevice, Usedmem: 30000, Usedcores: 60}}
+	appDevices := device.ContainerDevices{{UUID: "GPU-a", Type: nvidia.NvidiaGPUDevice, Usedmem: 20000, Usedcores: 30}}
+	reserved := device.PodSingleDevice{initDevices, appDevices}
+	pod := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			UID: refitPodUID, Name: refitPodName, Namespace: "default",
+			Annotations: map[string]string{
+				device.InRequestDevices[nvidia.NvidiaGPUDevice]: device.EncodePodSingleDevice(reserved),
+				device.SupportDevices[nvidia.NvidiaGPUDevice]:   device.EncodePodSingleDevice(reserved),
+			},
+		},
+		Spec: corev1.PodSpec{
+			InitContainers: []corev1.Container{{Name: "init"}},
+			Containers:     []corev1.Container{{Name: "main"}},
+		},
+	}
+	pods := device.NewPodManager()
+	pods.AddPod(pod, refitNode, device.PodDevices{nvidia.NvidiaGPUDevice: reserved})
+	// Simulate the informer's post-init shrink, which arms the released flag.
+	steady := device.SteadyStateDeviceUsage(pod, device.PodDevices{nvidia.NvidiaGPUDevice: reserved})
+	pods.UpdatePodDevice(pod, steady)
+
+	s := &Scheduler{nodeManager: nodes, podManager: pods, quotaManager: device.NewQuotaManager()}
+	s.quotaManager.Quotas = map[string]*device.DeviceQuota{}
+	stubRefitPatch(t, nil)
+
+	request := refitTestRequestFor("GPU-b")
+	request.ContainerIndex = 1
+	request.ContainerName = "main"
+	response := s.RefitNumaAllocation(request)
+	assert.Equal(t, response.Succeeded, true, "refit failed: %s", response.FailureReason)
+
+	pi, ok := s.podManager.GetPod(&corev1.Pod{ObjectMeta: metav1.ObjectMeta{UID: refitPodUID}})
+	assert.Equal(t, ok, true)
+	assert.Equal(t, pi.InitContainerResourceReleased, true)
+	// Steady state is app-only: the refitted GPU-b at the app amount, with no
+	// re-inflated init-container reservation on GPU-a.
+	total := int32(0)
+	for _, containerDevices := range pi.Devices[nvidia.NvidiaGPUDevice] {
+		for _, d := range containerDevices {
+			total += d.Usedmem
+			assert.Equal(t, d.UUID, "GPU-b", "unexpected device %s after refit", d.UUID)
+		}
+	}
+	assert.Equal(t, total, int32(20000))
+}