diff --git a/cmd/vGPUmonitor/metrics.go b/cmd/vGPUmonitor/metrics.go index a666199b52..f8ca1c40c7 100644 --- a/cmd/vGPUmonitor/metrics.go +++ b/cmd/vGPUmonitor/metrics.go @@ -61,13 +61,13 @@ var ( hostGPUdesc = prometheus.NewDesc( "hami_host_gpu_memory_used_bytes", "GPU device memory usage in bytes", - []string{"device_index", "device_uuid", "device_type"}, nil, + []string{"node_name", "device_index", "device_uuid", "device_type"}, nil, ) hostGPUUtilizationdesc = prometheus.NewDesc( "hami_host_gpu_utilization_ratio", "GPU core utilization ratio (0-100)", - []string{"device_index", "device_uuid", "device_type"}, nil, + []string{"node_name", "device_index", "device_uuid", "device_type"}, nil, ) ctrvGPUdesc = prometheus.NewDesc( @@ -136,12 +136,12 @@ func initLegacyDescriptors() { legacyHostGPUdesc = prometheus.NewDesc( "HostGPUMemoryUsage", "GPU device memory usage", - []string{"deviceidx", "deviceuuid", "devicetype"}, nil, + []string{"nodeid", "deviceidx", "deviceuuid", "devicetype"}, nil, ) legacyHostGPUUtilizationdesc = prometheus.NewDesc( "HostCoreUtilization", "GPU core utilization", - []string{"deviceidx", "deviceuuid", "devicetype"}, nil, + []string{"nodeid", "deviceidx", "deviceuuid", "devicetype"}, nil, ) legacyCtrvGPUdesc = prometheus.NewDesc( "vGPU_device_memory_usage_in_bytes", @@ -239,6 +239,12 @@ func (cc ClusterManagerCollector) Collect(ch chan<- prometheus.Metric) { } func (cc ClusterManagerCollector) collectGPUInfo(ch chan<- prometheus.Metric) error { + nodeName := os.Getenv(util.NodeNameEnvName) + if nodeName == "" { + klog.Warningf("NODE_NAME env var not set, using 'unknown' for node label") + nodeName = "unknown" + } + if err := cc.initNVML(); err != nil { return err } @@ -250,7 +256,7 @@ func (cc ClusterManagerCollector) collectGPUInfo(ch chan<- prometheus.Metric) er } for ii := range devnum { - if err := cc.collectGPUDeviceMetrics(ch, ii); err != nil { + if err := cc.collectGPUDeviceMetrics(ch, nodeName, ii); err != nil { klog.Error("Failed to collect metrics for GPU device ", ii, ": ", err) } } @@ -274,24 +280,24 @@ func (cc ClusterManagerCollector) getDeviceCount() (int, error) { return devnum, nil } -func (cc ClusterManagerCollector) collectGPUDeviceMetrics(ch chan<- prometheus.Metric, index int) error { +func (cc ClusterManagerCollector) collectGPUDeviceMetrics(ch chan<- prometheus.Metric, nodeName string, index int) error { hdev, nvret := nvml.DeviceGetHandleByIndex(index) if nvret != nvml.SUCCESS { return fmt.Errorf("nvml DeviceGetHandleByIndex err: %s", nvml.ErrorString(nvret)) } - if err := cc.collectGPUMemoryMetrics(ch, hdev, index); err != nil { + if err := cc.collectGPUMemoryMetrics(ch, nodeName, hdev, index); err != nil { return err } - if err := cc.collectGPUUtilizationMetrics(ch, hdev, index); err != nil { + if err := cc.collectGPUUtilizationMetrics(ch, nodeName, hdev, index); err != nil { return err } return nil } -func (cc ClusterManagerCollector) collectGPUMemoryMetrics(ch chan<- prometheus.Metric, hdev nvml.Device, index int) error { +func (cc ClusterManagerCollector) collectGPUMemoryMetrics(ch chan<- prometheus.Metric, nodeName string, hdev nvml.Device, index int) error { memory, ret := hdev.GetMemoryInfo() if ret == nvml.ERROR_NOT_SUPPORTED { klog.V(3).Infof("Memory metrics not supported for device %d (unified memory architecture), skipping", index) @@ -317,17 +323,17 @@ func (cc ClusterManagerCollector) collectGPUMemoryMetrics(ch chan<- prometheus.M hostGPUdesc, prometheus.GaugeValue, float64(memory.Used), - fmt.Sprint(index), uuid, deviceName, + nodeName, fmt.Sprint(index), uuid, deviceName, ) sendLegacyMetric(ch, legacyHostGPUdesc, prometheus.GaugeValue, float64(memory.Used), - fmt.Sprint(index), uuid, deviceName, + nodeName, fmt.Sprint(index), uuid, deviceName, ) return nil } -func (cc ClusterManagerCollector) collectGPUUtilizationMetrics(ch chan<- prometheus.Metric, hdev nvml.Device, index int) error { +func (cc ClusterManagerCollector) collectGPUUtilizationMetrics(ch chan<- prometheus.Metric, nodeName string, hdev nvml.Device, index int) error { util, nvret := hdev.GetUtilizationRates() if nvret != nvml.SUCCESS { return fmt.Errorf("nvml GetUtilizationRates err: %s", nvml.ErrorString(nvret)) @@ -349,11 +355,11 @@ func (cc ClusterManagerCollector) collectGPUUtilizationMetrics(ch chan<- prometh hostGPUUtilizationdesc, prometheus.GaugeValue, float64(util.Gpu), - fmt.Sprint(index), uuid, deviceName, + nodeName, fmt.Sprint(index), uuid, deviceName, ) sendLegacyMetric(ch, legacyHostGPUUtilizationdesc, prometheus.GaugeValue, float64(util.Gpu), - fmt.Sprint(index), uuid, deviceName, + nodeName, fmt.Sprint(index), uuid, deviceName, ) return nil diff --git a/cmd/vGPUmonitor/metrics_test.go b/cmd/vGPUmonitor/metrics_test.go index 2c31e4d221..ae45d32879 100644 --- a/cmd/vGPUmonitor/metrics_test.go +++ b/cmd/vGPUmonitor/metrics_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" @@ -68,3 +69,93 @@ func TestDescribeCollectSync(t *testing.T) { t.Errorf("Gather failed (legacy): %v", err) } } + +func TestHostMetricsIncludeNodeLabel(t *testing.T) { + cases := []struct { + name string + legacyMetrics bool + setNodeName bool + wantNodeName string + }{ + {name: "non-legacy/env-set", legacyMetrics: false, setNodeName: true, wantNodeName: "test-node-123"}, + {name: "non-legacy/env-unset", legacyMetrics: false, setNodeName: false, wantNodeName: "unknown"}, + {name: "legacy/env-set", legacyMetrics: true, setNodeName: true, wantNodeName: "test-node-123"}, + {name: "legacy/env-unset", legacyMetrics: true, setNodeName: false, wantNodeName: "unknown"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + reg := prometheus.NewPedanticRegistry() + + if tc.setNodeName { + t.Setenv(util.NodeNameEnvName, tc.wantNodeName) + } else { + // Empty value is equivalent to "unset" for the os.Getenv("") == "" + // fallback check in collectGPUInfo. + t.Setenv(util.NodeNameEnvName, "") + } + + client := fake.NewSimpleClientset() + informerFactory := informers.NewSharedInformerFactory(client, 0) + podLister := informerFactory.Core().V1().Pods().Lister() + + if tc.legacyMetrics { + initLegacyDescriptors() + } + + c := &ClusterManager{ + Zone: "test-zone", + LegacyMetrics: tc.legacyMetrics, + PodLister: podLister, + containerLister: &nvidia.ContainerLister{}, + } + cc := ClusterManagerCollector{ClusterManager: c} + + if err := reg.Register(cc); err != nil { + t.Fatalf("Failed to register: %v", err) + } + + metrics, err := reg.Gather() + if err != nil { + t.Fatalf("Gather failed: %v", err) + } + + // Metrics may not be present if NVML initialization fails (no GPU + // hardware); that's expected in test environments, so absence + // doesn't fail the test, but any metric that IS present must + // carry the correct node_name label. + assertHostMetricNodeLabel(t, metrics, "hami_host_gpu_memory_used_bytes", tc.wantNodeName) + assertHostMetricNodeLabel(t, metrics, "hami_host_gpu_utilization_ratio", tc.wantNodeName) + }) + } +} + +func assertHostMetricNodeLabel(t *testing.T, metrics []*dto.MetricFamily, metricName, wantNodeName string) { + t.Helper() + + for _, mf := range metrics { + if mf.GetName() != metricName { + continue + } + for _, m := range mf.GetMetric() { + labels := m.GetLabel() + if len(labels) != 4 { + t.Errorf("%s has %d labels, expected 4", metricName, len(labels)) + } + + hasNode := false + for _, label := range labels { + if label.GetName() == "node_name" { + hasNode = true + if label.GetValue() != wantNodeName { + t.Errorf("node_name label = %s, want %s", label.GetValue(), wantNodeName) + } + } + } + if !hasNode { + t.Errorf("%s missing 'node_name' label", metricName) + } + } + t.Logf("%s found and validated", metricName) + } +}