diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 11c2256e..8d4586a2 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -2,25 +2,50 @@ This file provides guidance to Claude Code when working with code in this repository. - + -## 1. Project Overview and Architecture +## Start here + +This file is the short core. It carries the invariants an agent must not break, the +boundaries of what an agent may change on its own, and the conventions that are +specific to this codebase. Everything else lives in the document that already owns +it and is not repeated here: -Topograph discovers the physical network topology of a cluster (NVLink domains, InfiniBand/Ethernet switch fabric, cloud rack topology) and exposes it to workload schedulers — Slurm, Kubernetes, and Slurm-on-Kubernetes (Slinky). It has five runtime components: +| You need | Read | +|---|---| +| Prerequisites, build, local test loop, running a binary, container images, packaging, CI parity | [DEVELOPMENT.md](../DEVELOPMENT.md) | +| Issue-first workflow, DCO sign-off, commit and branch conventions, review process, AI-assisted contribution policy | [CONTRIBUTING.md](../CONTRIBUTING.md) | +| What each runtime component does and how a request flows between them | [docs/architecture.md](../docs/architecture.md) | +| Every label and annotation key the Kubernetes engine writes | [docs/reference/node-labels.md](../docs/reference/node-labels.md) | +| Provider selection, supported providers, the "Choosing a Provider" scenario table | `docs/overview.md` | +| API endpoints, request parameters, response fields, config schema | `docs/api.md` | +| How to report a suspected vulnerability | `SECURITY.md` | +| Release history and operator-facing migration notes | `CHANGELOG.md` | + +When you change behaviour that one of those documents describes, update that +document. Do not answer the same question here as well; two copies of a rule drift +apart, and the next agent reads the stale one. + +## 1. Project Overview and Architecture -- **API Server** — receives `/v1/generate` requests, aggregates bursts, dispatches to a Provider -- **Node Observer** — Kubernetes-only; watches configured node/pod changes and Topograph API readiness, then triggers regeneration -- **Node Data Broker** — Kubernetes-only DaemonSet; collects per-node attributes (NVLink clique IDs, etc.) as node annotations -- **Provider** — per-environment adapter that queries a topology source (CSP API, NetQ, `ibnetdiscover`, DRA labels) and returns a canonical representation -- **Engine** — per-scheduler translator that writes the canonical representation out as `topology.conf`, Kubernetes node labels, or a Slinky ConfigMap +Topograph discovers the physical network topology of a cluster (NVLink domains, +InfiniBand/Ethernet switch fabric, cloud rack topology) and exposes it to workload +schedulers: Slurm, Kubernetes, and Slurm-on-Kubernetes (Slinky). It has five runtime +components, the API Server, the Node Observer, the Node Data Broker, the Provider, +and the Engine. [docs/architecture.md](../docs/architecture.md) describes what each one +does and how a request flows between them. ### Key invariant -Providers differ by environment. The canonical `topology.Graph` is stable. Engines only translate — they do not discover. +Providers differ by environment. The canonical `topology.Graph` is stable. Engines +only translate; they do not discover. -Within a provider, network-fabric and accelerator-domain discovery may be composed independently through `pkg/accelerator`; the provider remains responsible for combining both dimensions into the canonical graph. +Within a provider, network-fabric and accelerator-domain discovery may be composed +independently through `pkg/accelerator`; the provider remains responsible for +combining both dimensions into the canonical graph. -This separation is load-bearing. If you find yourself reading the fabric in an engine, or emitting scheduler-specific output from a provider, stop and reconsider. +This separation is load-bearing. If you find yourself reading the fabric in an +engine, or emitting scheduler-specific output from a provider, stop and reconsider. ### Repository map @@ -28,8 +53,8 @@ This separation is load-bearing. If you find yourself reading the fabric in an e cmd/ # Entry points: topograph, node-observer, node-data-broker, kwok-nodes pkg/ accelerator/ # Pluggable accelerator-domain discovery composed by providers - providers/ # One directory per provider: aws, crusoe, gcp, oci, nebius, netq, dra, infiniband, lambdai, test - engines/ # One directory per engine: k8s, nfd, slinky, slurm + providers/ # One directory per provider: aws, crusoe, dra, dsx, gcp, infiniband, lambdai, nebius, netq, nscale, oci, test + engines/ # One directory per engine: graph, k8s, nfd, slinky, slurm topology/ # Canonical Graph, Vertex tree, and topology constants (DO NOT CHANGE CASUALLY) registry/ # Central NamedLoader wiring for providers + engines translate/ # topology.conf and block/tree generation shared by engines @@ -41,122 +66,187 @@ pkg/ models/ # Go types and loader for YAML simulation models (the YAML files live in tests/models/) test/ # Cross-package test helpers internal/ # Shared utilities not part of the public API - cluset, component, config, exec, files, httperr, httpreq, k8s, version + cluset, component, config, exec, files, httperr, httpreq, k8s, kwok, version charts/topograph/ # Helm chart for all Kubernetes components; tests/ holds the helm-unittest suites + snapshots CHANGELOG.md # Release history (Keep a Changelog format); update [Unreleased] for user-facing PRs -docs/ # Public-facing docs — overview.md, architecture.md, api.md + providers/, engines/, reference/ subdirectories +docs/ # Public-facing docs: overview.md, architecture.md, api.md + providers/, engines/, reference/ subdirectories demos/ # Interactive Kubernetes/KWOK deployment demos tests/models/ # YAML simulation fixtures config/ # Sample topograph-config.yaml scripts/ # Build scripts (deb, rpm, SSL, clean) -localdev/ # Developer-local workspace — not tracked; personal scratch files +localdev/ # Developer-local workspace, not tracked; personal scratch files ``` +## 2. What an Agent Is Permitted to Do + +### In scope without asking + +An agent is permitted to make these changes on its own initiative, as long as the +change ships with tests when it changes behavior, keeps `make qualify` green, and +carries the doc updates named in the Documentation Impact Evaluation table below. + +| Task in scope | Files an agent may modify | +|---|---| +| Add a provider, or fix or extend an existing one | `pkg/providers//`, the one-line registry entry in `pkg/registry/registry.go`, and the docs the provider checklist requires: `docs/providers/.md`, the provider list and "Choosing a Provider" table in `docs/overview.md`, and a `docs/index.yml` entry when the page is new | +| Fix a bug or add tests inside an existing engine, without changing its output format | `pkg/engines//`, `pkg/translate/` | +| Server plumbing, config parsing, metrics, shared helpers | `pkg/server/`, `pkg/config/`, `pkg/metrics/`, `pkg/ib/`, `pkg/node_observer/`, `internal/` | +| Command-line entry points and flags | `cmd/` | +| Helm chart templates, values, and the helm-unittest suites and snapshots | `charts/topograph/` | +| Documentation, release notes, simulation fixtures, demos, build scripts | `docs/`, `CHANGELOG.md`, `tests/models/`, `demos/`, `scripts/` | +| Tests for any of the above | any `*_test.go`, `charts/topograph/tests/` | + +Stay inside the directories the task names. A change that starts in one provider and +ends up editing `pkg/topology/` is a signal that the task was scoped wrongly, not a +licence to widen it. Preparing a commit is in scope; `git push`, opening or +commenting on a pull request or issue, and publishing an image or chart are the human +contributor's call, not the agent's. + ### Do not change without discussion -These structures propagate across every provider and engine. Changing them in a single PR usually means the PR is too broad. +These structures propagate across every provider and engine. Changing them in a +single PR usually means the PR is too broad. Open an issue and get maintainer +agreement before writing code that touches one of them. | Surface | Why it's load-bearing | |---|---| -| `pkg/topology/` — `Graph`, the `Vertex` tree, and topology constants | Every provider returns it; every engine consumes it. A shape change ripples to all of them. | +| `pkg/topology/`: `Graph`, the `Vertex` tree, and topology constants | Every provider returns it; every engine consumes it. A shape change ripples to all of them. | | Helm `provider.name` / `engine.name` | External contract for operators deploying Topograph. | | The variable fabric labels `fabric.topograph.run/tier-N` and accelerator labels `accelerator.topograph.run/domain` and `accelerator.topograph.run/sub-domain` | Consumed by downstream projects (KAI Scheduler, NVSentinel, Kueue); fabric tier 0 is closest to the node. | +| Adding a new engine | Implies a new output format that every provider's output must be translatable into. | +| The vulnerability reporting route in `SECURITY.md` | Reports go to NVIDIA PSIRT, not to a GitHub issue. | +| `LICENSE`, `CODEOWNERS`, `MAINTAINERS.md`, `GOVERNANCE.md` | Project governance, owned by the maintainers. | + +### Never commit credentials, secrets, API keys, or tokens + +Credentials, secrets, API keys, access tokens, passwords, private keys, certificates, +and the values of environment variables must never be committed to this repository. +That holds for Go source, test fixtures, Helm values files, container images, the +config samples under `config/`, commit messages, and PR descriptions alike. Refer to +a secret by its name; never by its value. + +- Provider credentials are supplied at runtime through environment variables or a + Kubernetes Secret. `docs/providers/.md` documents which environment variables + each provider reads. Document the variable name, never a working value. +- A test that needs a credential uses an obviously fake placeholder that cannot + authenticate against anything real. +- `localdev/` is untracked on purpose. Keep scratch files that hold real endpoints or + tokens there, and read `git status` before staging. +- If you find a credential that is already committed, do not describe it in a public + issue or PR. Report it privately through the route in `SECURITY.md`. It has to be + rotated, not only deleted, because git history keeps the old value. + +## 3. Coding Style and Conventions + +Build, lint, and test commands live in [DEVELOPMENT.md](../DEVELOPMENT.md). This section +covers only what the tooling cannot tell you. -## 2. Setup and Installation +### Formatting and linting -### Prerequisites +- `go fmt ./...` is authoritative; do not hand-format +- `golangci-lint` runs in CI with `--new-from-rev` so only new issues block; fix warnings in code you touch +- Copyright header on every new Go file: `Copyright (c) , NVIDIA CORPORATION. All rights reserved.` followed by the Apache 2.0 boilerplate matching existing files -- **Go 1.26.6** (see `go.mod`) — newer minor versions are fine; older will not build -- **make** -- **golangci-lint** — `brew install golangci-lint` or via `go install` -- **helm 3.10+ or 4.x** — required for `make chart-test`; the `helm-unittest` plugin is installed automatically by the target (`brew install helm`). CI pins helm `v4.1.1` in `.github/workflows/chart-test.yaml`. -- **docker** — for container image builds (the main image includes `rdma-core` / `ibnetdiscover` for InfiniBand deployments) +### Provider interface -### Clone and build +The contract lives in `pkg/providers/providers.go`: -```bash -git clone https://github.com/NVIDIA/topograph.git -cd topograph -make build # produces bin/topograph, bin/node-observer, bin/node-data-broker, bin/kwok-nodes +```go +type Provider interface { + GenerateTopologyConfig( + ctx context.Context, + pageSize *int, + instances []topology.ComputeInstances, + ) (*topology.Graph, *httperr.Error) +} ``` -Cross-compile with `make build-linux-amd64`, `make build-darwin-arm64`, etc. +A provider returns a `*topology.Graph` of the discovered topology. Providers using +`ClusterTopology` populate `InstanceTopology.FabricTiers` closest-first, +`InstanceTopology.XclrDomainID` for the optional accelerator domain, and +`InstanceTopology.XclrSubDomainID` for an optional sub-domain nested within it, then +call `ToGraph`; the fabric path has no fixed depth. `Graph.Tiers` is the fabric +hierarchy, and `Graph.Domains` is the `topology/block` source. Leaf vertices are +compute nodes; interior tier vertices are switches. -## 3. Testing and Deployment Workflows +### Preferred and deprecated patterns -### Local test loop +#### Error type at the provider boundary -```bash -make qualify # runs fmt, vet, lint, and test in sequence — pre-push aggregator -make fmt # go fmt ./... -make vet # go vet ./... -make lint # golangci-lint run (only flags new issues vs. main) -make test # go test -race -coverprofile=coverage.out ./... -make chart-test # helm lint + helm-unittest suites (charts/topograph/tests/) -make chart-test-update-snapshot # refresh helm-unittest snapshots (review before commit) -make coverage # human-readable per-package summary -``` +Preferred. Return `*httperr.Error` so the API server can propagate a meaningful HTTP +status code to the caller: -Run `make qualify` before pushing. The individual targets are available if you want to run a single check during iteration. Run `make chart-test` when you change `charts/topograph/`; CI runs it on every workflow trigger. - -### Coverage policy +```go +func (p *Provider) GenerateTopologyConfig(ctx context.Context, pageSize *int, + instances []topology.ComputeInstances) (*topology.Graph, *httperr.Error) { -From `codecov.yml`: -- **Project coverage**: 60% target, 5% threshold for drops -- **Patch coverage**: 50% target, 5% threshold + cluster, err := p.discover(ctx, pageSize, instances) + if err != nil { + // The upstream API answered and rejected us: 502, not a bare 500. + return nil, httperr.NewError(http.StatusBadGateway, err.Error()) + } -Coverage checks run on pull requests. A drop below target with no matching uplift in the touched files will fail the Codecov check. + return cluster.ToGraph(NAME, instances, p.trimTiers, false), nil +} +``` -### CI workflows +Deprecated. A plain `error` is not acceptable at this boundary. The API server has no +status code to work with, so every provider failure collapses into one generic +response and callers cannot tell a bad request from an upstream outage: -- `.github/workflows/go.yml` — build, test, lint, and `govulncheck` on every push and PR -- `.github/workflows/chart-test.yaml` — Helm chart lint + helm-unittest suites (`make chart-test`) on every push and PR -- `.github/workflows/docker.yml` — container image build on release tags or manual trigger -- `.github/workflows/helm-release.yaml` — Helm chart release-candidate publication (manual trigger) -- `.github/workflows/release.yml` — official tag-driven Helm, checksum, and GitHub Release publication +```go +// Do not do this. +func (p *Provider) GenerateTopologyConfig(ctx context.Context, pageSize *int, + instances []topology.ComputeInstances) (*topology.Graph, error) { -### Deployment surfaces + cluster, err := p.discover(ctx, pageSize, instances) + if err != nil { + return nil, fmt.Errorf("discovery failed: %w", err) + } -- **Binaries** — `deb` and `rpm` packages via `make deb` / `make rpm` (consumed by Slurm users) -- **Container images** — `ghcr.io/nvidia/topograph` (consumed by Kubernetes users) -- **Helm chart** — `charts/topograph/` (API server, node-observer, and node-data-broker) + return cluster.ToGraph(NAME, instances, p.trimTiers, false), nil +} +``` -## 4. Coding Style and Conventions +#### Building the fabric path -### Formatting and linting +Preferred. Hand the switch IDs to the tier helper that matches your source ordering +and let `ToGraph` build the vertex tree. The helper names the ordering, so a provider +whose API returns root-first IDs uses `topology.RootFirstFabricTiers` instead of +reversing a slice by hand: -- `go fmt ./...` is authoritative — do not hand-format -- `golangci-lint` runs in CI with `--new-from-rev` so only new issues block; fix warnings in code you touch -- Copyright header on every new Go file: `Copyright (c) , NVIDIA CORPORATION. All rights reserved.` followed by the Apache 2.0 boilerplate matching existing files +```go +cluster.Append(&topology.InstanceTopology{ + InstanceID: node.ID, + FabricTiers: topology.ClosestFirstFabricTiers(node.LeafID, node.SpineID, node.CoreID), + XclrDomainID: node.NVLinkDomainID, +}) +``` -### Provider interface - -The contract lives in `pkg/providers/providers.go`: +Deprecated. Assembling `topology.Vertex` values inside a provider, or hardcoding a +tier count, couples the provider to a shape that only `pkg/topology` owns: ```go -type Provider interface { - GenerateTopologyConfig( - ctx context.Context, - pageSize *int, - instances []topology.ComputeInstances, - ) (*topology.Graph, *httperr.Error) -} +// Do not do this: the fabric path has no fixed depth, and building the vertex +// tree is pkg/topology's job, not the provider's. +leaf := &topology.Vertex{ID: node.LeafID} +spine := &topology.Vertex{ID: node.SpineID, Vertices: map[string]*topology.Vertex{node.LeafID: leaf}} ``` -A provider returns a `*topology.Graph` of the discovered topology. Providers using `ClusterTopology` populate `InstanceTopology.FabricTiers` closest-first, `InstanceTopology.XclrDomainID` for the optional accelerator domain, and `InstanceTopology.XclrSubDomainID` for an optional sub-domain nested within it, then call `ToGraph`; the fabric path has no fixed depth. `Graph.Tiers` is the fabric hierarchy, and `Graph.Domains` is the `topology/block` source. Leaf vertices are compute nodes; interior tier vertices are switches. Return `*httperr.Error` so the API server can propagate the correct HTTP status code — plain `error` is not acceptable at this boundary. - ### Adding a new provider 1. Create `pkg/providers//` with at minimum `provider.go` and `provider_test.go` -2. Expose a `NamedLoader` function with signature `func NamedLoader() (string, providers.Loader)` — this is how the registry wires the provider +2. Expose a `NamedLoader` function with signature `func NamedLoader() (string, providers.Loader)`; this is how the registry wires the provider 3. Register in `pkg/registry/registry.go` by adding `.NamedLoader` to the `providers.NewRegistry(...)` call list 4. Add `docs/providers/.md` following the shape of `aws.md` / `netq.md` (prerequisites, credentials, parameters, how it works, verification) -5. Update `docs/overview.md` — add the provider to the "Currently supported providers" list and the "Choosing a Provider" scenario table +5. Update `docs/overview.md`: add the provider to the "Currently supported providers" list and the "Choosing a Provider" scenario table 6. If the provider has a simulated variant for testing, export a second `NamedLoaderSim` and register it alongside (see `aws`, `gcp`, `oci`, `lambdai`) ### Adding a new engine -Engines are much rarer (four exist: slurm, k8s, nfd, slinky). Follow the same registry pattern but register in `engines.NewRegistry(...)`. Coordinate with maintainers before starting — adding an engine implies a new output format that every provider's output must be translatable into. +Engines are much rarer (five exist: `graph`, `k8s`, `nfd`, `slinky`, `slurm`). Follow +the same registry pattern but register in `engines.NewRegistry(...)`. Coordinate with +maintainers before starting; adding an engine implies a new output format that every +provider's output must be translatable into. ### Anti-patterns @@ -166,77 +256,74 @@ Engines are much rarer (four exist: slurm, k8s, nfd, slinky). Follow the same re | Emit scheduler-specific output from a provider | Same invariant in reverse | | Change `pkg/topology/Vertex` fields without discussion | Every provider and engine depends on the shape | | Add a new provider in `pkg/providers//` without also updating `pkg/registry/registry.go` | Orphaned code; provider will not be loadable | -| Modify an AGENTS.md-described surface (new Makefile target, top-level directory, chart template, invariant) without updating `AGENTS.md` + `.claude/CLAUDE.md` in the same PR | Drift between the code and its agent-facing description; the next contributor / agent reads stale guidance | +| Modify an AGENTS.md-described surface (new Makefile target, top-level directory, chart template, invariant) without updating `AGENTS.md` + `.claude/CLAUDE.md` in the same PR | Drift between the code and its agent-facing description; the next contributor or agent reads stale guidance | | Skip DCO sign-off to "fix later" | The DCO bot will block the PR; rebase with `--signoff` is always available | | Use plain `error` at the provider interface boundary | Must be `*httperr.Error` so the API server returns the correct HTTP status | +| Commit a credential, API key, token, or environment variable value | Anything reaching git history has to be rotated, not just deleted | | Enable both `ingress.enabled` and `gatewayAPI.enabled` in the same Helm release | Mutually exclusive; deploying both routing resources against the same Service is almost always a misconfiguration. Enforced by `charts/topograph/templates/_validation.tpl`. | | Add implementation-specific annotations, CRDs, or extensions to `charts/topograph/templates/httproute.yaml` | The default `HTTPRoute` must use only standard `gateway.networking.k8s.io/v1` fields so it renders and functions against any conformant Gateway API implementation. Implementation-specific examples (kgateway `TrafficPolicy`, etc.) belong in `values.k8s.gateway-api-example.yaml` as separate attached resources, not in the chart's default template. | ### Label and annotation reference -Label keys written by the Kubernetes engine are documented in `docs/reference/node-labels.md`. Do not invent new keys in provider code — values flow through the canonical graph. Optional custom keys are configured through the k8s engine's closest-first `fabricLabels` array and singular `acceleratorLabel`; the accelerator sub-domain key is fixed at `accelerator.topograph.run/sub-domain`. When `fabricLabels` is provided, only explicitly listed fabric tiers are labeled. - -## 5. Pull Request Guidelines - -### Branch naming - -Use a prefix that matches the change type: `feat/`, `fix/`, `docs/`, `chore/`, `refactor/`, `test/`. Example: `docs/agents-md`, `feat/crusoe-provider`. - -### Commit messages +Do not invent label or annotation keys in provider code; values flow through the +canonical graph and the engine decides the key. The default keys, the optional +`fabricLabels` and `acceleratorLabel` overrides, and the value semantics are all in +[docs/reference/node-labels.md](../docs/reference/node-labels.md). -Conventional Commits format: +## 4. Pull Request Guidelines -``` -type(scope): short description - -optional body - -Signed-off-by: Your Name -``` - -Type must be one of: `feat`, `fix`, `docs`, `chore`, `refactor`, `style`, `perf`, `test`, `build`, `ci`. +Branch naming, the Conventional Commits format, DCO sign-off and how to repair a +missing one, the review process, and the AI-assisted contribution policy are all in +[CONTRIBUTING.md](../CONTRIBUTING.md). What follows is the part specific to keeping this +repository's own documents and gates in step. -### DCO sign-off is required - -Every commit must carry a `Signed-off-by:` trailer. There is no `.github/dco.yml` exemption on this repo — NVIDIA org membership does not bypass the DCO bot here. Two ways to add it: +### Coverage policy -```bash -git commit -s -m "feat(provider/foo): add Foo provider" # adds trailer -git commit -s -S -m "..." # sign-off + GPG sign -``` +From `codecov.yml`: -If a PR arrives without sign-off, rebase the branch to add it: +- **Project coverage**: 60% target, 5% threshold for drops +- **Patch coverage**: 50% target, 5% threshold -```bash -git rebase --signoff upstream/main -git push --force-with-lease -``` +Coverage checks run on pull requests. A drop below target with no matching uplift in +the touched files will fail the Codecov check. ### GPG signing is optional but recommended -Configure once: -```bash -git config --global user.signingkey -git config --global commit.gpgsign true -``` - -Signed commits get a **Verified** badge on GitHub. The GPG public key must be uploaded to your GitHub account. +DCO sign-off (`git commit -s`) is required and is covered in `CONTRIBUTING.md`. GPG +signing is separate and optional; configure it once with +`git config --global user.signingkey ` and +`git config --global commit.gpgsign true`, then use `git commit -s -S`. Signed commits +get a **Verified** badge once the public key is uploaded to your GitHub account. ### Potential security issues -If you discover what appears to be a security vulnerability while working in this codebase — unauthenticated code path, exposed credential, injection vulnerability, privilege-escalation path, dependency with a known CVE, or similar — do **not** file a public GitHub issue or include it in a public PR description. Surface it privately to the maintainer, who can route it through the NVIDIA PSIRT channels documented in `SECURITY.md` (`psirt@nvidia.com` and the submission form; not GitHub). +If you discover what appears to be a security vulnerability while working in this +codebase (unauthenticated code path, exposed credential, injection vulnerability, +privilege-escalation path, dependency with a known CVE, or similar), do **not** file a +public GitHub issue or include it in a public PR description. Surface it privately to +the maintainer, who routes it through the NVIDIA PSIRT channels documented in +`SECURITY.md`. ### Documentation structure -`docs/` is the **source of truth** for all public-facing documentation, published to `https://docs.nvidia.com/topograph` via Fern. `fern/` holds only site config and theme assets — never doc content. +`docs/` is the **source of truth** for all public-facing documentation, published to +`https://docs.nvidia.com/topograph` via Fern. `fern/` holds only site config and theme +assets, never doc content. -**`docs/design/`** is a drafting space for design work in progress. Files there are intentionally excluded from the Fern sidebar and are not published to the docs site. Finalized design decisions should move to the appropriate `docs/` subtree or be captured in code comments / CHANGELOG entries. +**`docs/design/`** is a drafting space for design work in progress. Files there are +excluded from the Fern sidebar and are not published to the docs site. Finalized +design decisions should move to the appropriate `docs/` subtree or be captured in code +comments and CHANGELOG entries. -**Every `.md` file added to `docs/` (outside `docs/design/`) must also be added to `docs/index.yml`**, which drives the Fern sidebar. CI enforces this: `fern-docs-ci.yml` fails if any `docs/**/*.md` outside `docs/design/` is absent from `docs/index.yml`. +**Every `.md` file added to `docs/` (outside `docs/design/`) must also be added to +`docs/index.yml`**, which drives the Fern sidebar. CI enforces this: +`fern-docs-ci.yml` fails if any `docs/**/*.md` outside `docs/design/` is absent from +`docs/index.yml`. ### Documentation Impact Evaluation -Every PR should be evaluated for documentation impact before pre-push qualification. The following changes imply specific doc updates in the same PR: +Every PR should be evaluated for documentation impact before pre-push qualification. +The following changes imply specific doc updates in the same PR: | Change | Docs update required | |---|---| @@ -250,27 +337,24 @@ Every PR should be evaluated for documentation impact before pre-push qualificat | User-facing feature, fix, breaking change, or Helm migration worth calling out in release notes | `CHANGELOG.md` under `[Unreleased]` (Added / Changed / Fixed / Removed); move entries into a version section at release time | | New invariant or "do not change without discussion" surface | `AGENTS.md` + `.claude/CLAUDE.md` in the same PR | | New Makefile target, top-level directory, or repository-layout change described by the repository map | `AGENTS.md` + `.claude/CLAUDE.md` in the same PR | +| New build, test, or local-run instruction | `DEVELOPMENT.md` | | New `.md` file added to `docs/` (outside `docs/design/`) | Add an entry to `docs/index.yml`; CI will fail if omitted | -If a change falls outside these categories, it still warrants a moment's review for collateral doc drift. +If a change falls outside these categories, it still warrants a moment's review for +collateral doc drift. ### Pre-push checklist -When filing a PR (`gh pr create` or the GitHub UI), `.github/PULL_REQUEST_TEMPLATE.md` auto-populates the body with a Description section and a Checklist. Fill in the Description and tick the checklist items as completed — do not delete or replace the template wholesale. +When filing a PR (`gh pr create` or the GitHub UI), `.github/PULL_REQUEST_TEMPLATE.md` +auto-populates the body with a Description section and a Checklist. Fill in the +Description and tick the checklist items as completed; do not delete or replace the +template wholesale. - [ ] `make qualify` passes (runs fmt, vet, lint, test) +- [ ] `make chart-test` passes when `charts/topograph/` changed - [ ] New or changed public behavior is covered by a test -- [ ] Documentation impact evaluated per the table above — applicable doc updates are included in this PR +- [ ] Documentation impact evaluated per the table above, and applicable doc updates are included in this PR - [ ] User-facing changes recorded in `CHANGELOG.md` `[Unreleased]` when applicable - [ ] `pkg/topology/` changes were discussed in an issue first +- [ ] No credential, API key, token, or environment variable value appears anywhere in the diff - [ ] Every commit has a DCO sign-off - -### Review expectations - -- All CI checks must be green before merge (Go build/test/lint, Codecov, DCO) -- Reviewers look for: adherence to the provider/engine boundary, test coverage on new code paths, doc updates when contract changes -- Breaking changes to the config schema, label keys, or `Vertex` shape are rejected unless discussed in an issue first - -### When in doubt - -Read `docs/` before asking. Provider-specific questions usually have answers in `docs/providers/.md`. Label semantics are in `docs/reference/node-labels.md`. The scenario-to-provider mapping is in the "Choosing a Provider" table in `docs/overview.md`. API endpoints and config schema live in `docs/api.md`. Release history and operator-facing migration notes live in `CHANGELOG.md`. diff --git a/AGENTS.md b/AGENTS.md index 1450e057..1e48231c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,25 +2,50 @@ This file provides guidance to Codex, Cursor, Copilot, and other coding agents when working with code in this repository. - + -## 1. Project Overview and Architecture +## Start here + +This file is the short core. It carries the invariants an agent must not break, the +boundaries of what an agent may change on its own, and the conventions that are +specific to this codebase. Everything else lives in the document that already owns +it and is not repeated here: -Topograph discovers the physical network topology of a cluster (NVLink domains, InfiniBand/Ethernet switch fabric, cloud rack topology) and exposes it to workload schedulers — Slurm, Kubernetes, and Slurm-on-Kubernetes (Slinky). It has five runtime components: +| You need | Read | +|---|---| +| Prerequisites, build, local test loop, running a binary, container images, packaging, CI parity | [DEVELOPMENT.md](DEVELOPMENT.md) | +| Issue-first workflow, DCO sign-off, commit and branch conventions, review process, AI-assisted contribution policy | [CONTRIBUTING.md](CONTRIBUTING.md) | +| What each runtime component does and how a request flows between them | [docs/architecture.md](docs/architecture.md) | +| Every label and annotation key the Kubernetes engine writes | [docs/reference/node-labels.md](docs/reference/node-labels.md) | +| Provider selection, supported providers, the "Choosing a Provider" scenario table | `docs/overview.md` | +| API endpoints, request parameters, response fields, config schema | `docs/api.md` | +| How to report a suspected vulnerability | `SECURITY.md` | +| Release history and operator-facing migration notes | `CHANGELOG.md` | + +When you change behaviour that one of those documents describes, update that +document. Do not answer the same question here as well; two copies of a rule drift +apart, and the next agent reads the stale one. + +## 1. Project Overview and Architecture -- **API Server** — receives `/v1/generate` requests, aggregates bursts, dispatches to a Provider -- **Node Observer** — Kubernetes-only; watches configured node/pod changes and Topograph API readiness, then triggers regeneration -- **Node Data Broker** — Kubernetes-only DaemonSet; collects per-node attributes (NVLink clique IDs, etc.) as node annotations -- **Provider** — per-environment adapter that queries a topology source (CSP API, NetQ, `ibnetdiscover`, DRA labels) and returns a canonical representation -- **Engine** — per-scheduler translator that writes the canonical representation out as `topology.conf`, Kubernetes node labels, or a Slinky ConfigMap +Topograph discovers the physical network topology of a cluster (NVLink domains, +InfiniBand/Ethernet switch fabric, cloud rack topology) and exposes it to workload +schedulers: Slurm, Kubernetes, and Slurm-on-Kubernetes (Slinky). It has five runtime +components, the API Server, the Node Observer, the Node Data Broker, the Provider, +and the Engine. [docs/architecture.md](docs/architecture.md) describes what each one +does and how a request flows between them. ### Key invariant -Providers differ by environment. The canonical `topology.Graph` is stable. Engines only translate — they do not discover. +Providers differ by environment. The canonical `topology.Graph` is stable. Engines +only translate; they do not discover. -Within a provider, network-fabric and accelerator-domain discovery may be composed independently through `pkg/accelerator`; the provider remains responsible for combining both dimensions into the canonical graph. +Within a provider, network-fabric and accelerator-domain discovery may be composed +independently through `pkg/accelerator`; the provider remains responsible for +combining both dimensions into the canonical graph. -This separation is load-bearing. If you find yourself reading the fabric in an engine, or emitting scheduler-specific output from a provider, stop and reconsider. +This separation is load-bearing. If you find yourself reading the fabric in an +engine, or emitting scheduler-specific output from a provider, stop and reconsider. ### Repository map @@ -28,8 +53,8 @@ This separation is load-bearing. If you find yourself reading the fabric in an e cmd/ # Entry points: topograph, node-observer, node-data-broker, kwok-nodes pkg/ accelerator/ # Pluggable accelerator-domain discovery composed by providers - providers/ # One directory per provider: aws, crusoe, gcp, oci, nebius, netq, dra, infiniband, lambdai, test - engines/ # One directory per engine: k8s, nfd, slinky, slurm + providers/ # One directory per provider: aws, crusoe, dra, dsx, gcp, infiniband, lambdai, nebius, netq, nscale, oci, test + engines/ # One directory per engine: graph, k8s, nfd, slinky, slurm topology/ # Canonical Graph, Vertex tree, and topology constants (DO NOT CHANGE CASUALLY) registry/ # Central NamedLoader wiring for providers + engines translate/ # topology.conf and block/tree generation shared by engines @@ -41,122 +66,187 @@ pkg/ models/ # Go types and loader for YAML simulation models (the YAML files live in tests/models/) test/ # Cross-package test helpers internal/ # Shared utilities not part of the public API - cluset, component, config, exec, files, httperr, httpreq, k8s, version + cluset, component, config, exec, files, httperr, httpreq, k8s, kwok, version charts/topograph/ # Helm chart for all Kubernetes components; tests/ holds the helm-unittest suites + snapshots CHANGELOG.md # Release history (Keep a Changelog format); update [Unreleased] for user-facing PRs -docs/ # Public-facing docs — overview.md, architecture.md, api.md + providers/, engines/, reference/ subdirectories +docs/ # Public-facing docs: overview.md, architecture.md, api.md + providers/, engines/, reference/ subdirectories demos/ # Interactive Kubernetes/KWOK deployment demos tests/models/ # YAML simulation fixtures config/ # Sample topograph-config.yaml scripts/ # Build scripts (deb, rpm, SSL, clean) -localdev/ # Developer-local workspace — not tracked; personal scratch files +localdev/ # Developer-local workspace, not tracked; personal scratch files ``` +## 2. What an Agent Is Permitted to Do + +### In scope without asking + +An agent is permitted to make these changes on its own initiative, as long as the +change ships with tests when it changes behavior, keeps `make qualify` green, and +carries the doc updates named in the Documentation Impact Evaluation table below. + +| Task in scope | Files an agent may modify | +|---|---| +| Add a provider, or fix or extend an existing one | `pkg/providers//`, the one-line registry entry in `pkg/registry/registry.go`, and the docs the provider checklist requires: `docs/providers/.md`, the provider list and "Choosing a Provider" table in `docs/overview.md`, and a `docs/index.yml` entry when the page is new | +| Fix a bug or add tests inside an existing engine, without changing its output format | `pkg/engines//`, `pkg/translate/` | +| Server plumbing, config parsing, metrics, shared helpers | `pkg/server/`, `pkg/config/`, `pkg/metrics/`, `pkg/ib/`, `pkg/node_observer/`, `internal/` | +| Command-line entry points and flags | `cmd/` | +| Helm chart templates, values, and the helm-unittest suites and snapshots | `charts/topograph/` | +| Documentation, release notes, simulation fixtures, demos, build scripts | `docs/`, `CHANGELOG.md`, `tests/models/`, `demos/`, `scripts/` | +| Tests for any of the above | any `*_test.go`, `charts/topograph/tests/` | + +Stay inside the directories the task names. A change that starts in one provider and +ends up editing `pkg/topology/` is a signal that the task was scoped wrongly, not a +licence to widen it. Preparing a commit is in scope; `git push`, opening or +commenting on a pull request or issue, and publishing an image or chart are the human +contributor's call, not the agent's. + ### Do not change without discussion -These structures propagate across every provider and engine. Changing them in a single PR usually means the PR is too broad. +These structures propagate across every provider and engine. Changing them in a +single PR usually means the PR is too broad. Open an issue and get maintainer +agreement before writing code that touches one of them. | Surface | Why it's load-bearing | |---|---| -| `pkg/topology/` — `Graph`, the `Vertex` tree, and topology constants | Every provider returns it; every engine consumes it. A shape change ripples to all of them. | +| `pkg/topology/`: `Graph`, the `Vertex` tree, and topology constants | Every provider returns it; every engine consumes it. A shape change ripples to all of them. | | Helm `provider.name` / `engine.name` | External contract for operators deploying Topograph. | | The variable fabric labels `fabric.topograph.run/tier-N` and accelerator labels `accelerator.topograph.run/domain` and `accelerator.topograph.run/sub-domain` | Consumed by downstream projects (KAI Scheduler, NVSentinel, Kueue); fabric tier 0 is closest to the node. | +| Adding a new engine | Implies a new output format that every provider's output must be translatable into. | +| The vulnerability reporting route in `SECURITY.md` | Reports go to NVIDIA PSIRT, not to a GitHub issue. | +| `LICENSE`, `CODEOWNERS`, `MAINTAINERS.md`, `GOVERNANCE.md` | Project governance, owned by the maintainers. | + +### Never commit credentials, secrets, API keys, or tokens + +Credentials, secrets, API keys, access tokens, passwords, private keys, certificates, +and the values of environment variables must never be committed to this repository. +That holds for Go source, test fixtures, Helm values files, container images, the +config samples under `config/`, commit messages, and PR descriptions alike. Refer to +a secret by its name; never by its value. + +- Provider credentials are supplied at runtime through environment variables or a + Kubernetes Secret. `docs/providers/.md` documents which environment variables + each provider reads. Document the variable name, never a working value. +- A test that needs a credential uses an obviously fake placeholder that cannot + authenticate against anything real. +- `localdev/` is untracked on purpose. Keep scratch files that hold real endpoints or + tokens there, and read `git status` before staging. +- If you find a credential that is already committed, do not describe it in a public + issue or PR. Report it privately through the route in `SECURITY.md`. It has to be + rotated, not only deleted, because git history keeps the old value. + +## 3. Coding Style and Conventions + +Build, lint, and test commands live in [DEVELOPMENT.md](DEVELOPMENT.md). This section +covers only what the tooling cannot tell you. -## 2. Setup and Installation +### Formatting and linting -### Prerequisites +- `go fmt ./...` is authoritative; do not hand-format +- `golangci-lint` runs in CI with `--new-from-rev` so only new issues block; fix warnings in code you touch +- Copyright header on every new Go file: `Copyright (c) , NVIDIA CORPORATION. All rights reserved.` followed by the Apache 2.0 boilerplate matching existing files -- **Go 1.26.6** (see `go.mod`) — newer minor versions are fine; older will not build -- **make** -- **golangci-lint** — `brew install golangci-lint` or via `go install` -- **helm 3.10+ or 4.x** — required for `make chart-test`; the `helm-unittest` plugin is installed automatically by the target (`brew install helm`). CI pins helm `v4.1.1` in `.github/workflows/chart-test.yaml`. -- **docker** — for container image builds (the main image includes `rdma-core` / `ibnetdiscover` for InfiniBand deployments) +### Provider interface -### Clone and build +The contract lives in `pkg/providers/providers.go`: -```bash -git clone https://github.com/NVIDIA/topograph.git -cd topograph -make build # produces bin/topograph, bin/node-observer, bin/node-data-broker, bin/kwok-nodes +```go +type Provider interface { + GenerateTopologyConfig( + ctx context.Context, + pageSize *int, + instances []topology.ComputeInstances, + ) (*topology.Graph, *httperr.Error) +} ``` -Cross-compile with `make build-linux-amd64`, `make build-darwin-arm64`, etc. +A provider returns a `*topology.Graph` of the discovered topology. Providers using +`ClusterTopology` populate `InstanceTopology.FabricTiers` closest-first, +`InstanceTopology.XclrDomainID` for the optional accelerator domain, and +`InstanceTopology.XclrSubDomainID` for an optional sub-domain nested within it, then +call `ToGraph`; the fabric path has no fixed depth. `Graph.Tiers` is the fabric +hierarchy, and `Graph.Domains` is the `topology/block` source. Leaf vertices are +compute nodes; interior tier vertices are switches. -## 3. Testing and Deployment Workflows +### Preferred and deprecated patterns -### Local test loop +#### Error type at the provider boundary -```bash -make qualify # runs fmt, vet, lint, and test in sequence — pre-push aggregator -make fmt # go fmt ./... -make vet # go vet ./... -make lint # golangci-lint run (only flags new issues vs. main) -make test # go test -race -coverprofile=coverage.out ./... -make chart-test # helm lint + helm-unittest suites (charts/topograph/tests/) -make chart-test-update-snapshot # refresh helm-unittest snapshots (review before commit) -make coverage # human-readable per-package summary -``` +Preferred. Return `*httperr.Error` so the API server can propagate a meaningful HTTP +status code to the caller: -Run `make qualify` before pushing. The individual targets are available if you want to run a single check during iteration. Run `make chart-test` when you change `charts/topograph/`; CI runs it on every workflow trigger. - -### Coverage policy +```go +func (p *Provider) GenerateTopologyConfig(ctx context.Context, pageSize *int, + instances []topology.ComputeInstances) (*topology.Graph, *httperr.Error) { -From `codecov.yml`: -- **Project coverage**: 60% target, 5% threshold for drops -- **Patch coverage**: 50% target, 5% threshold + cluster, err := p.discover(ctx, pageSize, instances) + if err != nil { + // The upstream API answered and rejected us: 502, not a bare 500. + return nil, httperr.NewError(http.StatusBadGateway, err.Error()) + } -Coverage checks run on pull requests. A drop below target with no matching uplift in the touched files will fail the Codecov check. + return cluster.ToGraph(NAME, instances, p.trimTiers, false), nil +} +``` -### CI workflows +Deprecated. A plain `error` is not acceptable at this boundary. The API server has no +status code to work with, so every provider failure collapses into one generic +response and callers cannot tell a bad request from an upstream outage: -- `.github/workflows/go.yml` — build, test, lint, and `govulncheck` on every push and PR -- `.github/workflows/chart-test.yaml` — Helm chart lint + helm-unittest suites (`make chart-test`) on every push and PR -- `.github/workflows/docker.yml` — container image build on release tags or manual trigger -- `.github/workflows/helm-release.yaml` — Helm chart release-candidate publication (manual trigger) -- `.github/workflows/release.yml` — official tag-driven Helm, checksum, and GitHub Release publication +```go +// Do not do this. +func (p *Provider) GenerateTopologyConfig(ctx context.Context, pageSize *int, + instances []topology.ComputeInstances) (*topology.Graph, error) { -### Deployment surfaces + cluster, err := p.discover(ctx, pageSize, instances) + if err != nil { + return nil, fmt.Errorf("discovery failed: %w", err) + } -- **Binaries** — `deb` and `rpm` packages via `make deb` / `make rpm` (consumed by Slurm users) -- **Container images** — `ghcr.io/nvidia/topograph` (consumed by Kubernetes users) -- **Helm chart** — `charts/topograph/` (API server, node-observer, and node-data-broker) + return cluster.ToGraph(NAME, instances, p.trimTiers, false), nil +} +``` -## 4. Coding Style and Conventions +#### Building the fabric path -### Formatting and linting +Preferred. Hand the switch IDs to the tier helper that matches your source ordering +and let `ToGraph` build the vertex tree. The helper names the ordering, so a provider +whose API returns root-first IDs uses `topology.RootFirstFabricTiers` instead of +reversing a slice by hand: -- `go fmt ./...` is authoritative — do not hand-format -- `golangci-lint` runs in CI with `--new-from-rev` so only new issues block; fix warnings in code you touch -- Copyright header on every new Go file: `Copyright (c) , NVIDIA CORPORATION. All rights reserved.` followed by the Apache 2.0 boilerplate matching existing files +```go +cluster.Append(&topology.InstanceTopology{ + InstanceID: node.ID, + FabricTiers: topology.ClosestFirstFabricTiers(node.LeafID, node.SpineID, node.CoreID), + XclrDomainID: node.NVLinkDomainID, +}) +``` -### Provider interface - -The contract lives in `pkg/providers/providers.go`: +Deprecated. Assembling `topology.Vertex` values inside a provider, or hardcoding a +tier count, couples the provider to a shape that only `pkg/topology` owns: ```go -type Provider interface { - GenerateTopologyConfig( - ctx context.Context, - pageSize *int, - instances []topology.ComputeInstances, - ) (*topology.Graph, *httperr.Error) -} +// Do not do this: the fabric path has no fixed depth, and building the vertex +// tree is pkg/topology's job, not the provider's. +leaf := &topology.Vertex{ID: node.LeafID} +spine := &topology.Vertex{ID: node.SpineID, Vertices: map[string]*topology.Vertex{node.LeafID: leaf}} ``` -A provider returns a `*topology.Graph` of the discovered topology. Providers using `ClusterTopology` populate `InstanceTopology.FabricTiers` closest-first, `InstanceTopology.XclrDomainID` for the optional accelerator domain, and `InstanceTopology.XclrSubDomainID` for an optional sub-domain nested within it, then call `ToGraph`; the fabric path has no fixed depth. `Graph.Tiers` is the fabric hierarchy, and `Graph.Domains` is the `topology/block` source. Leaf vertices are compute nodes; interior tier vertices are switches. Return `*httperr.Error` so the API server can propagate the correct HTTP status code — plain `error` is not acceptable at this boundary. - ### Adding a new provider 1. Create `pkg/providers//` with at minimum `provider.go` and `provider_test.go` -2. Expose a `NamedLoader` function with signature `func NamedLoader() (string, providers.Loader)` — this is how the registry wires the provider +2. Expose a `NamedLoader` function with signature `func NamedLoader() (string, providers.Loader)`; this is how the registry wires the provider 3. Register in `pkg/registry/registry.go` by adding `.NamedLoader` to the `providers.NewRegistry(...)` call list 4. Add `docs/providers/.md` following the shape of `aws.md` / `netq.md` (prerequisites, credentials, parameters, how it works, verification) -5. Update `docs/overview.md` — add the provider to the "Currently supported providers" list and the "Choosing a Provider" scenario table +5. Update `docs/overview.md`: add the provider to the "Currently supported providers" list and the "Choosing a Provider" scenario table 6. If the provider has a simulated variant for testing, export a second `NamedLoaderSim` and register it alongside (see `aws`, `gcp`, `oci`, `lambdai`) ### Adding a new engine -Engines are much rarer (four exist: slurm, k8s, nfd, slinky). Follow the same registry pattern but register in `engines.NewRegistry(...)`. Coordinate with maintainers before starting — adding an engine implies a new output format that every provider's output must be translatable into. +Engines are much rarer (five exist: `graph`, `k8s`, `nfd`, `slinky`, `slurm`). Follow +the same registry pattern but register in `engines.NewRegistry(...)`. Coordinate with +maintainers before starting; adding an engine implies a new output format that every +provider's output must be translatable into. ### Anti-patterns @@ -166,77 +256,74 @@ Engines are much rarer (four exist: slurm, k8s, nfd, slinky). Follow the same re | Emit scheduler-specific output from a provider | Same invariant in reverse | | Change `pkg/topology/Vertex` fields without discussion | Every provider and engine depends on the shape | | Add a new provider in `pkg/providers//` without also updating `pkg/registry/registry.go` | Orphaned code; provider will not be loadable | -| Modify an AGENTS.md-described surface (new Makefile target, top-level directory, chart template, invariant) without updating `AGENTS.md` + `.claude/CLAUDE.md` in the same PR | Drift between the code and its agent-facing description; the next contributor / agent reads stale guidance | +| Modify an AGENTS.md-described surface (new Makefile target, top-level directory, chart template, invariant) without updating `AGENTS.md` + `.claude/CLAUDE.md` in the same PR | Drift between the code and its agent-facing description; the next contributor or agent reads stale guidance | | Skip DCO sign-off to "fix later" | The DCO bot will block the PR; rebase with `--signoff` is always available | | Use plain `error` at the provider interface boundary | Must be `*httperr.Error` so the API server returns the correct HTTP status | +| Commit a credential, API key, token, or environment variable value | Anything reaching git history has to be rotated, not just deleted | | Enable both `ingress.enabled` and `gatewayAPI.enabled` in the same Helm release | Mutually exclusive; deploying both routing resources against the same Service is almost always a misconfiguration. Enforced by `charts/topograph/templates/_validation.tpl`. | | Add implementation-specific annotations, CRDs, or extensions to `charts/topograph/templates/httproute.yaml` | The default `HTTPRoute` must use only standard `gateway.networking.k8s.io/v1` fields so it renders and functions against any conformant Gateway API implementation. Implementation-specific examples (kgateway `TrafficPolicy`, etc.) belong in `values.k8s.gateway-api-example.yaml` as separate attached resources, not in the chart's default template. | ### Label and annotation reference -Label keys written by the Kubernetes engine are documented in `docs/reference/node-labels.md`. Do not invent new keys in provider code — values flow through the canonical graph. Optional custom keys are configured through the k8s engine's closest-first `fabricLabels` array and singular `acceleratorLabel`; the accelerator sub-domain key is fixed at `accelerator.topograph.run/sub-domain`. When `fabricLabels` is provided, only explicitly listed fabric tiers are labeled. - -## 5. Pull Request Guidelines - -### Branch naming - -Use a prefix that matches the change type: `feat/`, `fix/`, `docs/`, `chore/`, `refactor/`, `test/`. Example: `docs/agents-md`, `feat/crusoe-provider`. - -### Commit messages +Do not invent label or annotation keys in provider code; values flow through the +canonical graph and the engine decides the key. The default keys, the optional +`fabricLabels` and `acceleratorLabel` overrides, and the value semantics are all in +[docs/reference/node-labels.md](docs/reference/node-labels.md). -Conventional Commits format: +## 4. Pull Request Guidelines -``` -type(scope): short description - -optional body - -Signed-off-by: Your Name -``` - -Type must be one of: `feat`, `fix`, `docs`, `chore`, `refactor`, `style`, `perf`, `test`, `build`, `ci`. +Branch naming, the Conventional Commits format, DCO sign-off and how to repair a +missing one, the review process, and the AI-assisted contribution policy are all in +[CONTRIBUTING.md](CONTRIBUTING.md). What follows is the part specific to keeping this +repository's own documents and gates in step. -### DCO sign-off is required - -Every commit must carry a `Signed-off-by:` trailer. There is no `.github/dco.yml` exemption on this repo — NVIDIA org membership does not bypass the DCO bot here. Two ways to add it: +### Coverage policy -```bash -git commit -s -m "feat(provider/foo): add Foo provider" # adds trailer -git commit -s -S -m "..." # sign-off + GPG sign -``` +From `codecov.yml`: -If a PR arrives without sign-off, rebase the branch to add it: +- **Project coverage**: 60% target, 5% threshold for drops +- **Patch coverage**: 50% target, 5% threshold -```bash -git rebase --signoff upstream/main -git push --force-with-lease -``` +Coverage checks run on pull requests. A drop below target with no matching uplift in +the touched files will fail the Codecov check. ### GPG signing is optional but recommended -Configure once: -```bash -git config --global user.signingkey -git config --global commit.gpgsign true -``` - -Signed commits get a **Verified** badge on GitHub. The GPG public key must be uploaded to your GitHub account. +DCO sign-off (`git commit -s`) is required and is covered in `CONTRIBUTING.md`. GPG +signing is separate and optional; configure it once with +`git config --global user.signingkey ` and +`git config --global commit.gpgsign true`, then use `git commit -s -S`. Signed commits +get a **Verified** badge once the public key is uploaded to your GitHub account. ### Potential security issues -If you discover what appears to be a security vulnerability while working in this codebase — unauthenticated code path, exposed credential, injection vulnerability, privilege-escalation path, dependency with a known CVE, or similar — do **not** file a public GitHub issue or include it in a public PR description. Surface it privately to the maintainer, who can route it through the NVIDIA PSIRT channels documented in `SECURITY.md` (`psirt@nvidia.com` and the submission form; not GitHub). +If you discover what appears to be a security vulnerability while working in this +codebase (unauthenticated code path, exposed credential, injection vulnerability, +privilege-escalation path, dependency with a known CVE, or similar), do **not** file a +public GitHub issue or include it in a public PR description. Surface it privately to +the maintainer, who routes it through the NVIDIA PSIRT channels documented in +`SECURITY.md`. ### Documentation structure -`docs/` is the **source of truth** for all public-facing documentation, published to `https://docs.nvidia.com/topograph` via Fern. `fern/` holds only site config and theme assets — never doc content. +`docs/` is the **source of truth** for all public-facing documentation, published to +`https://docs.nvidia.com/topograph` via Fern. `fern/` holds only site config and theme +assets, never doc content. -**`docs/design/`** is a drafting space for design work in progress. Files there are intentionally excluded from the Fern sidebar and are not published to the docs site. Finalized design decisions should move to the appropriate `docs/` subtree or be captured in code comments / CHANGELOG entries. +**`docs/design/`** is a drafting space for design work in progress. Files there are +excluded from the Fern sidebar and are not published to the docs site. Finalized +design decisions should move to the appropriate `docs/` subtree or be captured in code +comments and CHANGELOG entries. -**Every `.md` file added to `docs/` (outside `docs/design/`) must also be added to `docs/index.yml`**, which drives the Fern sidebar. CI enforces this: `fern-docs-ci.yml` fails if any `docs/**/*.md` outside `docs/design/` is absent from `docs/index.yml`. +**Every `.md` file added to `docs/` (outside `docs/design/`) must also be added to +`docs/index.yml`**, which drives the Fern sidebar. CI enforces this: +`fern-docs-ci.yml` fails if any `docs/**/*.md` outside `docs/design/` is absent from +`docs/index.yml`. ### Documentation Impact Evaluation -Every PR should be evaluated for documentation impact before pre-push qualification. The following changes imply specific doc updates in the same PR: +Every PR should be evaluated for documentation impact before pre-push qualification. +The following changes imply specific doc updates in the same PR: | Change | Docs update required | |---|---| @@ -250,27 +337,24 @@ Every PR should be evaluated for documentation impact before pre-push qualificat | User-facing feature, fix, breaking change, or Helm migration worth calling out in release notes | `CHANGELOG.md` under `[Unreleased]` (Added / Changed / Fixed / Removed); move entries into a version section at release time | | New invariant or "do not change without discussion" surface | `AGENTS.md` + `.claude/CLAUDE.md` in the same PR | | New Makefile target, top-level directory, or repository-layout change described by the repository map | `AGENTS.md` + `.claude/CLAUDE.md` in the same PR | +| New build, test, or local-run instruction | `DEVELOPMENT.md` | | New `.md` file added to `docs/` (outside `docs/design/`) | Add an entry to `docs/index.yml`; CI will fail if omitted | -If a change falls outside these categories, it still warrants a moment's review for collateral doc drift. +If a change falls outside these categories, it still warrants a moment's review for +collateral doc drift. ### Pre-push checklist -When filing a PR (`gh pr create` or the GitHub UI), `.github/PULL_REQUEST_TEMPLATE.md` auto-populates the body with a Description section and a Checklist. Fill in the Description and tick the checklist items as completed — do not delete or replace the template wholesale. +When filing a PR (`gh pr create` or the GitHub UI), `.github/PULL_REQUEST_TEMPLATE.md` +auto-populates the body with a Description section and a Checklist. Fill in the +Description and tick the checklist items as completed; do not delete or replace the +template wholesale. - [ ] `make qualify` passes (runs fmt, vet, lint, test) +- [ ] `make chart-test` passes when `charts/topograph/` changed - [ ] New or changed public behavior is covered by a test -- [ ] Documentation impact evaluated per the table above — applicable doc updates are included in this PR +- [ ] Documentation impact evaluated per the table above, and applicable doc updates are included in this PR - [ ] User-facing changes recorded in `CHANGELOG.md` `[Unreleased]` when applicable - [ ] `pkg/topology/` changes were discussed in an issue first +- [ ] No credential, API key, token, or environment variable value appears anywhere in the diff - [ ] Every commit has a DCO sign-off - -### Review expectations - -- All CI checks must be green before merge (Go build/test/lint, Codecov, DCO) -- Reviewers look for: adherence to the provider/engine boundary, test coverage on new code paths, doc updates when contract changes -- Breaking changes to the config schema, label keys, or `Vertex` shape are rejected unless discussed in an issue first - -### When in doubt - -Read `docs/` before asking. Provider-specific questions usually have answers in `docs/providers/.md`. Label semantics are in `docs/reference/node-labels.md`. The scenario-to-provider mapping is in the "Choosing a Provider" table in `docs/overview.md`. API endpoints and config schema live in `docs/api.md`. Release history and operator-facing migration notes live in `CHANGELOG.md`.