refactor: move vpcd to runtime/roles/vpcd

This commit is contained in:
joshsiv-mulga
2026-10-11 10:38:17 +10:00
parent eb53eded04
commit 7bf6c10d79
31 changed files with 21 additions and 21 deletions
+1 -1
View File
@@ -11,5 +11,5 @@
# see spinifex/domains/network/invariants/
# mulga docs/development/proposals/region-local-architecture/0006-spinifex-network-layer-contract.md (proposal, not accepted)
/spinifex/domains/network/ @joshsiv-mulga
/spinifex/vpcd/ @joshsiv-mulga
/spinifex/runtime/roles/vpcd/ @joshsiv-mulga
/spinifex/domains/network/invariants/ @joshsiv-mulga
-1
View File
@@ -169,5 +169,4 @@ var allowlist = []allowance{
{From: "operator/cli", To: "gateway/bedrock", Debt: "operator/cli (dependency)", Reason: "command tree imports implementations; needs public capabilities"},
{From: "operator/cli", To: "handlers/eks", Debt: "operator/cli (dependency)", Reason: "command tree imports implementations; needs public capabilities"},
{From: "operator/cli", To: "handlers/iam", Debt: "operator/cli (dependency)", Reason: "command tree imports implementations; needs public capabilities"},
{From: "operator/cli", To: "vpcd", Debt: "operator/cli (dependency)", Reason: "command tree imports implementations; needs public capabilities"},
}
+2 -2
View File
@@ -60,7 +60,7 @@ func (c class) isLegacy(roots ...string) bool {
// legacyRoots are pre-ADR-0001 application roots still standing.
var legacyRoots = map[string]bool{
"handlers": true, "gateway": true, "daemon": true,
"vpcd": true, "admin": true, "accountteardown": true, "lbagent": true,
"admin": true, "accountteardown": true, "lbagent": true,
}
// gatewayDomains names the domain each gateway/<service> adapter belongs to;
@@ -73,7 +73,7 @@ var gatewayDomains = map[string]string{
// runtimeLegacy and operatorLegacy are the legacy roots whose ADR-0001 target
// is runtime/ and operator/ respectively.
var (
runtimeLegacy = []string{"daemon", "vpcd"}
runtimeLegacy = []string{"daemon"}
operatorLegacy = []string{"admin", "accountteardown"}
)
+1 -2
View File
@@ -32,12 +32,11 @@ var frozenLegacyPackages = []string{
"handlers/rds",
"handlers/sts",
"lbagent",
"vpcd",
}
// retiredLegacyRoots were legacy roots whose every package has moved to its
// ADR-0001 target; recreating one would reopen a closed migration row.
var retiredLegacyRoots = []string{"services", "utils"}
var retiredLegacyRoots = []string{"services", "utils", "vpcd"}
// TestADR0001_LegacyRootsFrozen: the current-to-target map gives every legacy
// root a destination, so none may gain a package and no retired root may return.
+4 -2
View File
@@ -335,6 +335,7 @@ while the branch is in flight; they do not describe the umbrella repository's
| `c617914a1` | (codec behaviour only) | — | A changed array whose elements as read carry an unknown member at any depth is refused with `ErrRecordExtensionConflict` instead of being replaced and losing the member; unchanged arrays and changed arrays without element extensions behave as before. Still no production caller. Full preflight passed except the recorded Go 1.27.0 advisories. |
| `0d7b4266d` | (codec behaviour only) | — | The extension-conflict refusal becomes uniform: a capable writer may not remove or replace a known object, map entry or array whose stored value carries unknown structured extension fields, and `Encode` returns `ErrRecordExtensionConflict` naming the changed container and the extension. Removal without extensions stays allowed. Still no production caller. Full preflight passed except the recorded Go 1.27.0 advisories. |
| `e40e699e5` | `spinifex/vpcd/imds_instance_state.go` | `spinifex/domains/ec2/instance/state/imds_readers.go` | The local-file reader (`LocalVMReader`/`NewLocalVMReader`, formerly `localVMStateReader`) and the instance-record loader opener (`OpenInstanceRecordLoader`, formerly `newInstanceRecordLoader`) move unchanged to the instance-state boundary; `vpcd.Config` gains `LocalVMs guestmetadata.LocalStateReader` and `OpenInstanceRecords func(ctx, nc) (guestmetadata.RecordLoader, error)`, wired by `operator/cli` with `state.NewLocalVMReader(nodeConfig.DataDir)` and `state.OpenInstanceRecordLoader`, so `vpcd` no longer imports `daemon` directly. `guestmetadata`'s own narrow interfaces are exported (`LocalStateReader`, `RecordLoader`) with no semantic change so `vpcd.Config` can name them. The opener's absence is now a fatal startup error rather than an unchecked call, matching the existing mandatory-open behaviour. The instance-state ratchet's INV-13 entries move with the file; `OpenInstanceRecordLoader` returning `guestmetadata.RecordLoader` directly (so the two packages' function types match exactly) removes the file's local `recordLoader` interface and its `vm.InstanceRecord` reference, so INV-13 now cites 5 symbols rather than 6. `vpcd` still runs the legacy bucket-opening path (`InitKVBucket`, the version gate and the instance migrations) through the injected opener; that is ADR-0007 debt (INV-13) and is deliberately not fixed here. Full preflight passed except the recorded Go 1.27.0 advisories. |
| (uncommitted; hash added at commit) | `spinifex/vpcd` | `spinifex/runtime/roles/vpcd` | The whole vpcd role moves unchanged to its ADR-0001 runtime-role target: all sixteen production and test files (`vpcd.go`, the lease reaper, the OVN controller watchdog, the host-EIP loop, lease datapath reconcile and their tests), with no split and no lifecycle-behaviour change. The package name stays `vpcd`. `operator/cli/service.go` and `runtime/service/{service,service_test}.go` update their import path. `docs/service-interfaces.yaml`'s `vpcd` service entry, `.github/CODEOWNERS` and the real-manifest case in `tests/e2e/select-suites/selector_test.go` follow the new path; that package's own synthetic `testManifest()` fixture keeps its illustrative `spinifex/vpcd` string by design, decoupled from the real manifest. The network invariant scan roots in `domains/network/invariants/{nat_mode,nats_subjects,names}_test.go` and the S3 exemption for `vpcd.go` follow the new path; injected `UplinkMode()` and `vpc-`-prefixed concat faults in moved files failed S3 and S4 respectively, confirming the scan still covers them, and were reverted by hand. `ingress/aws/invariants/deps_test.go` keeps `vpcd` in its `forbiddenRoots` as defence in depth: `runtime` already rejects an import of the new path, confirmed by an injected import of `runtime/roles/vpcd` into an `ingress/aws` file, and the retained entry still rejects an ingress import of a recreated legacy `vpcd` directly. The layering package drops `vpcd` from `legacyRoots`, `runtimeLegacy` and `frozenLegacyPackages` and adds it to `retiredLegacyRoots`, so `runtime/roles/vpcd` classifies as runtime automatically and a revived `spinifex/vpcd` directory fails the frozen-root guard, both confirmed by fault injection and reverted; the stale `operator/cli` → `vpcd` allowlist entry is deleted rather than repointed, because operator already imports other `runtime/roles/*` packages with no allowlist entry and no rule forbids it. The instance-state ratchet's allowlist cites no vpcd path and is unchanged. |
The EC2 contract row is intentionally different from the directory moves: it creates
a compatibility boundary. `ec2.cmd.*` retains its deployed one-token NATS
@@ -408,12 +409,13 @@ These residuals were recorded by individual slices and must be resolved once the
- `handlers/sts/oidc_jwks.go:14` imports `handlers/eks` for `AccountBucketName` (line 101) and `OIDCJWKSKey` (line 110), the Q-22 STS-to-EKS dependency ADR-0001 prohibits in its present form. Removal condition: an EKS-owned OIDC projection or an identity-owned contract that STS consumes instead of the EKS implementation package.
- `handlers/elbv2/{agent_types,haproxy,health_checker,nginx}.go` import `lbagent` (`HealthReport`, `ServerStatus`, `CertDir`, `NginxPIDPath`, `NginxCertDir`), a domain reading an agent's implementation package rather than a declared wire contract. Removal condition: the health-report and cert/PID-path vocabulary moves behind a guest/controller contract the domain and the agent both consume.
- `handlers/iam/service_impl.go:30` imports `admin` for `GenerateAWSSecretKey` (line 641) only, a domain-to-operator edge; the `admin.AccountID`, `admin.AccountName` and `admin.EncryptedSecret` uses are fields of a local `*AdminBootstrapData` parameter, not the package. It clears with the remaining `admin` entry, once the key generators have an IAM-credentials owner.
- `domains/ec2/instance/state/imds_readers.go` (dependency, IMDS readers) imports `daemon` for `LocalState`, `ReadLocalState`, `LocalStatePath`, `NewJetStreamManager` and `InstanceStateBucket`. The local-file reader and the instance-record loader's `JetStreamManager` open, which `vpcd/imds_instance_state.go` held directly, moved here unchanged and are now injected into `vpcd` through `guestmetadata.LocalStateReader` and `guestmetadata.RecordLoader`. Removal condition: the local-file reader and the instance-record loader/bucket open move into `domains/ec2/instance/state` under ADR-0007's state work, with the daemon consuming them.
- `domains/ec2/instance/state/imds_readers.go` (dependency, IMDS readers) imports `daemon` for `LocalState`, `ReadLocalState`, `LocalStatePath`, `NewJetStreamManager` and `InstanceStateBucket`. The local-file reader and the instance-record loader's `JetStreamManager` open, which `vpcd/imds_instance_state.go` held directly, moved here unchanged and are now injected into `vpcd` through `guestmetadata.LocalStateReader` and `guestmetadata.RecordLoader`. The readers already live in `domains/ec2/instance/state`. Removal condition: the legacy dependencies they call are extracted from `daemon` — `LocalStatePath`, `ReadLocalState` and `LocalState`, the instance bucket constants, and `JetStreamManager` with its bucket initialisation (`InitKVBucket`) — leaving `domains/ec2/instance/state` with no `daemon` import.
- Advisory: 27 production files across `domains/{acm,dns,ec2/*,network/host,network/ovn,ochre}` import `bootstrap/config` directly rather than through typed constructor inputs, which ADR-0001's `config` row expects. `bootstrap/preflight/preflight.go:15` imports `runtime/host/command` and `services.go:13` imports `operator/host/systemd`; both are expected bootstrap-to-runtime/operator edges for startup validation and are not domain-rule violations.
- `admin` (remaining): the operator-only code moved to `operator/admin`. What remains has non-operator consumers and awaits owners: `DefaultAccountID`/`DefaultAccountName` (IAM/account bootstrap), `GenerateAWSAccessKey`/`GenerateAWSSecretKey` (IAM credentials) and `PromoteSystemImage` (the existing `domains/ec2/image`).
- `domains/dns`: imports `runtime/compute/vm` (domain-to-runtime), `domains/network/reconcile` for the reconcile lock and `bootstrap/config` for node configuration.
- ACM: the IAM AES-GCM helper import (ACM relocation residual).
- `runtime/roles/awsgw`: the quota reconcile loop belongs to `domains/admission/quota`, and the EKS token-verify responder belongs with STS or EKS identity.
- `runtime/roles/vpcd` (placement): the role still mixes process/role supervision with network-controller responsibilities. `lease_reaper.go` (orphaned external-pool DHCP lease sweep), `ovn_controller_watchdog.go` (OVN controller liveness and restart), `host_eip_loop.go` (host-EIP reconcile loop) and `lease_datapath_reconcile.go` (lease datapath reconcile) are periodic network-control loops, not role wiring; `vpcd.go` itself also carries bridge-mode detection and verification, chassis-list parsing, external-CIDR resolution, DHCP manager startup and lease/IP-change handling, and gateway/host-EIP allocator and neighbour flush/prime construction alongside `Service.Start`. The future placement question is whether these belong in `domains/network`. Removal condition: an accepted decision on network-controller placement, and extraction of those loops and helpers behind the role.
- ECR control-plane registration: `awsapi.Deps.NATS` is a raw `*nats.Conn`, not a narrow capability. Every NATS-backed `Handler` passes it to `domains/ecr.NewNATSMetaStore`, which calls `natsmsg.NATSRequest`, so narrowing it means changing the exported `Handler` type, every handler and the meta store; it must not be broadened further meanwhile.
- ACM control-plane registration: `domains/acm/awsapi.Deps.NATS` (`registration.go`) is likewise a raw `*nats.Conn` carried by every `Handler`, for the same reason; it must not be broadened further meanwhile.
- `cmd/aws-model-coverage` imports `ingress/aws/dispatch`, `domains/ecr/awsapi`, `domains/acm/awsapi` and `gateway` to read ECR's and ACM's declared inventories. It is a non-serving tool and the only named exception to the ADR-0002 composition invariant; it must not construct a registration.
@@ -659,7 +661,7 @@ series above; "needs decision" is open.
| `gateway/tagging` | The cross-service tagging API (`GetResources`) and its authorization. | Decided: a new `domains/tagging` boundary owns the cross-resource tagging API and aggregation; individual resource domains keep ownership of their own tag storage and expose narrow tagging capabilities. Stays debt until those per-domain capabilities exist; clears when `domains/tagging` aggregates only through them. |
| `services/viperblockd` | — | Done. The adapter is `providers/ebs/viperblock` (with the read-only `dirty` reader), the shared vocabulary is `contracts/viperblockd/legacy/v1`, and the role is `runtime/roles/viperblockd`. `services/` is empty. |
| `utils` | — | Done and deleted. `ec2_subject.go` went to `contracts/ec2/v1`, the vpcd protocol to `contracts/network/v1` with the typed client in `domains/network/projection`, and the master-key loader to `domains/ec2/ebs/encryption`. |
| `vpcd` | Nothing of its own; the IMDS local-file reader and record-space opener moved to `domains/ec2/instance/state` and are injected through `guestmetadata.LocalStateReader`/`RecordLoader` (recorded above), so `vpcd` no longer imports `daemon` directly. | Its inter-process boundary now exists (`contracts/network/v1`, with `domains/network/subscribers` as the consumer and `domains/network/projection` as the producer client), and the `vpcd` → `daemon` dependency is removed. Moving `vpcd` to `runtime/roles/vpcd` is next, pending review. |
| `vpcd` | — | Done. The IMDS local-file reader and record-space opener moved to `domains/ec2/instance/state` and are injected through `guestmetadata.LocalStateReader`/`RecordLoader` (recorded above), so `vpcd` no longer imports `daemon` directly; its inter-process boundary now exists (`contracts/network/v1`, with `domains/network/subscribers` as the consumer and `domains/network/projection` as the producer client). The package moved unchanged to `runtime/roles/vpcd` (recorded below), and the legacy directory is removed. |
| `daemon` | Reserved as `runtime/daemon`'s home only; not split or moved. | Deferred: Q-28's write-side consolidation is a prerequisite ADR-0001 names explicitly. |
| `admin` | `DefaultAccountID`/`DefaultAccountName` (consumed by `daemon`, `gateway`, `runtime/roles/awsgw`, `operator/cli`), `GenerateAWSAccessKey`/`GenerateAWSSecretKey` (`handlers/iam`, `operator/cli`) and `images_promote.go` (`daemon`, `gateway/spx`, `operator/cli`). The operator use cases moved to `operator/admin`. | Deferred to their owners: IAM/account bootstrap, IAM credentials, and the existing `domains/ec2/image`. |
| `accountteardown` | Direct calls into `domains/ec2/awsapi/*` action adapters (recorded above). | Deferred: the ADR's `accountteardown` row replaces this with domain-owned cleanup capabilities. |
+1 -1
View File
@@ -241,7 +241,7 @@ services:
- "s3.*"
vpcd:
path: spinifex/vpcd
path: spinifex/runtime/roles/vpcd
# OVN-backed VPC / subnet / SG / NAT / IGW data plane.
subscribes:
- "vpc.create"
@@ -53,9 +53,9 @@ type LocalStateReader interface {
LocalVM(instanceID string) (*vm.VM, error)
}
// RecordLoader is the shared instance record space accessor the fallback
// path needs. Satisfied by *daemon.JetStreamManager; narrowed to an interface
// so the fallback can be exercised without a live JetStream connection.
// RecordLoader is a consumer contract: the shared instance-record accessor
// the fallback path needs, narrowed so the fallback can be tested without
// JetStream. The composition root supplies the implementation.
type RecordLoader interface {
LoadInstanceRecord(instanceID string) (*vm.InstanceRecord, error)
}
@@ -247,7 +247,7 @@ func s4ScanRoots(t *testing.T) []string {
root := repoRoot(t)
return []string{
filepath.Join(root, "spinifex", "domains", "network"),
filepath.Join(root, "spinifex", "vpcd"),
filepath.Join(root, "spinifex", "runtime", "roles", "vpcd"),
filepath.Join(root, "spinifex", "daemon"),
filepath.Join(root, "spinifex", "handlers"),
filepath.Join(root, "spinifex", "runtime", "compute", "vm"),
@@ -33,7 +33,7 @@ func TestS3_NATModeInitTimeConstant(t *testing.T) {
roots := []string{
filepath.Join(repoRoot(t), "spinifex", "domains", "network"),
filepath.Join(repoRoot(t), "spinifex", "vpcd"),
filepath.Join(repoRoot(t), "spinifex", "runtime", "roles", "vpcd"),
filepath.Join(repoRoot(t), "spinifex", "daemon"),
}
for _, root := range roots {
@@ -134,7 +134,7 @@ func isS3Exempt(path string) bool {
return true
case strings.HasSuffix(clean, "/network/policy/policy.go"):
return true
case strings.HasSuffix(clean, "/vpcd/vpcd.go"):
case strings.HasSuffix(clean, "/runtime/roles/vpcd/vpcd.go"):
return true
case strings.Contains(clean, "/network/invariants/"):
return true
@@ -33,7 +33,7 @@ func TestS7_NATSSubjectsCarryAZPrefix(t *testing.T) {
roots := []string{
filepath.Join(repoRoot(t), "spinifex", "domains", "network"),
filepath.Join(repoRoot(t), "spinifex", "vpcd"),
filepath.Join(repoRoot(t), "spinifex", "runtime", "roles", "vpcd"),
filepath.Join(repoRoot(t), "spinifex", "daemon"),
filepath.Join(repoRoot(t), "spinifex", "handlers"),
filepath.Join(repoRoot(t), "internal", "testkit"),
+1 -1
View File
@@ -22,8 +22,8 @@ import (
"github.com/mulgadc/spinifex/spinifex/runtime/roles/qemunbdd"
"github.com/mulgadc/spinifex/spinifex/runtime/roles/qmpcollector"
"github.com/mulgadc/spinifex/spinifex/runtime/roles/spinifexui"
"github.com/mulgadc/spinifex/spinifex/runtime/roles/vpcd"
"github.com/mulgadc/spinifex/spinifex/runtime/service"
"github.com/mulgadc/spinifex/spinifex/vpcd"
"github.com/spf13/cobra"
"github.com/spf13/viper"
)
+1 -1
View File
@@ -14,7 +14,7 @@ import (
"github.com/mulgadc/spinifex/spinifex/runtime/roles/spinifex"
"github.com/mulgadc/spinifex/spinifex/runtime/roles/spinifexui"
"github.com/mulgadc/spinifex/spinifex/runtime/roles/viperblockd"
"github.com/mulgadc/spinifex/spinifex/vpcd"
"github.com/mulgadc/spinifex/spinifex/runtime/roles/vpcd"
)
// Service is a Spinifex component the CLI can launch. Start runs it and
+1 -1
View File
@@ -8,7 +8,7 @@ import (
"github.com/mulgadc/spinifex/spinifex/runtime/roles/nats"
"github.com/mulgadc/spinifex/spinifex/runtime/roles/predastore"
"github.com/mulgadc/spinifex/spinifex/runtime/roles/spinifexui"
"github.com/mulgadc/spinifex/spinifex/vpcd"
"github.com/mulgadc/spinifex/spinifex/runtime/roles/vpcd"
"github.com/stretchr/testify/assert"
)
+1 -1
View File
@@ -298,7 +298,7 @@ func TestSelect_RealManifest_Smoke(t *testing.T) {
if err != nil {
t.Fatalf("load real manifest: %v", err)
}
res := Select(m, DefaultConfig(), []string{"spinifex/vpcd/topology.go"}, nil)
res := Select(m, DefaultConfig(), []string{"spinifex/runtime/roles/vpcd/topology.go"}, nil)
if len(res.Suites) == 0 {
t.Fatalf("real manifest vpcd change selected no suites")
}
+1 -1
View File
@@ -43,7 +43,7 @@ type Fixture struct {
// it. The server and client are torn down via t.Cleanup.
//
// The managers are the production types a live vpcd constructs
// (spinifex/vpcd/vpcd.go), minus the host-dependent options: NAT runs in
// (spinifex/runtime/roles/vpcd/vpcd.go), minus the host-dependent options: NAT runs in
// distributed mode with no chassis list, IGW allocates link-local addresses
// rather than drawing from a configured pool, and nothing seeds a nexthop MAC
// or waits on an OpenFlow barrier. Those all reach for the host datapath,