You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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.
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.yamlpaths: 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/**:
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.
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.
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 true ⇔ Disabled; 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.
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 CamelCase → snake_case with the first char lowercased and underscores between subsequent transitions; reproduce exactly so existing serialized blobs in flight stay valid.
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.
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.
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.
Summary
Lay the foundation for the in-progress Python → Go agent rewrite: add a nested Go module at
agent/go/(with its owngo.mod,Makefile, license-header tooling, and a paths-scoped CI workflow) and in the same PR port the pure-data layer from Python — theSchemaVersion,Mode,Idempotenceenums; theStep/UpgradeStepstructs; and the fiveInterruptsubclasses (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/.dockerignoreand apaths: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
agent/go/is invisible to production — the Python Docker build context excludes it via.dockerignoreand the Python CI'spaths:filter excludes it too. At cutover the Go module is flattened up one level intoagent/.Feature description
Two intertwined deliverables:
agent/go/Go module with Makefile, license tooling, a CI workflow that runsgo testandgolangci-linton changes toagent/go/**, and the two guards (.dockerignore+ Python CIpaths:exclusion) that keep the new subdir invisible to the existing Python build.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.moddeclaring modulegithub.com/NVIDIA/nodewright/agentat Go 1.26.2 (matches operator). The module path deliberately omits the/gosuffix so it stays correct after [FEA]: Cutover: flip default to Go, delete Python, flatten dir, update docs #222's cutover flattensagent/go/*→agent/*— no import-path rewrite at cutover, only a directory move.cmd/agent/main.go— minimalfunc main()with a--versionflag. Buildable artifact so CI exercises the fullgo buildpath.Makefileexposingbuild,test,lint,fmt,license-fmt, mirroring the operator/agent Makefile target names so muscle memory works..gofile viascripts/format_license.py. Update that script if*.gounderagent/go/is not picked up automatically.2. Root Makefile fan-out
Update the repo-root Makefile so
maketargets fan intoagent/go/as a separate target (e.g.agent-go-test,agent-go-build) alongside the existing Pythonagenttargets. Do not modify agent/Makefile — Python keeps shipping unchanged, andagent/Makefileonly ever recurses intoagent/skyhook-agent/, so it won't accidentally walk intoagent/go/.3. Keep the new dir invisible to the Python image
containers/agent.Dockerfilebuilds with the build context atagent/and usesCOPY . /code(line 25). Without a guard, every Go source file would land in the Python builder image. Add a newagent/.dockerignorecontaining: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/codedirectory contains nogo/subdir.4. Keep the new dir invisible to the Python CI
The existing .github/workflows/agent-ci.yaml
paths:filter isagent/**, which would now matchagent/go/**too — every Go-only PR would trigger the full Python pipeline (build, push, operator-agent e2e). Update the filter on both thepull_requestandpushtriggers to excludeagent/go/**:This is the only edit to
agent-ci.yamlin #213 — no job logic changes. Verify with a no-op test commit underagent/go/that the Python workflow does not enqueue.5. New CI workflow
Add
.github/workflows/agent-go-ci.yamlmodeled on .github/workflows/agent-ci.yaml:paths:filter scoped toagent/go/**and.github/workflows/agent-go-ci.yaml.test(runsgo test ./...) andlint(runsgolangci-lint run).GO_VERSION: 1.26.2).6.
internal/enumsPort agent/skyhook-agent/src/skyhook_agent/enums.py:
SchemaVersion(V1 = "v1",Latest = "latest").Latest()helper returning the highest non-latestversion (matches Pythonget_latest_schema()).compare(a, b SchemaVersion) intcollapsing theSortableEnummachinery —latestalways sorts highest, otherwise numeric compare on the digits after the leadingv.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 stringwith 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 inverseCheckToApply.var NonStepModes = []Mode{Interrupt}.type Idempotence stringwithAuto/Disabledand aValidate()returning error.type Step structwith fieldsName 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'sStep.dump()byte-for-byte (note Python's quirks:idempotenceserializes as aboolwheretrue⇔Disabled;upgrade_step: true|falsediscriminator; omitenvwhen empty).type UpgradeStepeither as a struct embeddingStepor as aStepwith anIsUpgradeStep boolflag — pick whichever round-trips most cleanly. Document the choice in the PR description.Step.Load(data []byte) (Step, error)andStep.Dump() ([]byte, error)matching Python's classmethods. NoSteps.validatein this PR.8.
internal/interruptsPort agent/skyhook-agent/src/skyhook_agent/interrupts.py:
type Interrupt interface { Type() string; InterruptCmd() [][]string; Serialize() ([]byte, error) }plus a registry pattern (replaces Python'sinspect-based_make_map).NodeRestart,ServiceRestart(carries[]stringservices),RestartAllServices,NoOp,ScriptInterrupt.func Inflate(b64 string) (Interrupt, error)— base64-decode → JSON-parse → look uptype→ construct. Same error messages as Python where reasonable (operator logs may grep them)._type()Python helper convertsCamelCase→snake_casewith the first char lowercased and underscores between subsequent transitions; reproduce exactly so existing serialized blobs in flight stay valid.9. Tests
Port:
Steps.validateor touch the filesystem).Add a golden-bytes round-trip test that loads a known Python-produced
config.jsonfragment and verifiesStep.Load(data).Dump()produces the same bytes — this is the one hard parity guarantee for this PR.Scope boundaries
In scope:
agent/go/, Makefile, CI workflow, license header tooling.agent/.dockerignorewithgo/, and apaths:exclusion (!agent/go/**) on.github/workflows/agent-ci.yaml. No other Python-side changes.Out of scope:
Steps.validateand check-pair / file-existence rules ([FEA]: Config loader with embedded JSON schemas +Steps.validate#214).os.Exec, or talks to the network.agent-goDockerfile and container CI #220).Acceptance criteria
cd agent/go && make build test lintsucceeds locally.Agent Go CIworkflow passes; the existingAgent CIworkflow does not run onagent/go/**-only changes (verified by pushing a no-op file underagent/go/and observing the Python workflow stays idle).bin/agent --versionprints a version string.go test ./internal/enums/... ./internal/step/... ./internal/interrupts/...passes.Interrupt.make_controller_input()returns the equivalent GoInterrupt.osoutside test fixtures.docker build -f containers/agent.Dockerfile agent/builds successfully and the resulting image's/codetree contains nogo/subdir (proves.dockerignoreworks and the Python image stays untouched).agent/skyhook-agent/,agent/vendor/,agent/Makefile,agent/hatch.toml,containers/agent.Dockerfile,operator/, orchart/. The only edit outsideagent/go/is thepaths:filter in.github/workflows/agent-ci.yamlplus the newagent/.dockerignore.Open questions
.golangci.yml(copy verbatim) or define a slimmer config for the agent? Recommend copy-verbatim for consistency, deviate later if needed.-mod=vendorlike 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.UpgradeSteprepresentation — embedded struct vs. discriminator flag. Resolve in the PR.Stepas a sealed interface to forbid arbitraryStepimpls outside this package, or is the struct fine? (Python uses subclassing.) Recommend struct-with-flag for simplicity.References (codebase)
Alternatives considered
agent-go/at the repo root. Rejected — keeps the Pythonagent/dir cleaner during the rewrite (no.dockerignoreorpaths: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 underagent/localizes the churn to a single subdir move at cutover.agent/skyhook-agent/). Rejected — would force the production image to stop building from the very first PR.operator/cmd/agent/). Rejected — would couple agent versioning to operator versioning and break per-component release tagging (agent/vX.Y.Z).Code of Conduct