Skip to content

[FEA]: Bootstrap agent/go/ module + port pure-data types #213

Description

@rice-riley

Summary

Lay the foundation for the in-progress Python → Go agent rewrite: add a nested Go module at agent/go/ (with its own go.mod, Makefile, license-header tooling, and a paths-scoped CI workflow) and in the same PR port the pure-data layer from Python — the SchemaVersion, Mode, Idempotence enums; the Step / UpgradeStep structs; and the five Interrupt subclasses (NodeRestart, ServiceRestart, RestartAllServices, NoOp, ScriptInterrupt) including their base64+JSON serialize/inflate round-trip.

Bundling these two concerns avoids a hello-world first PR — the toolchain is exercised by real, non-trivial code from day one. The Python agent source under agent/skyhook-agent/ and its production image at containers/agent.Dockerfile remain untouched. The two surgical exceptions are an agent/.dockerignore and a paths: exclusion on the existing .github/workflows/agent-ci.yaml — both required so the Python build doesn't pick up the new Go subdir (see §3 and §4 below).

This is the first of 10 small, reviewable PRs that incrementally rewrite the agent in Go at full feature parity.

Motivation

  • The current Python agent is a separate build surface (Python wheel in distroless, hatch, vendored deps under agent/vendor/) maintained alongside an otherwise all-Go control plane.
  • A Go rewrite has been scoped, but landing 1.2K LOC of source + 2.4K LOC of tests in one PR would be unreviewable.
  • We need a place for Go code to live and a CI lane that gates it before any feature code lands.
  • The chosen strategy is in-place flip with a nested module: Python keeps shipping until a single cutover PR at the very end. Until then agent/go/ is invisible to production — the Python Docker build context excludes it via .dockerignore and the Python CI's paths: filter excludes it too. At cutover the Go module is flattened up one level into agent/.
  • The data layer (enums, step types, interrupt classes) forms the type-level vocabulary every other Go module will import, so landing it up-front lets follow-up PRs each focus on one concern.

Feature description

