From c1afd927b08f3b77e4501c47e782735f0bd0b519 Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Tue, 26 May 2026 13:50:55 -0700 Subject: [PATCH 01/16] docs: update supported platform docs (#207) Signed-off-by: Jingxiang Zhang --- README.md | 17 ++++++++--------- docs/usage.md | 4 ++-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 609db62f..9d0d3e94 100644 --- a/README.md +++ b/README.md @@ -6,9 +6,6 @@ Built on top of [leptonai/gpud](https://github.com/leptonai/gpud) ## Overview -For installation prerequisites and setup details, see: -[Helm Installation](docs/install-helm.md), [DEB Installation](docs/install-deb.md), and [RPM Installation](docs/install-rpm.md). - **What It Monitors:** - GPU Metrics: Power, temperature, clocks, utilization, memory, Xid events - System Metrics: CPU, memory, disk, network usage @@ -20,7 +17,7 @@ For installation prerequisites and setup details, see: - Remote Export: Sends telemetry data to OpenTelemetry-compatible endpoints via OTLP over HTTP **Key Features:** -- Lightweight: <100MB RAM, <1% CPU usage +- Lightweight: <500MB RAM, <1% CPU usage - Non-intrusive: Read-only operations, no system modifications - Production-ready: 24/7 datacenter operation @@ -28,14 +25,16 @@ For installation prerequisites and setup details, see: | OS Family | Supported Versions | Architecture | GPU | |-----------|--------------------|--------------|-----| -| Ubuntu | 22.04, 24.04 | x86_64, ARM64 | Hopper, Blackwell, Rubin | -| RHEL | 8, 9, 10 | x86_64, ARM64 | Hopper, Blackwell, Rubin | -| Rocky Linux | 8, 9, 10 | x86_64, ARM64 | Hopper, Blackwell, Rubin | -| AlmaLinux | 8, 9, 10 | x86_64, ARM64 | Hopper, Blackwell, Rubin | -| Amazon Linux | 2023 | x86_64, ARM64 | Hopper, Blackwell, Rubin | +| Ubuntu | 22.04, 24.04 | x86_64, ARM64 | Ampere, Ada Lovelace, Hopper, Blackwell, Rubin | +| RHEL | 8, 9, 10 | x86_64, ARM64 | Ampere, Ada Lovelace, Hopper, Blackwell, Rubin | +| Rocky Linux | 8, 9, 10 | x86_64, ARM64 | Ampere, Ada Lovelace, Hopper, Blackwell, Rubin | +| AlmaLinux | 8, 9, 10 | x86_64, ARM64 | Ampere, Ada Lovelace, Hopper, Blackwell, Rubin | +| Amazon Linux | 2023 | x86_64, ARM64 | Ampere, Ada Lovelace, Hopper, Blackwell, Rubin | ## Documentation +**Important:** Documentation links are relative to the branch or tag you are viewing. The default GitHub view uses `main`, which may describe unreleased changes. When installing or upgrading a specific agent version, switch to that version's release tag first. + - [Helm Installation](docs/install-helm.md) - Kubernetes (Helm) installation and troubleshooting - [DEB Installation](docs/install-deb.md) - Ubuntu package install, update, and uninstall - [RPM Installation](docs/install-rpm.md) - RHEL/Rocky/Alma/Amazon package install, update, and uninstall diff --git a/docs/usage.md b/docs/usage.md index 0dfc5cc4..630e3b8e 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -92,7 +92,7 @@ Validates the local prerequisites required for installation and enrollment. **What it checks:** - NVIDIA GPU presence -- supported GPU architecture (`Hopper`, `Blackwell`, `Rubin`, `Ampere`, `Ada Lovelace`) +- supported GPU architecture (`Ampere`, `Ada Lovelace`, `Hopper`, `Blackwell`, `Rubin`) - NVIDIA driver major version (`510` or newer) - DCGM HostEngine reachability - DCGM HostEngine minimum version (`4.2.3`) @@ -384,7 +384,7 @@ scrape_configs: ### High resource usage -The agent should use <100MB RAM and <1% CPU. Higher usage might indicate: +The agent should use <500MB RAM and <1% CPU. Higher usage might indicate: - Very frequent collection intervals (check `FLEETINT_COLLECT_INTERVAL`) - Large lookback windows (check `FLEETINT_METRICS_LOOKBACK` and `FLEETINT_EVENTS_LOOKBACK`) From 039c90896ec874b7894ac4622e62b01c10de43dc Mon Sep 17 00:00:00 2001 From: ambermingxin Date: Tue, 26 May 2026 14:07:40 -0700 Subject: [PATCH 02/16] feat: add validation for ng/cz input (#206) Signed-off-by: Amber Xue --- cmd/fleetint/enroll.go | 55 ++++++++- cmd/fleetint/enroll_test.go | 115 ++++++++++++++---- .../pkg/metadata/metadata.go | 25 +--- .../pkg/metadata/metadata_test.go | 20 +++ 4 files changed, 167 insertions(+), 48 deletions(-) diff --git a/cmd/fleetint/enroll.go b/cmd/fleetint/enroll.go index d12ade5e..c07f44e7 100644 --- a/cmd/fleetint/enroll.go +++ b/cmd/fleetint/enroll.go @@ -20,6 +20,7 @@ import ( "fmt" "io" "os" + "regexp" "strings" "time" @@ -35,6 +36,9 @@ var ( ) const defaultEnrollTimeout = time.Minute +const reservedUnassignedName = "Unassigned" + +var enrollMetadataNamePattern = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9 ._-]*$`) // resolveToken returns the SAK token from --token, --token-file, or stdin. func resolveToken(cliContext *cli.Context) (string, error) { @@ -84,9 +88,20 @@ func resolveToken(cliContext *cli.Context) (string, error) { func enrollCommand(cliContext *cli.Context) error { baseEndpoint := cliContext.String("endpoint") force := cliContext.Bool("force") + nodeGroup, err := validatedOptionalMetadataFlagValue(cliContext, "node-group", "Node group") + if err != nil { + return err + } + computeZone, err := validatedOptionalMetadataFlagValue(cliContext, "compute-zone", "Compute zone") + if err != nil { + return err + } + if err := validateReservedPairMetadata(nodeGroup, computeZone); err != nil { + return err + } metadata := &enrollment.EnrollMetadata{ - NodeGroup: optionalFlagValue(cliContext, "node-group"), - ComputeZone: optionalFlagValue(cliContext, "compute-zone"), + NodeGroup: nodeGroup, + ComputeZone: computeZone, } sakToken, err := resolveToken(cliContext) @@ -124,10 +139,40 @@ func enrollCommand(cliContext *cli.Context) error { return performEnrollWorkflow(ctx, baseEndpoint, sakToken, cfg, metadata) } -func optionalFlagValue(cliContext *cli.Context, name string) *string { +func validatedOptionalMetadataFlagValue(cliContext *cli.Context, name, fieldName string) (*string, error) { if !cliContext.IsSet(name) { - return nil + return nil, nil } value := strings.TrimSpace(cliContext.String(name)) - return &value + // Explicit empty means clear/unassign for reserved assignment fields. + if value == "" { + return &value, nil + } + if strings.EqualFold(value, reservedUnassignedName) { + return nil, fmt.Errorf("%s name %q is reserved; use empty value to clear assignment", fieldName, reservedUnassignedName) + } + if len(value) > 255 { + return nil, fmt.Errorf("%s name must be 255 characters or fewer", fieldName) + } + if !enrollMetadataNamePattern.MatchString(value) { + return nil, fmt.Errorf("%s name must start with a letter and contain only letters, numbers, spaces, hyphens, underscores, or periods", fieldName) + } + return &value, nil +} + +func validateReservedPairMetadata(nodeGroup, computeZone *string) error { + nodeGroupSet := nodeGroup != nil + computeZoneSet := computeZone != nil + + if !nodeGroupSet && !computeZoneSet { + return nil + } + if nodeGroupSet && computeZoneSet { + nodeGroupEmpty := *nodeGroup == "" + computeZoneEmpty := *computeZone == "" + if nodeGroupEmpty == computeZoneEmpty { + return nil + } + } + return fmt.Errorf("--node-group and --compute-zone must be both omitted, both empty, or both non-empty") } diff --git a/cmd/fleetint/enroll_test.go b/cmd/fleetint/enroll_test.go index e9d9fa55..96378895 100644 --- a/cmd/fleetint/enroll_test.go +++ b/cmd/fleetint/enroll_test.go @@ -18,14 +18,17 @@ package main import ( "bytes" "context" + "flag" "fmt" "os" "path/filepath" + "strings" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/urfave/cli" "github.com/NVIDIA/fleet-intelligence-agent/internal/config" "github.com/NVIDIA/fleet-intelligence-agent/internal/enrollment" @@ -288,33 +291,85 @@ func TestEnrollCommandPassesOptionalMetadata(t *testing.T) { require.NoError(t, err) } -func TestEnrollCommandTreatsExplicitEmptyMetadataAsClear(t *testing.T) { - useMissingFleetintEnvFile(t) +func TestValidatedOptionalMetadataFlagValueAllowsExplicitEmpty(t *testing.T) { + flagSet := flag.NewFlagSet("enroll", flag.ContinueOnError) + flagSet.String("node-group", "", "") + require.NoError(t, flagSet.Set("node-group", "")) + cliContext := cli.NewContext(cli.NewApp(), flagSet, nil) - originalRunPrecheck := runPrecheck - originalEnrollWorkflow := performEnrollWorkflow - t.Cleanup(func() { - runPrecheck = originalRunPrecheck - performEnrollWorkflow = originalEnrollWorkflow + value, err := validatedOptionalMetadataFlagValue(cliContext, "node-group", "Node group") + require.NoError(t, err) + require.NotNil(t, value) + require.Empty(t, *value) +} + +func TestValidatedOptionalMetadataFlagValueRejectsReservedUnassignedName(t *testing.T) { + flagSet := flag.NewFlagSet("enroll", flag.ContinueOnError) + flagSet.String("node-group", "", "") + require.NoError(t, flagSet.Set("node-group", "unassigned")) + cliContext := cli.NewContext(cli.NewApp(), flagSet, nil) + + _, err := validatedOptionalMetadataFlagValue(cliContext, "node-group", "Node group") + require.ErrorContains(t, err, `Node group name "Unassigned" is reserved; use empty value to clear assignment`) +} + +func TestEnrollCommandRejectsInvalidMetadataCharacters(t *testing.T) { + app := App() + app.Writer = &bytes.Buffer{} + + err := app.Run([]string{ + "fleetint", "enroll", + "--endpoint", "https://example.com", + "--token", "token", + "--compute-zone", "@bad-zone", }) + require.ErrorContains(t, err, "Compute zone name must start with a letter") +} - runPrecheck = func() (precheck.Result, error) { - return precheck.Result{ - Checks: []precheck.Check{ - {Name: "gpu-present", Message: "ok", Passed: true}, - }, - }, nil +func TestEnrollCommandRejectsOverlongMetadataNames(t *testing.T) { + app := App() + app.Writer = &bytes.Buffer{} + longName := strings.Repeat("a", 256) + + err := app.Run([]string{ + "fleetint", "enroll", + "--endpoint", "https://example.com", + "--token", "token", + "--node-group", longName, + }) + require.ErrorContains(t, err, "Node group name must be 255 characters or fewer") +} + +func TestValidateReservedPairMetadata(t *testing.T) { + strPtr := func(v string) *string { return &v } + tests := []struct { + name string + nodeGroup *string + computeZone *string + wantErr bool + }{ + {name: "both omitted", nodeGroup: nil, computeZone: nil, wantErr: false}, + {name: "both empty", nodeGroup: strPtr(""), computeZone: strPtr(""), wantErr: false}, + {name: "both non-empty", nodeGroup: strPtr("ng-a"), computeZone: strPtr("cz-a"), wantErr: false}, + {name: "node-group only", nodeGroup: strPtr("ng-a"), computeZone: nil, wantErr: true}, + {name: "compute-zone only", nodeGroup: nil, computeZone: strPtr("cz-a"), wantErr: true}, + {name: "node-group empty compute-zone non-empty", nodeGroup: strPtr(""), computeZone: strPtr("cz-a"), wantErr: true}, + {name: "node-group non-empty compute-zone empty", nodeGroup: strPtr("ng-a"), computeZone: strPtr(""), wantErr: true}, } - performEnrollWorkflow = func(ctx context.Context, baseEndpoint, sakToken string, cfg *config.Config, metadata *enrollment.EnrollMetadata) error { - require.NotNil(t, metadata) - require.NotNil(t, metadata.NodeGroup) - require.Empty(t, *metadata.NodeGroup) - require.NotNil(t, metadata.ComputeZone) - require.Empty(t, *metadata.ComputeZone) - return nil + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateReservedPairMetadata(tc.nodeGroup, tc.computeZone) + if tc.wantErr { + require.ErrorContains(t, err, "--node-group and --compute-zone must be both omitted, both empty, or both non-empty") + return + } + require.NoError(t, err) + }) } +} +func TestEnrollCommandRejectsMixedReservedPairValues(t *testing.T) { app := App() app.Writer = &bytes.Buffer{} @@ -322,8 +377,22 @@ func TestEnrollCommandTreatsExplicitEmptyMetadataAsClear(t *testing.T) { "fleetint", "enroll", "--endpoint", "https://example.com", "--token", "token", - "--node-group=", - "--compute-zone=", + "--node-group", "", + "--compute-zone", "cz-a", }) - require.NoError(t, err) + require.ErrorContains(t, err, "--node-group and --compute-zone must be both omitted, both empty, or both non-empty") +} + +func TestEnrollCommandRejectsReservedUnassignedName(t *testing.T) { + app := App() + app.Writer = &bytes.Buffer{} + + err := app.Run([]string{ + "fleetint", "enroll", + "--endpoint", "https://example.com", + "--token", "token", + "--node-group", "Unassigned", + "--compute-zone", "cz-a", + }) + require.ErrorContains(t, err, `Node group name "Unassigned" is reserved; use empty value to clear assignment`) } diff --git a/third_party/fleet-intelligence-sdk/pkg/metadata/metadata.go b/third_party/fleet-intelligence-sdk/pkg/metadata/metadata.go index 2ca203ad..da468246 100644 --- a/third_party/fleet-intelligence-sdk/pkg/metadata/metadata.go +++ b/third_party/fleet-intelligence-sdk/pkg/metadata/metadata.go @@ -21,6 +21,9 @@ const ( columnKey = "key" columnValue = "value" ) +const setMetadataUpsertQuery = ` +INSERT INTO gpud_metadata (key, value) VALUES (?, ?) +ON CONFLICT(key) DO UPDATE SET value = excluded.value` // CreateTableMetadata creates the table for the metadata. func CreateTableMetadata(ctx context.Context, dbRW *sql.DB) error { @@ -51,28 +54,10 @@ const ( // SetMetadata sets the value of a metadata entry. // If the metadata entry is not found, it is created. -// If the metadata entry is found and the value is the same, it is not updated. -// If the metadata entry is found and the value is different, it is updated. +// If the metadata entry is found, it is updated in-place. func SetMetadata(ctx context.Context, dbRW *sql.DB, key string, value string) error { - prev, err := ReadMetadata(ctx, dbRW, key) - if err != nil { - return err - } - - if prev == value { - return nil - } - start := time.Now() - if prev == "" { - // the "name" is not in the table, so we need to insert it - _, err = dbRW.ExecContext(ctx, fmt.Sprintf(` -INSERT INTO %s (%s, %s) VALUES (?, ?)`, tableNameGPUdMetadata, columnKey, columnValue), key, value) - } else { - // the "name" is already in the table, so we need to update it - _, err = dbRW.ExecContext(ctx, fmt.Sprintf(` -UPDATE %s SET %s = ? WHERE %s = ?`, tableNameGPUdMetadata, columnValue, columnKey), value, key) - } + _, err := dbRW.ExecContext(ctx, setMetadataUpsertQuery, key, value) pkgmetricsrecorder.RecordSQLiteInsertUpdate(time.Since(start).Seconds()) return err diff --git a/third_party/fleet-intelligence-sdk/pkg/metadata/metadata_test.go b/third_party/fleet-intelligence-sdk/pkg/metadata/metadata_test.go index 4f1bad9b..284aebc7 100644 --- a/third_party/fleet-intelligence-sdk/pkg/metadata/metadata_test.go +++ b/third_party/fleet-intelligence-sdk/pkg/metadata/metadata_test.go @@ -126,3 +126,23 @@ func TestSetAndReadMetadata(t *testing.T) { _, err = ReadMetadata(canceledCtx, dbRO, testKey) assert.Error(t, err) } + +func TestSetMetadata_AllowsTransitionFromEmptyToNonEmpty(t *testing.T) { + t.Parallel() + dbRW, dbRO, cleanup := sqlite.OpenTestDB(t) + defer cleanup() + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + require.NoError(t, CreateTableMetadata(ctx, dbRW)) + + const key = "nodegroup" + require.NoError(t, SetMetadata(ctx, dbRW, key, "group-a")) + require.NoError(t, SetMetadata(ctx, dbRW, key, "")) + require.NoError(t, SetMetadata(ctx, dbRW, key, "group-b")) + + value, err := ReadMetadata(ctx, dbRO, key) + require.NoError(t, err) + assert.Equal(t, "group-b", value) +} From c0100c6d73772fa2d0d719909074225da741aac7 Mon Sep 17 00:00:00 2001 From: ambermingxin Date: Wed, 27 May 2026 11:25:28 -0700 Subject: [PATCH 03/16] chore: disable accelerator-nvidia-processes by default (#209) (#210) Signed-off-by: Amber Xue --- docs/configuration.md | 1 - internal/registry/registry.go | 6 - internal/registry/registry_test.go | 1 + .../accelerator/nvidia/processes/component.go | 309 --------- .../nvidia/processes/component_test.go | 632 ------------------ .../accelerator/nvidia/processes/metrics.go | 32 - .../nvidia/processes/process_env.go | 20 - .../accelerator/nvidia/processes/processes.go | 217 ------ .../nvidia/processes/processes_test.go | 447 ------------- 9 files changed, 1 insertion(+), 1664 deletions(-) delete mode 100644 third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/component.go delete mode 100644 third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/component_test.go delete mode 100644 third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/metrics.go delete mode 100644 third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/process_env.go delete mode 100644 third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/processes.go delete mode 100644 third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/processes_test.go diff --git a/docs/configuration.md b/docs/configuration.md index 8656f873..5078fe20 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -162,7 +162,6 @@ Available component names: - `accelerator-nvidia-nccl` - `accelerator-nvidia-peermem` - `accelerator-nvidia-persistence-mode` -- `accelerator-nvidia-processes` - `accelerator-nvidia-error-sxid` - `accelerator-nvidia-error-xid` diff --git a/internal/registry/registry.go b/internal/registry/registry.go index b3964d93..ccd7b404 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -35,7 +35,6 @@ import ( componentsnvml "github.com/NVIDIA/fleet-intelligence-sdk/components/accelerator/nvidia/nvml" componentspeermem "github.com/NVIDIA/fleet-intelligence-sdk/components/accelerator/nvidia/peermem" componentspersistencemode "github.com/NVIDIA/fleet-intelligence-sdk/components/accelerator/nvidia/persistence-mode" - componentsprocesses "github.com/NVIDIA/fleet-intelligence-sdk/components/accelerator/nvidia/processes" componentssxid "github.com/NVIDIA/fleet-intelligence-sdk/components/accelerator/nvidia/sxid" componentsxid "github.com/NVIDIA/fleet-intelligence-sdk/components/accelerator/nvidia/xid" componentscpu "github.com/NVIDIA/fleet-intelligence-sdk/components/cpu" @@ -78,11 +77,6 @@ func All() []Component { InitFunc: componentspersistencemode.New, EnabledByDefault: true, }, - { - Name: componentsprocesses.Name, - InitFunc: componentsprocesses.New, - EnabledByDefault: true, - }, { Name: componentsnvml.Name, InitFunc: componentsnvml.New, diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go index 0c1a51ad..af7a6817 100644 --- a/internal/registry/registry_test.go +++ b/internal/registry/registry_test.go @@ -138,6 +138,7 @@ func TestAllComponentsHaveUniqueNames(t *testing.T) { func TestRemovedComponentsAbsent(t *testing.T) { removed := []string{ + "accelerator-nvidia-processes", "accelerator-nvidia-fabric-manager", "accelerator-nvidia-gpu-counts", "accelerator-nvidia-nvlink", diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/component.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/component.go deleted file mode 100644 index d55b1169..00000000 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/component.go +++ /dev/null @@ -1,309 +0,0 @@ -// Copyright 2024 Lepton AI Inc -// Source: https://github.com/leptonai/gpud - -// Package processes tracks the NVIDIA per-GPU processes. -package processes - -import ( - "bytes" - "context" - "encoding/json" - "errors" - "fmt" - "sync" - "time" - - "github.com/olekukonko/tablewriter" - "github.com/prometheus/client_golang/prometheus" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - apiv1 "github.com/NVIDIA/fleet-intelligence-sdk/api/v1" - "github.com/NVIDIA/fleet-intelligence-sdk/components" - "github.com/NVIDIA/fleet-intelligence-sdk/pkg/log" - nvidianvml "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia-query/nvml" - "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia-query/nvml/device" - nvmlerrors "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia/errors" -) - -const Name = "accelerator-nvidia-processes" - -const ( - defaultHealthCheckInterval = time.Minute -) - -var _ components.Component = &component{} - -type component struct { - ctx context.Context - cancel context.CancelFunc - - getTimeNowFunc func() time.Time - healthCheckInterval time.Duration - - nvmlInstance nvidianvml.Instance - getProcessesFunc func(uuid string, dev device.Device) (Processes, error) - - lastMu sync.RWMutex - lastCheckResult *checkResult -} - -func New(gpudInstance *components.GPUdInstance) (components.Component, error) { - cctx, ccancel := context.WithCancel(gpudInstance.RootCtx) - - healthCheckInterval := defaultHealthCheckInterval - if gpudInstance.HealthCheckInterval > 0 { - healthCheckInterval = gpudInstance.HealthCheckInterval - } - - c := &component{ - ctx: cctx, - cancel: ccancel, - getTimeNowFunc: func() time.Time { - return time.Now().UTC() - }, - - healthCheckInterval: healthCheckInterval, - - nvmlInstance: gpudInstance.NVMLInstance, - getProcessesFunc: GetProcesses, - } - return c, nil -} - -// InjectFault injects a fault into the processes component by replacing the getProcessesFunc -func (c *component) InjectFault(errMsg string) { - c.getProcessesFunc = func(uuid string, dev device.Device) (Processes, error) { - return Processes{}, errors.New(errMsg) - } -} - -// ClearFault clears any injected faults and restores the original processes checking function -func (c *component) ClearFault() { - c.getProcessesFunc = GetProcesses -} - -func (c *component) Name() string { return Name } - -func (c *component) Tags() []string { - return []string{ - "accelerator", - "gpu", - "nvidia", - Name, - } -} - -func (c *component) IsSupported() bool { - if c.nvmlInstance == nil { - return false - } - return c.nvmlInstance.NVMLExists() && c.nvmlInstance.ProductName() != "" -} - -func (c *component) Start() error { - go func() { - ticker := time.NewTicker(c.healthCheckInterval) - defer ticker.Stop() - - for { - _ = c.Check() - - select { - case <-c.ctx.Done(): - return - case <-ticker.C: - } - } - }() - return nil -} - -func (c *component) LastHealthStates() apiv1.HealthStates { - c.lastMu.RLock() - lastCheckResult := c.lastCheckResult - c.lastMu.RUnlock() - return lastCheckResult.HealthStates() -} - -func (c *component) Events(ctx context.Context, since time.Time) (apiv1.Events, error) { - return nil, nil -} - -func (c *component) Close() error { - log.Logger.Debugw("closing component") - - c.cancel() - - return nil -} - -func (c *component) Check() components.CheckResult { - log.Logger.Infow("checking nvidia gpu processes") - - cr := &checkResult{ - ts: c.getTimeNowFunc(), - } - defer func() { - c.lastMu.Lock() - c.lastCheckResult = cr - c.lastMu.Unlock() - }() - - if c.nvmlInstance == nil { - cr.health = apiv1.HealthStateTypeHealthy - cr.reason = "NVIDIA NVML instance is nil" - return cr - } - if !c.nvmlInstance.NVMLExists() { - cr.health = apiv1.HealthStateTypeHealthy - cr.reason = "NVIDIA NVML library is not loaded" - return cr - } - if c.nvmlInstance.ProductName() == "" { - cr.health = apiv1.HealthStateTypeHealthy - cr.reason = "NVIDIA NVML is loaded but GPU is not detected (missing product name)" - return cr - } - - devs := c.nvmlInstance.Devices() - for uuid, dev := range devs { - procs, err := c.getProcessesFunc(uuid, dev) - if err != nil { - cr.err = err - cr.health = apiv1.HealthStateTypeUnhealthy - cr.reason = "error getting processes" - - if errors.Is(err, nvmlerrors.ErrGPURequiresReset) { - cr.reason = nvmlerrors.ErrGPURequiresReset.Error() - cr.suggestedActions = &apiv1.SuggestedActions{ - Description: nvmlerrors.ErrGPURequiresReset.Error(), - RepairActions: []apiv1.RepairActionType{ - apiv1.RepairActionTypeRebootSystem, - }, - } - } - - if errors.Is(err, nvmlerrors.ErrGPULost) { - cr.reason = nvmlerrors.ErrGPULost.Error() - cr.suggestedActions = &apiv1.SuggestedActions{ - Description: nvmlerrors.ErrGPULost.Error(), - RepairActions: []apiv1.RepairActionType{ - apiv1.RepairActionTypeRebootSystem, - }, - } - } - - log.Logger.Warnw(cr.reason, "error", err) - return cr - } - - cr.Processes = append(cr.Processes, procs) - - metricRunningProcesses.With(prometheus.Labels{"uuid": uuid}).Set(float64(len(procs.RunningProcesses))) - } - - cr.health = apiv1.HealthStateTypeHealthy - cr.reason = fmt.Sprintf("all %d GPU(s) were checked, no process issue found", len(devs)) - - return cr -} - -var _ components.CheckResult = &checkResult{} - -type checkResult struct { - Processes []Processes `json:"processes,omitempty"` - - // timestamp of the last check - ts time.Time - // error from the last check - err error - - // tracks the healthy evaluation result of the last check - health apiv1.HealthStateType - // tracks the suggested actions for the last check - suggestedActions *apiv1.SuggestedActions - // tracks the reason of the last check - reason string -} - -func (cr *checkResult) ComponentName() string { - return Name -} - -func (cr *checkResult) String() string { - if cr == nil { - return "" - } - if len(cr.Processes) == 0 { - return "no data" - } - - buf := bytes.NewBuffer(nil) - table := tablewriter.NewWriter(buf) - table.SetAlignment(tablewriter.ALIGN_CENTER) - table.SetHeader([]string{"GPU UUID", "Processes"}) - for _, proc := range cr.Processes { - table.Append([]string{ - proc.UUID, - fmt.Sprintf("%d", len(proc.RunningProcesses)), - }) - } - table.Render() - - return buf.String() -} - -func (cr *checkResult) Summary() string { - if cr == nil { - return "" - } - return cr.reason -} - -func (cr *checkResult) HealthStateType() apiv1.HealthStateType { - if cr == nil { - return "" - } - return cr.health -} - -func (cr *checkResult) getError() string { - if cr == nil || cr.err == nil { - return "" - } - return cr.err.Error() -} - -func (cr *checkResult) HealthStates() apiv1.HealthStates { - if cr == nil { - return apiv1.HealthStates{ - { - Time: metav1.NewTime(time.Now().UTC()), - Component: Name, - Name: Name, - Health: apiv1.HealthStateTypeHealthy, - Reason: "no data yet", - }, - } - } - - state := apiv1.HealthState{ - Time: metav1.NewTime(cr.ts), - Component: Name, - Name: Name, - Reason: cr.reason, - Error: cr.getError(), - Health: cr.health, - } - - // propagate suggested actions to health state if present - if cr.suggestedActions != nil { - state.SuggestedActions = cr.suggestedActions - } - - if len(cr.Processes) > 0 { - b, _ := json.Marshal(cr) - state.ExtraInfo = map[string]string{"data": string(b)} - } - return apiv1.HealthStates{state} -} diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/component_test.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/component_test.go deleted file mode 100644 index eec6e8d5..00000000 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/component_test.go +++ /dev/null @@ -1,632 +0,0 @@ -// Copyright 2024 Lepton AI Inc -// Source: https://github.com/leptonai/gpud - -package processes - -import ( - "context" - "errors" - "testing" - "time" - - "github.com/NVIDIA/go-nvml/pkg/nvml" - "github.com/NVIDIA/go-nvml/pkg/nvml/mock" - "github.com/prometheus/client_golang/prometheus" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - apiv1 "github.com/NVIDIA/fleet-intelligence-sdk/api/v1" - "github.com/NVIDIA/fleet-intelligence-sdk/components" - "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia-query/nvml/device" - nvmllib "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia-query/nvml/lib" - "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia-query/nvml/testutil" - nvmlerrors "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia/errors" - nvidiaproduct "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia/product" -) - -// Override the metricRunningProcesses for testing -var _ = prometheus.Register(metricRunningProcesses) // This will fail silently if metrics are already registered - -// mockNVMLInstance is a mock implementation of nvidianvml.Instance -type mockNVMLInstance struct { - devicesFunc func() map[string]device.Device - nvmlExists bool -} - -func (m *mockNVMLInstance) Devices() map[string]device.Device { - if m.devicesFunc != nil { - return m.devicesFunc() - } - return nil -} - -// Override other InstanceV2 methods to return empty values -func (m *mockNVMLInstance) NVMLExists() bool { return m.nvmlExists } -func (m *mockNVMLInstance) Library() nvmllib.Library { return nil } -func (m *mockNVMLInstance) ProductName() string { return "Test GPU" } -func (m *mockNVMLInstance) Architecture() string { return "" } -func (m *mockNVMLInstance) Brand() string { return "" } -func (m *mockNVMLInstance) DriverVersion() string { return "1.0" } -func (m *mockNVMLInstance) DriverMajor() int { return 1 } -func (m *mockNVMLInstance) CUDAVersion() string { return "1.0" } -func (m *mockNVMLInstance) FabricManagerSupported() bool { return true } -func (m *mockNVMLInstance) FabricStateSupported() bool { return false } -func (m *mockNVMLInstance) GetMemoryErrorManagementCapabilities() nvidiaproduct.MemoryErrorManagementCapabilities { - return nvidiaproduct.MemoryErrorManagementCapabilities{} -} -func (m *mockNVMLInstance) Shutdown() error { return nil } - -func createMockDevice(uuid string, runningProcs []nvml.ProcessInfo) device.Device { - mockDevice := &mock.Device{ - GetComputeRunningProcessesFunc: func() ([]nvml.ProcessInfo, nvml.Return) { - return runningProcs, nvml.SUCCESS - }, - GetUUIDFunc: func() (string, nvml.Return) { - return uuid, nvml.SUCCESS - }, - GetProcessUtilizationFunc: func(pid uint64) ([]nvml.ProcessUtilizationSample, nvml.Return) { - return []nvml.ProcessUtilizationSample{ - { - Pid: uint32(pid), - TimeStamp: 123456789, - SmUtil: 50, - MemUtil: 30, - EncUtil: 0, - DecUtil: 0, - }, - }, nvml.SUCCESS - }, - } - - return testutil.NewMockDevice(mockDevice, "test-arch", "test-brand", "test-cuda", "test-pci") -} - -func TestNew(t *testing.T) { - ctx := context.Background() - mockInstance := &mockNVMLInstance{nvmlExists: true} - - // Create a mock GPUdInstance with the required fields - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - - assert.NoError(t, err) - assert.NotNil(t, comp) - assert.Equal(t, Name, comp.Name()) -} - -func TestComponentName(t *testing.T) { - ctx := context.Background() - mockInstance := &mockNVMLInstance{nvmlExists: true} - - // Create a mock GPUdInstance with the required fields - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - - assert.NoError(t, err) - assert.Equal(t, Name, comp.Name()) -} - -func TestTags(t *testing.T) { - ctx := context.Background() - mockInstance := &mockNVMLInstance{nvmlExists: true} - - // Create a mock GPUdInstance with the required fields - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - expectedTags := []string{ - "accelerator", - "gpu", - "nvidia", - Name, - } - - tags := comp.Tags() - assert.Equal(t, expectedTags, tags, "Component tags should match expected values") - assert.Len(t, tags, 4, "Component should return exactly 4 tags") -} - -func TestStartAndClose(t *testing.T) { - ctx := context.Background() - mockInstance := &mockNVMLInstance{nvmlExists: true} - - // Create a mock GPUdInstance with the required fields - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - // Test Start method - err = comp.Start() - assert.NoError(t, err) - - // Test Close method - err = comp.Close() - assert.NoError(t, err) -} - -func TestEvents(t *testing.T) { - ctx := context.Background() - mockInstance := &mockNVMLInstance{nvmlExists: true} - - // Create a mock GPUdInstance with the required fields - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - events, err := comp.Events(ctx, time.Now()) - assert.NoError(t, err) - assert.Empty(t, events) -} - -func TestCheck(t *testing.T) { - ctx := context.Background() - - // Create mock devices with running processes - mockDev1 := createMockDevice("gpu-uuid-1", []nvml.ProcessInfo{ - {Pid: 1234, UsedGpuMemory: 100000000}, - }) - - mockDevices := map[string]device.Device{ - "gpu-uuid-1": mockDev1, - } - - mockInstance := &mockNVMLInstance{ - nvmlExists: true, - devicesFunc: func() map[string]device.Device { - return mockDevices - }, - } - - // Create a mock GPUdInstance with the required fields - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - // Cast to component to access internal Check method - c := comp.(*component) - - // Set the getProcessesFunc for testing - c.getProcessesFunc = func(uuid string, dev device.Device) (Processes, error) { - return Processes{ - UUID: uuid, - RunningProcesses: []Process{ - { - PID: 1234, - GPUUsedMemoryBytes: 100000000, - }, - }, - }, nil - } - - // Run Check - result := c.Check() - - // Verify result is correct - data, ok := result.(*checkResult) - require.True(t, ok) - assert.Equal(t, apiv1.HealthStateTypeHealthy, data.health) - assert.Equal(t, 1, len(data.Processes)) -} - -func TestCheckError(t *testing.T) { - ctx := context.Background() - - // Create mock devices with no processes - mockDev1 := createMockDevice("gpu-uuid-1", nil) - - mockDevices := map[string]device.Device{ - "gpu-uuid-1": mockDev1, - } - - mockInstance := &mockNVMLInstance{ - nvmlExists: true, - devicesFunc: func() map[string]device.Device { - return mockDevices - }, - } - - // Create a mock GPUdInstance with the required fields - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - // Cast to component to access internal methods - c := comp.(*component) - - // Create a getProcessesFunc that returns an error - testErr := errors.New("test error") - c.getProcessesFunc = func(uuid string, dev device.Device) (Processes, error) { - return Processes{}, testErr - } - - // Run Check - result := c.Check() - - // Verify result is correct - data, ok := result.(*checkResult) - require.True(t, ok) - assert.Equal(t, apiv1.HealthStateTypeUnhealthy, data.health) - assert.Equal(t, testErr, data.err) -} - -func TestLastHealthStates(t *testing.T) { - ctx := context.Background() - mockInstance := &mockNVMLInstance{nvmlExists: true} - - // Create a mock GPUdInstance with the required fields - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - // Cast to component to access internal lastCheckResult - c := comp.(*component) - - // At this point, lastCheckResult should be nil - states := c.LastHealthStates() - assert.Equal(t, 1, len(states)) - - // Default values for nil data - assert.Equal(t, Name, states[0].Name) - assert.Equal(t, apiv1.HealthStateTypeHealthy, states[0].Health) - assert.Equal(t, "no data yet", states[0].Reason) -} - -func TestDataGetLastHealthStates(t *testing.T) { - // Test healthy data - healthyData := &checkResult{ - Processes: []Processes{ - { - UUID: "gpu-uuid-1", - RunningProcesses: []Process{ - {PID: 1234}, - }, - }, - }, - health: apiv1.HealthStateTypeHealthy, - reason: "all GPUs healthy", - } - - states := healthyData.HealthStates() - assert.Equal(t, 1, len(states)) - assert.Equal(t, apiv1.HealthStateTypeHealthy, states[0].Health) - - // Test unhealthy data - testErr := errors.New("test error") - unhealthyData := &checkResult{ - err: testErr, - health: apiv1.HealthStateTypeUnhealthy, - reason: "GPU issue detected", - } - - states = unhealthyData.HealthStates() - assert.Equal(t, 1, len(states)) - assert.Equal(t, apiv1.HealthStateTypeUnhealthy, states[0].Health) - assert.Equal(t, testErr.Error(), states[0].Error) -} - -func TestDataGetError(t *testing.T) { - // Test nil data - var nilData *checkResult - assert.Equal(t, "", nilData.getError()) - - // Test nil error - noErrorData := &checkResult{} - assert.Equal(t, "", noErrorData.getError()) - - // Test with error - testErr := errors.New("test error") - errorData := &checkResult{err: testErr} - assert.Equal(t, testErr.Error(), errorData.getError()) -} - -// Test Data.String() method -func TestDataString(t *testing.T) { - // Test nil data - var nilData *checkResult - assert.Equal(t, "", nilData.String()) - - // Test empty data - emptyData := &checkResult{} - assert.Equal(t, "no data", emptyData.String()) - - // Test data with processes - dataWithProcesses := &checkResult{ - Processes: []Processes{ - { - UUID: "gpu-uuid-1", - RunningProcesses: []Process{ - {PID: 1234}, - {PID: 5678}, - }, - }, - }, - } - - result := dataWithProcesses.String() - assert.Contains(t, result, "gpu-uuid-1") - assert.Contains(t, result, "2") // Number of processes -} - -// Test Data.Summary() method -func TestDataSummary(t *testing.T) { - // Test nil data - var nilData *checkResult - assert.Equal(t, "", nilData.Summary()) - - // Test data with reason - dataWithReason := &checkResult{ - reason: "test reason", - } - assert.Equal(t, "test reason", dataWithReason.Summary()) -} - -// Test Data.HealthState() method -func TestDataHealthState(t *testing.T) { - // Test nil data - var nilData *checkResult - assert.Equal(t, apiv1.HealthStateType(""), nilData.HealthStateType()) - - // Test data with health state - dataWithHealth := &checkResult{ - health: apiv1.HealthStateTypeHealthy, - } - assert.Equal(t, apiv1.HealthStateTypeHealthy, dataWithHealth.HealthStateType()) - - // Test data with unhealthy state - dataUnhealthy := &checkResult{ - health: apiv1.HealthStateTypeUnhealthy, - } - assert.Equal(t, apiv1.HealthStateTypeUnhealthy, dataUnhealthy.HealthStateType()) -} - -// Test Check edge cases -func TestCheckEdgeCases(t *testing.T) { - ctx := context.Background() - - t.Run("nil NVML instance", func(t *testing.T) { - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: nil, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - c := comp.(*component) - result := c.Check() - - data, ok := result.(*checkResult) - require.True(t, ok) - assert.Equal(t, apiv1.HealthStateTypeHealthy, data.health) - assert.Equal(t, "NVIDIA NVML instance is nil", data.reason) - }) - - t.Run("NVML not loaded", func(t *testing.T) { - mockInstance := &mockNVMLInstance{ - nvmlExists: false, - } - - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - c := comp.(*component) - result := c.Check() - - data, ok := result.(*checkResult) - require.True(t, ok) - assert.Equal(t, apiv1.HealthStateTypeHealthy, data.health) - assert.Equal(t, "NVIDIA NVML library is not loaded", data.reason) - }) - - t.Run("multiple devices", func(t *testing.T) { - // Create mock devices with running processes - mockDev1 := createMockDevice("gpu-uuid-1", []nvml.ProcessInfo{ - {Pid: 1234, UsedGpuMemory: 100000000}, - }) - mockDev2 := createMockDevice("gpu-uuid-2", []nvml.ProcessInfo{ - {Pid: 5678, UsedGpuMemory: 200000000}, - {Pid: 9012, UsedGpuMemory: 300000000}, - }) - - mockDevices := map[string]device.Device{ - "gpu-uuid-1": mockDev1, - "gpu-uuid-2": mockDev2, - } - - mockInstance := &mockNVMLInstance{ - nvmlExists: true, - devicesFunc: func() map[string]device.Device { - return mockDevices - }, - } - - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - c := comp.(*component) - - // Set up the getProcessesFunc to match device UUID - c.getProcessesFunc = func(uuid string, dev device.Device) (Processes, error) { - if uuid == "gpu-uuid-1" { - return Processes{ - UUID: uuid, - RunningProcesses: []Process{ - {PID: 1234, GPUUsedMemoryBytes: 100000000}, - }, - }, nil - } else { - return Processes{ - UUID: uuid, - RunningProcesses: []Process{ - {PID: 5678, GPUUsedMemoryBytes: 200000000}, - {PID: 9012, GPUUsedMemoryBytes: 300000000}, - }, - }, nil - } - } - - result := c.Check() - - data, ok := result.(*checkResult) - require.True(t, ok) - assert.Equal(t, apiv1.HealthStateTypeHealthy, data.health) - assert.Equal(t, 2, len(data.Processes)) - assert.Equal(t, "all 2 GPU(s) were checked, no process issue found", data.reason) - }) -} - -func TestCheck_GPULostError(t *testing.T) { - ctx := context.Background() - - // Create mock devices with no processes - mockDev1 := createMockDevice("gpu-uuid-1", nil) - - mockDevices := map[string]device.Device{ - "gpu-uuid-1": mockDev1, - } - - mockInstance := &mockNVMLInstance{ - nvmlExists: true, - devicesFunc: func() map[string]device.Device { - return mockDevices - }, - } - - // Create a mock GPUdInstance with the required fields - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - // Cast to component to access internal methods - c := comp.(*component) - - // Create a getProcessesFunc that returns GPU lost error - c.getProcessesFunc = func(uuid string, dev device.Device) (Processes, error) { - return Processes{}, nvmlerrors.ErrGPULost - } - - // Run Check - result := c.Check() - - // Verify result is correct - data, ok := result.(*checkResult) - require.True(t, ok) - assert.Equal(t, apiv1.HealthStateTypeUnhealthy, data.health) - assert.True(t, errors.Is(data.err, nvmlerrors.ErrGPULost), "error should be nvmlerrors.ErrGPULost") - assert.Equal(t, nvmlerrors.ErrGPULost.Error(), data.reason) - - // Verify suggested actions for GPU lost case - if assert.NotNil(t, data.suggestedActions) { - assert.Equal(t, nvmlerrors.ErrGPULost.Error(), data.suggestedActions.Description) - assert.Contains(t, data.suggestedActions.RepairActions, apiv1.RepairActionTypeRebootSystem) - } - - // Verify suggested actions propagates to health state output - states := c.LastHealthStates() - require.Len(t, states, 1) - assert.NotNil(t, states[0].SuggestedActions) - assert.Contains(t, states[0].SuggestedActions.RepairActions, apiv1.RepairActionTypeRebootSystem) -} - -func TestCheck_GPURequiresResetSuggestedActions(t *testing.T) { - ctx := context.Background() - - // Create mock devices with no processes - mockDev1 := createMockDevice("gpu-uuid-1", nil) - - mockDevices := map[string]device.Device{ - "gpu-uuid-1": mockDev1, - } - - mockInstance := &mockNVMLInstance{ - nvmlExists: true, - devicesFunc: func() map[string]device.Device { - return mockDevices - }, - } - - // Create a mock GPUdInstance with the required fields - gpudInstance := &components.GPUdInstance{ - RootCtx: ctx, - NVMLInstance: mockInstance, - } - - comp, err := New(gpudInstance) - assert.NoError(t, err) - - // Cast to component to access internal methods - c := comp.(*component) - - // Create a getProcessesFunc that returns GPU requires reset error - c.getProcessesFunc = func(uuid string, dev device.Device) (Processes, error) { - return Processes{}, nvmlerrors.ErrGPURequiresReset - } - - // Run Check - result := c.Check() - - // Verify check result carries suggested actions - data, ok := result.(*checkResult) - require.True(t, ok, "result should be of type *checkResult") - require.NotNil(t, data) - assert.Equal(t, apiv1.HealthStateTypeUnhealthy, data.health) - assert.True(t, errors.Is(data.err, nvmlerrors.ErrGPURequiresReset)) - assert.Equal(t, "GPU requires reset", data.reason) - if assert.NotNil(t, data.suggestedActions) { - assert.Equal(t, "GPU requires reset", data.suggestedActions.Description) - assert.Contains(t, data.suggestedActions.RepairActions, apiv1.RepairActionTypeRebootSystem) - } - - // Verify suggested actions propagates to health state output - states := c.LastHealthStates() - require.Len(t, states, 1) - assert.NotNil(t, states[0].SuggestedActions) - assert.Contains(t, states[0].SuggestedActions.RepairActions, apiv1.RepairActionTypeRebootSystem) -} diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/metrics.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/metrics.go deleted file mode 100644 index a52b134e..00000000 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/metrics.go +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright 2024 Lepton AI Inc -// Source: https://github.com/leptonai/gpud - -package processes - -import ( - pkgmetrics "github.com/NVIDIA/fleet-intelligence-sdk/pkg/metrics" - - "github.com/prometheus/client_golang/prometheus" -) - -const SubSystem = "accelerator_nvidia_processes" - -var ( - componentLabel = prometheus.Labels{ - pkgmetrics.MetricComponentLabelKey: Name, - } - - metricRunningProcesses = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Namespace: "", - Subsystem: SubSystem, - Name: "running_total", - Help: "tracks the current per-GPU process counter", - }, - []string{pkgmetrics.MetricComponentLabelKey, "uuid"}, // label is GPU ID - ).MustCurryWith(componentLabel) -) - -func init() { - pkgmetrics.MustRegister(metricRunningProcesses) -} diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/process_env.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/process_env.go deleted file mode 100644 index be16543c..00000000 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/process_env.go +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright 2024 Lepton AI Inc -// Source: https://github.com/leptonai/gpud - -package processes - -// ports "DCGM_FR_BAD_CUDA_ENV"; The environment has variables that hurt CUDA -// This is derived from "DCGM_FR_BAD_CUDA_ENV" in DCGM. -// ref. https://github.com/NVIDIA/DCGM/blob/903d745504f50153be8293f8566346f9de3b3c93/nvvs/plugin_src/software/Software.cpp#L839-L876 -var BAD_CUDA_ENV_KEYS = map[string]string{ - "NSIGHT_CUDA_DEBUGGER": "Setting NSIGHT_CUDA_DEBUGGER=1 can degrade the performance of an application, since the debugger is made resident. See https://docs.nvidia.com/nsight-visual-studio-edition/3.2/Content/Attach_CUDA_to_Process.htm.", - "CUDA_INJECTION32_PATH": "Captures information about CUDA execution trace. See https://docs.nvidia.com/nsight-systems/2020.3/tracing/index.html.", - "CUDA_INJECTION64_PATH": "Captures information about CUDA execution trace. See https://docs.nvidia.com/nsight-systems/2020.3/tracing/index.html.", - "CUDA_AUTO_BOOST": "Automatically selects the highest possible clock rate allowed by the thermal and power budget. Independent of the global default setting the autoboost behavior can be overridden by setting the environment variable CUDA_AUTO_BOOST. Set CUDA_AUTO_BOOST=0 to disable frequency throttling/boosting. You may run 'nvidia-smi --auto-boost-default=0' to disable autoboost by default. See https://developer.nvidia.com/blog/increase-performance-gpu-boost-k80-autoboost/.", - "CUDA_ENABLE_COREDUMP_ON_EXCEPTION": "Enables GPU core dumps.", - "CUDA_COREDUMP_FILE": "Enables GPU core dumps.", - "CUDA_DEVICE_WAITS_ON_EXCEPTION": "CUDA kernel will pause when an exception occurs. This is only useful for debugging.", - "CUDA_PROFILE": "Enables CUDA profiling.", - "COMPUTE_PROFILE": "Enables compute profiling.", - "OPENCL_PROFILE": "Enables OpenCL profiling.", -} diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/processes.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/processes.go deleted file mode 100644 index 9e670426..00000000 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/processes.go +++ /dev/null @@ -1,217 +0,0 @@ -// Copyright 2024 Lepton AI Inc -// Source: https://github.com/leptonai/gpud - -package processes - -import ( - "errors" - "fmt" - "sort" - "strings" - - "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia-query/nvml/device" - "github.com/NVIDIA/go-nvml/pkg/nvml" - "github.com/dustin/go-humanize" - "github.com/shirou/gopsutil/v4/process" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/NVIDIA/fleet-intelligence-sdk/pkg/log" - nvmlerrors "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia/errors" -) - -// Processes represents the current clock events from the nvmlDeviceGetCurrentClocksEventReasons API. -// ref. https://docs.nvidia.com/deploy/nvml-api/group__nvmlDeviceQueries.html#group__nvmlDeviceQueries_1g7e505374454a0d4fc7339b6c885656d6 -// ref. https://docs.nvidia.com/deploy/nvml-api/group__nvmlDeviceQueries.html#group__nvmlDeviceQueries_1ga115e41a14b747cb334a0e7b49ae1941 -// ref. https://docs.nvidia.com/deploy/nvml-api/group__nvmlClocksEventReasons.html#group__nvmlClocksEventReasons -type Processes struct { - // Represents the GPU UUID. - UUID string `json:"uuid"` - - // BusID is the GPU bus ID from the nvml API. - // e.g., "0000:0f:00.0" - BusID string `json:"bus_id"` - - // A list of running processes. - RunningProcesses []Process `json:"running_processes"` - - // GetComputeRunningProcessesSupported is true if the device supports the getComputeRunningProcesses API. - GetComputeRunningProcessesSupported bool `json:"get_compute_running_processes_supported"` - - // GetProcessUtilizationSupported is true if the device supports the getProcessUtilization API. - GetProcessUtilizationSupported bool `json:"get_process_utilization_supported"` -} - -type Process struct { - PID uint32 `json:"pid"` - Status []string `json:"status,omitempty"` - - // ZombieStatus is set to true if the process is defunct - // (terminated but not reaped by its parent). - ZombieStatus bool `json:"zombie_status,omitempty"` - - // BadEnvVarsForCUDA is a map of environment variables that are known to hurt CUDA - // that is set for this specific process. - // Empty if there is no bad environment variable found for this process. - // This implements "DCGM_FR_BAD_CUDA_ENV" logic in DCGM. - BadEnvVarsForCUDA map[string]string `json:"bad_env_vars_for_cuda,omitempty"` - - CmdArgs []string `json:"cmd_args,omitempty"` - CreateTime metav1.Time `json:"create_time,omitempty"` - GPUUsedPercent uint32 `json:"gpu_used_percent,omitempty"` - GPUUsedMemoryBytes uint64 `json:"gpu_used_memory_bytes,omitempty"` - GPUUsedMemoryBytesHumanized string `json:"gpu_used_memory_bytes_humanized,omitempty"` -} - -func GetProcesses(uuid string, dev device.Device) (Processes, error) { - return getProcesses(uuid, dev, process.NewProcess) -} - -func getProcesses(uuid string, dev device.Device, newProcessFunc func(pid int32) (*process.Process, error)) (Processes, error) { - procs := Processes{ - UUID: uuid, - BusID: dev.PCIBusID(), - - GetComputeRunningProcessesSupported: true, - GetProcessUtilizationSupported: true, - } - - // ref. https://docs.nvidia.com/deploy/nvml-api/group__nvmlDeviceQueries.html#group__nvmlDeviceQueries_1g34afcba3d32066db223265aa022a6b80 - computeProcs, ret := dev.GetComputeRunningProcesses() - if nvmlerrors.IsNotSupportError(ret) { - procs.GetComputeRunningProcessesSupported = false - return procs, nil - } - if nvmlerrors.IsGPULostError(ret) { - return procs, nvmlerrors.ErrGPULost - } - if nvmlerrors.IsGPURequiresReset(ret) { - return procs, nvmlerrors.ErrGPURequiresReset - } - if ret != nvml.SUCCESS { // not a "not supported" error, not a success return, thus return an error here - return procs, fmt.Errorf("failed to get device compute processes: %v", nvml.ErrorString(ret)) - } - - for _, proc := range computeProcs { - procObject, err := newProcessFunc(int32(proc.Pid)) - if err != nil { - // ref. process does not exist - if errors.Is(err, process.ErrorProcessNotRunning) { - log.Logger.Debugw("process not running -- skipping", "pid", proc.Pid, "error", err) - continue - } - if nvmlerrors.IsNoSuchFileOrDirectoryError(err) { - log.Logger.Debugw("process not running -- skipping", "pid", proc.Pid, "error", err) - continue - } - return Processes{}, fmt.Errorf("failed to get process %d: %v", proc.Pid, err) - } - - args, err := procObject.CmdlineSlice() - if err != nil { - if nvmlerrors.IsNoSuchFileOrDirectoryError(err) { - log.Logger.Debugw("process not running -- skipping", "pid", proc.Pid, "error", err) - continue - } - return Processes{}, fmt.Errorf("failed to get process %d args: %v", proc.Pid, err) - } - createTimeUnixMS, err := procObject.CreateTime() - if err != nil { - if nvmlerrors.IsNoSuchFileOrDirectoryError(err) { - log.Logger.Debugw("process not running -- skipping", "pid", proc.Pid, "error", err) - continue - } - return Processes{}, fmt.Errorf("failed to get process %d create time: %v", proc.Pid, err) - } - createTime := metav1.Unix(createTimeUnixMS/1000, 0) - - // ref. https://docs.nvidia.com/deploy/nvml-api/group__nvmlDeviceQueries.html#group__nvmlDeviceQueries_1gb0ea5236f5e69e63bf53684a11c233bd - memUtil := uint32(0) - utils, ret := dev.GetProcessUtilization(uint64(proc.Pid)) - if nvmlerrors.IsNotSupportError(ret) { - procs.GetProcessUtilizationSupported = false - return procs, nil - } - if nvmlerrors.IsNotFoundError(ret) { - continue - } - if nvmlerrors.IsGPULostError(ret) { - return procs, nvmlerrors.ErrGPULost - } - if nvmlerrors.IsGPURequiresReset(ret) { - return procs, nvmlerrors.ErrGPURequiresReset - } - if ret != nvml.SUCCESS { // not a "not supported" error, not a success return, thus return an error here - return procs, fmt.Errorf("failed to get process %d utilization (%v)", proc.Pid, nvml.ErrorString(ret)) - } - - if len(utils) > 0 { - // sort by last seen timestamp, so that first is the latest - sort.Slice(utils, func(i, j int) bool { - return utils[i].TimeStamp > utils[j].TimeStamp - }) - - // ref. https://docs.nvidia.com/deploy/nvml-api/structnvmlProcessUtilizationSample__t.html#structnvmlProcessUtilizationSample__t - memUtil = utils[0].MemUtil - } - - status, err := procObject.Status() - if err != nil { - if nvmlerrors.IsNoSuchFileOrDirectoryError(err) { - continue - } - return procs, fmt.Errorf("failed to get process %d status: %v", proc.Pid, err) - } - isZombie := false - for _, s := range status { - if s == process.Zombie { - isZombie = true - break - } - } - - envs, err := procObject.Environ() - if err != nil { - if nvmlerrors.IsNoSuchFileOrDirectoryError(err) { - continue - } - return procs, fmt.Errorf("failed to get process %d environ: %v", proc.Pid, err) - } - - badEnvVars := make(map[string]string) - for _, env := range envs { - parts := strings.SplitN(env, "=", 2) - if len(parts) == 2 { - key, value := parts[0], parts[1] - - // implementing "DCGM_FR_BAD_CUDA_ENV" - if _, ok := BAD_CUDA_ENV_KEYS[key]; ok { - badEnvVars[key] = value - } - } - } - if len(badEnvVars) == 0 { - badEnvVars = nil - } - - procs.RunningProcesses = append(procs.RunningProcesses, Process{ - PID: proc.Pid, - - Status: status, - ZombieStatus: isZombie, - - BadEnvVarsForCUDA: badEnvVars, - - CmdArgs: args, - CreateTime: createTime, - - GPUUsedPercent: memUtil, - - // "Amount of used GPU memory in bytes." - // ref. https://docs.nvidia.com/deploy/nvml-api/structnvmlProcessInfo__t.html#structnvmlProcessInfo__t - GPUUsedMemoryBytes: proc.UsedGpuMemory, - GPUUsedMemoryBytesHumanized: humanize.IBytes(proc.UsedGpuMemory), - }) - } - - return procs, nil -} diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/processes_test.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/processes_test.go deleted file mode 100644 index ba4ee248..00000000 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/processes/processes_test.go +++ /dev/null @@ -1,447 +0,0 @@ -// Copyright 2024 Lepton AI Inc -// Source: https://github.com/leptonai/gpud - -package processes - -import ( - "encoding/json" - "errors" - "testing" - - "github.com/NVIDIA/go-nvml/pkg/nvml" - "github.com/NVIDIA/go-nvml/pkg/nvml/mock" - "github.com/shirou/gopsutil/v4/process" - "github.com/stretchr/testify/assert" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia-query/nvml/device" - "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia-query/nvml/testutil" - nvmlerrors "github.com/NVIDIA/fleet-intelligence-sdk/pkg/nvidia/errors" -) - -func TestGetProcessesWithNilDevice(t *testing.T) { - var nilDevice device.Device = nil - testUUID := "GPU-NILTEST" - - // We expect the function to panic with a nil device - assert.Panics(t, func() { - // Call the function with a nil device - _, _ = GetProcesses(testUUID, nilDevice) - }, "Expected panic when calling GetProcesses with nil device") -} - -// TestGetProcessesWithGPULostError tests that the function properly handles GPU lost errors -func TestGetProcessesWithGPULostError(t *testing.T) { - testUUID := "GPU-LOST" - - // Create a mock device that returns GPU_IS_LOST error - mockDevice := &testutil.MockDevice{ - Device: &mock.Device{ - GetComputeRunningProcessesFunc: func() ([]nvml.ProcessInfo, nvml.Return) { - return nil, nvml.ERROR_GPU_IS_LOST - }, - }, - } - - // Call the function - _, err := GetProcesses(testUUID, mockDevice) - - // Check that we get a GPU lost error - assert.Error(t, err) - assert.True(t, errors.Is(err, nvmlerrors.ErrGPULost), "Expected GPU lost error") -} - -// TestGetProcessesWithGPURequiresResetError tests handling of "GPU requires reset" errors -func TestGetProcessesWithGPURequiresResetError(t *testing.T) { - testUUID := "GPU-RESET" - - // Override nvml.ErrorString to simulate the message - originalErrorString := nvml.ErrorString - nvml.ErrorString = func(ret nvml.Return) string { - if ret == nvml.Return(4242) { - return "GPU requires reset" - } - return originalErrorString(ret) - } - defer func() { nvml.ErrorString = originalErrorString }() - - // Create a mock device that returns a custom error code - mockDevice := &testutil.MockDevice{ - Device: &mock.Device{ - GetComputeRunningProcessesFunc: func() ([]nvml.ProcessInfo, nvml.Return) { - return nil, nvml.Return(4242) - }, - }, - } - - // Call the function - _, err := GetProcesses(testUUID, mockDevice) - - // Check that we get a GPU requires reset error - assert.Error(t, err) - assert.True(t, errors.Is(err, nvmlerrors.ErrGPURequiresReset), "Expected GPU requires reset error") -} - -// TestGetProcesses_ProcessUtilizationGPURequiresReset tests reset error handling in utilization path -// Note: utilization path reset handling is exercised indirectly by other components. - -func TestProcessesJSON(t *testing.T) { - procs := Processes{ - UUID: "GPU-12345678", - RunningProcesses: []Process{ - { - PID: 1234, - Status: []string{"S", "R"}, - ZombieStatus: false, - CmdArgs: []string{"/usr/bin/python", "train.py"}, - CreateTime: metav1.Now(), - GPUUsedPercent: 75, - GPUUsedMemoryBytes: 1024 * 1024 * 100, - GPUUsedMemoryBytesHumanized: "100 MB", - }, - }, - GetComputeRunningProcessesSupported: true, - GetProcessUtilizationSupported: true, - } - - // Test JSON marshaling - jsonData, err := json.Marshal(procs) - assert.NoError(t, err) - assert.Contains(t, string(jsonData), "GPU-12345678") - assert.Contains(t, string(jsonData), "1234") - assert.Contains(t, string(jsonData), "train.py") -} - -// TestGetProcesses_NoSuchFileOrDirectoryError_NewProcess tests that when newProcessFunc -// fails with a "no such file or directory" error, the process is skipped -func TestGetProcesses_NoSuchFileOrDirectoryError_NewProcess(t *testing.T) { - testUUID := "GPU-TEST" - noSuchFileErr := errors.New("no such file or directory") - - // Create a mock device that returns one process - mockDevice := &testutil.MockDevice{ - Device: &mock.Device{ - GetComputeRunningProcessesFunc: func() ([]nvml.ProcessInfo, nvml.Return) { - return []nvml.ProcessInfo{ - {Pid: 1234, UsedGpuMemory: 1024}, - }, nvml.SUCCESS - }, - GetProcessUtilizationFunc: func(pid uint64) ([]nvml.ProcessUtilizationSample, nvml.Return) { - return []nvml.ProcessUtilizationSample{}, nvml.SUCCESS - }, - }, - } - - // Mock newProcessFunc that returns a "no such file or directory" error - mockNewProcessFunc := func(int32) (*process.Process, error) { - return nil, noSuchFileErr - } - - // Call the function - result, err := getProcesses(testUUID, mockDevice, func(pid int32) (*process.Process, error) { - return mockNewProcessFunc(pid) - }) - - // Should succeed but skip the process - assert.NoError(t, err) - assert.Equal(t, testUUID, result.UUID) - assert.Len(t, result.RunningProcesses, 0) // Process should be skipped - assert.True(t, result.GetComputeRunningProcessesSupported) -} - -// TestGetProcesses_NoSuchFileOrDirectoryError_CmdlineSlice tests that when CmdlineSlice -// fails with a "no such file or directory" error, the process is skipped -func TestGetProcesses_NoSuchFileOrDirectoryError_CmdlineSlice(t *testing.T) { - testUUID := "GPU-TEST" - noSuchFileErr := errors.New("not found") // Matches nvmlerrors.IsNoSuchFileOrDirectoryError - - // Create a mock device that returns one process - mockDevice := &testutil.MockDevice{ - Device: &mock.Device{ - GetComputeRunningProcessesFunc: func() ([]nvml.ProcessInfo, nvml.Return) { - return []nvml.ProcessInfo{ - {Pid: 1234, UsedGpuMemory: 1024}, - }, nvml.SUCCESS - }, - GetProcessUtilizationFunc: func(pid uint64) ([]nvml.ProcessUtilizationSample, nvml.Return) { - return []nvml.ProcessUtilizationSample{}, nvml.SUCCESS - }, - }, - } - - // Create a custom test version that simulates CmdlineSlice failure - testGetProcesses := func(uuid string, dev device.Device) (Processes, error) { - procs := Processes{ - UUID: uuid, - GetComputeRunningProcessesSupported: true, - GetProcessUtilizationSupported: true, - } - - computeProcs, ret := dev.GetComputeRunningProcesses() - if ret != nvml.SUCCESS { - return procs, nil - } - - for range computeProcs { - // Simulate the CmdlineSlice failure - if nvmlerrors.IsNoSuchFileOrDirectoryError(noSuchFileErr) { - continue // Process should be skipped - } - } - - return procs, nil - } - - // Call the test function - result, err := testGetProcesses(testUUID, mockDevice) - - // Should succeed but skip the process - assert.NoError(t, err) - assert.Equal(t, testUUID, result.UUID) - assert.Len(t, result.RunningProcesses, 0) // Process should be skipped -} - -// TestGetProcesses_NoSuchFileOrDirectoryError_CreateTime tests that when CreateTime -// fails with a "no such file or directory" error, the process is skipped -func TestGetProcesses_NoSuchFileOrDirectoryError_CreateTime(t *testing.T) { - testUUID := "GPU-TEST" - noSuchFileErr := errors.New("No such file or directory") - - // Create a mock device - mockDevice := &testutil.MockDevice{ - Device: &mock.Device{ - GetComputeRunningProcessesFunc: func() ([]nvml.ProcessInfo, nvml.Return) { - return []nvml.ProcessInfo{ - {Pid: 1234, UsedGpuMemory: 1024}, - }, nvml.SUCCESS - }, - GetProcessUtilizationFunc: func(pid uint64) ([]nvml.ProcessUtilizationSample, nvml.Return) { - return []nvml.ProcessUtilizationSample{}, nvml.SUCCESS - }, - }, - } - - // Test the CreateTime failure scenario - testGetProcesses := func(uuid string, dev device.Device) (Processes, error) { - procs := Processes{ - UUID: uuid, - GetComputeRunningProcessesSupported: true, - GetProcessUtilizationSupported: true, - } - - computeProcs, ret := dev.GetComputeRunningProcesses() - if ret != nvml.SUCCESS { - return procs, nil - } - - for range computeProcs { - // Simulate successful process creation and CmdlineSlice - // Then simulate CreateTime failure - if nvmlerrors.IsNoSuchFileOrDirectoryError(noSuchFileErr) { - continue // Process should be skipped (lines 106-109) - } - } - - return procs, nil - } - - result, err := testGetProcesses(testUUID, mockDevice) - - assert.NoError(t, err) - assert.Equal(t, testUUID, result.UUID) - assert.Len(t, result.RunningProcesses, 0) // Process should be skipped -} - -// TestGetProcesses_NoSuchFileOrDirectoryError_Status tests that when Status -// fails with a "no such file or directory" error, the process continues (no return) -func TestGetProcesses_NoSuchFileOrDirectoryError_Status(t *testing.T) { - testUUID := "GPU-TEST" - noSuchFileErr := errors.New("not found") - - // Create a mock device - mockDevice := &testutil.MockDevice{ - Device: &mock.Device{ - GetComputeRunningProcessesFunc: func() ([]nvml.ProcessInfo, nvml.Return) { - return []nvml.ProcessInfo{ - {Pid: 1234, UsedGpuMemory: 1024}, - }, nvml.SUCCESS - }, - GetProcessUtilizationFunc: func(pid uint64) ([]nvml.ProcessUtilizationSample, nvml.Return) { - return []nvml.ProcessUtilizationSample{}, nvml.SUCCESS - }, - }, - } - - // Test the Status failure scenario - should continue processing - testGetProcesses := func(uuid string, dev device.Device) (Processes, error) { - procs := Processes{ - UUID: uuid, - GetComputeRunningProcessesSupported: true, - GetProcessUtilizationSupported: true, - } - - computeProcs, ret := dev.GetComputeRunningProcesses() - if ret != nvml.SUCCESS { - return procs, nil - } - - for _, proc := range computeProcs { - // Simulate successful process creation, CmdlineSlice, CreateTime - // Then simulate Status failure - should continue (lines 143-145) - if nvmlerrors.IsNoSuchFileOrDirectoryError(noSuchFileErr) { - // Should continue to Environ call, not return - // Simulate Environ success - procs.RunningProcesses = append(procs.RunningProcesses, Process{ - PID: proc.Pid, - CmdArgs: []string{"test"}, - CreateTime: metav1.Now(), - }) - } - } - - return procs, nil - } - - result, err := testGetProcesses(testUUID, mockDevice) - - assert.NoError(t, err) - assert.Equal(t, testUUID, result.UUID) - assert.Len(t, result.RunningProcesses, 1) // Process should be included despite Status failure -} - -// TestGetProcesses_NoSuchFileOrDirectoryError_Environ tests that when Environ -// fails with a "no such file or directory" error, the process continues (no return) -func TestGetProcesses_NoSuchFileOrDirectoryError_Environ(t *testing.T) { - testUUID := "GPU-TEST" - noSuchFileErr := errors.New("No such file or directory") - - // Create a mock device - mockDevice := &testutil.MockDevice{ - Device: &mock.Device{ - GetComputeRunningProcessesFunc: func() ([]nvml.ProcessInfo, nvml.Return) { - return []nvml.ProcessInfo{ - {Pid: 1234, UsedGpuMemory: 1024}, - }, nvml.SUCCESS - }, - GetProcessUtilizationFunc: func(pid uint64) ([]nvml.ProcessUtilizationSample, nvml.Return) { - return []nvml.ProcessUtilizationSample{}, nvml.SUCCESS - }, - }, - } - - // Test the Environ failure scenario - should continue processing - testGetProcesses := func(uuid string, dev device.Device) (Processes, error) { - procs := Processes{ - UUID: uuid, - GetComputeRunningProcessesSupported: true, - GetProcessUtilizationSupported: true, - } - - computeProcs, ret := dev.GetComputeRunningProcesses() - if ret != nvml.SUCCESS { - return procs, nil - } - - for _, proc := range computeProcs { - // Simulate successful process creation, CmdlineSlice, CreateTime, Status - // Then simulate Environ failure - should continue (lines 158-160) - if nvmlerrors.IsNoSuchFileOrDirectoryError(noSuchFileErr) { - // Should continue and complete the process - procs.RunningProcesses = append(procs.RunningProcesses, Process{ - PID: proc.Pid, - Status: []string{"S"}, - CmdArgs: []string{"test"}, - CreateTime: metav1.Now(), - // BadEnvVarsForCUDA should be nil since Environ failed - }) - } - } - - return procs, nil - } - - result, err := testGetProcesses(testUUID, mockDevice) - - assert.NoError(t, err) - assert.Equal(t, testUUID, result.UUID) - assert.Len(t, result.RunningProcesses, 1) // Process should be included despite Environ failure - assert.Nil(t, result.RunningProcesses[0].BadEnvVarsForCUDA) // Should be nil due to Environ failure -} - -// TestGetProcesses_Integration_NoSuchFileOrDirectoryError provides a comprehensive test -// that uses the actual getProcesses function with a mock newProcessFunc that returns -// "no such file or directory" errors at different stages -func TestGetProcesses_Integration_NoSuchFileOrDirectoryError(t *testing.T) { - testUUID := "GPU-INTEGRATION-TEST" - - // Create a mock device that returns multiple processes - mockDevice := &testutil.MockDevice{ - Device: &mock.Device{ - GetComputeRunningProcessesFunc: func() ([]nvml.ProcessInfo, nvml.Return) { - return []nvml.ProcessInfo{ - {Pid: 1001, UsedGpuMemory: 1024}, // This will fail on newProcessFunc - {Pid: 1002, UsedGpuMemory: 2048}, // This will succeed - }, nvml.SUCCESS - }, - GetProcessUtilizationFunc: func(pid uint64) ([]nvml.ProcessUtilizationSample, nvml.Return) { - return []nvml.ProcessUtilizationSample{}, nvml.SUCCESS - }, - }, - } - - // Mock newProcessFunc that fails for PID 1001 but succeeds for PID 1002 - mockNewProcessFunc := func(pid int32) (*process.Process, error) { - if pid == 1001 { - // Return "no such file or directory" error - this process should be skipped - return nil, errors.New("no such file or directory") - } - // For PID 1002, return nil (we can't easily create a real process.Process) - // but this simulates the case where newProcessFunc succeeds but subsequent calls fail - return nil, errors.New("operation not supported") // This will trigger a different error path - } - - // Call the actual getProcesses function - _, err := getProcesses(testUUID, mockDevice, mockNewProcessFunc) - - // PID 1001 should be skipped due to nvmlerrors.IsNoSuchFileOrDirectoryError - // PID 1002 should cause the function to return an error since it's not a recognized error type - assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to get process 1002") -} - -// TestGetProcesses_Integration_AllProcessesSkipped tests the case where all processes -// are skipped due to nvmlerrors.IsNoSuchFileOrDirectoryError, but the function still succeeds -func TestGetProcesses_Integration_AllProcessesSkipped(t *testing.T) { - testUUID := "GPU-SKIP-ALL-TEST" - - // Create a mock device that returns processes - mockDevice := &testutil.MockDevice{ - Device: &mock.Device{ - GetComputeRunningProcessesFunc: func() ([]nvml.ProcessInfo, nvml.Return) { - return []nvml.ProcessInfo{ - {Pid: 1001, UsedGpuMemory: 1024}, - {Pid: 1002, UsedGpuMemory: 2048}, - {Pid: 1003, UsedGpuMemory: 4096}, - }, nvml.SUCCESS - }, - GetProcessUtilizationFunc: func(pid uint64) ([]nvml.ProcessUtilizationSample, nvml.Return) { - return []nvml.ProcessUtilizationSample{}, nvml.SUCCESS - }, - }, - } - - // Mock newProcessFunc that always returns "no such file or directory" error - mockNewProcessFunc := func(int32) (*process.Process, error) { - return nil, errors.New("not found") // Matches nvmlerrors.IsNoSuchFileOrDirectoryError - } - - // Call the actual getProcesses function - result, err := getProcesses(testUUID, mockDevice, mockNewProcessFunc) - - // Should succeed with empty process list - assert.NoError(t, err) - assert.Equal(t, testUUID, result.UUID) - assert.Len(t, result.RunningProcesses, 0) // All processes should be skipped - assert.True(t, result.GetComputeRunningProcessesSupported) - assert.True(t, result.GetProcessUtilizationSupported) -} From db1b1b7bd400e0b1a2ecf96468251e072a351744 Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Thu, 28 May 2026 16:28:38 -0700 Subject: [PATCH 04/16] fix: allow user to only set one field for ng or cz (#211) (#213) Signed-off-by: Amber Xue Signed-off-by: Jingxiang Zhang Co-authored-by: ambermingxin --- cmd/fleetint/enroll.go | 20 ---- cmd/fleetint/enroll_test.go | 94 +++++++++++++------ .../helm/fleet-intelligence-agent/values.yaml | 11 +-- internal/agentstate/sqlite_test.go | 10 ++ internal/agentstate/state.go | 2 +- internal/enrollment/enrollment.go | 2 +- internal/exporter/exporter.go | 2 +- internal/inventory/sink/backend.go | 2 +- 8 files changed, 82 insertions(+), 61 deletions(-) diff --git a/cmd/fleetint/enroll.go b/cmd/fleetint/enroll.go index c07f44e7..be0c08cf 100644 --- a/cmd/fleetint/enroll.go +++ b/cmd/fleetint/enroll.go @@ -96,9 +96,6 @@ func enrollCommand(cliContext *cli.Context) error { if err != nil { return err } - if err := validateReservedPairMetadata(nodeGroup, computeZone); err != nil { - return err - } metadata := &enrollment.EnrollMetadata{ NodeGroup: nodeGroup, ComputeZone: computeZone, @@ -159,20 +156,3 @@ func validatedOptionalMetadataFlagValue(cliContext *cli.Context, name, fieldName } return &value, nil } - -func validateReservedPairMetadata(nodeGroup, computeZone *string) error { - nodeGroupSet := nodeGroup != nil - computeZoneSet := computeZone != nil - - if !nodeGroupSet && !computeZoneSet { - return nil - } - if nodeGroupSet && computeZoneSet { - nodeGroupEmpty := *nodeGroup == "" - computeZoneEmpty := *computeZone == "" - if nodeGroupEmpty == computeZoneEmpty { - return nil - } - } - return fmt.Errorf("--node-group and --compute-zone must be both omitted, both empty, or both non-empty") -} diff --git a/cmd/fleetint/enroll_test.go b/cmd/fleetint/enroll_test.go index 96378895..e6c2d6dd 100644 --- a/cmd/fleetint/enroll_test.go +++ b/cmd/fleetint/enroll_test.go @@ -340,49 +340,83 @@ func TestEnrollCommandRejectsOverlongMetadataNames(t *testing.T) { require.ErrorContains(t, err, "Node group name must be 255 characters or fewer") } -func TestValidateReservedPairMetadata(t *testing.T) { +func TestValidatedOptionalMetadataFlagValueSupportsIndependentReservedUpdates(t *testing.T) { strPtr := func(v string) *string { return &v } tests := []struct { - name string - nodeGroup *string - computeZone *string - wantErr bool + name string + nodeGroupArg *string + computeZoneArg *string + expectNodeGroup *string + expectComputeZone *string }{ - {name: "both omitted", nodeGroup: nil, computeZone: nil, wantErr: false}, - {name: "both empty", nodeGroup: strPtr(""), computeZone: strPtr(""), wantErr: false}, - {name: "both non-empty", nodeGroup: strPtr("ng-a"), computeZone: strPtr("cz-a"), wantErr: false}, - {name: "node-group only", nodeGroup: strPtr("ng-a"), computeZone: nil, wantErr: true}, - {name: "compute-zone only", nodeGroup: nil, computeZone: strPtr("cz-a"), wantErr: true}, - {name: "node-group empty compute-zone non-empty", nodeGroup: strPtr(""), computeZone: strPtr("cz-a"), wantErr: true}, - {name: "node-group non-empty compute-zone empty", nodeGroup: strPtr("ng-a"), computeZone: strPtr(""), wantErr: true}, + { + name: "clear compute-zone and set node-group", + nodeGroupArg: strPtr("xx"), + computeZoneArg: strPtr(""), + expectNodeGroup: strPtr("xx"), + expectComputeZone: strPtr(""), + }, + { + name: "set compute-zone and clear node-group", + nodeGroupArg: strPtr(""), + computeZoneArg: strPtr("xx"), + expectNodeGroup: strPtr(""), + expectComputeZone: strPtr("xx"), + }, + { + name: "set compute-zone and omit node-group", + nodeGroupArg: nil, + computeZoneArg: strPtr("xx"), + expectNodeGroup: nil, + expectComputeZone: strPtr("xx"), + }, + { + name: "set node-group and omit compute-zone", + nodeGroupArg: strPtr("xx"), + computeZoneArg: nil, + expectNodeGroup: strPtr("xx"), + expectComputeZone: nil, + }, + { + name: "clear compute-zone and omit node-group", + nodeGroupArg: nil, + computeZoneArg: strPtr(""), + expectNodeGroup: nil, + expectComputeZone: strPtr(""), + }, + { + name: "clear node-group and omit compute-zone", + nodeGroupArg: strPtr(""), + computeZoneArg: nil, + expectNodeGroup: strPtr(""), + expectComputeZone: nil, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - err := validateReservedPairMetadata(tc.nodeGroup, tc.computeZone) - if tc.wantErr { - require.ErrorContains(t, err, "--node-group and --compute-zone must be both omitted, both empty, or both non-empty") - return + flagSet := flag.NewFlagSet("enroll", flag.ContinueOnError) + flagSet.String("node-group", "", "") + flagSet.String("compute-zone", "", "") + if tc.nodeGroupArg != nil { + require.NoError(t, flagSet.Set("node-group", *tc.nodeGroupArg)) } + if tc.computeZoneArg != nil { + require.NoError(t, flagSet.Set("compute-zone", *tc.computeZoneArg)) + } + cliContext := cli.NewContext(cli.NewApp(), flagSet, nil) + + nodeGroup, err := validatedOptionalMetadataFlagValue(cliContext, "node-group", "Node group") require.NoError(t, err) + computeZone, err := validatedOptionalMetadataFlagValue(cliContext, "compute-zone", "Compute zone") + require.NoError(t, err) + + assert.Equal(t, tc.expectNodeGroup, nodeGroup) + assert.Equal(t, tc.expectComputeZone, computeZone) }) } } -func TestEnrollCommandRejectsMixedReservedPairValues(t *testing.T) { - app := App() - app.Writer = &bytes.Buffer{} - - err := app.Run([]string{ - "fleetint", "enroll", - "--endpoint", "https://example.com", - "--token", "token", - "--node-group", "", - "--compute-zone", "cz-a", - }) - require.ErrorContains(t, err, "--node-group and --compute-zone must be both omitted, both empty, or both non-empty") -} - func TestEnrollCommandRejectsReservedUnassignedName(t *testing.T) { app := App() app.Writer = &bytes.Buffer{} diff --git a/deployments/helm/fleet-intelligence-agent/values.yaml b/deployments/helm/fleet-intelligence-agent/values.yaml index 747bc608..e256b524 100644 --- a/deployments/helm/fleet-intelligence-agent/values.yaml +++ b/deployments/helm/fleet-intelligence-agent/values.yaml @@ -61,13 +61,10 @@ enroll: tokenSecretName: "" tokenSecretKey: "token" tokenValue: "" - # Optional enrollment metadata: - # - omit key to preserve existing stored value - # - set non-empty string to overwrite - # - set empty string ("") to clear - # - # nodeGroup: "prod-a" - # computeZone: "us-east-1c" + # Optional enrollment metadata. Omit these keys to preserve stored values. + # Set to "" explicitly to clear stored values. + # nodeGroup: "" + # computeZone: "" ports: http: 15133 diff --git a/internal/agentstate/sqlite_test.go b/internal/agentstate/sqlite_test.go index af28c849..724fc689 100644 --- a/internal/agentstate/sqlite_test.go +++ b/internal/agentstate/sqlite_test.go @@ -23,6 +23,7 @@ import ( "testing" "time" + pkgmetadata "github.com/NVIDIA/fleet-intelligence-sdk/pkg/metadata" "github.com/NVIDIA/fleet-intelligence-sdk/pkg/sqlite" "github.com/stretchr/testify/require" ) @@ -84,6 +85,15 @@ func TestSQLiteStateRoundTrip(t *testing.T) { require.True(t, ok) require.Equal(t, "group-a", value) + stateFile, err := state.stateFileFn() + require.NoError(t, err) + db, err := sqlite.Open(stateFile, sqlite.WithReadOnly(true)) + require.NoError(t, err) + defer db.Close() + value, err = pkgmetadata.ReadMetadata(ctx, db, "node_group") + require.NoError(t, err) + require.Equal(t, "group-a", value) + value, ok, err = state.GetComputeZone(ctx) require.NoError(t, err) require.True(t, ok) diff --git a/internal/agentstate/state.go b/internal/agentstate/state.go index 23da576a..2f705f3d 100644 --- a/internal/agentstate/state.go +++ b/internal/agentstate/state.go @@ -25,7 +25,7 @@ const ( MetadataKeyBackendBaseURL = "backend_base_url" MetadataKeySAKToken = "sak_token" MetadataKeyEnrolledAt = "enrolled_at" - MetadataKeyNodeGroup = "nodegroup" + MetadataKeyNodeGroup = "node_group" MetadataKeyComputeZone = "compute_zone" ) diff --git a/internal/enrollment/enrollment.go b/internal/enrollment/enrollment.go index 7e50303c..77ce9f1b 100644 --- a/internal/enrollment/enrollment.go +++ b/internal/enrollment/enrollment.go @@ -175,7 +175,7 @@ func storeConfigInMetadata(ctx context.Context, baseURL, jwtToken, sakToken stri } if metadata.NodeGroup != nil { if err := pkgmetadata.SetMetadata(ctx, dbRW, agentstate.MetadataKeyNodeGroup, *metadata.NodeGroup); err != nil { - return fmt.Errorf("failed to set nodegroup: %w", err) + return fmt.Errorf("failed to set node_group: %w", err) } } if metadata.ComputeZone != nil { diff --git a/internal/exporter/exporter.go b/internal/exporter/exporter.go index 52e67d2d..e903fa5f 100644 --- a/internal/exporter/exporter.go +++ b/internal/exporter/exporter.go @@ -196,7 +196,7 @@ func (e *healthExporter) populateOptionalResourceMetadata(ctx context.Context, d nodeGroup, err := pkgmetadata.ReadMetadata(ctx, e.options.dbRO, agentstate.MetadataKeyNodeGroup) if err != nil { - log.Logger.Debugw("nodegroup metadata not available for telemetry resource", "error", err) + log.Logger.Debugw("node_group metadata not available for telemetry resource", "error", err) } else { data.NodeGroup = nodeGroup } diff --git a/internal/inventory/sink/backend.go b/internal/inventory/sink/backend.go index 45a3f90a..7a1f1888 100644 --- a/internal/inventory/sink/backend.go +++ b/internal/inventory/sink/backend.go @@ -83,7 +83,7 @@ func (s *backendSink) Export(ctx context.Context, snap *inventory.Snapshot) erro req := mapper.ToNodeUpsertRequest(snap) nodeGroup, ok, err := s.state.GetNodeGroup(ctx) if err != nil { - log.Logger.Warnw("inventory export continuing without nodegroup metadata", "error", err) + log.Logger.Warnw("inventory export continuing without node_group metadata", "error", err) } else if ok { req.NodeGroup = nodeGroup } From cf0cffecb9076c5a39e43db2ebb47f793826c4c1 Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Tue, 26 May 2026 11:47:37 -0700 Subject: [PATCH 05/16] fix: preserve counter metric types in OTLP (#198) Signed-off-by: Jingxiang Zhang --- internal/exporter/converter/otlp.go | 56 +++-- internal/exporter/converter/otlp_test.go | 58 ++++- .../accelerator/nvidia/dcgm/mem/metrics.go | 40 ++-- .../accelerator/nvidia/dcgm/nvlink/metrics.go | 60 ++--- .../accelerator/nvidia/dcgm/pcie/metrics.go | 4 +- .../accelerator/nvidia/dcgm/power/metrics.go | 12 +- .../nvidia/dcgm/thermal/metrics.go | 5 +- .../accelerator/nvidia/infiniband/metrics.go | 4 +- .../components/network/ethernet/metrics.go | 32 +-- .../pkg/metrics/scraper/prometheus.go | 4 + .../pkg/metrics/scraper/prometheus_test.go | 54 ++++- .../pkg/metrics/settable_const_metric.go | 217 ++++++++++++++++++ .../pkg/metrics/settable_const_metric_test.go | 79 +++++++ .../pkg/metrics/store/sqlite.go | 85 ++++++- .../pkg/metrics/store/sqlite_test.go | 55 +++++ .../pkg/metrics/types.go | 9 + 16 files changed, 662 insertions(+), 112 deletions(-) create mode 100644 third_party/fleet-intelligence-sdk/pkg/metrics/settable_const_metric.go create mode 100644 third_party/fleet-intelligence-sdk/pkg/metrics/settable_const_metric_test.go diff --git a/internal/exporter/converter/otlp.go b/internal/exporter/converter/otlp.go index d6be8289..30348c88 100644 --- a/internal/exporter/converter/otlp.go +++ b/internal/exporter/converter/otlp.go @@ -25,6 +25,7 @@ import ( "time" apiv1 "github.com/NVIDIA/fleet-intelligence-sdk/api/v1" + pkgmetrics "github.com/NVIDIA/fleet-intelligence-sdk/pkg/metrics" commonv1 "go.opentelemetry.io/proto/otlp/common/v1" logsv1 "go.opentelemetry.io/proto/otlp/logs/v1" metricsv1 "go.opentelemetry.io/proto/otlp/metrics/v1" @@ -179,25 +180,7 @@ func (c *otlpConverter) convertMetricsToOTLP(data *collector.HealthData) []*metr // Convert regular metrics if available if len(data.Metrics) > 0 { for _, metric := range data.Metrics { - otlpMetric := &metricsv1.Metric{ - Name: metric.Name, - Description: fmt.Sprintf("Metric from component %s", metric.Component), - Unit: "1", - Data: &metricsv1.Metric_Gauge{ - Gauge: &metricsv1.Gauge{ - DataPoints: []*metricsv1.NumberDataPoint{ - { - TimeUnixNano: uint64(metric.UnixMilliseconds) * 1_000_000, - Value: &metricsv1.NumberDataPoint_AsDouble{ - AsDouble: metric.Value, - }, - Attributes: c.convertLabelsToOTLPAttributes(metric.Labels, gpuUUIDToIndex), - }, - }, - }, - }, - } - otlpMetrics = append(otlpMetrics, otlpMetric) + otlpMetrics = append(otlpMetrics, c.convertMetricToOTLP(metric, gpuUUIDToIndex)) } } @@ -205,7 +188,6 @@ func (c *otlpConverter) convertMetricsToOTLP(data *collector.HealthData) []*metr summaryMetric := &metricsv1.Metric{ Name: "fleetint_agent_collection_summary", Description: "Summary of Fleet Intelligence data collection including counts of metrics, events, and components", - Unit: "1", Data: &metricsv1.Metric_Gauge{ Gauge: &metricsv1.Gauge{ DataPoints: []*metricsv1.NumberDataPoint{ @@ -244,7 +226,6 @@ func (c *otlpConverter) convertMetricsToOTLP(data *collector.HealthData) []*metr upMetric := &metricsv1.Metric{ Name: "fleetint_agent_up", Description: "Fleet Intelligence agent liveness. A value of 1 indicates the agent was running when telemetry was exported.", - Unit: "1", Data: &metricsv1.Metric_Gauge{ Gauge: &metricsv1.Gauge{ DataPoints: []*metricsv1.NumberDataPoint{ @@ -263,6 +244,39 @@ func (c *otlpConverter) convertMetricsToOTLP(data *collector.HealthData) []*metr return otlpMetrics } +func (c *otlpConverter) convertMetricToOTLP(metric pkgmetrics.Metric, gpuUUIDToIndex map[string]string) *metricsv1.Metric { + dataPoint := &metricsv1.NumberDataPoint{ + TimeUnixNano: uint64(metric.UnixMilliseconds) * 1_000_000, + Value: &metricsv1.NumberDataPoint_AsDouble{ + AsDouble: metric.Value, + }, + Attributes: c.convertLabelsToOTLPAttributes(metric.Labels, gpuUUIDToIndex), + } + + otlpMetric := &metricsv1.Metric{ + Name: metric.Name, + Description: fmt.Sprintf("Metric from component %s", metric.Component), + } + + if metric.Type == pkgmetrics.MetricTypeCounter { + otlpMetric.Data = &metricsv1.Metric_Sum{ + Sum: &metricsv1.Sum{ + AggregationTemporality: metricsv1.AggregationTemporality_AGGREGATION_TEMPORALITY_CUMULATIVE, + IsMonotonic: true, + DataPoints: []*metricsv1.NumberDataPoint{dataPoint}, + }, + } + return otlpMetric + } + + otlpMetric.Data = &metricsv1.Metric_Gauge{ + Gauge: &metricsv1.Gauge{ + DataPoints: []*metricsv1.NumberDataPoint{dataPoint}, + }, + } + return otlpMetric +} + // convertLabelsToOTLPAttributes converts metric labels to OTLP attributes. // If the labels contain a "uuid" key but no "gpu" key, it enriches the // attributes with the GPU index looked up from the machine info mapping. diff --git a/internal/exporter/converter/otlp_test.go b/internal/exporter/converter/otlp_test.go index ca43a619..68ba4ead 100644 --- a/internal/exporter/converter/otlp_test.go +++ b/internal/exporter/converter/otlp_test.go @@ -99,6 +99,53 @@ func TestOTLPConverter_Convert_WithMetrics(t *testing.T) { assert.Contains(t, metrics[0].Description, "gpu") } +func TestOTLPConverter_Convert_CounterMetricsBecomeCumulativeSums(t *testing.T) { + data := &collector.HealthData{ + Timestamp: time.Now(), + MachineID: "test-machine", + Metrics: metrics.Metrics{ + { + Component: "gpu", + Name: "dcgm_fi_dev_pcie_replay_counter", + Type: metrics.MetricTypeCounter, + UnixMilliseconds: 1699200000000, + Value: 42, + Labels: map[string]string{"uuid": "GPU-0", "gpu": "0"}, + }, + { + Component: "gpu", + Name: "dcgm_fi_dev_gpu_temp", + Type: metrics.MetricTypeGauge, + UnixMilliseconds: 1699200001000, + Value: 65, + Labels: map[string]string{"uuid": "GPU-0", "gpu": "0"}, + }, + }, + } + + converter := NewOTLPConverter() + otlpData := converter.Convert(data) + + convertedMetrics := otlpData.Metrics.ResourceMetrics[0].ScopeMetrics[0].Metrics + counterMetric := findOTLPMetric(convertedMetrics, "dcgm_fi_dev_pcie_replay_counter") + require.NotNil(t, counterMetric) + assert.Empty(t, counterMetric.Unit) + sum := counterMetric.GetSum() + require.NotNil(t, sum) + assert.True(t, sum.IsMonotonic) + assert.Equal(t, metricsv1.AggregationTemporality_AGGREGATION_TEMPORALITY_CUMULATIVE, sum.AggregationTemporality) + require.Len(t, sum.DataPoints, 1) + assert.Equal(t, 42.0, sum.DataPoints[0].GetAsDouble()) + + gaugeMetric := findOTLPMetric(convertedMetrics, "dcgm_fi_dev_gpu_temp") + require.NotNil(t, gaugeMetric) + assert.Empty(t, gaugeMetric.Unit) + gauge := gaugeMetric.GetGauge() + require.NotNil(t, gauge) + require.Len(t, gauge.DataPoints, 1) + assert.Equal(t, 65.0, gauge.DataPoints[0].GetAsDouble()) +} + func TestOTLPConverter_Convert_WithEvents(t *testing.T) { data := &collector.HealthData{ Timestamp: time.Now(), @@ -733,7 +780,7 @@ func TestOTLPConverter_UpMetric(t *testing.T) { } require.NotNil(t, upMetric, "Should have fleetint_agent_up metric") - assert.Equal(t, "1", upMetric.Unit) + assert.Empty(t, upMetric.Unit) assert.Contains(t, upMetric.Description, "liveness") gauge := upMetric.Data.(*metricsv1.Metric_Gauge).Gauge @@ -745,6 +792,15 @@ func TestOTLPConverter_UpMetric(t *testing.T) { assert.Empty(t, point.Attributes) } +func findOTLPMetric(metrics []*metricsv1.Metric, name string) *metricsv1.Metric { + for _, metric := range metrics { + if metric.Name == name { + return metric + } + } + return nil +} + func TestOTLPConverter_ResourceAttributes(t *testing.T) { data := &collector.HealthData{ Timestamp: time.Now(), diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/mem/metrics.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/mem/metrics.go index 2cbf8184..e09a7b1d 100644 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/mem/metrics.go +++ b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/mem/metrics.go @@ -83,8 +83,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevUncorrectableRemappedRows = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevUncorrectableRemappedRows = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_uncorrectable_remapped_rows", @@ -93,8 +93,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevCorrectableRemappedRows = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevCorrectableRemappedRows = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_correctable_remapped_rows", @@ -133,8 +133,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevECCSBEVolTotal = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevECCSBEVolTotal = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_ecc_sbe_vol_total", @@ -143,8 +143,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevECCDBEVolTotal = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevECCDBEVolTotal = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_ecc_dbe_vol_total", @@ -153,8 +153,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevECCSBEAggTotal = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevECCSBEAggTotal = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_ecc_sbe_agg_total", @@ -163,8 +163,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevECCDBAggTotal = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevECCDBAggTotal = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_ecc_dbe_agg_total", @@ -173,32 +173,32 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevECCSBEVolDev = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevECCSBEVolDev = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Name: "dcgm_fi_dev_ecc_sbe_vol_dev", Help: "Device memory single bit volatile ECC errors.", }, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevECCDBEVolDev = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevECCDBEVolDev = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Name: "dcgm_fi_dev_ecc_dbe_vol_dev", Help: "Device memory double bit volatile ECC errors.", }, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevECCSBEAggDev = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevECCSBEAggDev = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Name: "dcgm_fi_dev_ecc_sbe_agg_dev", Help: "Device memory single bit aggregate (persistent) ECC errors. Note: monotonically increasing.", }, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevECCDBEAggDev = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevECCDBEAggDev = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Name: "dcgm_fi_dev_ecc_dbe_agg_dev", Help: "Device memory double bit aggregate (persistent) ECC errors. Note: monotonically increasing.", }, diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/nvlink/metrics.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/nvlink/metrics.go index 4107f3f5..be4ed8b1 100644 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/nvlink/metrics.go +++ b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/nvlink/metrics.go @@ -61,8 +61,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkErrorDLCrc = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevNvlinkErrorDLCrc = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_nvlink_error_dl_crc", @@ -71,8 +71,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkErrorDLRecovery = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevNvlinkErrorDLRecovery = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_nvlink_error_dl_recovery", @@ -81,8 +81,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkErrorDLReplay = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevNvlinkErrorDLReplay = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_nvlink_error_dl_replay", @@ -91,8 +91,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountLinkRecoverySuccessfulEvents = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevNvlinkCountLinkRecoverySuccessfulEvents = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_nvlink_count_link_recovery_successful_events", @@ -101,8 +101,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountLinkRecoveryFailedEvents = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevNvlinkCountLinkRecoveryFailedEvents = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_nvlink_count_link_recovery_failed_events", @@ -115,40 +115,40 @@ var ( prometheus.GaugeOpts{Name: "dcgm_fi_dev_fabric_manager_status", Help: "The status of the fabric manager - a value from dcgmFabricManagerStatus_t"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevC2CLinkErrorReplay = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_c2c_link_error_replay", Help: "C2C Link Replay Error Counter"}, + metricDCGMFIDevC2CLinkErrorReplay = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_c2c_link_error_replay", Help: "C2C Link Replay Error Counter"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountRxGeneralErrors = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_nvlink_count_rx_general_errors", Help: "Total number of packets Rx with header mismatch"}, + metricDCGMFIDevNvlinkCountRxGeneralErrors = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_nvlink_count_rx_general_errors", Help: "Total number of packets Rx with header mismatch"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountRxErrors = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_nvlink_count_rx_errors", Help: "Total number of packets with errors Rx on a link"}, + metricDCGMFIDevNvlinkCountRxErrors = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_nvlink_count_rx_errors", Help: "Total number of packets with errors Rx on a link"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountRxMalformedPacketErrors = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_nvlink_count_rx_malformed_packet_errors", Help: "Number of packets Rx on a link where packets are malformed"}, + metricDCGMFIDevNvlinkCountRxMalformedPacketErrors = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_nvlink_count_rx_malformed_packet_errors", Help: "Number of packets Rx on a link where packets are malformed"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountRxRemoteErrors = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_nvlink_count_rx_remote_errors", Help: "Total number of packets Rx - stomp/EBP marker"}, + metricDCGMFIDevNvlinkCountRxRemoteErrors = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_nvlink_count_rx_remote_errors", Help: "Total number of packets Rx - stomp/EBP marker"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountRxSymbolErrors = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_nvlink_count_rx_symbol_errors", Help: "Number of errors in rx symbols"}, + metricDCGMFIDevNvlinkCountRxSymbolErrors = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_nvlink_count_rx_symbol_errors", Help: "Number of errors in rx symbols"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountRxBufferOverrunErrors = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_nvlink_count_rx_buffer_overrun_errors", Help: "Number of packets that were discarded on Rx due to buffer overrun"}, + metricDCGMFIDevNvlinkCountRxBufferOverrunErrors = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_nvlink_count_rx_buffer_overrun_errors", Help: "Number of packets that were discarded on Rx due to buffer overrun"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountLocalLinkIntegrityErrors = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_nvlink_count_local_link_integrity_errors", Help: "Total number of times that the count of local errors exceeded a threshold"}, + metricDCGMFIDevNvlinkCountLocalLinkIntegrityErrors = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_nvlink_count_local_link_integrity_errors", Help: "Total number of times that the count of local errors exceeded a threshold"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountEffectiveErrors = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_nvlink_count_effective_errors", Help: "Sum of the number of errors in each Nvlink packet"}, + metricDCGMFIDevNvlinkCountEffectiveErrors = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_nvlink_count_effective_errors", Help: "Sum of the number of errors in each Nvlink packet"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) metricDCGMFIDevNvlinkCountEffectiveBERFloat = prometheus.NewGaugeVec( @@ -159,8 +159,8 @@ var ( prometheus.GaugeOpts{Name: "dcgm_fi_dev_nvlink_count_symbol_ber_float", Help: "BER for symbol errors - decoded float value"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevNvlinkCountTxDiscards = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_nvlink_count_tx_discards", Help: "Total number of tx error packets that were discarded"}, + metricDCGMFIDevNvlinkCountTxDiscards = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_nvlink_count_tx_discards", Help: "Total number of tx error packets that were discarded"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) ) diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/pcie/metrics.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/pcie/metrics.go index 6decc753..ebd2b8d3 100644 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/pcie/metrics.go +++ b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/pcie/metrics.go @@ -33,8 +33,8 @@ var ( pkgmetrics.MetricComponentLabelKey: Name, } - metricDCGMFIDevPCIeReplayCounter = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricDCGMFIDevPCIeReplayCounter = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: "", Name: "dcgm_fi_dev_pcie_replay_counter", diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/power/metrics.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/power/metrics.go index edc79bd1..d386de03 100644 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/power/metrics.go +++ b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/power/metrics.go @@ -57,18 +57,18 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevPowerViolation = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_power_violation", Help: "Power Violation time in ns"}, + metricDCGMFIDevPowerViolation = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_power_violation", Help: "Power Violation time in ns"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevReliabilityViolation = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_reliability_violation", Help: "Reliability violation limit"}, + metricDCGMFIDevReliabilityViolation = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_reliability_violation", Help: "Reliability violation limit"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevBoardLimitViolation = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_board_limit_violation", Help: "Board violation limit"}, + metricDCGMFIDevBoardLimitViolation = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_board_limit_violation", Help: "Board violation limit"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) ) diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/thermal/metrics.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/thermal/metrics.go index d70a5492..c711ba8e 100644 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/thermal/metrics.go +++ b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/thermal/metrics.go @@ -67,8 +67,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - metricDCGMFIDevThermalViolation = prometheus.NewGaugeVec( - prometheus.GaugeOpts{Name: "dcgm_fi_dev_thermal_violation", Help: "Thermal Violation time in ns"}, + metricDCGMFIDevThermalViolation = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{Name: "dcgm_fi_dev_thermal_violation", Help: "Thermal Violation time in ns"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) @@ -76,7 +76,6 @@ var ( prometheus.GaugeOpts{Name: "dcgm_fi_dev_gpu_temp_limit", Help: "Thermal margin temperature (distance to nearest slowdown threshold) for this GPU"}, []string{pkgmetrics.MetricComponentLabelKey, "uuid", "gpu"}, ).MustCurryWith(componentLabel) - ) func init() { diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/infiniband/metrics.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/infiniband/metrics.go index f940fcc8..b65a5c28 100644 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/infiniband/metrics.go +++ b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/infiniband/metrics.go @@ -16,8 +16,8 @@ var ( pkgmetrics.MetricComponentLabelKey: Name, } - metricIbLinkedDowned = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricIbLinkedDowned = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: SubSystem, Name: "link_downed", diff --git a/third_party/fleet-intelligence-sdk/components/network/ethernet/metrics.go b/third_party/fleet-intelligence-sdk/components/network/ethernet/metrics.go index 26f5f8e6..a95ce502 100644 --- a/third_party/fleet-intelligence-sdk/components/network/ethernet/metrics.go +++ b/third_party/fleet-intelligence-sdk/components/network/ethernet/metrics.go @@ -16,8 +16,8 @@ var ( pkgmetrics.MetricComponentLabelKey: Name, } - metricRxBytes = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricRxBytes = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: SubSystem, Name: "rx_bytes", @@ -26,8 +26,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "interface"}, ).MustCurryWith(componentLabel) - metricRxPackets = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricRxPackets = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: SubSystem, Name: "rx_packets", @@ -36,8 +36,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "interface"}, ).MustCurryWith(componentLabel) - metricRxErrors = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricRxErrors = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: SubSystem, Name: "rx_errors", @@ -46,8 +46,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "interface"}, ).MustCurryWith(componentLabel) - metricRxDropped = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricRxDropped = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: SubSystem, Name: "rx_dropped", @@ -56,8 +56,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "interface"}, ).MustCurryWith(componentLabel) - metricTxBytes = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricTxBytes = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: SubSystem, Name: "tx_bytes", @@ -66,8 +66,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "interface"}, ).MustCurryWith(componentLabel) - metricTxPackets = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricTxPackets = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: SubSystem, Name: "tx_packets", @@ -76,8 +76,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "interface"}, ).MustCurryWith(componentLabel) - metricTxErrors = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricTxErrors = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: SubSystem, Name: "tx_errors", @@ -86,8 +86,8 @@ var ( []string{pkgmetrics.MetricComponentLabelKey, "interface"}, ).MustCurryWith(componentLabel) - metricTxDropped = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + metricTxDropped = pkgmetrics.NewSettableCounterVec( + prometheus.CounterOpts{ Namespace: "", Subsystem: SubSystem, Name: "tx_dropped", diff --git a/third_party/fleet-intelligence-sdk/pkg/metrics/scraper/prometheus.go b/third_party/fleet-intelligence-sdk/pkg/metrics/scraper/prometheus.go index 6236367f..96cca597 100644 --- a/third_party/fleet-intelligence-sdk/pkg/metrics/scraper/prometheus.go +++ b/third_party/fleet-intelligence-sdk/pkg/metrics/scraper/prometheus.go @@ -71,9 +71,13 @@ func (s *promScraper) Scrape(_ context.Context) (pkgmetrics.Metrics, error) { // for now, only support counter and gauge switch { case mtRaw.GetCounter() != nil: + m.Type = pkgmetrics.MetricTypeCounter m.Value = mtRaw.GetCounter().GetValue() case mtRaw.GetGauge() != nil: + m.Type = pkgmetrics.MetricTypeGauge m.Value = mtRaw.GetGauge().GetValue() + default: + continue } ms = append(ms, m) diff --git a/third_party/fleet-intelligence-sdk/pkg/metrics/scraper/prometheus_test.go b/third_party/fleet-intelligence-sdk/pkg/metrics/scraper/prometheus_test.go index 5ccd49f0..577af352 100644 --- a/third_party/fleet-intelligence-sdk/pkg/metrics/scraper/prometheus_test.go +++ b/third_party/fleet-intelligence-sdk/pkg/metrics/scraper/prometheus_test.go @@ -89,16 +89,19 @@ func TestPrometheusScraper(t *testing.T) { require.Equal(t, "component-1", ms[0].Component) require.Equal(t, "sqlite_insert_update_total", ms[0].Name) + require.Equal(t, pkgmetrics.MetricTypeCounter, ms[0].Type) require.Nil(t, ms[0].Labels, "Expected no labels for the first metric") require.Equal(t, float64(1), ms[0].Value) require.Equal(t, "component-1", ms[1].Component) require.Equal(t, "test_current_celsius", ms[1].Name) + require.Equal(t, pkgmetrics.MetricTypeGauge, ms[1].Type) require.Equal(t, "GPU-0", ms[1].Labels["label_uuid"]) require.Equal(t, float64(100), ms[1].Value) require.Equal(t, "component-1", ms[2].Component) require.Equal(t, "test_slowdown_used_percent", ms[2].Name) + require.Equal(t, pkgmetrics.MetricTypeGauge, ms[2].Type) require.Equal(t, "GPU-0", ms[2].Labels["label_uuid"]) require.Equal(t, float64(98), ms[2].Value) } @@ -112,6 +115,14 @@ func (m *mockGathererWithError) Gather() ([]*dto.MetricFamily, error) { return nil, m.err } +type mockGatherer struct { + metricFamilies []*dto.MetricFamily +} + +func (m *mockGatherer) Gather() ([]*dto.MetricFamily, error) { + return m.metricFamilies, nil +} + func TestPrometheusScraper_GatherError(t *testing.T) { t.Parallel() @@ -134,6 +145,39 @@ func TestPrometheusScraper_GatherError(t *testing.T) { require.Nil(t, metrics) } +func TestPrometheusScraper_SkipsUnsupportedMetricKinds(t *testing.T) { + t.Parallel() + + scraper := &promScraper{ + gatherer: &mockGatherer{ + metricFamilies: []*dto.MetricFamily{ + { + Name: stringPtr("test_summary"), + Type: dto.MetricType_SUMMARY.Enum(), + Metric: []*dto.Metric{ + { + Label: []*dto.LabelPair{ + { + Name: stringPtr(pkgmetrics.MetricComponentLabelKey), + Value: stringPtr("component-1"), + }, + }, + Summary: &dto.Summary{}, + }, + }, + }, + }, + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + metrics, err := scraper.Scrape(ctx) + require.NoError(t, err) + require.Empty(t, metrics) +} + func TestPrometheusScraper_NilGatherer(t *testing.T) { t.Parallel() @@ -151,6 +195,10 @@ func TestPrometheusScraper_NilGatherer(t *testing.T) { require.Nil(t, metrics) } +func stringPtr(s string) *string { + return &s +} + func TestPrometheusScraper_NilScraper(t *testing.T) { t.Parallel() @@ -254,19 +302,19 @@ func TestPrometheusScraper_MultipleMetricTypes(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, ms) - // Should have more than 4 metrics because histograms and summaries - // generate multiple underlying metrics - require.True(t, len(ms) >= 4, "Expected at least 4 metrics, got %d", len(ms)) + require.Len(t, ms, 2, "Only counter and gauge metrics should be returned") // Verify the counter and gauge are included var foundCounter, foundGauge bool for _, m := range ms { if m.Name == "test_operations_total" && m.Component == "component1" { foundCounter = true + require.Equal(t, pkgmetrics.MetricTypeCounter, m.Type) require.Equal(t, float64(1), m.Value) } if m.Name == "test_resources_utilization" && m.Component == "component1" { foundGauge = true + require.Equal(t, pkgmetrics.MetricTypeGauge, m.Type) if labels := m.Labels; labels != nil { require.Equal(t, "resource1", labels["label_uuid"]) } diff --git a/third_party/fleet-intelligence-sdk/pkg/metrics/settable_const_metric.go b/third_party/fleet-intelligence-sdk/pkg/metrics/settable_const_metric.go new file mode 100644 index 00000000..09972590 --- /dev/null +++ b/third_party/fleet-intelligence-sdk/pkg/metrics/settable_const_metric.go @@ -0,0 +1,217 @@ +// Copyright 2024 Lepton AI Inc +// Source: https://github.com/leptonai/gpud + +package metrics + +import ( + "fmt" + "sort" + "strings" + "sync" + + "github.com/prometheus/client_golang/prometheus" +) + +// SettableConstMetricVec stores externally-collected absolute values and emits +// them as constant Prometheus metrics with the configured ValueType. +type SettableConstMetricVec struct { + opts prometheus.Opts + desc *prometheus.Desc + valueType prometheus.ValueType + variableLabels []string + constLabels prometheus.Labels + + mu sync.RWMutex + samples map[string]settableConstMetricSample +} + +type settableConstMetricSample struct { + labelValues []string + value float64 +} + +// NewSettableCounterVec creates a collector for counters whose absolute values +// are read from an external source. +func NewSettableCounterVec(opts prometheus.CounterOpts, variableLabels []string) *SettableConstMetricVec { + return newSettableConstMetricVec(prometheus.Opts(opts), prometheus.CounterValue, variableLabels, nil) +} + +func newSettableConstMetricVec(opts prometheus.Opts, valueType prometheus.ValueType, variableLabels []string, constLabels prometheus.Labels) *SettableConstMetricVec { + copiedVariableLabels := append([]string(nil), variableLabels...) + copiedConstLabels := mergeLabels(opts.ConstLabels, constLabels) + + return &SettableConstMetricVec{ + opts: opts, + desc: newConstMetricDesc(opts, copiedVariableLabels, copiedConstLabels), + valueType: valueType, + variableLabels: copiedVariableLabels, + constLabels: copiedConstLabels, + samples: make(map[string]settableConstMetricSample), + } +} + +// MustCurryWith returns a collector with the provided labels emitted as constant +// labels. It panics if any provided label is not one of the variable labels. +func (v *SettableConstMetricVec) MustCurryWith(labels prometheus.Labels) *SettableConstMetricVec { + if v == nil { + panic("nil SettableConstMetricVec") + } + + curriedLabels := copyLabels(v.constLabels) + if curriedLabels == nil { + curriedLabels = make(prometheus.Labels, len(labels)) + } + for name, value := range labels { + if !containsLabel(v.variableLabels, name) { + panic(fmt.Sprintf("unknown label %q", name)) + } + curriedLabels[name] = value + } + + var remainingLabels []string + for _, name := range v.variableLabels { + if _, ok := labels[name]; ok { + continue + } + remainingLabels = append(remainingLabels, name) + } + + return newSettableConstMetricVec(v.opts, v.valueType, remainingLabels, curriedLabels) +} + +// With returns a handle for setting one labeled metric sample. +func (v *SettableConstMetricVec) With(labels prometheus.Labels) *SettableConstMetric { + if v == nil { + panic("nil SettableConstMetricVec") + } + + labelValues := make([]string, 0, len(v.variableLabels)) + for _, name := range v.variableLabels { + value, ok := labels[name] + if !ok { + panic(fmt.Sprintf("missing label %q", name)) + } + labelValues = append(labelValues, value) + } + for name := range labels { + if !containsLabel(v.variableLabels, name) { + panic(fmt.Sprintf("unknown label %q", name)) + } + } + + return &SettableConstMetric{ + vec: v, + labelValues: labelValues, + key: strings.Join(labelValues, "\xff"), + } +} + +// Delete removes the sample for the provided label set. +func (v *SettableConstMetricVec) Delete(labels prometheus.Labels) bool { + metric := v.With(labels) + + v.mu.Lock() + defer v.mu.Unlock() + + if _, ok := v.samples[metric.key]; !ok { + return false + } + delete(v.samples, metric.key) + return true +} + +// Reset removes all samples from the collector. +func (v *SettableConstMetricVec) Reset() { + v.mu.Lock() + defer v.mu.Unlock() + v.samples = make(map[string]settableConstMetricSample) +} + +// Describe implements prometheus.Collector. +func (v *SettableConstMetricVec) Describe(ch chan<- *prometheus.Desc) { + ch <- v.desc +} + +// Collect implements prometheus.Collector. +func (v *SettableConstMetricVec) Collect(ch chan<- prometheus.Metric) { + v.mu.RLock() + keys := make([]string, 0, len(v.samples)) + for key := range v.samples { + keys = append(keys, key) + } + sort.Strings(keys) + + samples := make([]settableConstMetricSample, 0, len(keys)) + for _, key := range keys { + samples = append(samples, v.samples[key]) + } + v.mu.RUnlock() + + for _, sample := range samples { + metric, err := prometheus.NewConstMetric(v.desc, v.valueType, sample.value, sample.labelValues...) + if err != nil { + ch <- prometheus.NewInvalidMetric(v.desc, err) + continue + } + ch <- metric + } +} + +// SettableConstMetric is a handle for one labeled sample. +type SettableConstMetric struct { + vec *SettableConstMetricVec + labelValues []string + key string +} + +// Set stores the current absolute value for this sample. +func (m *SettableConstMetric) Set(value float64) { + m.vec.mu.Lock() + defer m.vec.mu.Unlock() + m.vec.samples[m.key] = settableConstMetricSample{ + labelValues: append([]string(nil), m.labelValues...), + value: value, + } +} + +func newConstMetricDesc(opts prometheus.Opts, variableLabels []string, constLabels prometheus.Labels) *prometheus.Desc { + return prometheus.NewDesc( + prometheus.BuildFQName(opts.Namespace, opts.Subsystem, opts.Name), + opts.Help, + variableLabels, + constLabels, + ) +} + +func containsLabel(labels []string, name string) bool { + for _, label := range labels { + if label == name { + return true + } + } + return false +} + +func copyLabels(labels prometheus.Labels) prometheus.Labels { + if labels == nil { + return nil + } + copied := make(prometheus.Labels, len(labels)) + for name, value := range labels { + copied[name] = value + } + return copied +} + +func mergeLabels(labelSets ...prometheus.Labels) prometheus.Labels { + var merged prometheus.Labels + for _, labels := range labelSets { + for name, value := range labels { + if merged == nil { + merged = make(prometheus.Labels) + } + merged[name] = value + } + } + return merged +} diff --git a/third_party/fleet-intelligence-sdk/pkg/metrics/settable_const_metric_test.go b/third_party/fleet-intelligence-sdk/pkg/metrics/settable_const_metric_test.go new file mode 100644 index 00000000..b5434a38 --- /dev/null +++ b/third_party/fleet-intelligence-sdk/pkg/metrics/settable_const_metric_test.go @@ -0,0 +1,79 @@ +// Copyright 2024 Lepton AI Inc +// Source: https://github.com/leptonai/gpud + +package metrics + +import ( + "testing" + + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" +) + +func TestSettableCounterVecEmitsCounterMetric(t *testing.T) { + registry := prometheus.NewRegistry() + counter := NewSettableCounterVec( + prometheus.CounterOpts{ + Name: "external_absolute_total", + Help: "external absolute counter", + }, + []string{MetricComponentLabelKey, "device"}, + ).MustCurryWith(prometheus.Labels{MetricComponentLabelKey: "component1"}) + + if err := registry.Register(counter); err != nil { + t.Fatalf("Register() failed: %v", err) + } + + counter.With(prometheus.Labels{"device": "gpu0"}).Set(42) + + families, err := registry.Gather() + if err != nil { + t.Fatalf("Gather() failed: %v", err) + } + + family := findMetricFamily(families, "external_absolute_total") + if family == nil { + t.Fatalf("metric family was not gathered") + } + if family.GetType() != dto.MetricType_COUNTER { + t.Fatalf("metric type = %v, want %v", family.GetType(), dto.MetricType_COUNTER) + } + if got := family.GetMetric()[0].GetCounter().GetValue(); got != 42 { + t.Fatalf("counter value = %v, want 42", got) + } +} + +func TestSettableCounterVecResetAndDelete(t *testing.T) { + counter := NewSettableCounterVec( + prometheus.CounterOpts{ + Name: "resettable_external_absolute_total", + Help: "external absolute counter", + }, + []string{"device"}, + ) + + labels := prometheus.Labels{"device": "gpu0"} + counter.With(labels).Set(42) + + if !counter.Delete(labels) { + t.Fatalf("Delete() = false, want true") + } + if counter.Delete(labels) { + t.Fatalf("Delete() after delete = true, want false") + } + + counter.With(labels).Set(43) + counter.Reset() + if counter.Delete(labels) { + t.Fatalf("Delete() after reset = true, want false") + } +} + +func findMetricFamily(families []*dto.MetricFamily, name string) *dto.MetricFamily { + for _, family := range families { + if family.GetName() == name { + return family + } + } + return nil +} diff --git a/third_party/fleet-intelligence-sdk/pkg/metrics/store/sqlite.go b/third_party/fleet-intelligence-sdk/pkg/metrics/store/sqlite.go index fc378219..d0518e73 100644 --- a/third_party/fleet-intelligence-sdk/pkg/metrics/store/sqlite.go +++ b/third_party/fleet-intelligence-sdk/pkg/metrics/store/sqlite.go @@ -10,6 +10,7 @@ import ( "encoding/json" "errors" "fmt" + "regexp" "strings" "time" @@ -43,6 +44,9 @@ const ( // columnMetricValue represents the numeric value of the metric. columnMetricValue = "metric_value" + + // columnMetricType represents the Prometheus metric type. + columnMetricType = "metric_type" ) // TODO: drop the old table "gpud_metrics" @@ -58,6 +62,8 @@ var ( var _ pkgmetrics.Store = &sqliteStore{} +var sqliteIdentifierRE = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) + type sqliteStore struct { dbRW *sql.DB dbRO *sql.DB @@ -99,13 +105,17 @@ CREATE TABLE IF NOT EXISTS %s ( %s TEXT NOT NULL, %s TEXT, %s REAL NOT NULL, + %s TEXT NOT NULL DEFAULT '%s', PRIMARY KEY (%s, %s, %s, %s) ) WITHOUT ROWID;`, table, - columnUnixMilliseconds, columnComponentName, columnMetricName, columnMetricLabels, columnMetricValue, // columns + columnUnixMilliseconds, columnComponentName, columnMetricName, columnMetricLabels, columnMetricValue, columnMetricType, pkgmetrics.MetricTypeGauge, // columns columnUnixMilliseconds, columnComponentName, columnMetricName, columnMetricLabels, // primary keys )) - return err + if err != nil { + return err + } + return ensureMetricTypeColumn(ctx, dbRW, table) } func insert(ctx context.Context, dbRW *sql.DB, table string, ms ...pkgmetrics.Metric) error { @@ -129,23 +139,24 @@ func insert(ctx context.Context, dbRW *sql.DB, table string, ms ...pkgmetrics.Me // Build the query with placeholders for all metrics query := fmt.Sprintf( - "INSERT OR REPLACE INTO %s (%s, %s, %s, %s, %s) VALUES ", + "INSERT OR REPLACE INTO %s (%s, %s, %s, %s, %s, %s) VALUES ", table, columnUnixMilliseconds, columnComponentName, columnMetricName, columnMetricLabels, columnMetricValue, + columnMetricType, ) // Create proper placeholders with commas between value sets placeholders := make([]string, len(ms)) for i := range placeholders { - placeholders[i] = "(?, ?, ?, ?, ?)" + placeholders[i] = "(?, ?, ?, ?, ?, ?)" } query += strings.Join(placeholders, ", ") - args := make([]interface{}, 0, len(ms)*5) + args := make([]interface{}, 0, len(ms)*6) for _, m := range ms { labels := "" if len(m.Labels) > 0 { @@ -155,7 +166,11 @@ func insert(ctx context.Context, dbRW *sql.DB, table string, ms ...pkgmetrics.Me } labels = string(b) } - args = append(args, m.UnixMilliseconds, m.Component, m.Name, labels, m.Value) + metricType := m.Type + if metricType == "" { + metricType = pkgmetrics.MetricTypeGauge + } + args = append(args, m.UnixMilliseconds, m.Component, m.Name, labels, m.Value, metricType) } log.Logger.Infow("inserting metrics", "metrics", len(ms)) @@ -205,7 +220,7 @@ func read(ctx context.Context, dbRO *sql.DB, table string, opts ...pkgmetrics.Op whereStatement = fmt.Sprintf("WHERE %s", whereStatement) } - query := fmt.Sprintf(`SELECT %s, %s, %s, %s, %s + query := fmt.Sprintf(`SELECT %s, %s, %s, %s, %s, %s FROM %s `, columnUnixMilliseconds, @@ -213,6 +228,7 @@ FROM %s columnMetricName, columnMetricLabels, columnMetricValue, + columnMetricType, table, ) if whereStatement != "" { @@ -240,9 +256,12 @@ FROM %s for queryRows.Next() { m := pkgmetrics.Metric{} var labels sql.NullString - if err := queryRows.Scan(&m.UnixMilliseconds, &m.Component, &m.Name, &labels, &m.Value); err != nil { + if err := queryRows.Scan(&m.UnixMilliseconds, &m.Component, &m.Name, &labels, &m.Value, &m.Type); err != nil { return nil, err } + if m.Type == "" { + m.Type = pkgmetrics.MetricTypeGauge + } if labels.Valid && labels.String != "" { lm := make(map[string]string, 0) if err := json.Unmarshal([]byte(labels.String), &lm); err != nil { @@ -258,6 +277,56 @@ FROM %s return rows, nil } +func ensureMetricTypeColumn(ctx context.Context, dbRW *sql.DB, table string) error { + quotedTable, err := quoteSQLiteIdentifier(table) + if err != nil { + return err + } + + rows, err := dbRW.QueryContext(ctx, fmt.Sprintf("PRAGMA table_info(%s);", quotedTable)) + if err != nil { + return err + } + defer func() { + _ = rows.Close() + }() + + for rows.Next() { + var cid int + var name, columnType string + var notNull int + var defaultValue sql.NullString + var pk int + if err := rows.Scan(&cid, &name, &columnType, ¬Null, &defaultValue, &pk); err != nil { + return err + } + if name == columnMetricType { + return nil + } + } + if err := rows.Err(); err != nil { + return err + } + + _, err = dbRW.ExecContext(ctx, fmt.Sprintf( + "ALTER TABLE %s ADD COLUMN %s TEXT NOT NULL DEFAULT '%s';", + quotedTable, + columnMetricType, + pkgmetrics.MetricTypeGauge, + )) + return err +} + +func quoteSQLiteIdentifier(name string) (string, error) { + if name == "" { + return "", ErrEmptyTableName + } + if !sqliteIdentifierRE.MatchString(name) { + return "", fmt.Errorf("invalid sqlite identifier %q", name) + } + return `"` + name + `"`, nil +} + // purge purges the data for the corresponding component that is older // than the given time. func purge(ctx context.Context, dbRW *sql.DB, table string, before time.Time) (int, error) { diff --git a/third_party/fleet-intelligence-sdk/pkg/metrics/store/sqlite_test.go b/third_party/fleet-intelligence-sdk/pkg/metrics/store/sqlite_test.go index 90593f35..392e4931 100644 --- a/third_party/fleet-intelligence-sdk/pkg/metrics/store/sqlite_test.go +++ b/third_party/fleet-intelligence-sdk/pkg/metrics/store/sqlite_test.go @@ -34,6 +34,55 @@ func TestSQLiteNewStore(t *testing.T) { assert.Equal(t, ErrEmptyTableName, err) } +func TestSQLiteNewStore_AddsMetricTypeColumnToExistingTable(t *testing.T) { + dbRW, dbRO, cleanup := pkgsqlite.OpenTestDB(t) + defer cleanup() + + ctx := context.Background() + table := "test_metrics_legacy" + _, err := dbRW.ExecContext(ctx, fmt.Sprintf(` +CREATE TABLE %s ( + %s INTEGER NOT NULL, + %s TEXT NOT NULL, + %s TEXT NOT NULL, + %s TEXT, + %s REAL NOT NULL, + PRIMARY KEY (%s, %s, %s, %s) +) WITHOUT ROWID;`, + table, + columnUnixMilliseconds, columnComponentName, columnMetricName, columnMetricLabels, columnMetricValue, + columnUnixMilliseconds, columnComponentName, columnMetricName, columnMetricLabels, + )) + require.NoError(t, err) + + store, err := NewSQLiteStore(ctx, dbRW, dbRO, table) + require.NoError(t, err) + + err = store.Record(ctx, pkgmetrics.Metric{ + UnixMilliseconds: time.Now().UnixMilli(), + Component: "test-component", + Name: "counter_metric", + Type: pkgmetrics.MetricTypeCounter, + Value: 10, + }) + require.NoError(t, err) + + results, err := store.Read(ctx) + require.NoError(t, err) + require.Len(t, results, 1) + assert.Equal(t, pkgmetrics.MetricTypeCounter, results[0].Type) +} + +func TestEnsureMetricTypeColumnRejectsInvalidTableName(t *testing.T) { + dbRW, _, cleanup := pkgsqlite.OpenTestDB(t) + defer cleanup() + + ctx := context.Background() + err := ensureMetricTypeColumn(ctx, dbRW, `test_metrics; DROP TABLE test_metrics;`) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid sqlite identifier") +} + func TestSQLiteStore_Record(t *testing.T) { // Setup test database dbRW, dbRO, cleanup := pkgsqlite.OpenTestDB(t) @@ -52,6 +101,7 @@ func TestSQLiteStore_Record(t *testing.T) { UnixMilliseconds: now, Component: "test-component", Name: "metric1", + Type: pkgmetrics.MetricTypeCounter, Value: 42.0, }, { @@ -90,6 +140,7 @@ func TestSQLiteStore_Record(t *testing.T) { UnixMilliseconds: now, Component: "test-component", Name: "metric1", + Type: pkgmetrics.MetricTypeCounter, Value: 99.9, } err = store.Record(ctx, updatedMetric) @@ -102,8 +153,12 @@ func TestSQLiteStore_Record(t *testing.T) { for _, m := range results { if m.Component == "test-component" && m.Name == "metric1" { assert.Equal(t, 99.9, m.Value) + assert.Equal(t, pkgmetrics.MetricTypeCounter, m.Type) found = true } + if m.Component == "test-component" && m.Name == "metric2" { + assert.Equal(t, pkgmetrics.MetricTypeGauge, m.Type) + } } assert.True(t, found, "Updated metric not found in results") } diff --git a/third_party/fleet-intelligence-sdk/pkg/metrics/types.go b/third_party/fleet-intelligence-sdk/pkg/metrics/types.go index f04ba4d7..759efc40 100644 --- a/third_party/fleet-intelligence-sdk/pkg/metrics/types.go +++ b/third_party/fleet-intelligence-sdk/pkg/metrics/types.go @@ -11,6 +11,13 @@ import ( // MetricComponentLabelKey is the key for the component of the metric. const MetricComponentLabelKey = "gpud_component" +type MetricType string + +const ( + MetricTypeGauge MetricType = "gauge" + MetricTypeCounter MetricType = "counter" +) + // Metric represents a metric row in the database table. type Metric struct { // UnixMilliseconds represents the Unix timestamp of the metric. @@ -19,6 +26,8 @@ type Metric struct { Component string `json:"component"` // Name represents the name of the metric. Name string `json:"name"` + // Type represents the Prometheus metric type. + Type MetricType `json:"type,omitempty"` // Value represents the numeric value of the metric. Value float64 `json:"value"` From da5ff1854aeda7763c2935aeb3b2c141f8c40f30 Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Wed, 27 May 2026 11:19:35 -0700 Subject: [PATCH 06/16] ci: harden github actions workflows (#205) Signed-off-by: Jingxiang Zhang --- .github/workflows/ci.yml | 93 ++++++++++++----------------------- .github/workflows/release.yml | 56 ++------------------- sonar-project.properties | 23 --------- 3 files changed, 34 insertions(+), 138 deletions(-) delete mode 100644 sonar-project.properties diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cb26847f..cc9cba44 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,10 +10,13 @@ env: GO_VERSION: '1.26.3' GOFLAGS: '-trimpath' +permissions: + contents: read + jobs: secret-detection: name: Secret Detection - runs-on: [self-hosted, rocky8] + runs-on: ubuntu-latest steps: - name: Checkout code uses: actions/checkout@v4 @@ -53,7 +56,7 @@ jobs: lint: name: Lint - runs-on: [self-hosted, rocky8] + runs-on: ubuntu-latest steps: - name: Checkout code uses: actions/checkout@v4 @@ -91,7 +94,7 @@ jobs: unit_test: name: Unit Tests - runs-on: [self-hosted, rocky8] + runs-on: ubuntu-latest steps: - name: Checkout code uses: actions/checkout@v4 @@ -121,7 +124,7 @@ jobs: build_check: name: Build Check - runs-on: [self-hosted, rocky8] + runs-on: ubuntu-latest needs: [lint] strategy: matrix: @@ -139,6 +142,12 @@ jobs: - name: Download dependencies run: go mod download + - name: Install ARM64 cross compiler + if: matrix.goarch == 'arm64' + run: | + sudo apt-get update + sudo apt-get install -y gcc-aarch64-linux-gnu g++-aarch64-linux-gnu + - name: Clean previous build artifacts run: | rm -f fleetint-${{ matrix.goos }}-${{ matrix.goarch }} || true @@ -168,45 +177,13 @@ jobs: path: fleetint-${{ matrix.goos }}-${{ matrix.goarch }} retention-days: 7 - sonarqube: - name: SonarQube Analysis - runs-on: [self-hosted, rocky8] - needs: [unit_test] - steps: - - name: Checkout code - uses: actions/checkout@v4 - with: - fetch-depth: 0 # Shallow clones should be disabled for better relevancy of analysis - - - name: Set up Go - uses: actions/setup-go@v4 - with: - go-version: ${{ env.GO_VERSION }} - - - name: Download dependencies - run: go mod download - - - name: Run unit tests with coverage - run: make test - - - name: SonarQube Scan - uses: SonarSource/sonarqube-scan-action@v7 - env: - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }} - - # Optional: Fail the job if the Quality Gate fails - # Uncomment if you want to enforce quality gate in CI - # - name: SonarQube Quality Gate check - # uses: SonarSource/sonarqube-quality-gate-action@v1 - # timeout-minutes: 5 - # env: - # SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - - package_snapshot: - name: Package Snapshot - runs-on: [self-hosted, rocky8] - needs: [unit_test, build_check] + codeql: + name: CodeQL Analysis + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write steps: - name: Checkout code uses: actions/checkout@v4 @@ -221,26 +198,18 @@ jobs: - name: Download dependencies run: go mod download - - name: Clean previous dist directory - run: | - rm -rf ./dist || true - - - name: Run GoReleaser snapshot - uses: goreleaser/goreleaser-action@v5 + - name: Initialize CodeQL + uses: github/codeql-action/init@v4 with: - distribution: goreleaser - version: v2.4.1 - args: release --snapshot --clean --config .goreleaser.yaml + languages: go + build-mode: manual env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + CODEQL_EXTRACTOR_GO_BUILD_TRACING: on - - name: Upload snapshot packages - uses: actions/upload-artifact@v4 + - name: Build with CodeQL + run: go build ./... + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v4 with: - name: snapshot-packages - path: | - dist/*.deb - dist/*.rpm - dist/*.tar.gz - dist/checksums.txt - retention-days: 7 + category: "/language:go" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b1c15005..8531640a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -61,6 +61,7 @@ jobs: run: | VERSION=${GITHUB_REF#refs/tags/v} COMMIT_SHORT=$(git rev-parse --short=7 HEAD) + BUILD_TIMESTAMP=$(git show -s --format=%cI HEAD) BRANCH="release" SOURCE="https://github.com/${{ github.repository }}/archive/refs/tags/${{ github.ref_name }}.tar.gz" @@ -75,24 +76,11 @@ jobs: echo "version=$VERSION" >> $GITHUB_OUTPUT echo "commit=$COMMIT_SHORT" >> $GITHUB_OUTPUT + echo "build_timestamp=$BUILD_TIMESTAMP" >> $GITHUB_OUTPUT echo "branch=$BRANCH" >> $GITHUB_OUTPUT echo "source=$SOURCE" >> $GITHUB_OUTPUT echo "is_prerelease=$IS_PRERELEASE" >> $GITHUB_OUTPUT - - name: Prepare fleet-intelligence-agent image tags - id: agent_image - run: | - set -euo pipefail - VERSION="${{ steps.version.outputs.version }}" - IMAGE="nvcr.io/0753215517602916/agent-artifact/fleet-intelligence-agent" - echo "image=${IMAGE}" >> "$GITHUB_OUTPUT" - echo "tags<> "$GITHUB_OUTPUT" - echo "${IMAGE}:${VERSION}" >> "$GITHUB_OUTPUT" - if [ "${{ steps.version.outputs.is_prerelease }}" = "false" ]; then - echo "${IMAGE}:latest" >> "$GITHUB_OUTPUT" - fi - echo "EOF" >> "$GITHUB_OUTPUT" - - name: Prepare fleet-intelligence-agent GHCR image tags id: agent_image_ghcr run: | @@ -115,13 +103,6 @@ jobs: - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 - - name: Login to NVCR (nvcr.io) - uses: docker/login-action@v3 - with: - registry: nvcr.io - username: $oauthtoken - password: ${{ secrets.NVCR_API_KEY }} - - name: Login to GHCR (ghcr.io) uses: docker/login-action@v3 with: @@ -137,12 +118,11 @@ jobs: platforms: linux/amd64,linux/arm64 push: true tags: | - ${{ steps.agent_image.outputs.tags }} ${{ steps.agent_image_ghcr.outputs.tags }} build-args: | VERSION=${{ steps.version.outputs.version }} REVISION=${{ steps.version.outputs.commit }} - BUILD_TIMESTAMP=${{ github.event.head_commit.timestamp }} + BUILD_TIMESTAMP=${{ steps.version.outputs.build_timestamp }} - name: Package Helm chart run: | @@ -151,31 +131,6 @@ jobs: --version "${{ steps.version.outputs.version }}" \ --app-version "${{ steps.version.outputs.version }}" - - name: Ensure Helm push plugin is installed - run: | - set -euo pipefail - if helm plugin list | awk '{print $1}' | grep -qx 'cm-push'; then - echo "helm-push plugin already installed" - else - helm plugin install https://github.com/chartmuseum/helm-push - fi - - - name: Add NGC Helm repo - env: - NGC_API_KEY: ${{ secrets.NVCR_API_KEY }} - run: | - set -euo pipefail - helm repo remove ngc || true - helm repo add ngc https://helm.ngc.nvidia.com/0753215517602916/agent-artifact \ - --username '$oauthtoken' \ - --password "${NGC_API_KEY}" - - - name: Push Helm chart to NGC Helm repo - run: | - set -euo pipefail - CHART_VERSION="${{ steps.version.outputs.version }}" - helm cm-push "dist/fleet-intelligence-agent-${CHART_VERSION}.tgz" ngc - - name: Login to GHCR Helm registry run: | set -euo pipefail @@ -234,13 +189,8 @@ jobs: echo "**Pre-release:** ${{ steps.version.outputs.is_prerelease }}" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY echo "**fleet-intelligence-agent image tags:**" >> $GITHUB_STEP_SUMMARY - echo "${{ steps.agent_image.outputs.tags }}" >> $GITHUB_STEP_SUMMARY echo "${{ steps.agent_image_ghcr.outputs.tags }}" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY - echo "**Helm chart (NGC Helm repo):**" >> $GITHUB_STEP_SUMMARY - echo "repo: https://helm.ngc.nvidia.com/0753215517602916/agent-artifact" >> $GITHUB_STEP_SUMMARY - echo "chart: fleet-intelligence-agent-${{ steps.version.outputs.version }}.tgz" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY echo "**Helm chart (GHCR OCI):**" >> $GITHUB_STEP_SUMMARY echo "repo: oci://ghcr.io/${{ steps.agent_image_ghcr.outputs.owner_lc }}/charts" >> $GITHUB_STEP_SUMMARY echo "chart: fleet-intelligence-agent:${{ steps.version.outputs.version }}" >> $GITHUB_STEP_SUMMARY diff --git a/sonar-project.properties b/sonar-project.properties deleted file mode 100644 index bd6c5367..00000000 --- a/sonar-project.properties +++ /dev/null @@ -1,23 +0,0 @@ -# Project identification -sonar.projectKey=SW-Cloud_GPU-Health_GPU-Health_gpu_health_agent -sonar.projectName=GPU Health Agent -sonar.projectVersion=1.0 - -# Quality Gate - Wait for quality gate result (blocks pipeline if quality gate fails) -sonar.qualitygate.wait=true - -# Source code -sonar.sources=. -sonar.exclusions=**/*_test.go,**/mock_*.go,Dockerfile.citests,third_party/fleet-intelligence-sdk/pkg/providers/** - -# Test sources -sonar.tests=. -sonar.test.inclusions=**/*_test.go -sonar.test.exclusions=**/vendor/**,**/third_party/** - -# Go specific settings -sonar.sourceEncoding=UTF-8 -sonar.go.coverage.reportPaths=coverage/coverage.out - -# Coverage exclusions -sonar.coverage.exclusions=**/cmd/fleetint/**,**/docs/**,**/routes/**,**/testutil/**,**/*_test.go,**/mock_*.go,**/internal/server/**,**/third_party/** From f850721560e5dbb72069ae7d26b97023fc13bd5a Mon Sep 17 00:00:00 2001 From: ambermingxin Date: Tue, 2 Jun 2026 09:35:06 -0700 Subject: [PATCH 07/16] fix: remove warning message (#214) Signed-off-by: Amber Xue --- docs/install-deb.md | 2 +- docs/install-rpm.md | 2 +- internal/validation/outbound/validator.go | 18 +++++--- .../validation/outbound/validator_test.go | 45 +++++++++++++++++++ 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/docs/install-deb.md b/docs/install-deb.md index 6aa9fb04..6ea3e246 100644 --- a/docs/install-deb.md +++ b/docs/install-deb.md @@ -59,7 +59,7 @@ sudo apt install ./fleetint_VERSION_arm64.deb Verify: ```bash -fleetint --version +sudo fleetint --version systemctl status fleetintd ``` diff --git a/docs/install-rpm.md b/docs/install-rpm.md index c9cf8b03..ce32f4ff 100644 --- a/docs/install-rpm.md +++ b/docs/install-rpm.md @@ -84,7 +84,7 @@ sudo dnf install ./fleetint-VERSION-1.aarch64.rpm Verify: ```bash -fleetint --version +sudo fleetint --version systemctl status fleetintd ``` diff --git a/internal/validation/outbound/validator.go b/internal/validation/outbound/validator.go index 11074ab7..ebe92775 100644 --- a/internal/validation/outbound/validator.go +++ b/internal/validation/outbound/validator.go @@ -106,7 +106,7 @@ func ValidateNodeUpsertRequest(req *backendclient.NodeUpsertRequest) []Issue { prefix := fmt.Sprintf("resources.gpuInfo.gpus[%d]", i) validateLen(&issues, "length", prefix+".uuid", gpu.UUID, 255) validateLen(&issues, "length", prefix+".gpuIndex", gpu.GPUIndex, 64) - validateUUID(&issues, "format", prefix+".uuid", gpu.UUID) + validateGPUUUID(&issues, "format", prefix+".uuid", gpu.UUID) if gpu.UUID != "" { if _, exists := seenGPUUUID[gpu.UUID]; exists { issues = append(issues, newIssue(SeverityWarning, "dedup", prefix+".uuid", "duplicate GPU UUID in payload")) @@ -317,12 +317,20 @@ func validateIP(issues *[]Issue, category, field, value string) { } } -func validateUUID(issues *[]Issue, category, field, value string) { - if strings.TrimSpace(value) == "" { +// validateGPUUUID checks NVIDIA GPU UUIDs as returned by NVML/nvidia-smi (e.g. +// "GPU-b1631f34-4ee6-b7a8-5afc-00c88d0fcae1"). The "GPU-" prefix is optional; +// the remainder must be a valid RFC 4122 UUID. +func validateGPUUUID(issues *[]Issue, category, field, value string) { + value = strings.TrimSpace(value) + if value == "" { return } - if _, err := uuid.Parse(strings.TrimSpace(value)); err != nil { - *issues = append(*issues, newIssue(SeverityWarning, category, field, "invalid UUID format")) + uuidPart := value + if strings.HasPrefix(value, "GPU-") { + uuidPart = strings.TrimPrefix(value, "GPU-") + } + if _, err := uuid.Parse(uuidPart); err != nil { + *issues = append(*issues, newIssue(SeverityWarning, category, field, "invalid GPU UUID format")) } } diff --git a/internal/validation/outbound/validator_test.go b/internal/validation/outbound/validator_test.go index 8c0cecd7..f5ac6a73 100644 --- a/internal/validation/outbound/validator_test.go +++ b/internal/validation/outbound/validator_test.go @@ -72,6 +72,51 @@ func TestValidateNodeUpsertRequest(t *testing.T) { }) } +func TestValidateNodeUpsertRequest_GPUUUID(t *testing.T) { + t.Run("accepts NVIDIA GPU UUID format", func(t *testing.T) { + req := &backendclient.NodeUpsertRequest{ + Hostname: "host", + MachineID: "machine-1", + SystemUUID: "4c4c4544-004a-3410-804c-c7c04f313434", + Resources: backendclient.NodeResources{ + GPUInfo: backendclient.GPUInfo{ + GPUs: []backendclient.GPUDevice{{ + UUID: "GPU-b1631f34-4ee6-b7a8-5afc-00c88d0fcae1", + }}, + }, + }, + } + + for _, issue := range ValidateNodeUpsertRequest(req) { + require.NotEqual(t, "resources.gpuInfo.gpus[0].uuid", issue.Field, + "unexpected GPU UUID validation issue: %+v", issue) + } + }) + + t.Run("rejects malformed GPU UUID", func(t *testing.T) { + req := &backendclient.NodeUpsertRequest{ + Hostname: "host", + MachineID: "machine-1", + SystemUUID: "4c4c4544-004a-3410-804c-c7c04f313434", + Resources: backendclient.NodeResources{ + GPUInfo: backendclient.GPUInfo{ + GPUs: []backendclient.GPUDevice{{ + UUID: "GPU-not-a-valid-uuid", + }}, + }, + }, + } + + issues := ValidateNodeUpsertRequest(req) + require.Contains(t, issues, Issue{ + Severity: SeverityWarning, + Category: "format", + Field: "resources.gpuInfo.gpus[0].uuid", + Message: "invalid GPU UUID format", + }) + }) +} + func TestValidateAttestationRequest(t *testing.T) { req := &backendclient.AttestationRequest{ AttestationData: backendclient.AttestationData{ From f981591b2d112edc68aa9626e1c0722a81ac2854 Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Thu, 4 Jun 2026 10:10:21 -0700 Subject: [PATCH 08/16] chore: update go version to 1.26.4 (#215) Signed-off-by: Jingxiang Zhang --- .github/workflows/ci.yml | 2 +- .github/workflows/release.yml | 2 +- .golangci.yml | 2 +- CONTRIBUTING.md | 2 +- Dockerfile | 2 +- Dockerfile.citests | 2 +- docs/development.md | 2 +- go.mod | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cc9cba44..0a78ad27 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,7 @@ on: branches: [ main, 'release/**' ] env: - GO_VERSION: '1.26.3' + GO_VERSION: '1.26.4' GOFLAGS: '-trimpath' permissions: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8531640a..927d0ef7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -6,7 +6,7 @@ on: - 'v*' # Triggers on version tags like v1.0.0, v2.1.3, etc. env: - GO_VERSION: '1.26.3' + GO_VERSION: '1.26.4' permissions: contents: write # Needed for creating GitHub releases diff --git a/.golangci.yml b/.golangci.yml index 40ec47f7..32ff19e9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,6 @@ version: "2" run: - go: 1.26.3 + go: 1.26.4 linters: default: none enable: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1a3e50bd..01725e16 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,7 +7,7 @@ Please start all enhancement, bugfix, or change requests by opening a GitHub iss ## Development ### Prerequisites -- Go 1.24+ (see `go.mod`) +- Go 1.26.4+ (see `go.mod`) - Make - golangci-lint (optional locally, required in CI) diff --git a/Dockerfile b/Dockerfile index 4fc07e20..7adb9a08 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,6 @@ ARG DCGM_VERSION="4.4.2-1-ubuntu22.04" -FROM golang:1.26.3 AS build +FROM golang:1.26.4 AS build RUN apt-get update && apt-get install -y --no-install-recommends \ ca-certificates \ diff --git a/Dockerfile.citests b/Dockerfile.citests index 48d488f5..227bf94e 100644 --- a/Dockerfile.citests +++ b/Dockerfile.citests @@ -16,7 +16,7 @@ # Run specific package tests: # docker run --rm fleet-intelligence-agent:test go test -v ./internal/... # -FROM golang:1.26.3 +FROM golang:1.26.4 ARG TARGETARCH diff --git a/docs/development.md b/docs/development.md index 593a849e..27ffc528 100644 --- a/docs/development.md +++ b/docs/development.md @@ -11,7 +11,7 @@ cd fleet-intelligence-agent ### Prerequisites -- **Go**: 1.26.3 +- **Go**: 1.26.4 - **GoReleaser**: For building packages (install from [goreleaser.com](https://goreleaser.com/install/)) - **For ARM64 cross-compilation** (optional): `gcc-aarch64-linux-gnu` and `g++-aarch64-linux-gnu` diff --git a/go.mod b/go.mod index 8a6f8fd5..e70c18eb 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/NVIDIA/fleet-intelligence-agent -go 1.26.3 +go 1.26.4 require ( github.com/NVIDIA/fleet-intelligence-sdk v0.0.0-00010101000000-000000000000 From 1e6871bf10847f98ea92bb1c6857c281de5142ca Mon Sep 17 00:00:00 2001 From: Michael O'Neill Date: Thu, 4 Jun 2026 23:48:19 +0100 Subject: [PATCH 09/16] fix: OTLP export silently skips when endpoints are configured but no auth token is present. (#199) Signed-off-by: Michael O'Neill --- internal/exporter/exporter.go | 3 +- internal/exporter/exporter_test.go | 75 ++++++++++++++++++++---------- 2 files changed, 52 insertions(+), 26 deletions(-) diff --git a/internal/exporter/exporter.go b/internal/exporter/exporter.go index e903fa5f..cccabad5 100644 --- a/internal/exporter/exporter.go +++ b/internal/exporter/exporter.go @@ -243,8 +243,7 @@ func (e *healthExporter) exportToHTTP(ctx context.Context, data *collector.Healt } if e.options.config.AuthToken == "" { - log.Logger.Infow("No auth token configured, skipping HTTP export") - return nil + log.Logger.Warnw("No auth token configured, exporting without authentication") } newToken, err := e.httpWriter.Send(ctx, data, e.options.config.MetricsEndpoint, e.options.config.LogsEndpoint, e.options.config.RetryMaxAttempts, e.options.config.AuthToken) diff --git a/internal/exporter/exporter_test.go b/internal/exporter/exporter_test.go index 8bc470ab..c85304ea 100644 --- a/internal/exporter/exporter_test.go +++ b/internal/exporter/exporter_test.go @@ -25,6 +25,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "sync/atomic" "testing" "time" @@ -639,17 +640,36 @@ func TestExportToHTTP(t *testing.T) { require.NoError(t, err) }) - t.Run("skips when no auth token configured", func(t *testing.T) { + t.Run("updates token from server response", func(t *testing.T) { + newToken := "new-test-token" + + // Create mock server that returns new token + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("jwt_assertion", newToken) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("SUCCESS")) + })) + defer server.Close() + cfg := &config.HealthExporterConfig{ Interval: metav1.Duration{Duration: 1 * time.Minute}, Timeout: metav1.Duration{Duration: 30 * time.Second}, - MetricsEndpoint: "http://example.com", - LogsEndpoint: "http://example.com", - AuthToken: "", + MetricsEndpoint: server.URL, + LogsEndpoint: server.URL, + AuthToken: "old-token", } ctx := context.Background() - exporter, err := New(ctx, WithConfig(cfg), WithMachineID("test-machine-id")) + + // Create temporary database for testing token updates + tmpDB := setupTestDB(t) + defer tmpDB.Close() + + exporter, err := New(ctx, + WithConfig(cfg), + WithDatabaseConnections(tmpDB, tmpDB), + WithMachineID("test-machine-id"), + ) require.NoError(t, err) require.NotNil(t, exporter) @@ -658,24 +678,39 @@ func TestExportToHTTP(t *testing.T) { healthData := &collector.HealthData{ MachineID: "test-machine", Timestamp: time.Now(), + Metrics: []pkgmetrics.Metric{ + { + Name: "test_metric", + Value: 42.0, + UnixMilliseconds: time.Now().UnixMilli(), + }, + }, } err = he.exportToHTTP(ctx, healthData) - require.NoError(t, err) // Should not error, just skip gracefully + require.NoError(t, err) + + // Verify token was updated in config + assert.Equal(t, newToken, he.options.config.AuthToken, "token should be refreshed in config") + + // Verify token was persisted to DB + storedToken, err := pkgmetadata.ReadMetadata(ctx, tmpDB, pkgmetadata.MetadataKeyToken) + require.NoError(t, err) + assert.Equal(t, newToken, storedToken, "token should be persisted to database") // Cleanup err = exporter.Stop() require.NoError(t, err) }) - t.Run("updates token from server response", func(t *testing.T) { - newToken := "new-test-token" - - // Create mock server that returns new token + t.Run("exports without auth token when endpoints configured", func(t *testing.T) { + // Create mock server that accepts requests without Authorization header + var requestCount atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("X-New-JWT-Token", newToken) + requestCount.Add(1) + assert.Equal(t, http.MethodPost, r.Method) + assert.Empty(t, r.Header.Get("Authorization"), "should not send Authorization header when token is empty") w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("SUCCESS")) })) defer server.Close() @@ -684,20 +719,11 @@ func TestExportToHTTP(t *testing.T) { Timeout: metav1.Duration{Duration: 30 * time.Second}, MetricsEndpoint: server.URL, LogsEndpoint: server.URL, - AuthToken: "old-token", + AuthToken: "", // No auth token - should still export } ctx := context.Background() - - // Create temporary database for testing token updates - tmpDB := setupTestDB(t) - defer tmpDB.Close() - - exporter, err := New(ctx, - WithConfig(cfg), - WithDatabaseConnections(tmpDB, tmpDB), - WithMachineID("test-machine-id"), - ) + exporter, err := New(ctx, WithConfig(cfg), WithMachineID("test-machine-id")) require.NoError(t, err) require.NotNil(t, exporter) @@ -717,11 +743,12 @@ func TestExportToHTTP(t *testing.T) { err = he.exportToHTTP(ctx, healthData) require.NoError(t, err) + assert.Greater(t, requestCount.Load(), int32(0), "expected at least one request to the server") - // Cleanup err = exporter.Stop() require.NoError(t, err) }) + } // TestExportNow tests the ExportNow function From c3095e9f8cb3b561c85a1ecc6afaae26fc0a453d Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Thu, 11 Jun 2026 11:00:48 -0700 Subject: [PATCH 10/16] fix: ubuntu deb package upgrade restart the service (#218) Signed-off-by: Jingxiang Zhang --- deployments/packages/scripts/postinst | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/deployments/packages/scripts/postinst b/deployments/packages/scripts/postinst index 6a8078e0..7d7c082e 100644 --- a/deployments/packages/scripts/postinst +++ b/deployments/packages/scripts/postinst @@ -18,17 +18,32 @@ else systemctl daemon-reload || echo "[WARNING] Failed to reload systemd daemon" case "$1" in - configure|install|1) + configure) + if [ -n "$2" ]; then + # $2 is the previous version — Debian upgrade path + echo "Upgrade detected - restarting service if already running..." + if systemctl is-active --quiet fleetintd.service; then + systemctl restart fleetintd.service || echo "[WARNING] Failed to restart service" + else + echo "Service is not running - not starting during upgrade" + fi + else + echo "Fresh installation - enabling and starting service..." + systemctl enable fleetintd.service || echo "[WARNING] Failed to enable service" + systemctl start fleetintd.service || echo "[WARNING] Failed to start service" + fi + ;; + install|1) echo "Fresh installation - enabling and starting service..." systemctl enable fleetintd.service || echo "[WARNING] Failed to enable service" systemctl start fleetintd.service || echo "[WARNING] Failed to start service" ;; upgrade|2) - echo "Upgrade detected - (re)starting service..." + echo "Upgrade detected - restarting service if already running..." if systemctl is-active --quiet fleetintd.service; then systemctl restart fleetintd.service || echo "[WARNING] Failed to restart service" else - systemctl start fleetintd.service || echo "[WARNING] Failed to start service" + echo "Service is not running - not starting during upgrade" fi ;; abort-upgrade|abort-remove|abort-deconfigure) From 0df5b9089e9eaf91306a40a624887119a8206c31 Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Wed, 24 Jun 2026 09:44:37 -0700 Subject: [PATCH 11/16] fix: apt remove preserve service enable (#221) Signed-off-by: Jingxiang Zhang --- .goreleaser.yaml | 26 ++++--- deployments/packages/scripts/deb/postinst | 57 ++++++++++++++ deployments/packages/scripts/deb/postrm | 32 ++++++++ deployments/packages/scripts/deb/preinst | 15 ++++ deployments/packages/scripts/deb/prerm | 24 ++++++ deployments/packages/scripts/postinst | 79 -------------------- deployments/packages/scripts/postrm | 37 --------- deployments/packages/scripts/preinst | 18 ----- deployments/packages/scripts/prerm | 36 --------- deployments/packages/scripts/rpm/postinstall | 49 ++++++++++++ deployments/packages/scripts/rpm/postremove | 18 +++++ deployments/packages/scripts/rpm/preinstall | 12 +++ deployments/packages/scripts/rpm/preremove | 18 +++++ 13 files changed, 239 insertions(+), 182 deletions(-) create mode 100755 deployments/packages/scripts/deb/postinst create mode 100755 deployments/packages/scripts/deb/postrm create mode 100755 deployments/packages/scripts/deb/preinst create mode 100755 deployments/packages/scripts/deb/prerm delete mode 100644 deployments/packages/scripts/postinst delete mode 100644 deployments/packages/scripts/postrm delete mode 100644 deployments/packages/scripts/preinst delete mode 100644 deployments/packages/scripts/prerm create mode 100755 deployments/packages/scripts/rpm/postinstall create mode 100755 deployments/packages/scripts/rpm/postremove create mode 100755 deployments/packages/scripts/rpm/preinstall create mode 100755 deployments/packages/scripts/rpm/preremove diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 8024eacf..b2d8b8de 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -74,12 +74,13 @@ nfpms: bindir: /usr/bin contents: # License and notices + - src: LICENSE + dst: /usr/share/doc/fleetint/copyright - src: NOTICE dst: /usr/share/doc/fleetint/NOTICE # SystemD service file - src: deployments/packages/systemd/fleetintd.service dst: /usr/lib/systemd/system/fleetintd.service - type: config|noreplace # Environment configuration - src: deployments/packages/systemd/fleetint.env dst: /etc/default/fleetint @@ -88,12 +89,12 @@ nfpms: - dst: /var/lib/fleetint type: dir file_info: - mode: 0755 + mode: 0700 scripts: - preinstall: deployments/packages/scripts/preinst - postinstall: deployments/packages/scripts/postinst - preremove: deployments/packages/scripts/prerm - postremove: deployments/packages/scripts/postrm + preinstall: deployments/packages/scripts/deb/preinst + postinstall: deployments/packages/scripts/deb/postinst + preremove: deployments/packages/scripts/deb/prerm + postremove: deployments/packages/scripts/deb/postrm - id: fleetint-rpm package_name: fleetint @@ -113,12 +114,13 @@ nfpms: bindir: /usr/bin contents: # License and notices + - src: LICENSE + dst: /usr/share/licenses/fleetint/LICENSE - src: NOTICE dst: /usr/share/doc/fleetint/NOTICE # SystemD service file - src: deployments/packages/systemd/fleetintd.service dst: /usr/lib/systemd/system/fleetintd.service - type: config|noreplace # Environment configuration (RPM uses /etc/default like Debian) - src: deployments/packages/systemd/fleetint.env dst: /etc/default/fleetint @@ -127,12 +129,12 @@ nfpms: - dst: /var/lib/fleetint type: dir file_info: - mode: 0755 + mode: 0700 scripts: - preinstall: deployments/packages/scripts/preinst - postinstall: deployments/packages/scripts/postinst - preremove: deployments/packages/scripts/prerm - postremove: deployments/packages/scripts/postrm + preinstall: deployments/packages/scripts/rpm/preinstall + postinstall: deployments/packages/scripts/rpm/postinstall + preremove: deployments/packages/scripts/rpm/preremove + postremove: deployments/packages/scripts/rpm/postremove # Checksum configuration checksum: diff --git a/deployments/packages/scripts/deb/postinst b/deployments/packages/scripts/deb/postinst new file mode 100755 index 00000000..ba7d2ac5 --- /dev/null +++ b/deployments/packages/scripts/deb/postinst @@ -0,0 +1,57 @@ +#!/bin/sh +set -e + +service=fleetintd.service + +service_was_stopped=false +service_action="installed" + +if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ]; then + if command -v deb-systemd-helper >/dev/null 2>&1; then + deb-systemd-helper unmask "$service" >/dev/null || true + if deb-systemd-helper --quiet was-enabled "$service"; then + deb-systemd-helper enable "$service" >/dev/null || true + else + deb-systemd-helper update-state "$service" >/dev/null || true + fi + fi + + if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ]; then + systemctl --system daemon-reload >/dev/null || true + + if [ "$1" = "configure" ] && [ -n "$2" ]; then + service_action="upgraded" + if systemctl is-active --quiet "$service"; then + deb-systemd-invoke restart "$service" >/dev/null || true + else + service_was_stopped=true + fi + else + deb-systemd-invoke start "$service" >/dev/null || true + fi + else + echo "Systemd not available - manual service management required" + fi +fi + +echo "" +if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ]; then + if systemctl is-active --quiet "$service"; then + echo "[SUCCESS] Fleet Intelligence Agent ${service_action} and service is running" + echo "" + echo "Configuration: sudo vi /etc/default/fleetint" + echo "Logs: sudo journalctl -u fleetintd -f" + elif [ "$service_was_stopped" = true ]; then + echo "[INFO] Fleet Intelligence Agent upgraded; service was not running and was left stopped" + echo "Start manually: sudo systemctl start fleetintd" + else + echo "[WARNING] Fleet Intelligence Agent ${service_action} but service is not running" + echo "Start manually: sudo systemctl start fleetintd" + fi +else + echo "[SUCCESS] Fleet Intelligence Agent ${service_action} (systemd not available)" + echo "Manual start: /usr/bin/fleetint run --log-level=warn" +fi +echo "" + +exit 0 diff --git a/deployments/packages/scripts/deb/postrm b/deployments/packages/scripts/deb/postrm new file mode 100755 index 00000000..e9bf8a67 --- /dev/null +++ b/deployments/packages/scripts/deb/postrm @@ -0,0 +1,32 @@ +#!/bin/sh +set -e + +service=fleetintd.service + +case "$1" in + remove) + if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ]; then + systemctl --system daemon-reload >/dev/null || true + fi + echo "Fleet Intelligence Agent removed (configuration preserved)" + echo "To completely remove including data: sudo apt purge fleetint" + ;; + purge) + if command -v deb-systemd-helper >/dev/null 2>&1; then + deb-systemd-helper purge "$service" >/dev/null || true + fi + if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ]; then + systemctl --system daemon-reload >/dev/null || true + fi + rm -rf /var/lib/fleetint 2>/dev/null || echo "[WARNING] Failed to remove data directory" + rm -f /etc/default/fleetint 2>/dev/null || echo "[WARNING] Failed to remove configuration" + echo "Fleet Intelligence Agent purged" + ;; + upgrade|failed-upgrade|abort-install|abort-upgrade|disappear) + if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ]; then + systemctl --system daemon-reload >/dev/null || true + fi + ;; +esac + +exit 0 diff --git a/deployments/packages/scripts/deb/preinst b/deployments/packages/scripts/deb/preinst new file mode 100755 index 00000000..ddefac26 --- /dev/null +++ b/deployments/packages/scripts/deb/preinst @@ -0,0 +1,15 @@ +#!/bin/sh +set -e + +case "$1" in + install) + if [ -z "${DPKG_ROOT:-}" ] && [ ! -d /run/systemd/system ]; then + echo "[WARNING] Systemd not detected - service management will be limited" + fi + ;; + abort-upgrade) + echo "Aborting Fleet Intelligence Agent upgrade..." + ;; +esac + +exit 0 diff --git a/deployments/packages/scripts/deb/prerm b/deployments/packages/scripts/deb/prerm new file mode 100755 index 00000000..8aee5e95 --- /dev/null +++ b/deployments/packages/scripts/deb/prerm @@ -0,0 +1,24 @@ +#!/bin/sh +set -e + +service=fleetintd.service + +case "$1" in + remove|deconfigure) + echo "Stopping Fleet Intelligence Agent service..." + if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ]; then + deb-systemd-invoke stop "$service" >/dev/null || true + fi + ;; + upgrade) + # Leave the service running during upgrade. postinst restarts it only if it is already active. + ;; + failed-upgrade) + echo "Stopping Fleet Intelligence Agent service after failed upgrade..." + if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ]; then + deb-systemd-invoke stop "$service" >/dev/null || true + fi + ;; +esac + +exit 0 diff --git a/deployments/packages/scripts/postinst b/deployments/packages/scripts/postinst deleted file mode 100644 index 7d7c082e..00000000 --- a/deployments/packages/scripts/postinst +++ /dev/null @@ -1,79 +0,0 @@ -#!/bin/bash -set -e - -echo "Configuring Fleet Intelligence Agent..." - -# Setup directories and permissions -mkdir -p /var/lib/fleetint -chmod 700 /var/lib/fleetint -chmod 755 /usr/bin/fleetint - -echo "Setting up Fleet Intelligence Agent service..." - -# Handle both Debian ("configure", with optional previous version in $2) -# and RPM ($1 numeric: 1=install, 2=upgrade) -if [ ! -d /run/systemd/system ]; then - echo "Systemd not available - manual service management required" -else - systemctl daemon-reload || echo "[WARNING] Failed to reload systemd daemon" - - case "$1" in - configure) - if [ -n "$2" ]; then - # $2 is the previous version — Debian upgrade path - echo "Upgrade detected - restarting service if already running..." - if systemctl is-active --quiet fleetintd.service; then - systemctl restart fleetintd.service || echo "[WARNING] Failed to restart service" - else - echo "Service is not running - not starting during upgrade" - fi - else - echo "Fresh installation - enabling and starting service..." - systemctl enable fleetintd.service || echo "[WARNING] Failed to enable service" - systemctl start fleetintd.service || echo "[WARNING] Failed to start service" - fi - ;; - install|1) - echo "Fresh installation - enabling and starting service..." - systemctl enable fleetintd.service || echo "[WARNING] Failed to enable service" - systemctl start fleetintd.service || echo "[WARNING] Failed to start service" - ;; - upgrade|2) - echo "Upgrade detected - restarting service if already running..." - if systemctl is-active --quiet fleetintd.service; then - systemctl restart fleetintd.service || echo "[WARNING] Failed to restart service" - else - echo "Service is not running - not starting during upgrade" - fi - ;; - abort-upgrade|abort-remove|abort-deconfigure) - if systemctl is-enabled --quiet fleetintd.service; then - systemctl start fleetintd.service || true - fi - ;; - *) - # Fallback: try to start if enabled - if systemctl is-enabled --quiet fleetintd.service; then - systemctl start fleetintd.service || true - fi - ;; - esac -fi - -# Final status and guidance -echo "" -if [ -d /run/systemd/system ]; then - if systemctl is-active --quiet fleetintd.service; then - echo "[SUCCESS] Fleet Intelligence Agent installed and started successfully!" - echo "" - echo "Configuration: sudo vi /etc/default/fleetint" - echo "Logs: sudo journalctl -u fleetintd -f" - else - echo "[WARNING] Fleet Intelligence Agent installed but service is not running" - echo "Start manually: sudo systemctl start fleetintd" - fi -else - echo "[SUCCESS] Fleet Intelligence Agent installed (systemd not available)" - echo "Manual start: /usr/bin/fleetint run --log-level=warn" -fi -echo "" diff --git a/deployments/packages/scripts/postrm b/deployments/packages/scripts/postrm deleted file mode 100644 index dfa19e68..00000000 --- a/deployments/packages/scripts/postrm +++ /dev/null @@ -1,37 +0,0 @@ -#!/bin/bash -set -e - -case "$1" in - remove|0) - echo "Fleet Intelligence Agent removed (configuration preserved)" - - # Simple systemd cleanup - if [ -d /run/systemd/system ]; then - systemctl daemon-reload || echo "[WARNING] Failed to reload systemd daemon" - fi - - echo "To completely remove including data: sudo apt purge fleetint" - ;; - - purge) - echo "Completely removing Fleet Intelligence Agent..." - - # Basic systemd cleanup - if [ -d /run/systemd/system ]; then - systemctl daemon-reload || echo "[WARNING] Failed to reload systemd daemon" - fi - - # Remove data directory and configuration - rm -rf /var/lib/fleetint 2>/dev/null || echo "[WARNING] Failed to remove data directory" - rm -f /etc/default/fleetint 2>/dev/null || echo "[WARNING] Failed to remove configuration" - - echo "[SUCCESS] Fleet Intelligence Agent completely removed" - ;; - - upgrade|1|2|failed-upgrade|abort-install|abort-upgrade|disappear) - # Just reload systemd for other scenarios - if [ -d /run/systemd/system ]; then - systemctl daemon-reload || true - fi - ;; -esac diff --git a/deployments/packages/scripts/preinst b/deployments/packages/scripts/preinst deleted file mode 100644 index f977f4ac..00000000 --- a/deployments/packages/scripts/preinst +++ /dev/null @@ -1,18 +0,0 @@ -#!/bin/bash -set -e - -case "$1" in - install|1) - echo "Preparing to install Fleet Intelligence Agent..." - # Warn if systemd not available - if [ ! -d /run/systemd/system ]; then - echo "[WARNING] Systemd not detected - service management will be limited" - fi - ;; - upgrade|2) - echo "Preparing to upgrade Fleet Intelligence Agent${2:+ from version $2}..." - ;; - abort-upgrade) - echo "Aborting Fleet Intelligence Agent upgrade..." - ;; -esac diff --git a/deployments/packages/scripts/prerm b/deployments/packages/scripts/prerm deleted file mode 100644 index 7eca48e8..00000000 --- a/deployments/packages/scripts/prerm +++ /dev/null @@ -1,36 +0,0 @@ -#!/bin/bash -set -e - -case "$1" in - remove|0) - echo "Removing Fleet Intelligence Agent..." - - if [ -d /run/systemd/system ]; then - # Stop if active - if systemctl is-active --quiet fleetintd.service 2>/dev/null; then - echo "Stopping fleetintd service..." - systemctl stop fleetintd.service || echo "[WARNING] Failed to stop service" - fi - - # Disable if enabled - if systemctl is-enabled --quiet fleetintd.service 2>/dev/null; then - echo "Disabling fleetintd service..." - systemctl disable fleetintd.service || echo "[WARNING] Failed to disable service" - fi - fi - - echo "Fleet Intelligence Agent service stopped and disabled" - ;; - - upgrade|1|2) - echo "Preparing Fleet Intelligence Agent for upgrade..." - # Don't stop service during upgrade - ;; - - failed-upgrade|deconfigure) - echo "Fleet Intelligence Agent operation failed, stopping service..." - if [ -d /run/systemd/system ] && systemctl is-active --quiet fleetintd.service 2>/dev/null; then - systemctl stop fleetintd.service || true - fi - ;; -esac diff --git a/deployments/packages/scripts/rpm/postinstall b/deployments/packages/scripts/rpm/postinstall new file mode 100755 index 00000000..73222a15 --- /dev/null +++ b/deployments/packages/scripts/rpm/postinstall @@ -0,0 +1,49 @@ +#!/bin/sh +set -e + +service=fleetintd.service + +service_was_stopped=false +service_action="installed" + +if [ -d /run/systemd/system ]; then + systemctl daemon-reload >/dev/null || true + + case "$1" in + 1) + systemctl enable --now "$service" >/dev/null || true + ;; + 2) + service_action="upgraded" + if systemctl is-active --quiet "$service"; then + systemctl restart "$service" >/dev/null || true + else + service_was_stopped=true + fi + ;; + esac +else + echo "Systemd not available - manual service management required" +fi + +echo "" +if [ -d /run/systemd/system ]; then + if systemctl is-active --quiet "$service"; then + echo "[SUCCESS] Fleet Intelligence Agent ${service_action} and service is running" + echo "" + echo "Configuration: sudo vi /etc/default/fleetint" + echo "Logs: sudo journalctl -u fleetintd -f" + elif [ "$service_was_stopped" = true ]; then + echo "[INFO] Fleet Intelligence Agent upgraded; service was not running and was left stopped" + echo "Start manually: sudo systemctl start fleetintd" + else + echo "[WARNING] Fleet Intelligence Agent ${service_action} but service is not running" + echo "Start manually: sudo systemctl start fleetintd" + fi +else + echo "[SUCCESS] Fleet Intelligence Agent ${service_action} (systemd not available)" + echo "Manual start: /usr/bin/fleetint run --log-level=warn" +fi +echo "" + +exit 0 diff --git a/deployments/packages/scripts/rpm/postremove b/deployments/packages/scripts/rpm/postremove new file mode 100755 index 00000000..ca9e2da1 --- /dev/null +++ b/deployments/packages/scripts/rpm/postremove @@ -0,0 +1,18 @@ +#!/bin/sh +set -e + +case "$1" in + 0) + if [ -d /run/systemd/system ]; then + systemctl daemon-reload >/dev/null || true + fi + echo "Fleet Intelligence Agent erased" + ;; + 1) + if [ -d /run/systemd/system ]; then + systemctl daemon-reload >/dev/null || true + fi + ;; +esac + +exit 0 diff --git a/deployments/packages/scripts/rpm/preinstall b/deployments/packages/scripts/rpm/preinstall new file mode 100755 index 00000000..612e5e98 --- /dev/null +++ b/deployments/packages/scripts/rpm/preinstall @@ -0,0 +1,12 @@ +#!/bin/sh +set -e + +case "$1" in + 1) + if [ ! -d /run/systemd/system ]; then + echo "[WARNING] Systemd not detected - service management will be limited" + fi + ;; +esac + +exit 0 diff --git a/deployments/packages/scripts/rpm/preremove b/deployments/packages/scripts/rpm/preremove new file mode 100755 index 00000000..c9826bcb --- /dev/null +++ b/deployments/packages/scripts/rpm/preremove @@ -0,0 +1,18 @@ +#!/bin/sh +set -e + +service=fleetintd.service + +case "$1" in + 0) + echo "Stopping and disabling Fleet Intelligence Agent service..." + if [ -d /run/systemd/system ]; then + systemctl --no-reload disable --now "$service" >/dev/null || true + fi + ;; + 1) + # Leave the service running during upgrade. postinstall try-restarts it only if it is already active. + ;; +esac + +exit 0 From f438717010438412e3bf7f9e8a1efad9b1148ce5 Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Wed, 24 Jun 2026 09:47:53 -0700 Subject: [PATCH 12/16] fix: make DCGM group name configurable and different for each process (#220) Signed-off-by: Jingxiang Zhang --- internal/scan/scan.go | 7 +- internal/server/server.go | 7 +- .../accelerator/nvidia/dcgm/cpu/component.go | 5 +- .../accelerator/nvidia/dcgm/prof/component.go | 197 +++++++++--------- .../components/registry.go | 57 ++++- .../pkg/nvidia-query/dcgm/field_cache.go | 2 +- .../pkg/nvidia-query/dcgm/instance.go | 81 ++++++- .../pkg/nvidia-query/dcgm/instance_test.go | 35 ++++ 8 files changed, 272 insertions(+), 119 deletions(-) diff --git a/internal/scan/scan.go b/internal/scan/scan.go index 02327447..2b6a05f5 100644 --- a/internal/scan/scan.go +++ b/internal/scan/scan.go @@ -124,8 +124,10 @@ func Scan(ctx context.Context, opts ...Option) error { } } + dcgmGroupNames := components.NewDCGMGroupNames(fmt.Sprintf("scan-%d", os.Getpid())) + // Initialize DCGM instance for DCGM-based health checks - dcgmInstance, err := nvidiadcgm.New() + dcgmInstance, err := nvidiadcgm.NewWithGroupName(dcgmGroupNames.HealthMonitoringGroup) if err != nil { return err } @@ -159,6 +161,7 @@ func Scan(ctx context.Context, opts ...Option) error { DCGMInstance: dcgmInstance, DCGMHealthCache: dcgmHealthCache, DCGMFieldValueCache: dcgmFieldValueCache, + DCGMGroupNames: dcgmGroupNames, NVIDIAToolOverwrites: nvidiacommon.ToolOverwrites{ InfinibandClassRootDir: op.infinibandClassRootDir, }, @@ -191,7 +194,7 @@ func Scan(ctx context.Context, opts ...Option) error { } // Set up DCGM field watching after all components have registered their fields - if err := dcgmFieldValueCache.SetupFieldWatching(); err != nil { + if err := dcgmFieldValueCache.SetupFieldWatchingWithName(dcgmGroupNames.GPUFieldGroup); err != nil { log.Logger.Warnw("failed to set up DCGM field watching", "error", err) } diff --git a/internal/server/server.go b/internal/server/server.go index b3d69174..8525056a 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -296,9 +296,11 @@ func New(ctx context.Context, auditLogger log.AuditLogger, config *config.Config return nil, fmt.Errorf("failed to create NVML instance: %w", err) } + dcgmGroupNames := components.NewDCGMGroupNames(fmt.Sprintf("daemon-%d", stdos.Getpid())) + // Initialize DCGM instance dcgmInitCtx, dcgmInitCancel := context.WithTimeout(ctx, time.Minute) - dcgmInstance, err := nvidiadcgm.NewWithContext(dcgmInitCtx) + dcgmInstance, err := nvidiadcgm.NewWithContextAndGroupName(dcgmInitCtx, dcgmGroupNames.HealthMonitoringGroup) dcgmInitCancel() if err != nil { return nil, fmt.Errorf("failed to create DCGM instance: %w", err) @@ -337,6 +339,7 @@ func New(ctx context.Context, auditLogger log.AuditLogger, config *config.Config DCGMInstance: dcgmInstance, DCGMHealthCache: dcgmHealthCache, DCGMFieldValueCache: dcgmFieldValueCache, + DCGMGroupNames: dcgmGroupNames, NVIDIAToolOverwrites: config.NvidiaToolOverwrites, DBRW: dbRW, DBRO: dbRO, @@ -364,7 +367,7 @@ func New(ctx context.Context, auditLogger log.AuditLogger, config *config.Config // Set up DCGM field watching after all components have registered their fields if dcgmFieldValueCache != nil { - if err := dcgmFieldValueCache.SetupFieldWatching(); err != nil { + if err := dcgmFieldValueCache.SetupFieldWatchingWithName(dcgmGroupNames.GPUFieldGroup); err != nil { log.Logger.Errorw("failed to set up DCGM field watching, DCGM metrics collection unavailable", "error", err) } } diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/cpu/component.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/cpu/component.go index 13cf55ee..03bc6bae 100644 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/cpu/component.go +++ b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/cpu/component.go @@ -65,6 +65,7 @@ type component struct { func New(gpudInstance *components.GPUdInstance) (components.Component, error) { cctx, ccancel := context.WithCancel(gpudInstance.RootCtx) + dcgmGroupNames := gpudInstance.DCGMGroupNames.WithDefaults() healthCheckInterval := defaultHealthCheckInterval if gpudInstance.HealthCheckInterval > 0 { @@ -100,7 +101,7 @@ func New(gpudInstance *components.GPUdInstance) (components.Component, error) { c.cpuEntities = cpuEntities // Create a dedicated DCGM group for CPUs - cpuGroupHandle, err := dcgm.CreateGroup("gpud-cpu-group") + cpuGroupHandle, err := dcgm.CreateGroup(dcgmGroupNames.CPUGroup) if err != nil { log.Logger.Warnw("failed to create DCGM group for CPUs", "error", err) c.setupDegradedReason = fmt.Sprintf("failed to create DCGM CPU group: %v", err) @@ -119,7 +120,7 @@ func New(gpudInstance *components.GPUdInstance) (components.Component, error) { } // Create a field group for this component's CPU fields - fieldGroupName := "gpud-cpu-fields" + fieldGroupName := dcgmGroupNames.CPUFieldGroup fieldGroupID, err := dcgm.FieldGroupCreate(fieldGroupName, cpuLevelFields) if err != nil { log.Logger.Warnw("failed to create DCGM field group for CPU fields", "error", err) diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/prof/component.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/prof/component.go index e8a882c5..77057696 100644 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/prof/component.go +++ b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/dcgm/prof/component.go @@ -68,6 +68,7 @@ type component struct { // fieldValidator validates fields based on hardware support func New(gpudInstance *components.GPUdInstance) (components.Component, error) { cctx, ccancel := context.WithCancel(gpudInstance.RootCtx) + dcgmGroupNames := gpudInstance.DCGMGroupNames.WithDefaults() healthCheckInterval := defaultHealthCheckInterval if gpudInstance.HealthCheckInterval > 0 { @@ -119,7 +120,7 @@ func New(gpudInstance *components.GPUdInstance) (components.Component, error) { c.watchedFields = validFields // Create field group with ONLY hardware-validated fields - fieldGroupName := "gpud-prof-fields" + fieldGroupName := dcgmGroupNames.ProfilingFieldGroup fieldGroupID, err := dcgm.FieldGroupCreate(fieldGroupName, validFields) if err != nil { log.Logger.Warnw("failed to create DCGM field group", "error", err) @@ -280,7 +281,7 @@ func (c *component) Check() components.CheckResult { "action", "systemd/k8s will restart agent and recreate DCGM resources") os.Exit(1) } - + // Check if this is a transient error (benign, skip) if nvidiadcgm.IsTransientError(err) { log.Logger.Infow("DCGM transient error, will retry", @@ -289,7 +290,7 @@ func (c *component) Check() components.CheckResult { "error", err) continue } - + // For unhealthy or unknown errors, set component state if nvidiadcgm.IsUnhealthyAPIError(err) { cr.health = apiv1.HealthStateTypeUnhealthy @@ -314,101 +315,101 @@ func (c *component) Check() components.CheckResult { // Value is valid - export metric based on field type switch val.FieldID { - case dcgm.DCGM_FI_PROF_GR_ENGINE_ACTIVE: - metricDCGMFIProfGrEngineActive.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_SM_ACTIVE: - metricDCGMFIProfSmActive.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_SM_OCCUPANCY: - metricDCGMFIProfSmOccupancy.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_PIPE_TENSOR_ACTIVE: - metricDCGMFIProfPipeTensorActive.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_PIPE_TENSOR_IMMA_ACTIVE: - metricDCGMFIProfPipeTensorImmaActive.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_PIPE_TENSOR_HMMA_ACTIVE: - metricDCGMFIProfPipeTensorHmmaActive.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_PIPE_TENSOR_DFMA_ACTIVE: - metricDCGMFIProfPipeTensorDfmaActive.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_PIPE_INT_ACTIVE: - metricDCGMFIProfPipeIntActive.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_DRAM_ACTIVE: - metricDCGMFIProfDramActive.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_PIPE_FP64_ACTIVE: - metricDCGMFIProfPipeFp64Active.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_PIPE_FP32_ACTIVE: - metricDCGMFIProfPipeFp32Active.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_PIPE_FP16_ACTIVE: - metricDCGMFIProfPipeFp16Active.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(val.Float64()) - - case dcgm.DCGM_FI_PROF_PCIE_TX_BYTES: - metricDCGMFIProfPcieTxBytes.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(float64(val.Int64())) - - case dcgm.DCGM_FI_PROF_PCIE_RX_BYTES: - metricDCGMFIProfPcieRxBytes.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(float64(val.Int64())) - - case dcgm.DCGM_FI_PROF_NVLINK_TX_BYTES: - metricDCGMFIProfNvlinkTxBytes.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(float64(val.Int64())) - - case dcgm.DCGM_FI_PROF_NVLINK_RX_BYTES: - metricDCGMFIProfNvlinkRxBytes.With(prometheus.Labels{ - "uuid": device.UUID, - "gpu": fmt.Sprintf("%d", device.ID), - }).Set(float64(val.Int64())) + case dcgm.DCGM_FI_PROF_GR_ENGINE_ACTIVE: + metricDCGMFIProfGrEngineActive.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_SM_ACTIVE: + metricDCGMFIProfSmActive.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_SM_OCCUPANCY: + metricDCGMFIProfSmOccupancy.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_PIPE_TENSOR_ACTIVE: + metricDCGMFIProfPipeTensorActive.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_PIPE_TENSOR_IMMA_ACTIVE: + metricDCGMFIProfPipeTensorImmaActive.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_PIPE_TENSOR_HMMA_ACTIVE: + metricDCGMFIProfPipeTensorHmmaActive.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_PIPE_TENSOR_DFMA_ACTIVE: + metricDCGMFIProfPipeTensorDfmaActive.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_PIPE_INT_ACTIVE: + metricDCGMFIProfPipeIntActive.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_DRAM_ACTIVE: + metricDCGMFIProfDramActive.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_PIPE_FP64_ACTIVE: + metricDCGMFIProfPipeFp64Active.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_PIPE_FP32_ACTIVE: + metricDCGMFIProfPipeFp32Active.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_PIPE_FP16_ACTIVE: + metricDCGMFIProfPipeFp16Active.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(val.Float64()) + + case dcgm.DCGM_FI_PROF_PCIE_TX_BYTES: + metricDCGMFIProfPcieTxBytes.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(float64(val.Int64())) + + case dcgm.DCGM_FI_PROF_PCIE_RX_BYTES: + metricDCGMFIProfPcieRxBytes.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(float64(val.Int64())) + + case dcgm.DCGM_FI_PROF_NVLINK_TX_BYTES: + metricDCGMFIProfNvlinkTxBytes.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(float64(val.Int64())) + + case dcgm.DCGM_FI_PROF_NVLINK_RX_BYTES: + metricDCGMFIProfNvlinkRxBytes.With(prometheus.Labels{ + "uuid": device.UUID, + "gpu": fmt.Sprintf("%d", device.ID), + }).Set(float64(val.Int64())) } } } diff --git a/third_party/fleet-intelligence-sdk/components/registry.go b/third_party/fleet-intelligence-sdk/components/registry.go index cde14b0a..8e6351ae 100644 --- a/third_party/fleet-intelligence-sdk/components/registry.go +++ b/third_party/fleet-intelligence-sdk/components/registry.go @@ -35,11 +35,12 @@ type GPUdInstance struct { KernelModulesToCheck []string - DCGMInstance nvidiadcgm.Instance - DCGMHealthCache *nvidiadcgm.HealthCache // Shared cache for DCGM health check results - DCGMFieldValueCache *nvidiadcgm.FieldValueCache // Shared cache for DCGM field values (GPU devices only) - NVMLInstance nvidianvml.Instance - NVIDIAToolOverwrites nvidiacommon.ToolOverwrites + DCGMInstance nvidiadcgm.Instance + DCGMHealthCache *nvidiadcgm.HealthCache // Shared cache for DCGM health check results + DCGMFieldValueCache *nvidiadcgm.FieldValueCache // Shared cache for DCGM field values (GPU devices only) + DCGMGroupNames DCGMGroupNames + NVMLInstance nvidianvml.Instance + NVIDIAToolOverwrites nvidiacommon.ToolOverwrites DBRW *sql.DB DBRO *sql.DB @@ -56,6 +57,52 @@ type GPUdInstance struct { FailureInjector *FailureInjector } +// DCGMGroupNames names the DCGM groups and field groups owned by one fleetint process. +type DCGMGroupNames struct { + HealthMonitoringGroup string + GPUFieldGroup string + CPUGroup string + CPUFieldGroup string + ProfilingFieldGroup string +} + +// NewDCGMGroupNames returns a fleetint-owned set of DCGM names for a single owner. +func NewDCGMGroupNames(owner string) DCGMGroupNames { + return DCGMGroupNames{ + HealthMonitoringGroup: fmt.Sprintf("fleetint-%s-health", owner), + GPUFieldGroup: fmt.Sprintf("fleetint-%s-gpu-fields", owner), + CPUGroup: fmt.Sprintf("fleetint-%s-cpu", owner), + CPUFieldGroup: fmt.Sprintf("fleetint-%s-cpu-fields", owner), + ProfilingFieldGroup: fmt.Sprintf("fleetint-%s-prof-fields", owner), + } +} + +// DefaultDCGMGroupNames returns fleetint defaults for callers that do not provide names. +func DefaultDCGMGroupNames() DCGMGroupNames { + return NewDCGMGroupNames("default") +} + +// WithDefaults fills any missing names with fleetint defaults. +func (n DCGMGroupNames) WithDefaults() DCGMGroupNames { + defaults := DefaultDCGMGroupNames() + if n.HealthMonitoringGroup == "" { + n.HealthMonitoringGroup = defaults.HealthMonitoringGroup + } + if n.GPUFieldGroup == "" { + n.GPUFieldGroup = defaults.GPUFieldGroup + } + if n.CPUGroup == "" { + n.CPUGroup = defaults.CPUGroup + } + if n.CPUFieldGroup == "" { + n.CPUFieldGroup = defaults.CPUFieldGroup + } + if n.ProfilingFieldGroup == "" { + n.ProfilingFieldGroup = defaults.ProfilingFieldGroup + } + return n +} + type FailureInjector struct { GPUUUIDsWithRowRemappingPending []string GPUUUIDsWithRowRemappingFailed []string diff --git a/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/field_cache.go b/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/field_cache.go index 84abe84d..9a46870a 100644 --- a/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/field_cache.go +++ b/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/field_cache.go @@ -56,7 +56,7 @@ func NewFieldValueCache(ctx context.Context, instance Instance, pollInterval tim cancel: ccancel, instance: instance, values: make(map[uint]map[dcgm.Short]dcgm.FieldValue_v1), - fieldGroupName: "gpud-gpu-fields", + fieldGroupName: "fleetint-default-gpu-fields", pollInterval: pollInterval, } diff --git a/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/instance.go b/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/instance.go index f975d847..663810d1 100644 --- a/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/instance.go +++ b/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/instance.go @@ -33,6 +33,7 @@ import ( var _ Instance = &instance{} const defaultDCGMReconnectInterval = 30 * time.Second +const defaultDCGMGroupName = "fleetint-default-health" var dcgmReconnectInterval = defaultDCGMReconnectInterval var errReconnectAborted = errors.New("dcgm reconnect aborted") @@ -172,10 +173,21 @@ func New() (Instance, error) { return newInitializedInstance() } +// NewWithGroupName creates a DCGM instance with a caller-owned DCGM group name. +func NewWithGroupName(groupName string) (Instance, error) { + return newInitializedInstanceWithGroupName(groupName) +} + // NewWithContext creates a DCGM instance with a bounded wait. If initialization // exceeds the context deadline, it returns a no-op instance so callers can // continue startup without blocking on slow DCGM device enumeration. func NewWithContext(ctx context.Context) (Instance, error) { + return NewWithContextAndGroupName(ctx, defaultDCGMGroupName) +} + +// NewWithContextAndGroupName creates a DCGM instance with a bounded wait and +// caller-owned DCGM group name. +func NewWithContextAndGroupName(ctx context.Context, groupName string) (Instance, error) { type result struct { inst Instance err error @@ -186,7 +198,7 @@ func NewWithContext(ctx context.Context) (Instance, error) { initFn := newInstanceFunc go func() { - inst, err := initFn() + inst, err := initFnWithGroupName(initFn, groupName) select { case resultCh <- result{inst: inst, err: err}: case <-abandonCh: @@ -207,7 +219,7 @@ func NewWithContext(ctx context.Context) (Instance, error) { "retryInterval", dcgmReconnectInterval.String(), ) } - return newReconnectingInstance(res.inst, dcgmReconnectInterval), nil + return newReconnectingInstanceWithGroupName(res.inst, dcgmReconnectInterval, groupName), nil case <-ctx.Done(): close(abandonCh) log.Logger.Warnw( @@ -215,14 +227,31 @@ func NewWithContext(ctx context.Context) (Instance, error) { "error", ctx.Err(), "retryInterval", dcgmReconnectInterval.String(), ) - return newReconnectingInstance(NewNoOp(), dcgmReconnectInterval), nil + return newReconnectingInstanceWithGroupName(NewNoOp(), dcgmReconnectInterval, groupName), nil } } +func initFnWithGroupName(initFn func() (Instance, error), groupName string) (Instance, error) { + if groupName == "" || groupName == defaultDCGMGroupName { + return initFn() + } + return newInitializedInstanceWithGroupName(groupName) +} + var newConnectedInstanceFunc = func() (Instance, error) { - return newConnectedInstance() + return newConnectedInstance(defaultDCGMGroupName) } +var newConnectedInstanceWithGroupNameFunc = func(groupName string) (Instance, error) { + return newConnectedInstance(groupName) +} + +var dcgmInitFunc = func(initParams dcgmInitParams) (func(), error) { + return dcgm.Init(dcgm.Standalone, initParams.address, initParams.isUnixSocket) +} + +var dcgmNewDefaultGroupFunc = dcgm.NewDefaultGroup + func newInitializedInstance() (Instance, error) { connectedInst, err := newConnectedInstanceFunc() if err != nil { @@ -232,10 +261,32 @@ func newInitializedInstance() (Instance, error) { return connectedInst, nil } -func newConnectedInstance() (Instance, error) { +func newInitializedInstanceWithGroupName(groupName string) (Instance, error) { + if groupName == "" { + groupName = defaultDCGMGroupName + } + connectedInst, err := connectWithGroupName(groupName) + if err != nil { + log.Logger.Warnw("DCGM initialization failed, returning no-op instance", "error", err) + return NewNoOp(), nil + } + return connectedInst, nil +} + +func connectWithGroupName(groupName string) (Instance, error) { + if groupName == "" || groupName == defaultDCGMGroupName { + return newConnectedInstanceFunc() + } + return newConnectedInstanceWithGroupNameFunc(groupName) +} + +func newConnectedInstance(groupName string) (Instance, error) { + if groupName == "" { + groupName = defaultDCGMGroupName + } initParams := resolveInitFromEnv() - cleanup, err := dcgm.Init(dcgm.Standalone, initParams.address, initParams.isUnixSocket) + cleanup, err := dcgmInitFunc(initParams) if err != nil { return nil, err } @@ -243,12 +294,15 @@ func newConnectedInstance() (Instance, error) { log.Logger.Debugw("DCGM initialized successfully") // Create group with GPUs. Components add their own entities (e.g., NVSwitch). - groupHandle, err := dcgm.NewDefaultGroup("gpud-health-monitoring") + groupHandle, err := dcgmNewDefaultGroupFunc(groupName) if err != nil { + if cleanup != nil { + cleanup() + } return nil, fmt.Errorf("failed to create custom DCGM group: %w", err) } - log.Logger.Infow("created custom DCGM group for isolated health monitoring") + log.Logger.Infow("created custom DCGM group for isolated health monitoring", "groupName", groupName) // Fetch and cache device information once during initialization deviceIDs, err := dcgm.GetSupportedDevices() @@ -497,6 +551,7 @@ type reconnectingInstance struct { generation uint64 reconnectInterval time.Duration + groupName string stopCh chan struct{} shutdownOnce sync.Once } @@ -508,18 +563,26 @@ type reconnectStateSnapshot struct { } func newReconnectingInstance(initial Instance, reconnectInterval time.Duration) Instance { + return newReconnectingInstanceWithGroupName(initial, reconnectInterval, defaultDCGMGroupName) +} + +func newReconnectingInstanceWithGroupName(initial Instance, reconnectInterval time.Duration, groupName string) Instance { if initial == nil { initial = NewNoOp() } if reconnectInterval <= 0 { reconnectInterval = defaultDCGMReconnectInterval } + if groupName == "" { + groupName = defaultDCGMGroupName + } inst := &reconnectingInstance{ current: initial, watchedFields: make(map[dcgm.Short]struct{}), groupEntities: make(map[uint]struct{}), reconnectInterval: reconnectInterval, + groupName: groupName, stopCh: make(chan struct{}), } @@ -589,7 +652,7 @@ func (inst *reconnectingInstance) reconnectNow() error { expectedGeneration := inst.generation inst.reconnectMu.Unlock() - connectedInst, err := newConnectedInstanceFunc() + connectedInst, err := connectWithGroupName(inst.groupName) if err != nil { return err } diff --git a/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/instance_test.go b/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/instance_test.go index 6b2837f7..4c9aba95 100644 --- a/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/instance_test.go +++ b/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/instance_test.go @@ -59,6 +59,41 @@ func TestResolveInitFromEnv(t *testing.T) { } } +func TestNewConnectedInstanceCleansUpWhenGroupCreationFails(t *testing.T) { + originalDCGMInitFunc := dcgmInitFunc + originalDCGMNewDefaultGroupFunc := dcgmNewDefaultGroupFunc + defer func() { + dcgmInitFunc = originalDCGMInitFunc + dcgmNewDefaultGroupFunc = originalDCGMNewDefaultGroupFunc + }() + + cleanupCalled := false + dcgmInitFunc = func(_ dcgmInitParams) (func(), error) { + return func() { + cleanupCalled = true + }, nil + } + + expectedErr := errors.New("invalid group name") + dcgmNewDefaultGroupFunc = func(_ string) (dcgm.GroupHandle, error) { + return dcgm.GroupHandle{}, expectedErr + } + + inst, err := newConnectedInstance("invalid group") + if err == nil { + t.Fatalf("expected newConnectedInstance() to fail") + } + if inst != nil { + t.Fatalf("expected nil instance on group creation failure") + } + if !errors.Is(err, expectedErr) { + t.Fatalf("expected error %v, got %v", expectedErr, err) + } + if !cleanupCalled { + t.Fatalf("expected DCGM cleanup to be called when group creation fails") + } +} + func TestInstance(t *testing.T) { inst, err := New() if err != nil { From 9123632064a8625a170a300d7277d2da65d1d2e8 Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Wed, 24 Jun 2026 13:07:55 -0700 Subject: [PATCH 13/16] fix: degrade dcgm query timeouts (#222) Signed-off-by: Jingxiang Zhang --- .../pkg/nvidia-query/dcgm/query_validator.go | 4 +-- .../nvidia-query/dcgm/query_validator_test.go | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/query_validator.go b/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/query_validator.go index 7463946d..7626fc9a 100644 --- a/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/query_validator.go +++ b/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/query_validator.go @@ -210,9 +210,7 @@ func IsUnhealthyAPIError(err error) bool { case dcgm.DCGM_ST_NVML_ERROR, dcgm.DCGM_ST_GPU_IS_LOST, dcgm.DCGM_ST_RESET_REQUIRED, - dcgm.DCGM_ST_GPU_NOT_SUPPORTED, - dcgm.DCGM_ST_TIMEOUT, - dcgm.DCGM_ST_NVML_DRIVER_TIMEOUT: + dcgm.DCGM_ST_GPU_NOT_SUPPORTED: return true default: return false diff --git a/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/query_validator_test.go b/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/query_validator_test.go index e07b2089..c167a3cb 100644 --- a/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/query_validator_test.go +++ b/third_party/fleet-intelligence-sdk/pkg/nvidia-query/dcgm/query_validator_test.go @@ -16,7 +16,10 @@ package dcgm import ( + "fmt" "testing" + + dcgm "github.com/NVIDIA/go-dcgm/pkg/dcgm" ) // Note: Tests for CheckSentinel() and CheckSentinelV2() require actual DCGM library @@ -66,3 +69,28 @@ func TestSentinelType_String(t *testing.T) { }) } } + +func TestIsUnhealthyAPIError(t *testing.T) { + tests := []struct { + name string + code int32 + want bool + }{ + {"nvml error is unhealthy", dcgm.DCGM_ST_NVML_ERROR, true}, + {"gpu lost is unhealthy", dcgm.DCGM_ST_GPU_IS_LOST, true}, + {"reset required is unhealthy", dcgm.DCGM_ST_RESET_REQUIRED, true}, + {"gpu not supported is unhealthy", dcgm.DCGM_ST_GPU_NOT_SUPPORTED, true}, + {"dcgm timeout is degraded", dcgm.DCGM_ST_TIMEOUT, false}, + {"nvml driver timeout is degraded", dcgm.DCGM_ST_NVML_DRIVER_TIMEOUT, false}, + {"stale data is not unhealthy", dcgm.DCGM_ST_STALE_DATA, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := fmt.Errorf("dcgm request failed with error code %d", tt.code) + if got := IsUnhealthyAPIError(err); got != tt.want { + t.Errorf("IsUnhealthyAPIError() = %v, want %v", got, tt.want) + } + }) + } +} From 91c07184bf85e94367b85ab6151af337a032ce5e Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Wed, 24 Jun 2026 15:40:18 -0700 Subject: [PATCH 14/16] fix: include metric scrape interval in agent config (#223) Signed-off-by: Jingxiang Zhang --- internal/backendclient/types.go | 17 +++++++------- internal/config/config.go | 13 +++++++++++ internal/config/config_test.go | 8 +++++++ internal/enrollment/enrollment.go | 17 +++++++------- internal/inventory/mapper/backend.go | 17 +++++++------- internal/inventory/mapper/backend_test.go | 19 ++++++++------- internal/inventory/source/source_test.go | 18 ++++++++------- internal/inventory/types.go | 17 +++++++------- internal/server/server.go | 23 ++++++++----------- internal/validation/outbound/validator.go | 1 + .../validation/outbound/validator_test.go | 9 ++++++++ 11 files changed, 98 insertions(+), 61 deletions(-) diff --git a/internal/backendclient/types.go b/internal/backendclient/types.go index dfad2a6c..00420e1c 100644 --- a/internal/backendclient/types.go +++ b/internal/backendclient/types.go @@ -49,14 +49,15 @@ type NodeResources struct { } type AgentConfig struct { - TotalComponents int64 `json:"totalComponents"` - RetentionPeriodSeconds int64 `json:"retentionPeriodSeconds"` - EnabledComponents []string `json:"enabledComponents"` - DisabledComponents []string `json:"disabledComponents"` - InventoryEnabled bool `json:"inventoryEnabled"` - InventoryIntervalSeconds int64 `json:"inventoryIntervalSeconds"` - AttestationEnabled bool `json:"attestationEnabled"` - AttestationIntervalSeconds int64 `json:"attestationIntervalSeconds"` + TotalComponents int64 `json:"totalComponents"` + RetentionPeriodSeconds int64 `json:"retentionPeriodSeconds"` + MetricScrapeIntervalSeconds int64 `json:"metricScrapeIntervalSeconds"` + EnabledComponents []string `json:"enabledComponents"` + DisabledComponents []string `json:"disabledComponents"` + InventoryEnabled bool `json:"inventoryEnabled"` + InventoryIntervalSeconds int64 `json:"inventoryIntervalSeconds"` + AttestationEnabled bool `json:"attestationEnabled"` + AttestationIntervalSeconds int64 `json:"attestationIntervalSeconds"` } type CPUInfo struct { diff --git a/internal/config/config.go b/internal/config/config.go index 015aa435..2c1f45dc 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -345,6 +345,19 @@ func (config *Config) AttestationLoopAgentConfig() (enabled bool, intervalSecond return config.Attestation.Enabled, int64(config.Attestation.Interval.Seconds()) } +// HealthCheckInterval returns the resolved component health-check interval. +func (config *Config) HealthCheckInterval() time.Duration { + if config == nil || config.HealthExporter == nil || config.HealthExporter.HealthCheckInterval.Duration <= 0 { + return time.Minute + } + return config.HealthExporter.HealthCheckInterval.Duration +} + +// MetricScrapeIntervalSeconds returns the resolved metric scrape interval stored in inventory. +func (config *Config) MetricScrapeIntervalSeconds() int64 { + return int64(config.HealthCheckInterval().Seconds()) +} + // getComponentLists computes enabled/disabled lists from config rules against all available components. func (config *Config) getComponentLists(allComponentNames []string) (enabled, disabled []string) { enabled, disabled = []string{}, []string{} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c9503018..b4adc884 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -881,6 +881,9 @@ func TestInventoryAgentConfig(t *testing.T) { APIVersion: "v1", RetentionPeriod: metav1.Duration{Duration: 24 * time.Hour}, Components: []string{"*", "-memory", "-disk"}, + HealthExporter: &HealthExporterConfig{ + HealthCheckInterval: metav1.Duration{Duration: 30 * time.Second}, + }, Inventory: &InventoryConfig{ Enabled: true, Interval: metav1.Duration{Duration: time.Hour}, @@ -903,4 +906,9 @@ func TestInventoryAgentConfig(t *testing.T) { attestationEnabled, attestationIntervalSeconds := cfg.AttestationLoopAgentConfig() assert.True(t, attestationEnabled) assert.Equal(t, int64(86400), attestationIntervalSeconds) + + assert.Equal(t, 30*time.Second, cfg.HealthCheckInterval()) + assert.Equal(t, int64(30), cfg.MetricScrapeIntervalSeconds()) + assert.Equal(t, time.Minute, (*Config)(nil).HealthCheckInterval()) + assert.Equal(t, int64(60), (*Config)(nil).MetricScrapeIntervalSeconds()) } diff --git a/internal/enrollment/enrollment.go b/internal/enrollment/enrollment.go index 77ce9f1b..682494f9 100644 --- a/internal/enrollment/enrollment.go +++ b/internal/enrollment/enrollment.go @@ -219,14 +219,15 @@ func syncInventoryOnce(ctx context.Context, cfg *config.Config) error { return machineinfo.GetMachineInfo(nvmlInstance) }), &inventory.AgentConfig{ - TotalComponents: int64(len(allComponents)), - RetentionPeriodSeconds: retentionPeriodSeconds, - EnabledComponents: enabledComponents, - DisabledComponents: disabledComponents, - InventoryEnabled: inventoryEnabled, - InventoryIntervalSeconds: inventoryIntervalSeconds, - AttestationEnabled: attestationEnabled, - AttestationIntervalSeconds: attestationIntervalSeconds, + TotalComponents: int64(len(allComponents)), + RetentionPeriodSeconds: retentionPeriodSeconds, + MetricScrapeIntervalSeconds: cfg.MetricScrapeIntervalSeconds(), + EnabledComponents: enabledComponents, + DisabledComponents: disabledComponents, + InventoryEnabled: inventoryEnabled, + InventoryIntervalSeconds: inventoryIntervalSeconds, + AttestationEnabled: attestationEnabled, + AttestationIntervalSeconds: attestationIntervalSeconds, }, ) manager := inventory.NewManager(src, sink, inventory.InventoryConfig{}) diff --git a/internal/inventory/mapper/backend.go b/internal/inventory/mapper/backend.go index c197a5ed..b0603e3a 100644 --- a/internal/inventory/mapper/backend.go +++ b/internal/inventory/mapper/backend.go @@ -75,14 +75,15 @@ func ToNodeUpsertRequest(s *inventory.Snapshot) *backendclient.NodeUpsertRequest return &backendclient.NodeUpsertRequest{ Hostname: s.Hostname, AgentConfig: backendclient.AgentConfig{ - TotalComponents: s.AgentConfig.TotalComponents, - RetentionPeriodSeconds: s.AgentConfig.RetentionPeriodSeconds, - EnabledComponents: cloneStringSlice(s.AgentConfig.EnabledComponents), - DisabledComponents: cloneStringSlice(s.AgentConfig.DisabledComponents), - InventoryEnabled: s.AgentConfig.InventoryEnabled, - InventoryIntervalSeconds: s.AgentConfig.InventoryIntervalSeconds, - AttestationEnabled: s.AgentConfig.AttestationEnabled, - AttestationIntervalSeconds: s.AgentConfig.AttestationIntervalSeconds, + TotalComponents: s.AgentConfig.TotalComponents, + RetentionPeriodSeconds: s.AgentConfig.RetentionPeriodSeconds, + MetricScrapeIntervalSeconds: s.AgentConfig.MetricScrapeIntervalSeconds, + EnabledComponents: cloneStringSlice(s.AgentConfig.EnabledComponents), + DisabledComponents: cloneStringSlice(s.AgentConfig.DisabledComponents), + InventoryEnabled: s.AgentConfig.InventoryEnabled, + InventoryIntervalSeconds: s.AgentConfig.InventoryIntervalSeconds, + AttestationEnabled: s.AgentConfig.AttestationEnabled, + AttestationIntervalSeconds: s.AgentConfig.AttestationIntervalSeconds, }, MachineID: s.MachineID, SystemUUID: s.SystemUUID, diff --git a/internal/inventory/mapper/backend_test.go b/internal/inventory/mapper/backend_test.go index 5328990e..676c77f8 100644 --- a/internal/inventory/mapper/backend_test.go +++ b/internal/inventory/mapper/backend_test.go @@ -38,6 +38,7 @@ func TestToNodeUpsertRequestAgentConfigJSONIncludesZeroValues(t *testing.T) { require.JSONEq(t, `{ "totalComponents": 0, "retentionPeriodSeconds": 0, + "metricScrapeIntervalSeconds": 0, "enabledComponents": [], "disabledComponents": [], "inventoryEnabled": false, @@ -65,14 +66,15 @@ func TestToNodeUpsertRequest(t *testing.T) { ContainerRuntimeVersion: "containerd://1.7.13", NetPrivateIP: "10.0.0.10", AgentConfig: inventory.AgentConfig{ - TotalComponents: 30, - RetentionPeriodSeconds: 86400, - EnabledComponents: []string{"cpu", "gpu"}, - DisabledComponents: []string{"disk"}, - InventoryEnabled: true, - InventoryIntervalSeconds: 3600, - AttestationEnabled: true, - AttestationIntervalSeconds: 86400, + TotalComponents: 30, + RetentionPeriodSeconds: 86400, + MetricScrapeIntervalSeconds: 60, + EnabledComponents: []string{"cpu", "gpu"}, + DisabledComponents: []string{"disk"}, + InventoryEnabled: true, + InventoryIntervalSeconds: 3600, + AttestationEnabled: true, + AttestationIntervalSeconds: 86400, }, Resources: inventory.Resources{ CPUInfo: inventory.CPUInfo{ @@ -130,6 +132,7 @@ func TestToNodeUpsertRequest(t *testing.T) { require.Equal(t, bootTime.UTC(), *req.Uptime) require.Equal(t, int64(30), req.AgentConfig.TotalComponents) require.Equal(t, int64(86400), req.AgentConfig.RetentionPeriodSeconds) + require.Equal(t, int64(60), req.AgentConfig.MetricScrapeIntervalSeconds) require.Equal(t, []string{"cpu", "gpu"}, req.AgentConfig.EnabledComponents) require.Equal(t, []string{"disk"}, req.AgentConfig.DisabledComponents) require.True(t, req.AgentConfig.InventoryEnabled) diff --git a/internal/inventory/source/source_test.go b/internal/inventory/source/source_test.go index 3b857214..6bb6028e 100644 --- a/internal/inventory/source/source_test.go +++ b/internal/inventory/source/source_test.go @@ -129,14 +129,15 @@ func TestMachineInfoSourceCollectWithAgentConfig(t *testing.T) { }, }, &inventory.AgentConfig{ - TotalComponents: 42, - RetentionPeriodSeconds: 86400, - EnabledComponents: []string{"cpu", "gpu"}, - DisabledComponents: []string{"disk"}, - InventoryEnabled: true, - InventoryIntervalSeconds: 3600, - AttestationEnabled: true, - AttestationIntervalSeconds: 86400, + TotalComponents: 42, + RetentionPeriodSeconds: 86400, + MetricScrapeIntervalSeconds: 60, + EnabledComponents: []string{"cpu", "gpu"}, + DisabledComponents: []string{"disk"}, + InventoryEnabled: true, + InventoryIntervalSeconds: 3600, + AttestationEnabled: true, + AttestationIntervalSeconds: 86400, }, ) @@ -146,6 +147,7 @@ func TestMachineInfoSourceCollectWithAgentConfig(t *testing.T) { require.Equal(t, "machine-id", snap.MachineID) require.Equal(t, int64(42), snap.AgentConfig.TotalComponents) require.Equal(t, int64(86400), snap.AgentConfig.RetentionPeriodSeconds) + require.Equal(t, int64(60), snap.AgentConfig.MetricScrapeIntervalSeconds) require.Equal(t, []string{"cpu", "gpu"}, snap.AgentConfig.EnabledComponents) require.Equal(t, []string{"disk"}, snap.AgentConfig.DisabledComponents) require.True(t, snap.AgentConfig.InventoryEnabled) diff --git a/internal/inventory/types.go b/internal/inventory/types.go index e0f13f26..a3215133 100644 --- a/internal/inventory/types.go +++ b/internal/inventory/types.go @@ -48,14 +48,15 @@ type Snapshot struct { } type AgentConfig struct { - TotalComponents int64 - RetentionPeriodSeconds int64 - EnabledComponents []string - DisabledComponents []string - InventoryEnabled bool - InventoryIntervalSeconds int64 - AttestationEnabled bool - AttestationIntervalSeconds int64 + TotalComponents int64 + RetentionPeriodSeconds int64 + MetricScrapeIntervalSeconds int64 + EnabledComponents []string + DisabledComponents []string + InventoryEnabled bool + InventoryIntervalSeconds int64 + AttestationEnabled bool + AttestationIntervalSeconds int64 } type Resources struct { diff --git a/internal/server/server.go b/internal/server/server.go index 8525056a..adabeb98 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -175,11 +175,7 @@ func initializeMachineID(ctx context.Context, dbRW, dbRO *sql.DB) (string, error // getHealthCheckInterval determines the health check interval from config func getHealthCheckInterval(config *config.Config) time.Duration { - healthCheckInterval := time.Minute // default - if config.HealthExporter != nil && config.HealthExporter.HealthCheckInterval.Duration > 0 { - healthCheckInterval = config.HealthExporter.HealthCheckInterval.Duration - } - return healthCheckInterval + return config.HealthCheckInterval() } func getInventorySyncInterval(config *config.Config) time.Duration { @@ -474,14 +470,15 @@ func (s *Server) startInventoryLoop( return machineinfo.GetMachineInfo(nvmlInstance, machineinfo.WithDCGMGPUIndexes(dcgmGPUIndexes)) }), &inventory.AgentConfig{ - TotalComponents: int64(len(allComponents)), - RetentionPeriodSeconds: retentionPeriodSeconds, - EnabledComponents: enabledComponents, - DisabledComponents: disabledComponents, - InventoryEnabled: inventoryEnabled, - InventoryIntervalSeconds: inventoryIntervalSeconds, - AttestationEnabled: attestationEnabled, - AttestationIntervalSeconds: attestationIntervalSeconds, + TotalComponents: int64(len(allComponents)), + RetentionPeriodSeconds: retentionPeriodSeconds, + MetricScrapeIntervalSeconds: cfg.MetricScrapeIntervalSeconds(), + EnabledComponents: enabledComponents, + DisabledComponents: disabledComponents, + InventoryEnabled: inventoryEnabled, + InventoryIntervalSeconds: inventoryIntervalSeconds, + AttestationEnabled: attestationEnabled, + AttestationIntervalSeconds: attestationIntervalSeconds, }, ) sink := inventorysink.NewBackendSink(agentstate.NewSQLite()) diff --git a/internal/validation/outbound/validator.go b/internal/validation/outbound/validator.go index ebe92775..7c0ca808 100644 --- a/internal/validation/outbound/validator.go +++ b/internal/validation/outbound/validator.go @@ -94,6 +94,7 @@ func ValidateNodeUpsertRequest(req *backendclient.NodeUpsertRequest) []Issue { validateNonNegative(&issues, "numeric", "agentConfig.totalComponents", req.AgentConfig.TotalComponents) validateNonNegative(&issues, "numeric", "agentConfig.retentionPeriodSeconds", req.AgentConfig.RetentionPeriodSeconds) + validateNonNegative(&issues, "numeric", "agentConfig.metricScrapeIntervalSeconds", req.AgentConfig.MetricScrapeIntervalSeconds) validateNonNegative(&issues, "numeric", "agentConfig.inventoryIntervalSeconds", req.AgentConfig.InventoryIntervalSeconds) validateNonNegative(&issues, "numeric", "agentConfig.attestationIntervalSeconds", req.AgentConfig.AttestationIntervalSeconds) diff --git a/internal/validation/outbound/validator_test.go b/internal/validation/outbound/validator_test.go index f5ac6a73..9ef8a4c3 100644 --- a/internal/validation/outbound/validator_test.go +++ b/internal/validation/outbound/validator_test.go @@ -37,6 +37,9 @@ func TestValidateNodeUpsertRequest(t *testing.T) { MachineID: "machine-1", SystemUUID: "not-a-uuid", Uptime: &now, + AgentConfig: backendclient.AgentConfig{ + MetricScrapeIntervalSeconds: -1, + }, Resources: backendclient.NodeResources{ CPUInfo: backendclient.CPUInfo{LogicalCores: "-1"}, MemoryInfo: backendclient.MemoryInfo{ @@ -64,6 +67,12 @@ func TestValidateNodeUpsertRequest(t *testing.T) { Field: "hostname", Message: "field is required", }) + require.Contains(t, issues, Issue{ + Severity: SeverityWarning, + Category: "numeric", + Field: "agentConfig.metricScrapeIntervalSeconds", + Message: "value is negative", + }) require.Contains(t, issues, Issue{ Severity: SeverityCritical, Category: "numeric", From b42a68c59fd3e2b85eca915aec5a1b591a699c62 Mon Sep 17 00:00:00 2001 From: ambermingxin Date: Thu, 25 Jun 2026 16:33:08 -0700 Subject: [PATCH 15/16] feat: mark persistence mode diable as warning level (#225) Signed-off-by: Amber Xue --- .../accelerator/nvidia/persistence-mode/component.go | 6 +++++- .../accelerator/nvidia/persistence-mode/component_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/persistence-mode/component.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/persistence-mode/component.go index cdbaedd5..7f5482f1 100644 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/persistence-mode/component.go +++ b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/persistence-mode/component.go @@ -170,6 +170,7 @@ func (c *component) Check() components.CheckResult { persistenceMode, err := c.getPersistenceModeFunc(uuid, dev) if err != nil { cr.err = err + // deliberately stayed Unhealthy to indicate NVML query errors, GPU-lost, and GPU-requires-reset cr.health = apiv1.HealthStateTypeUnhealthy cr.reason = "error getting persistence mode" @@ -208,7 +209,10 @@ func (c *component) Check() components.CheckResult { } if len(notEnabled) > 0 { - cr.health = apiv1.HealthStateTypeUnhealthy + // Persistence mode being disabled is a configuration issue that does not + // affect running workloads, so it is flagged as a warning (Degraded) + // rather than critical (Unhealthy). + cr.health = apiv1.HealthStateTypeDegraded if len(notEnabled) == len(cr.PersistenceModes) { cr.reason = fmt.Sprintf("all %d GPU(s) disabled persistence mode", len(devs)) } else { diff --git a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/persistence-mode/component_test.go b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/persistence-mode/component_test.go index 7795d7fb..eead13cc 100644 --- a/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/persistence-mode/component_test.go +++ b/third_party/fleet-intelligence-sdk/components/accelerator/nvidia/persistence-mode/component_test.go @@ -337,8 +337,8 @@ func TestCheck_MultipleDevices(t *testing.T) { require.True(t, ok, "result should be of type *checkResult") require.NotNil(t, data, "data should not be nil") - // This should be unhealthy since one device has persistence mode supported but not enabled - assert.Equal(t, apiv1.HealthStateTypeUnhealthy, data.health, "data should be marked unhealthy") + // This should be degraded (warning) since one device has persistence mode supported but not enabled + assert.Equal(t, apiv1.HealthStateTypeDegraded, data.health, "data should be marked degraded") assert.Equal(t, "gpu-uuid-456 persistence mode supported but not enabled", data.reason) assert.Len(t, data.PersistenceModes, 2) @@ -734,7 +734,7 @@ func TestPersistenceModeCheck(t *testing.T) { {UUID: "GPU-1", Supported: true, Enabled: true}, {UUID: "GPU-2", Supported: true, Enabled: false}, }, - expectedHealth: apiv1.HealthStateTypeUnhealthy, + expectedHealth: apiv1.HealthStateTypeDegraded, expectedReasonCmp: "GPU-2 persistence mode supported but not enabled", }, { @@ -743,7 +743,7 @@ func TestPersistenceModeCheck(t *testing.T) { {UUID: "GPU-1", Supported: true, Enabled: false}, {UUID: "GPU-2", Supported: true, Enabled: false}, }, - expectedHealth: apiv1.HealthStateTypeUnhealthy, + expectedHealth: apiv1.HealthStateTypeDegraded, expectedReasonCmp: "all 2 GPU(s) disabled persistence mode", }, { From 6eaf5d2022b0f5530969156b0647edcb51f38426 Mon Sep 17 00:00:00 2001 From: Jingxiang Zhang <72907213+jingxiang-z@users.noreply.github.com> Date: Fri, 26 Jun 2026 14:00:04 -0700 Subject: [PATCH 16/16] fix: initialize node uuid during enrollment (#224) Signed-off-by: Jingxiang Zhang --- .../templates/daemonset.yaml | 3 + go.mod | 2 +- internal/agentstate/sqlite.go | 82 ++++++- internal/agentstate/sqlite_test.go | 38 ++++ internal/agentstate/state.go | 3 + internal/attestation/backend_test.go | 15 ++ internal/enrollment/enrollment.go | 8 +- internal/enrollment/enrollment_test.go | 44 ++++ internal/inventory/sink/backend_test.go | 14 ++ internal/nodeidentity/nodeidentity.go | 103 +++++++++ internal/nodeidentity/nodeidentity_test.go | 203 ++++++++++++++++++ internal/server/server.go | 47 ++-- internal/server/server_test.go | 25 +-- 13 files changed, 530 insertions(+), 57 deletions(-) create mode 100644 internal/nodeidentity/nodeidentity.go create mode 100644 internal/nodeidentity/nodeidentity_test.go diff --git a/deployments/helm/fleet-intelligence-agent/templates/daemonset.yaml b/deployments/helm/fleet-intelligence-agent/templates/daemonset.yaml index 8c059f83..1e738e43 100644 --- a/deployments/helm/fleet-intelligence-agent/templates/daemonset.yaml +++ b/deployments/helm/fleet-intelligence-agent/templates/daemonset.yaml @@ -101,6 +101,9 @@ spec: volumeMounts: - name: dev mountPath: /dev + - name: sys + mountPath: /sys + readOnly: true - name: state mountPath: /var/lib/fleetint {{- else if .Values.enroll.unenroll }} diff --git a/go.mod b/go.mod index e70c18eb..f21f8939 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/mitchellh/go-homedir v1.1.0 github.com/olekukonko/tablewriter v0.0.5 github.com/prometheus/client_golang v1.23.0 + github.com/prometheus/procfs v0.16.1 github.com/stretchr/testify v1.11.1 github.com/urfave/cli v1.22.17 go.opentelemetry.io/proto/otlp v1.8.0 @@ -76,7 +77,6 @@ require ( github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.65.0 // indirect - github.com/prometheus/procfs v0.16.1 // indirect github.com/quic-go/qpack v0.6.0 // indirect github.com/quic-go/quic-go v0.57.0 // indirect github.com/rivo/uniseg v0.4.4 // indirect diff --git a/internal/agentstate/sqlite.go b/internal/agentstate/sqlite.go index 523c682f..5f87e46e 100644 --- a/internal/agentstate/sqlite.go +++ b/internal/agentstate/sqlite.go @@ -112,6 +112,85 @@ func (s *sqliteState) SetNodeUUID(ctx context.Context, value string) error { return s.setMetadata(ctx, pkgmetadata.MetadataKeyMachineID, value) } +func (s *sqliteState) GetOrCreateNodeUUID(ctx context.Context, create func() (string, error)) (string, bool, error) { + db, err := s.openReadWrite() + if err != nil { + return "", false, err + } + defer db.Close() + + if stateFile, err := s.stateFileFn(); err == nil { + if err := config.SecureStateFilePermissions(stateFile); err != nil { + return "", false, fmt.Errorf("secure state file permissions: %w", err) + } + } + if err := pkgmetadata.CreateTableMetadata(ctx, db); err != nil { + return "", false, fmt.Errorf("create metadata table: %w", err) + } + + return GetOrCreateNodeUUIDMetadata(ctx, db, create) +} + +// GetOrCreateNodeUUIDMetadata atomically returns the committed node UUID +// metadata value, creating it when missing or repairing an empty value. +func GetOrCreateNodeUUIDMetadata(ctx context.Context, db *sql.DB, create func() (string, error)) (string, bool, error) { + if create == nil { + return "", false, fmt.Errorf("node UUID create function is required") + } + + existing, ok, err := readNodeUUIDDB(ctx, db) + if err != nil { + return "", false, err + } else if ok { + return existing, false, nil + } + + candidate, err := create() + if err != nil { + return "", false, err + } + if candidate == "" { + return "", false, fmt.Errorf("created node UUID is empty") + } + if _, err := db.ExecContext(ctx, ` +INSERT INTO gpud_metadata (key, value) VALUES (?, ?) +ON CONFLICT(key) DO UPDATE SET value = excluded.value +WHERE gpud_metadata.value = ''`, + pkgmetadata.MetadataKeyMachineID, + candidate, + ); err != nil { + return "", false, fmt.Errorf("insert node UUID metadata: %w", err) + } + + committed, ok, err := readNodeUUIDDB(ctx, db) + if err != nil { + return "", false, err + } else if !ok { + return "", false, fmt.Errorf("node UUID metadata was not committed") + } else if committed == "" { + return "", false, fmt.Errorf("node UUID metadata is empty") + } + return committed, committed == candidate, nil +} + +func readNodeUUIDDB(ctx context.Context, db *sql.DB) (string, bool, error) { + var value string + err := db.QueryRowContext(ctx, ` +SELECT value FROM gpud_metadata WHERE key = ?`, + pkgmetadata.MetadataKeyMachineID, + ).Scan(&value) + switch { + case err == nil && value != "": + return value, true, nil + case err == nil: + return "", false, nil + case errors.Is(err, sql.ErrNoRows): + return "", false, nil + default: + return "", false, fmt.Errorf("read node UUID metadata: %w", err) + } +} + func (s *sqliteState) GetNodeGroup(ctx context.Context) (string, bool, error) { return s.getMetadata(ctx, MetadataKeyNodeGroup) } @@ -174,8 +253,7 @@ func (s *sqliteState) setMetadata(ctx context.Context, key, value string) error } defer db.Close() - stateFile, err := s.stateFileFn() - if err == nil { + if stateFile, err := s.stateFileFn(); err == nil { if err := config.SecureStateFilePermissions(stateFile); err != nil { return fmt.Errorf("secure state file permissions: %w", err) } diff --git a/internal/agentstate/sqlite_test.go b/internal/agentstate/sqlite_test.go index 724fc689..c59c9fe8 100644 --- a/internal/agentstate/sqlite_test.go +++ b/internal/agentstate/sqlite_test.go @@ -120,6 +120,44 @@ func TestSQLiteStateMissingValue(t *testing.T) { require.Empty(t, value) } +func TestSQLiteStateGetOrCreateNodeUUID(t *testing.T) { + t.Parallel() + + ctx := context.Background() + state := newTestSQLiteState(t) + + value, created, err := state.GetOrCreateNodeUUID(ctx, func() (string, error) { + return "node-created", nil + }) + require.NoError(t, err) + require.True(t, created) + require.Equal(t, "node-created", value) + + value, created, err = state.GetOrCreateNodeUUID(ctx, func() (string, error) { + t.Fatal("create should not run when node UUID is already persisted") + return "", nil + }) + require.NoError(t, err) + require.False(t, created) + require.Equal(t, "node-created", value) + + emptyRowState := newTestSQLiteState(t) + err = emptyRowState.SetNodeUUID(ctx, "") + require.NoError(t, err) + + value, created, err = emptyRowState.GetOrCreateNodeUUID(ctx, func() (string, error) { + return "node-repaired", nil + }) + require.NoError(t, err) + require.True(t, created) + require.Equal(t, "node-repaired", value) + + value, ok, err := emptyRowState.GetNodeUUID(ctx) + require.NoError(t, err) + require.True(t, ok) + require.Equal(t, "node-repaired", value) +} + func TestSQLiteStateMissingMetadataTableIsTreatedAsAbsent(t *testing.T) { t.Parallel() diff --git a/internal/agentstate/state.go b/internal/agentstate/state.go index 2f705f3d..1e3230a6 100644 --- a/internal/agentstate/state.go +++ b/internal/agentstate/state.go @@ -40,8 +40,11 @@ type State interface { GetSAK(ctx context.Context) (value string, ok bool, err error) SetSAK(ctx context.Context, value string) error + // GetNodeUUID reads the committed node UUID without creating one. GetNodeUUID(ctx context.Context) (value string, ok bool, err error) SetNodeUUID(ctx context.Context, value string) error + // GetOrCreateNodeUUID creates and commits the node UUID when no value exists. + GetOrCreateNodeUUID(ctx context.Context, create func() (string, error)) (value string, created bool, err error) GetEnrollmentTime(ctx context.Context) (value time.Time, ok bool, err error) SetEnrollmentTime(ctx context.Context, value time.Time) error diff --git a/internal/attestation/backend_test.go b/internal/attestation/backend_test.go index 86b109ae..1066dea7 100644 --- a/internal/attestation/backend_test.go +++ b/internal/attestation/backend_test.go @@ -51,6 +51,21 @@ func (s *stubState) GetNodeUUID(context.Context) (string, bool, error) { return s.nodeUUID, s.nodeOK, s.nodeErr } func (s *stubState) SetNodeUUID(context.Context, string) error { return nil } +func (s *stubState) GetOrCreateNodeUUID(_ context.Context, create func() (string, error)) (string, bool, error) { + if s.nodeErr != nil { + return "", false, s.nodeErr + } + if s.nodeOK && s.nodeUUID != "" { + return s.nodeUUID, false, nil + } + value, err := create() + if err != nil { + return "", false, err + } + s.nodeUUID = value + s.nodeOK = true + return value, true, nil +} func (s *stubState) GetNodeGroup(context.Context) (string, bool, error) { return "", false, nil } diff --git a/internal/enrollment/enrollment.go b/internal/enrollment/enrollment.go index 682494f9..4f988f7d 100644 --- a/internal/enrollment/enrollment.go +++ b/internal/enrollment/enrollment.go @@ -36,12 +36,15 @@ import ( inventorysink "github.com/NVIDIA/fleet-intelligence-agent/internal/inventory/sink" inventorysource "github.com/NVIDIA/fleet-intelligence-agent/internal/inventory/source" "github.com/NVIDIA/fleet-intelligence-agent/internal/machineinfo" + "github.com/NVIDIA/fleet-intelligence-agent/internal/nodeidentity" "github.com/NVIDIA/fleet-intelligence-agent/internal/registry" ) var ( newBackendClient = backendclient.New syncInventoryAfterEnroll = syncInventoryOnce + ensureNodeUUID = nodeidentity.EnsureNodeUUID + storeEnrollmentConfig = storeConfigInMetadata postEnrollInventorySyncTimeout = time.Minute ) @@ -76,8 +79,11 @@ func EnrollWithConfigAndMetadata(ctx context.Context, baseEndpoint, sakToken str if err != nil { return err } + if _, err := ensureNodeUUID(ctx, agentstate.NewSQLite()); err != nil { + return fmt.Errorf("failed to initialize node UUID: %w", err) + } enrolledAt := time.Now().UTC() - if err := storeConfigInMetadata(ctx, baseURL.String(), jwtToken, sakToken, enrolledAt, normalizedEnrollMetadata(metadata)); err != nil { + if err := storeEnrollmentConfig(ctx, baseURL.String(), jwtToken, sakToken, enrolledAt, normalizedEnrollMetadata(metadata)); err != nil { return fmt.Errorf("failed to store configuration: %w", err) } syncCtx, cancel := context.WithTimeout(ctx, postEnrollInventorySyncTimeout) diff --git a/internal/enrollment/enrollment_test.go b/internal/enrollment/enrollment_test.go index a01f7b72..6b0fcda8 100644 --- a/internal/enrollment/enrollment_test.go +++ b/internal/enrollment/enrollment_test.go @@ -28,6 +28,7 @@ import ( "github.com/NVIDIA/fleet-intelligence-agent/internal/agentstate" "github.com/NVIDIA/fleet-intelligence-agent/internal/backendclient" "github.com/NVIDIA/fleet-intelligence-agent/internal/config" + "github.com/NVIDIA/fleet-intelligence-agent/internal/nodeidentity" ) type fakeBackendClient struct { @@ -60,9 +61,11 @@ func (f *fakeBackendClient) SubmitAttestation(context.Context, string, *backendc func TestEnrollWorkflow(t *testing.T) { originalFactory := newBackendClient originalSync := syncInventoryAfterEnroll + originalEnsure := ensureNodeUUID t.Cleanup(func() { newBackendClient = originalFactory syncInventoryAfterEnroll = originalSync + ensureNodeUUID = originalEnsure }) client := &fakeBackendClient{enrollJWT: "jwt-token"} @@ -72,7 +75,14 @@ func TestEnrollWorkflow(t *testing.T) { } syncCalled := false + ensureCalled := false + ensureNodeUUID = func(ctx context.Context, state nodeidentity.State) (string, error) { + require.NotNil(t, state) + ensureCalled = true + return "4c4c4544-0053-5210-8038-c8c04f583034", nil + } syncInventoryAfterEnroll = func(ctx context.Context, cfg *config.Config) error { + require.True(t, ensureCalled, "node UUID should be initialized before post-enroll inventory sync") syncCalled = true return nil } @@ -83,6 +93,7 @@ func TestEnrollWorkflow(t *testing.T) { err := Enroll(context.Background(), "https://example.com", "sak-token") require.NoError(t, err) require.Equal(t, "sak-token", client.enrollSAK) + require.True(t, ensureCalled) require.True(t, syncCalled) } @@ -340,6 +351,39 @@ func TestEnrollWorkflowErrors(t *testing.T) { require.ErrorContains(t, err, "enroll boom") }) + t.Run("node UUID initialization", func(t *testing.T) { + originalFactory := newBackendClient + originalEnsure := ensureNodeUUID + originalSync := syncInventoryAfterEnroll + originalStore := storeEnrollmentConfig + t.Cleanup(func() { + newBackendClient = originalFactory + ensureNodeUUID = originalEnsure + syncInventoryAfterEnroll = originalSync + storeEnrollmentConfig = originalStore + }) + newBackendClient = func(rawBaseURL string) (backendclient.Client, error) { + return &fakeBackendClient{enrollJWT: "jwt-token"}, nil + } + ensureNodeUUID = func(context.Context, nodeidentity.State) (string, error) { + return "", errors.New("identity store failed") + } + syncInventoryAfterEnroll = func(context.Context, *config.Config) error { + t.Fatal("inventory sync should not run when node UUID initialization fails") + return nil + } + storeEnrollmentConfig = func(context.Context, string, string, string, time.Time, EnrollMetadata) error { + t.Fatal("enrollment metadata should not be stored when node UUID initialization fails") + return nil + } + + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + + err := Enroll(context.Background(), "https://example.com", "sak-token") + require.ErrorContains(t, err, "failed to initialize node UUID") + }) + t.Run("localhost legacy endpoint allowed", func(t *testing.T) { originalFactory := newBackendClient originalSync := syncInventoryAfterEnroll diff --git a/internal/inventory/sink/backend_test.go b/internal/inventory/sink/backend_test.go index 12713457..628f8593 100644 --- a/internal/inventory/sink/backend_test.go +++ b/internal/inventory/sink/backend_test.go @@ -63,6 +63,20 @@ func (f *fakeState) GetNodeUUID(context.Context) (string, bool, error) { return f.nodeUUID, f.nodeUUID != "", nil } func (f *fakeState) SetNodeUUID(context.Context, string) error { return nil } +func (f *fakeState) GetOrCreateNodeUUID(_ context.Context, create func() (string, error)) (string, bool, error) { + if f.err != nil { + return "", false, f.err + } + if f.nodeUUID != "" { + return f.nodeUUID, false, nil + } + value, err := create() + if err != nil { + return "", false, err + } + f.nodeUUID = value + return value, true, nil +} func (f *fakeState) GetNodeGroup(context.Context) (string, bool, error) { if f.nodeGroupErr != nil { return "", false, f.nodeGroupErr diff --git a/internal/nodeidentity/nodeidentity.go b/internal/nodeidentity/nodeidentity.go new file mode 100644 index 00000000..153e1a2e --- /dev/null +++ b/internal/nodeidentity/nodeidentity.go @@ -0,0 +1,103 @@ +// 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 nodeidentity initializes and persists the stable node UUID used by +// enrollment, inventory, attestation, and metrics workflows. +// The persisted node UUID is stored in the legacy machine ID metadata key. +package nodeidentity + +import ( + "context" + "errors" + "fmt" + "strings" + + "github.com/google/uuid" + "github.com/prometheus/procfs/sysfs" + + "github.com/NVIDIA/fleet-intelligence-sdk/pkg/log" +) + +// State is the local storage contract needed to persist node identity. +type State interface { + GetOrCreateNodeUUID(ctx context.Context, create func() (string, error)) (value string, created bool, err error) +} + +var ( + readHardwareUUID = readDMIProductUUID + newNodeUUID = uuid.NewString +) + +// EnsureNodeUUID returns the persisted node UUID, or initializes and persists it +// from DMI sysfs with a random UUID fallback. +func EnsureNodeUUID(ctx context.Context, state State) (string, error) { + if state == nil { + return "", errors.New("node identity state is required") + } + + nodeUUID, created, err := state.GetOrCreateNodeUUID(ctx, func() (string, error) { + nodeUUID, err := readHardwareUUID() + if err != nil || nodeUUID == "" { + nodeUUID = newNodeUUID() + log.Logger.Warnw("Failed to get hardware UUID, generated random agent ID", + "error", err, + "generated_id", nodeUUID) + } else { + log.Logger.Infow("Initialized agent ID from hardware UUID", "machine_id", nodeUUID) + } + return nodeUUID, nil + }) + if err != nil { + return "", fmt.Errorf("failed to ensure node UUID: %w", err) + } + if created { + log.Logger.Infow("Persisted agent ID to database", "machine_id", nodeUUID) + } else { + log.Logger.Infow("Using persisted agent ID from database", "machine_id", nodeUUID) + } + return nodeUUID, nil +} + +func readDMIProductUUID() (string, error) { + return readDMIProductUUIDFromFS("/sys") +} + +func readDMIProductUUIDFromFS(root string) (string, error) { + fs, err := sysfs.NewFS(root) + if err != nil { + return "", fmt.Errorf("failed to open sysfs: %w", err) + } + + dmi, err := fs.DMIClass() + if err != nil { + return "", fmt.Errorf("failed to read DMI class: %w", err) + } + if dmi.ProductUUID == nil { + return "", errors.New("DMI product UUID not found") + } + + nodeUUID := strings.TrimSpace(*dmi.ProductUUID) + if nodeUUID == "" { + return "", errors.New("DMI product UUID is empty") + } + parsedUUID, err := uuid.Parse(nodeUUID) + if err != nil { + return "", fmt.Errorf("invalid DMI product UUID %q: %w", nodeUUID, err) + } + if parsedUUID == uuid.Nil { + return "", errors.New("DMI product UUID is unset") + } + return nodeUUID, nil +} diff --git a/internal/nodeidentity/nodeidentity_test.go b/internal/nodeidentity/nodeidentity_test.go new file mode 100644 index 00000000..783fceb4 --- /dev/null +++ b/internal/nodeidentity/nodeidentity_test.go @@ -0,0 +1,203 @@ +// 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 nodeidentity + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type fakeState struct { + nodeUUID string + ok bool + err error + calls int + commitAs string +} + +func (s *fakeState) GetOrCreateNodeUUID(_ context.Context, create func() (string, error)) (string, bool, error) { + s.calls++ + if s.err != nil { + return "", false, s.err + } + if s.ok && s.nodeUUID != "" { + return s.nodeUUID, false, nil + } + value, err := create() + if err != nil { + return "", false, err + } + created := s.commitAs == "" + if s.commitAs != "" { + value = s.commitAs + } + s.nodeUUID = value + s.ok = true + return value, created, nil +} + +func writeTestDMIProductUUID(t *testing.T, root, id string) { + t.Helper() + + dmiIDDir := filepath.Join(root, "class", "dmi", "id") + require.NoError(t, os.MkdirAll(dmiIDDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dmiIDDir, "product_uuid"), []byte(id), 0o644)) +} + +func overrideUUIDSources(t *testing.T, read func() (string, error), generate func() string) { + t.Helper() + + previousRead := readHardwareUUID + previousGenerate := newNodeUUID + readHardwareUUID = read + newNodeUUID = generate + t.Cleanup(func() { + readHardwareUUID = previousRead + newNodeUUID = previousGenerate + }) +} + +func TestReadDMIProductUUIDFromFS(t *testing.T) { + const validUUID = "4c4c4544-0053-5210-8038-c8c04f583034" + + t.Run("reads product uuid", func(t *testing.T) { + root := t.TempDir() + writeTestDMIProductUUID(t, root, validUUID+"\n") + + id, err := readDMIProductUUIDFromFS(root) + require.NoError(t, err) + assert.Equal(t, validUUID, id) + }) + + t.Run("errors when product uuid is missing", func(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(root, "class", "dmi", "id"), 0o755)) + + id, err := readDMIProductUUIDFromFS(root) + require.Error(t, err) + assert.Empty(t, id) + assert.Contains(t, err.Error(), "DMI product UUID not found") + }) + + t.Run("errors when product uuid is invalid", func(t *testing.T) { + root := t.TempDir() + writeTestDMIProductUUID(t, root, "Not Settable\n") + + id, err := readDMIProductUUIDFromFS(root) + require.Error(t, err) + assert.Empty(t, id) + assert.Contains(t, err.Error(), "invalid DMI product UUID") + }) + + t.Run("errors when product uuid is zero", func(t *testing.T) { + root := t.TempDir() + writeTestDMIProductUUID(t, root, "00000000-0000-0000-0000-000000000000\n") + + id, err := readDMIProductUUIDFromFS(root) + require.Error(t, err) + assert.Empty(t, id) + assert.Contains(t, err.Error(), "DMI product UUID is unset") + }) +} + +func TestEnsureNodeUUID(t *testing.T) { + const ( + dmiUUID = "4c4c4544-0053-5210-8038-c8c04f583034" + generatedID = "6f459d44-ee11-44c1-b633-e810390ab3f7" + ) + + t.Run("uses persisted UUID", func(t *testing.T) { + state := &fakeState{nodeUUID: dmiUUID, ok: true} + overrideUUIDSources(t, func() (string, error) { + t.Fatal("hardware UUID should not be read when UUID is already persisted") + return "", nil + }, func() string { + t.Fatal("random UUID should not be generated when UUID is already persisted") + return "" + }) + + id, err := EnsureNodeUUID(context.Background(), state) + require.NoError(t, err) + assert.Equal(t, dmiUUID, id) + assert.Equal(t, 1, state.calls) + }) + + t.Run("persists DMI product UUID", func(t *testing.T) { + state := &fakeState{} + overrideUUIDSources(t, func() (string, error) { + return dmiUUID, nil + }, func() string { + return generatedID + }) + + id, err := EnsureNodeUUID(context.Background(), state) + require.NoError(t, err) + assert.Equal(t, dmiUUID, id) + assert.Equal(t, dmiUUID, state.nodeUUID) + assert.Equal(t, 1, state.calls) + }) + + t.Run("persists random UUID when DMI fails", func(t *testing.T) { + state := &fakeState{} + overrideUUIDSources(t, func() (string, error) { + return "", errors.New("dmi unavailable") + }, func() string { + return generatedID + }) + + id, err := EnsureNodeUUID(context.Background(), state) + require.NoError(t, err) + assert.Equal(t, generatedID, id) + assert.Equal(t, generatedID, state.nodeUUID) + assert.Equal(t, 1, state.calls) + }) + + t.Run("returns committed UUID from state", func(t *testing.T) { + const committedID = "3758d746-9bad-4f47-b240-ab73d6f68c9a" + state := &fakeState{commitAs: committedID} + overrideUUIDSources(t, func() (string, error) { + return dmiUUID, nil + }, func() string { + return generatedID + }) + + id, err := EnsureNodeUUID(context.Background(), state) + require.NoError(t, err) + assert.Equal(t, committedID, id) + assert.Equal(t, committedID, state.nodeUUID) + assert.Equal(t, 1, state.calls) + }) + + t.Run("propagates state error", func(t *testing.T) { + state := &fakeState{err: errors.New("write failed")} + overrideUUIDSources(t, func() (string, error) { + return dmiUUID, nil + }, func() string { + return generatedID + }) + + id, err := EnsureNodeUUID(context.Background(), state) + require.Error(t, err) + assert.Empty(t, id) + assert.Contains(t, err.Error(), "failed to ensure node UUID") + }) +} diff --git a/internal/server/server.go b/internal/server/server.go index adabeb98..cabe6246 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -34,7 +34,6 @@ import ( "github.com/gin-contrib/gzip" "github.com/gin-gonic/gin" - "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/NVIDIA/fleet-intelligence-sdk/components" @@ -61,6 +60,7 @@ import ( inventorysink "github.com/NVIDIA/fleet-intelligence-agent/internal/inventory/sink" inventorysource "github.com/NVIDIA/fleet-intelligence-agent/internal/inventory/source" "github.com/NVIDIA/fleet-intelligence-agent/internal/machineinfo" + "github.com/NVIDIA/fleet-intelligence-agent/internal/nodeidentity" "github.com/NVIDIA/fleet-intelligence-agent/internal/registry" ) @@ -137,40 +137,23 @@ func initializeDatabases(ctx context.Context, cfg *config.Config) (*sql.DB, *sql return dbRW, dbRO, nil } -// initializeMachineID retrieves or creates a machine ID -// This establishes the agent's stable identity for metrics reporting. -// Priority: DB (persisted) → dmidecode (hardware UUID) → random UUID -func initializeMachineID(ctx context.Context, dbRW, dbRO *sql.DB) (string, error) { - // Try to read existing machine ID from database - machineID, err := pkgmetadata.ReadMachineID(ctx, dbRO) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - return "", fmt.Errorf("failed to read machine uid: %w", err) - } - - // If no machine ID found in database, initialize a new one - if machineID == "" { - // First, try to get hardware UUID from dmidecode - machineID, err = pkghost.GetDmidecodeUUID(ctx) - if err != nil || machineID == "" { - // If dmidecode fails (permissions, not available, etc.), generate a random UUID - machineID = uuid.New().String() - log.Logger.Warnw("Failed to get hardware UUID, generated random agent ID", - "error", err, - "generated_id", machineID) - } else { - log.Logger.Infow("Initialized agent ID from hardware UUID", "machine_id", machineID) - } +type dbNodeUUIDState struct { + dbRW *sql.DB + dbRO *sql.DB +} - // Store the machine ID in database for persistence - if err := pkgmetadata.SetMetadata(ctx, dbRW, pkgmetadata.MetadataKeyMachineID, machineID); err != nil { - return "", fmt.Errorf("failed to store machine ID in database: %w", err) - } - log.Logger.Infow("Persisted agent ID to database", "machine_id", machineID) - } else { - log.Logger.Infow("Using persisted agent ID from database", "machine_id", machineID) +func (s dbNodeUUIDState) GetOrCreateNodeUUID(ctx context.Context, create func() (string, error)) (string, bool, error) { + if err := pkgmetadata.CreateTableMetadata(ctx, s.dbRW); err != nil { + return "", false, fmt.Errorf("create metadata table: %w", err) } + return agentstate.GetOrCreateNodeUUIDMetadata(ctx, s.dbRW, create) +} - return machineID, nil +// initializeMachineID retrieves or creates a machine ID +// This establishes the agent's stable identity for metrics reporting. +// Priority: DB (persisted) → DMI product UUID → random UUID +func initializeMachineID(ctx context.Context, dbRW, dbRO *sql.DB) (string, error) { + return nodeidentity.EnsureNodeUUID(ctx, dbNodeUUIDState{dbRW: dbRW, dbRO: dbRO}) } // getHealthCheckInterval determines the health check interval from config diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 010e4ff9..ef278e4c 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -382,35 +382,18 @@ func TestInitializeDatabasesSecuresFreshStateFile(t *testing.T) { // TestInitializeMachineID tests the initializeMachineID function. func TestInitializeMachineID(t *testing.T) { ctx := context.Background() - - // Initialize databases first (required for machine ID storage) - config := &config.Config{ - State: "", // Use in-memory database - } - dbRW, dbRO, err := initializeDatabases(ctx, config) + dbRW, dbRO, err := initializeDatabases(ctx, &config.Config{State: filepath.Join(t.TempDir(), "fleetint.state")}) require.NoError(t, err) defer dbRW.Close() defer dbRO.Close() - // Test initializeMachineID machineID, err := initializeMachineID(ctx, dbRW, dbRO) - - // The function should either succeed or fail gracefully - if err != nil { - // If it fails, it's likely due to missing pkgmetadata functionality - // which is acceptable in a unit test environment - t.Logf("initializeMachineID returned error (expected in test environment): %v", err) - return - } - - // If successful, verify the behavior + require.NoError(t, err) assert.NotEmpty(t, machineID) - // Verify machine ID can be read back machineID2, err := initializeMachineID(ctx, dbRW, dbRO) - if err == nil { - assert.Equal(t, machineID, machineID2, "Machine ID should be consistent across calls") - } + require.NoError(t, err) + assert.Equal(t, machineID, machineID2, "Machine ID should be consistent across calls") } // TestServerStop tests the Stop method.