Add the GCP provider: four-call surface, fire-and-forget ops, zone-qualified IDs
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -12,7 +12,7 @@ Pairs with [docs/plans/2026-08-07-1747-proxy-operator.md](../plans/2026-08-07-17
|
||||
- [x] Step 5 — Health engine (`internal/health/`)
|
||||
- [x] Step 6 — Lease store (`internal/lease/`)
|
||||
- [x] Step 7 — Discovery API (`internal/discovery/`)
|
||||
- [ ] Step 8 — GCP provider (`internal/provider/gcp/`)
|
||||
- [x] Step 8 — GCP provider (`internal/provider/gcp/`)
|
||||
- [ ] Step 9 — Orphan GC + metrics
|
||||
- [ ] Step 10 — Wiring, config, docs
|
||||
- [ ] Step 11 — Tests
|
||||
@@ -811,3 +811,67 @@ indirect to direct (the server holds a `logr.Logger` field). The
|
||||
`--discovery-addr`, `--max-lease-ttl` flags and the `DISCOVERY_TOKEN`
|
||||
Secret mount arrive with `cmd/main.go` in Step 10. `docs/architecture.md`
|
||||
gained §7 covering the whole HTTP path and the store's sweep Runnable.
|
||||
|
||||
## Step 8 — GCP provider (`internal/provider/gcp/`)
|
||||
|
||||
Implemented per the plan: `gcp.go` (Provider + the flattened `instancesAPI`
|
||||
test seam + providerID handling + state mapping), `insert.go` (pure
|
||||
`buildInsertRequest` + config defaults), `errors.go` (HTTP-code → taxonomy
|
||||
classification). Only the four calls the spec allows — instances Insert /
|
||||
Get / Delete / AggregatedList — and `Operation.Wait` is never called:
|
||||
Create/Delete return once the operation is submitted, `409 alreadyExists`
|
||||
on Insert and `404` on Delete both count as success, which is what makes
|
||||
repeat calls after a crash correct.
|
||||
|
||||
Dependency added (the plan's environment check pinned it):
|
||||
|
||||
```bash
|
||||
go get cloud.google.com/go/compute@v1.65.0 google.golang.org/api@latest
|
||||
# resolved google.golang.org/api v0.292.0; go mod tidy pulled the auth/gax chain
|
||||
```
|
||||
|
||||
Key shapes, all straight from the plan:
|
||||
|
||||
- **providerID `zones/<zone>/instances/<name>`** — Get/Delete parse the
|
||||
zone out of the ID instead of re-reading `spec.placement.zone`, which is
|
||||
wrong exactly when a zone edit is the replacement being processed.
|
||||
- **The seam is not an SDK mirror** — verified the plan's premise against
|
||||
the vendored source before designing around it:
|
||||
`InstancesScopedListPairIterator` has an unexported `nextFunc`, so a
|
||||
fake cannot construct one. The seam flattens `AggregatedList` to
|
||||
`[]*computepb.Instance` and returns operations as just their name.
|
||||
- `AggregatedList` sets `ReturnPartialSuccess: true` (one unreachable
|
||||
zone must not fail a GC sweep) and filters by
|
||||
`labels.proxy-operator-managed = true`.
|
||||
- `RUNNING` without a `NatIP` maps to `Provisioning` — never publish an
|
||||
empty IP. Unknown/new GCP statuses map to `Stopped`: the reconciler's
|
||||
response is delete-and-recreate, always safe for cattle.
|
||||
- Classification: 404→NotFound; 429 and 403-with-
|
||||
`quotaExceeded`/`rateLimitExceeded`→Quota; 400/401/403-other→Permanent;
|
||||
everything else (408, 5xx, network, unknown)→Transient.
|
||||
|
||||
Judgment call: `placement.zone`/`machineType`/`image` are all required at
|
||||
`Create` — missing values fail as `ErrPermanent` with a message naming
|
||||
the empty fields, rather than inventing defaults the spec doesn't define.
|
||||
A wrong guess here would silently create billable VMs of an arbitrary
|
||||
shape; a Failed condition telling the user what to set is strictly better.
|
||||
|
||||
Tests (75.2%, `-race -count=2` clean, green on first run): the plan's
|
||||
primary field-by-field `buildInsertRequest` assertion (machine-type URL,
|
||||
boot disk, the exact `{External NAT, ONE_TO_ONE_NAT}` access config,
|
||||
user-data metadata, GC labels, network tag) plus config overrides and
|
||||
no-metadata-without-cloud-init; the full classification table including
|
||||
`errors.Is` AND `errors.As` through the multi-unwrap; and fake-seam tests
|
||||
for zone-qualified IDs, 409-is-success, permanent-on-bad-placement (no
|
||||
API call made), the nine-row state-mapping table, 404 paths, malformed
|
||||
providerIDs, and the ListByTag filter/partial-success assertions. The
|
||||
uncovered remainder is `New()` (dials real Google with ADC) and the
|
||||
`realInstances` adapter — the same deliberately-untested posture as the
|
||||
kubernetes provider's `New()`.
|
||||
|
||||
Worth noting: gopls suggested replacing `proto.String(x)` with Go 1.26's
|
||||
`new(x)` expression; left as `proto.String` — it is the universal
|
||||
protobuf-construction idiom and matches every example in the SDK docs.
|
||||
Registry wiring (`"gcp": gcp.New`) happens at the composition root in
|
||||
Step 10, as designed in Step 2. `docs/architecture.md` §5 now shows both
|
||||
providers' call mappings.
|
||||
|
||||
Reference in New Issue
Block a user