Two intertwined deliverables:

  1. An empty-but-compiling agent/go/ Go module with Makefile, license tooling, a CI workflow that runs go test and golangci-lint on changes to agent/go/**, and the two guards (.dockerignore + Python CI paths: exclusion) that keep the new subdir invisible to the existing Python build.
  2. Three new internal packages — internal/enums, internal/step, internal/interrupts — providing the parity-equivalent Go types with pure unit tests.

No I/O, no Docker image, no operator wiring — those come in #214#222.

Proposed direction

1. Module skeleton

Create agent/go/ containing:

  • go.mod declaring module github.com/NVIDIA/nodewright/agent at Go 1.26.2 (matches operator). The module path deliberately omits the /go suffix so it stays correct after [FEA]: Cutover: flip default to Go, delete Python, flatten dir, update docs #222's cutover flattens agent/go/*agent/* — no import-path rewrite at cutover, only a directory move.
  • cmd/agent/main.go — minimal func main() with a --version flag. Buildable artifact so CI exercises the full go build path.
  • Makefile exposing build, test, lint, fmt, license-fmt, mirroring the operator/agent Makefile target names so muscle memory works.
  • License headers on every .go file via scripts/format_license.py. Update that script if *.go under agent/go/ is not picked up automatically.

2. Root Makefile fan-out

Update the repo-root Makefile so make targets fan into agent/go/ as a separate target (e.g. agent-go-test, agent-go-build) alongside the existing Python agent targets. Do not modify agent/Makefile — Python keeps shipping unchanged, and agent/Makefile only ever recurses into agent/skyhook-agent/, so it won't accidentally walk into agent/go/.

3. Keep the new dir invisible to the Python image

containers/agent.Dockerfile builds with the build context at agent/ and uses COPY . /code (line 25). Without a guard, every Go source file would land in the Python builder image. Add a new agent/.dockerignore containing:

go/

This is the smallest, most targeted change to the Python build surface — the Dockerfile itself stays byte-identical. Acceptance test: docker build -f containers/agent.Dockerfile agent/ produces an image whose /code directory contains no go/ subdir.

4. Keep the new dir invisible to the Python CI

The existing .github/workflows/agent-ci.yaml paths: filter is agent/**, which would now match agent/go/** too — every Go-only PR would trigger the full Python pipeline (build, push, operator-agent e2e). Update the filter on both the pull_request and push triggers to exclude agent/go/**:

paths:
  - agent/**
  - '!agent/go/**'
  - containers/agent.Dockerfile
  - .github/workflows/agent-ci.yaml

This is the only edit to agent-ci.yaml in #213 — no job logic changes. Verify with a no-op test commit under agent/go/ that the Python workflow does not enqueue.

5. New CI workflow

Add .github/workflows/agent-go-ci.yaml modeled on .github/workflows/agent-ci.yaml:

  • paths: filter scoped to agent/go/** and .github/workflows/agent-go-ci.yaml.
  • Jobs: test (runs go test ./...) and lint (runs golangci-lint run).
  • No Docker build, no operator-agent integration tests yet — both come in later PRs.
  • Use the same Go version as the operator (GO_VERSION: 1.26.2).

6. internal/enums

Port agent/skyhook-agent/src/skyhook_agent/enums.py:

  • Typed string constants for SchemaVersion (V1 = "v1", Latest = "latest").
  • Latest() helper returning the highest non-latest version (matches Python get_latest_schema()).
  • compare(a, b SchemaVersion) int collapsing the SortableEnum machinery — latest always sorts highest, otherwise numeric compare on the digits after the leading v.

7. internal/step (data only)

Port the data parts of agent/skyhook-agent/src/skyhook_agent/step.py; validation rules land in #214.

  • type Mode string with constants for all 11 modes (apply, apply-check, config, config-check, interrupt, post-interrupt, post-interrupt-check, uninstall, uninstall-check, upgrade, upgrade-check).
  • var ApplyToCheck = map[Mode]Mode{...} and its inverse CheckToApply. var NonStepModes = []Mode{Interrupt}.
  • type Idempotence string with Auto / Disabled and a Validate() returning error.
  • type Step struct with fields Name string, Path string, Arguments []string, OnHost bool, Returncodes []int, Env map[string]string, Idempotence Idempotence, RequiresInterrupt bool. JSON tags must round-trip the wire format produced by Python's Step.dump() byte-for-byte (note Python's quirks: idempotence serializes as a bool where trueDisabled; upgrade_step: true|false discriminator; omit env when empty).
  • type UpgradeStep either as a struct embedding Step or as a Step with an IsUpgradeStep bool flag — pick whichever round-trips most cleanly. Document the choice in the PR description.
  • Step.Load(data []byte) (Step, error) and Step.Dump() ([]byte, error) matching Python's classmethods. No Steps.validate in this PR.

8. internal/interrupts

Port agent/skyhook-agent/src/skyhook_agent/interrupts.py:

  • type Interrupt interface { Type() string; InterruptCmd() [][]string; Serialize() ([]byte, error) } plus a registry pattern (replaces Python's inspect-based _make_map).
  • Concrete types: NodeRestart, ServiceRestart (carries []string services), RestartAllServices, NoOp, ScriptInterrupt.
  • func Inflate(b64 string) (Interrupt, error) — base64-decode → JSON-parse → look up type → construct. Same error messages as Python where reasonable (operator logs may grep them).
  • The _type() Python helper converts CamelCasesnake_case with the first char lowercased and underscores between subsequent transitions; reproduce exactly so existing serialized blobs in flight stay valid.

9. Tests

Port:

Add a golden-bytes round-trip test that loads a known Python-produced config.json fragment and verifies Step.Load(data).Dump() produces the same bytes — this is the one hard parity guarantee for this PR.

Scope boundaries

In scope:

  • Empty Go module under agent/go/, Makefile, CI workflow, license header tooling.
  • Root Makefile fan-out.
  • Pure types/enums and the base64+JSON interrupt round-trip.
  • Two surgical guards on the existing Python build surface so the new subdir is invisible to it: a new agent/.dockerignore with go/, and a paths: exclusion (!agent/go/**) on .github/workflows/agent-ci.yaml. No other Python-side changes.

Out of scope:

Acceptance criteria

  • cd agent/go && make build test lint succeeds locally.
  • New Agent Go CI workflow passes; the existing Agent CI workflow does not run on agent/go/**-only changes (verified by pushing a no-op file under agent/go/ and observing the Python workflow stays idle).
  • bin/agent --version prints a version string.
  • go test ./internal/enums/... ./internal/step/... ./internal/interrupts/... passes.
  • Golden round-trip test against a Python-produced config fixture passes byte-for-byte.
  • Inflate of a base64 blob produced by Python's Interrupt.make_controller_input() returns the equivalent Go Interrupt.
  • No imports of os outside test fixtures.
  • docker build -f containers/agent.Dockerfile agent/ builds successfully and the resulting image's /code tree contains no go/ subdir (proves .dockerignore works and the Python image stays untouched).
  • No changes to existing files under agent/skyhook-agent/, agent/vendor/, agent/Makefile, agent/hatch.toml, containers/agent.Dockerfile, operator/, or chart/. The only edit outside agent/go/ is the paths: filter in .github/workflows/agent-ci.yaml plus the new agent/.dockerignore.

Open questions

  • Linter set: re-use the operator's .golangci.yml (copy verbatim) or define a slimmer config for the agent? Recommend copy-verbatim for consistency, deviate later if needed.
  • Vendoring: vendor (-mod=vendor like the operator) from day one, or stay on a pure module-mode build until cutover? Recommend vendoring from day one to match the operator's CI-without-network discipline.
  • UpgradeStep representation — embedded struct vs. discriminator flag. Resolve in the PR.
  • Should we expose Step as a sealed interface to forbid arbitrary Step impls outside this package, or is the struct fine? (Python uses subclassing.) Recommend struct-with-flag for simplicity.

References (codebase)

Alternatives considered

  • Sibling agent-go/ at the repo root. Rejected — keeps the Python agent/ dir cleaner during the rewrite (no .dockerignore or paths: exclusion needed) but at cutover requires a top-level rename that touches every contributor's working tree, every PR's path filter, every doc reference, and the per-component tag prefix story. Nesting under agent/ localizes the churn to a single subdir move at cutover.
  • In-place dir replacement (write Go directly into agent/skyhook-agent/). Rejected — would force the production image to stop building from the very first PR.
  • Inside operator module (operator/cmd/agent/). Rejected — would couple agent versioning to operator versioning and break per-component release tagging (agent/vX.Y.Z).
  • Bootstrap and types as separate PRs. Rejected as overhead-heavy: the bootstrap PR would land code that does nothing, and the types PR would stall behind a trivial review.

Code of Conduct

  • I agree to follow Skyhook's Code of Conduct.

Metadata

Metadata

Assignees

Labels

component/agentSkyhook agent (package executor)

Fields

No fields configured for Enhancement.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions