Skip to content

development.md

Latest commit

 

History

History
245 lines (188 loc) · 17.1 KB

development.md

File metadata and controls

245 lines (188 loc) · 17.1 KB
Raw

Development

Purpose — every command you need to build, lint, test and regenerate the operator, plus how the vendored subcharts and patched dependencies work. Audience contributors · Prerequisites none (read Architecture before changing behavior) · Read time ~6 min

Contents

Commands

make <target> [args] runs hack/<target>.sh and forwards args to it. All builds use CGO_ENABLED=1, GODEBUG=gotypesalias=0 and the build tags goccy netgo.

  • make deps — vendor patched k8s staging modules into staging/ and upstream Helm charts into deploy/gpustack-operator/chart/charts/, then go mod tidy && go mod download; make deps update adds go get -u ./....
  • make generate — the gen/api generators: deepcopy, register, apiservice, CRDs, conversion, protobuf, webhooks. make generate binding regenerates the CGO bindings in binding/ via c-for-go.
  • make lint — golangci-lint (.golangci.yaml); make lint dirty also fails on a dirty tree. make lint docs checks the documentation contract instead — bash and awk over the corpus, a second or two, and no cluster; see the docs skill.
  • make build — cross-build cmd/gpustack-operator into .dist/build/, version ldflag-injected into pkg/utils/version; VERSION=vX.y.z+l.m make build sets it, BUILD_PLATFORMS="linux/amd64 linux/arm64" cross-compiles.
  • make test — go test -v -failfast -race -cover -shuffle=on -timeout=30m ./..., coverage to .dist/test/coverage.out. The order is shuffled on every run so an order-dependent test cannot hide behind the fixed one; a failure banner prints the seed, and go test -shuffle=<seed> reproduces that exact order. Trailing args are regexes of packages to exclude. RACE=false make test drops -race and changes nothing else.
  • make package — images via docker buildx from pack/*/Dockerfile (Linux only).

CI (hack/ci.sh) runs make generate && make deps && make lint && make build inside the image build. The unit tests run separately, in test.yml, as RACE=false make test on linux/amd64 and linux/arm64.

An image built from a git worktree carries the worktree's .git pointer file but not the gitdir it names, so git cannot read the tree inside the build. There the .agents shell gate, hack/check-agents-shell.sh, reports itself skipped with its reason instead of failing the build: in local mode it checks only uncommitted changes, so inside an image its set is empty even from a clone. The verdict on committed .agents shell comes from agents-shell.yml and from make lint on the host.

make lint writes. It runs goimports-reviser -output=file and golangci-lint --fix, so it edits the source rather than only reading it. Anything generated before it — make generate, make generate chart — was produced from a version of the source that no longer exists, and nothing downstream notices: the build passes, the tests pass, and git status is clean. Regenerate after linting. api.yml and chart.yml both fail when the committed artifacts do not match a fresh regeneration, which is what catches it when nobody remembers.

The image build lints with its own golangci-lint, and it can be stricter than the one on your host: the packaged build runs make lint inside the image, where the pinned .golangci.yaml applies against the image's golangci-lint release, and a rule your host binary does not enable — predeclared, which refuses parameter names like new — fails there while the tree passed locally minutes earlier. When the packaged build fails a rule your host never reported, fix the name rather than the config: the image's verdict is the one CI ships.

Helm chart

generate, lint and test take a chart argument, operating on deploy/gpustack-operator/chart via chart-testing, helm-docs and helm-schema:

  • make generate chart — regenerate README.md (from README.md.gotmpl) and values.schema.json. Never hand-edit those two: edit values.yaml/its annotations/README.md.gotmpl and re-run, after any values.yaml edit — the generated schema is what rejects a bad install. Nothing in values.yaml is generated: Kueue's resources.transformations is rendered at install time by a chart helper a patch adds to its config.
  • make lint chart — ct lint in a container, then assert the global.* image knobs reach every image the chart and its subcharts render (gpustack::helm::verify_images).
  • make test chart — ct install onto the current cluster in a container; needs a reachable cluster (e.g. kind) and ~/.kube/config. It installs CHART_TEST_IMAGE_REPOSITORY:CHART_TEST_IMAGE_TAG, by default the published gpustack/gpustack-operator:dev, which is built from main. To test a chart change together with the binary it needs, build the tree, load the image into the cluster, and name it in those two variables. The Chart workflow does this on every run.

Vendored subcharts

Kueue, Node Feature Discovery, csi-driver-nfs and csi-driver-s3 are vendored unpacked under deploy/gpustack-operator/chart/charts/<name>/ and committed, so helm install works from a bare clone and CI stays offline.

gpustack::chart_staging (hack/deps.sh) pulls each pinned archive, unpacks it, stamps _VERSION_ and applies hack/deploy/gpustack-operator/chart/charts/<name>/*.patch; a tree at the pinned stamp is skipped, so runs are idempotent and a patched tree is never clobbered.

Unpacked is what lets the patches exist: Helm merges subchart values rather than rendering them, so the parent cannot compose global.imageRegistry into a subchart's image.repository; each tree's global-image.patch makes the subchart's templates read .Values.global.*, which Helm does propagate.

To change an upstream chart:

  1. Never edit a staged tree in place — a version bump makes make deps delete and re-unpack it, so every change lives in a patch file.
  2. Write the patch against the unpacked tree (git diff from a scratch copy works), drop it into hack/deploy/gpustack-operator/chart/charts/<name>/, bump the pinned version in hack/deps.sh if that is the change, and re-run make deps.
  3. A patch that no longer applies, or leaves a .rej, fails make deps — otherwise a moved chart ships half-patched and silent. A shifted hunk is fine: patch runs -F0, so context still matches exactly, and two patches on one file shift each other.

Mirror the images before bumping a pinned version: every image the chart renders points at a gpustack/mirrored-* repository, an unmirrored bump lands every install in ImagePullBackOff, and make lint chart only checks that the override knobs reach each reference, not that it resolves.

chart.yml runs all three across the supported Kubernetes matrix and gates drift: make generate chart must leave README.md/values.schema.json unchanged, make deps the vendored trees; both fail with the command to run. For a full install → version-consistency → uninstall cycle on a real cluster use the gpustack-operator-chart-e2e skill, and gpustack-operator-e2e for scheduling-chain behavior.

Commit messages

Conventional Commits (type: subject), checked by commitsar (v1.0.3, pinned in hack/lib/style.sh) within make lint, but only over a clean tree (hack/lint.sh → gpustack::commit::lint) and only across the commits ahead of origin/main (scope in .commitsar.yml). Types: feat, fix, refactor, test, docs, chore.

Pull request review

code-review.yml runs an AI review on every pull request (opened/synchronize/reopened) and posts inline findings plus a summary under the org's GitHub App identity. The pipeline itself lives in gpustack/.github as a reusable workflow — this repository only pins its model backing and maps the org secrets. To re-review the latest head, comment /open-code-review on the PR (MEMBER/OWNER/COLLABORATOR only).

What each file is reviewed against is owned here, in .opencodereview/rule.json — its include/exclude lists and path-scoped rules are the single source of truth; a review-scope change lands there, not in this document. Verify a rule change before committing: npx -y @alibaba-group/open-code-review rules check <path> shows which rule a file resolves to; npx -y @alibaba-group/open-code-review review --preview shows which files a diff would send to review.

Running a single test

make test only excludes packages, so target one package or test with go test directly:

GODEBUG=gotypesalias=0 CGO_ENABLED=1 go test -race ./pkg/nodefeature/...
GODEBUG=gotypesalias=0 CGO_ENABLED=1 go test -race -run TestExtractGeneralNodeKey ./pkg/nodefeature/

Checks whose failure reads as success

REQUIRED: take a check's verdict from its return code and from the object under test, never from the shape of its output. Each trap below broke with the signal taken as the verdict reading as success, while the failure sat in a channel nobody was reading: stderr, a return code, or a line that was never printed. The shell traps are about the prompt these commands get typed at, which on macOS is zsh; the repository's own scripts run under bash with pipefail set.

An unquoted $var does not word-split in zsh. FILES="a b c"; cp $FILES $dir passes the list as one filename and the copy fails, where bash would split it into three arguments. Keep a list in an array and expand it as "${FILES[@]}": a bare $FILES over an array is three words in zsh but only its first element in bash. The comparison downstream then read a match, because both sides were the empty string a failed git hash-object returned.

zsh arrays are 1-indexed. A for i in 0 1 2 3 4 loop over ${IDS[$i]} drops one end. Two arrays stepped together stay aligned, so the other iterations land correctly and the single missing one reads as a flake rather than as a boundary error.

A pipeline reports only its last command. make lint | tail -5; echo "rc=$?" gives tail's status, not lint's, unless pipefail is set -- and neither zsh nor bash sets it by default. A green lint prints nothing after its closing banner, so "the code is 0" and "the last line is the banner" confirm each other. Redirect instead: make lint >/tmp/out.log 2>&1; echo "RC=$?".

An empty result is not a negative result. A grep -c of 0 is a real count from whatever search ran, and a dropped -i quietly changes which search that is. A command that never ran prints no count at all: a missing path, or -P on the macOS grep, exits 2 with its message on stderr, where || echo none prints the reassuring branch over it. Feed the check an input it MUST match first.

Let the check veto the cleanup. Verification and teardown joined by ; tear down even when the verification failed, costing the evidence needed to diagnose it; && is the guard, and it guards only a check that exits non-zero. Carry the verdict in the status -- [ -n "$a" ] && [ -n "$b" ] && [ "$a" = "$b" ] && rm -rf "$tree" -- rather than printing SAME or DIFF and returning 0 either way. The non-empty tests matter: two missing values compare equal.

Shipped specification corrections

Shipped specifications are historical design records. A later design change MUST be recorded in a new specification whose header names the earlier sections it supersedes; leave those sections unchanged.

An in-place edit is ALLOWED only when evidence proves a factual claim or its supporting reason wrong while the shipped design and conclusion remain unchanged. Mark prose **Corrected after shipping.**, or use Corrected after shipping. inside a preserved code block. Retain enough of the former claim to explain the correction, and state the replacement evidence at the same location. Keep the edit to that correction; terminology, formatting and later design belong elsewhere.

A bug fix writes no NEW specification into this repository. Three shipped ones predate that rule and stay exactly where they are, as the historical records they already were. What it changes is the FIRST paragraph above: a design change a bug fix makes has no new specification to be recorded in, so recording it there is impossible rather than merely skipped.

Record it in the superseded section instead, marked **Corrected after shipping.**. That is the SECOND paragraph's marker carrying one thing that paragraph otherwise forbids — the resulting rule does change here — so the note MUST name the page that carries the rule now. Everything else in that paragraph still applies: retain enough of the former text to explain what changed, and keep the edit to it.

A superseded section MUST NOT be left carrying a file name that resolves to nothing — the reader follows it and lands nowhere. Naming the document is fine and often necessary; what must go is the extension that makes the name a path. So write 2026-01-02-a-thing rather than 2026-01-02-a-thing.md once that file is gone. The prohibition is on the dangling path, not on the mention.

Runtime log verbosity

Every component registers PUT /debug/flags/v on its own secure port, so klog verbosity can be raised on a running pod and dropped again without a restart (pkg/manager/manager.go, pkg/worker/worker.go, pkg/workergateway/gateway.go; the device-manager's port is 32443, from pkg/devicemanager/option.go).

kubectl -n gpustack-system exec <pod> -- \
  curl -sk -X PUT -H "Host: 127.0.0.1" -d '4' https://127.0.0.1:32443/debug/flags/v
# → successfully set klog.logging.verbosity to 4
kubectl -n gpustack-system exec <pod> -- \
  curl -sk -X PUT -H "Host: 127.0.0.1" -d '2' https://127.0.0.1:32443/debug/flags/v

-H "Host: 127.0.0.1" is mandatory: httpx.LoopbackAccessHandlerFunc compares r.Host against the bare 127.0.0.1 / localhost / ::1 and an ordinary request carries the port (127.0.0.1:32443), so without it the guard answers a plain 404 that reads like a missing route. A GET with the header answers 406 unsupported http method — the guard passing.

Use it to see a decision logged above the deployment's verbosity. The device plugin is the sharpest case: its ResourceServers use Logger: logger.V(3) (pkg/devicemanager/allocator/allocator.go) while the DaemonSet runs -v=2, so Allocate/GetPreferredAllocation decisions — which accelerator a slice landed on — are discarded by default.

Raise v before creating the workload to trace; those lines fire only on an allocation, so a quiet window afterwards proves nothing. The gpustack-operator-e2e skill carries the same recipe as a triage step, with the operational caveats.

API groups & code generation

Path Group / Version Kind
api/v1 gpustack.ai/v1 Extension API (settings, status)
api/worker/v1 worker.gpustack.ai/v1 Extension API served by the aggregated apiserver: a proxy/conversion over the v1alpha1 CRDs plus the read-only (get, list, watch) InstanceTypeFlavor catalog
api/worker/v1alpha1 worker.gpustack.ai/v1alpha1 CRDs (Instance, Devices, InstanceType)

gen/api/main.go configures which packages are CRDs vs extension APIs and drives the custom generators in gen/api/generator (apireg-gen, crd-gen, webhook-gen). Never hand-edit generated files (zz_generated.*, generated.pb.go, generated.proto): edit the source *.go types or gen/api/main.go and run make generate (the gpustack-operator-generate skill automates this).

The patched applyconfiguration generator strips list and continuation indentation from API comments in staging/k8s.io/code-generator/cmd/applyconfiguration-gen/generators/applyconfiguration.go:317-326 (commentsWithoutMarkers). A comment scan of pkg/kubeclients/applyconfiguration/ can therefore report list formatting that cannot be fixed by editing the source comment; fix the generator patch instead.

Vendored / patched dependencies

go.mod replaces several k8s modules (k8s.io/api, apimachinery, code-generator, apiextensions-apiserver, kube-aggregator, klog) plus gogo/protobuf and go-logr/logr with patched copies under ./staging/, checked out and patched by make deps (sources + versions in hack/deps.sh, patches in hack/staging/). Don't hand-edit staging/; change the patch and re-run make deps. The subcharts are staged the same way, for the same reason — see Vendored subcharts.

See also — Internals (the invariants the code keeps) · Installation Modes · Settings

Next → All documentation — pick the next page on the contributor path.