kacerrandClaude Opus 5.5
3d72507894
Phase 10. Two review passes (go-operator-reviewer, /code-review high) over the branch, recorded in docs/reviews/2026-09-23-proxmox-provider-review/: 9 findings fixed, 3 accepted as documented limits, 7 rejected with a reason. - Delete removes the seed between the stop and the destroy, so a failed seed delete leaves the VM for the next Delete instead of leaking the ISO; a Permanent refusal on the every-node path fails the call. - Create skips a probe VMID whose listed name is someone else's without waiting on its lock. - The three answers Phase 0 never captured are fixtures now: a missing storage is a 400 (the Checker reports StorageNotFound), a hibernated VM is destroyable (Delete destroys ours), a paused VM stops. - TestProxmox_lifecycle and TestProxmoxCheck_realHost passed on pv1-1 (PVE 9.2.20); the docs record the run and drop "not yet run". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
794 lines
59 KiB
Markdown
794 lines
59 KiB
Markdown
# The existing providers
|
|
|
|
This is the appendix to [provider-contract.md](provider-contract.md): how the
|
|
five providers that exist today (kubernetes, gcp, kvm, kvm2, proxmox) meet
|
|
the contract, which profile each one chose on every axis, and what is still
|
|
missing. It is reference material for whoever writes the next provider, not
|
|
a user guide; the user guides are [kvm-provider.md](kvm-provider.md),
|
|
[kvm2-provider.md](kvm2-provider.md),
|
|
[proxmox-provider.md](proxmox-provider.md),
|
|
[gcp-vm-validation.md](gcp-vm-validation.md) and
|
|
[gcp-in-specific-project.md](gcp-in-specific-project.md). The README's
|
|
"Implemented providers" table stays the short public summary. kvm2 and
|
|
proxmox are the two providers built from the contract itself (kvm2
|
|
clean-room from [kvm2-provider-spec.md](kvm2-provider-spec.md) without
|
|
reading kvm's code, proxmox from
|
|
[proxmox-provider-spec.md](proxmox-provider-spec.md)), so their columns are
|
|
also a record of what the contract produced.
|
|
|
|
The per-provider call flows are drawn in
|
|
[architecture.md §5](architecture.md#5-what-provider-calls-do-in-the-outside-world),
|
|
the health checks are tabulated in
|
|
[§11](architecture.md#11-provider-health-checks-internalproviderhealth), and the
|
|
judgment calls behind several rows below are recorded under its "Decisions"
|
|
list. They are linked, not repeated.
|
|
|
|
Code references are a file path plus a function or symbol name.
|
|
|
|
## 1. Profile table
|
|
|
|
One row per axis of contract §4, in the same order, with the profile name
|
|
and the symbol that shows it.
|
|
|
|
| Axis | kubernetes | gcp | kvm | kvm2 | proxmox |
|
|
|---|---|---|---|---|---|
|
|
| `readiness` | immediate: a Pod is scheduled within seconds (`stateFromPod`) | provisioning-window: minutes of boot (`mapState`) | provisioning-window: boot plus, on first use, an image download (`imageCache.ensure`) | provisioning-window: boot plus, on first use, an image download (`ensureImage`) | provisioning-window: boot plus the guest agent's install at first boot, about 50 s to an address on the test host (`agentAddress`) |
|
|
| `create-return` | submit: returns after `client.Create` | submit: `Insert` is never awaited, `Operation.Wait` is not called | **blocking**: waits through `DomainCreate` (`ensureRunning`), and the first image fetch runs inside (`imageCache.fetch`) | **blocking**: waits through `DomainCreate` and a 15-second poll for running (`ensureRunning`, `waitRunning`), and the first image fetch runs inside (`fetchImage`) | **blocking**: waits through the clone, upload, resize and start tasks and a 60-second poll for running (`provision`, `waitRunning`) |
|
|
| `delete-return` | returns-early: `client.Delete`, Pod terminates on its own | returns-early: `Delete` submits an operation each time | waits: destroy, undefine and volume removal complete before return (`removeDomain`, `removeVolumes`) | waits: destroy, undefine and volume removal complete before return (`removeDomain`, `removeVolumes`) | waits: the stop, seed delete and destroy tasks complete before return (`destroy`, `deleteSeed`) |
|
|
| `ip-source` | assigned-at-create: `PodIP` (`instanceFromPod`) | assigned-at-create: the external NAT IP of the first NIC (`toInstance`) | discovered: ARP table first, then the guest agent, first routable IPv4 (`address`, `firstIPv4`) | discovered: the guest agent first, then the ARP table, first routable IPv4 (`addressSources`, `address`, `firstIPv4`) | discovered: the guest agent only, first routable IPv4 in the agent's order (`agentAddress`, `firstIPv4`) |
|
|
| `gc-mark` | native-labels: Pod labels (`buildPod`) | native-labels: instance labels (`buildInsertRequest`) | written-metadata: a namespaced `<metadata>` element written in `DomainDefineXML` (`metadata.go`) | written-metadata: an element in kvm2's own namespace written in `DomainDefineXML` (`metadataURI`, `newInstanceMetadata`) | written-metadata: three `key=value` lines in the VM's description, passed in the clone call itself (`formatDescription`, `parseDescription`) |
|
|
| `created-at` | backend-timestamp: `CreationTimestamp` | backend-timestamp: `CreationTimestamp`, parse error gives zero | written-at-create: `createdAt` in the metadata element (`newInstanceMetadata`) | written-at-create: `createdAt` in the metadata element, parse error gives zero (`newInstanceMetadata`, `createdAt`) | written-at-create: `proxy-operator-created-at` in the description, parse error gives zero (`parseDescription`) |
|
|
| `list-partial` | n/a: one call, all namespaces | omit-unreachable: `ReturnPartialSuccess` on `AggregatedList` | fail-whole per host, one unreadable domain skipped (`ListByTag`, `managedInstance`) | fail-whole per host, one unreadable domain skipped unless ctx ended; `ID`, `UID` and `CreatedAt` from the name and the mark only, no state or address RPC (`ListByTag`) | fail-whole for the one cluster-wide `/cluster/resources` call, one unreadable VM skipped unless ctx ended; `ID`, `UID` and `CreatedAt` from the description only, no status or agent call (`ListByTag`) |
|
|
| `quota` | absent: a quota 403 is Permanent (`classify`) | detected: 429, and 403 with reason `quotaExceeded` or `rateLimitExceeded` (`classify`, `hasReason`) | absent (`classify`) | detected: a pool-space precheck before any volume is created (`checkPoolSpace`, `neededBytes`) | absent: thin storage overprovisions, so no out-of-space answer exists to match; free space goes to the Checker's details (`classify`) |
|
|
| `not-found` | status-404: `apierrors.IsNotFound` | status-404: `isNotFound` | error-code: `ErrNoDomain`, `ErrNoStorageVol` (`hasCode`) | error-code: `ErrNoDomain`, `ErrNoStorageVol`, Transient inside Create (`classifyLibvirt`) | error-code: the 500 `Configuration file '…' does not exist`, plus list-and-miss on `/cluster/resources`; Transient inside Create (`isNotExist`, `classifyAPI`) |
|
|
| `primary-key` | name-is-key: `namespace/name` | name-is-key: `zones/<zone>/instances/<name>` | name-is-key: `hosts/<host>/domains/<name>` | name-is-key: `hosts/<host>/domains/<name>`, with a UID guard on adoption (`create`) | **allocated-id**: `vms/<vmid>/<name>`, the VMID from an FNV-1a hash of the UID with forward probing, each taken VMID put through an ownership check with a UID guard (`vmidFor`, `allocate`, `inspect`) |
|
|
| `provisioning-state` | native: `PodPending` | native: `PROVISIONING`, `STAGING`, `REPAIRING` | create-blocks: a domain is defined and started in one `Create`; `nostate` maps to Provisioning | create-blocks: defined, started and polled to running in one `Create`; `nostate` maps to Provisioning (`mapState`) | create-blocks: cloned, configured, started and polled to running in one `Create`; `qmpstatus: prelaunch` maps to Provisioning (`mapState`) |
|
|
| `async-ops` | none | task-handle, but the handle is discarded: an insert that fails after acceptance is never read | none | none | task-handle and backend-lock: every clone, upload, resize, start, stop, destroy and volume delete returns a UPID polled to its end within the call (`waitTask`); a VM's lock is waited on in Create and Transient in Get and Delete (`waitUnlocked`) |
|
|
| `secondary-resources` | none | cascade: `AutoDelete` on the boot disk (`buildInsertRequest`) | explicit: overlay and seed removed by name (`removeVolumes`); base image kept | explicit: overlay and seed removed by name (`removeVolumes`); base image kept | explicit: the seed ISO deleted by name (`deleteSeed`, `deleteSeedEverywhere`); the disks go with the destroy's `purge=1&destroy-unreferenced-disks=1` |
|
|
| `credentials` | workload-identity: in-cluster config or kubeconfig (`NewWithTransportWrapper`) | environment: Application Default Credentials | mounted-files: SSH key and known_hosts from the optional `kvm-ssh` Secret, read lazily (`newLazySSH`) | mounted-files: the same `kvm-ssh` Secret, read on every dial and never cached (`loadCredentials`) | mounted-files: the token from the optional `proxmox-token` Secret, read on every request (`readToken`); the CA from the same Secret, loaded once and cached (`httpClient`) |
|
|
| `client-kind` | vendor-sdk: controller-runtime `client.Client`, no seam interface | vendor-sdk: `cloud.google.com/go/compute` behind `instancesAPI` | rpc-over-ssh: `go-libvirt` over an SSH-forwarded unix socket, behind `libvirtAPI` (`conn.go`) | rpc-over-ssh: `go-libvirt` over an SSH-forwarded unix socket, behind the ctx-first `backend` seam (`backend.go`) | thin-client: `net/http` and `encoding/json` behind the ctx-first `api` seam, one method per endpoint (`api.go`, `httpAPI`) |
|
|
| `timeouts` | ctx-only: no `rest.Config.Timeout` is set | ctx-only: SDK defaults, with SDK retries on Get and List | per-call for the dial only (`defaultHandshakeTimeout` in `dialSSH`); RPCs are ctx-only and a cancelled `call` closes the connection | per-call: a budget around the dial, every RPC, the uploads and the download (`defaultBudgets`, `call`); a cancelled or overlong call kills the pooled connection | per-call: dial and TLS handshake 10 s, each request `requestTimeoutSeconds`, a budget per task wait (`defaultBudgets`); a cancelled request ends only itself |
|
|
| `logging` | none | tiered: V(1) outcomes and `logAPIError`, V(2) detail, V(5) wire log (`wirelog.go`) | tiered: V(1) outcomes and one `libvirt call` line per RPC (`trace.go`), V(2) XML and addresses; no wire tier | tiered: V(1) outcomes and one `libvirt call` line per RPC (`observe.go`), V(2) XML and addresses; no wire tier | tiered: V(1) outcomes and one `proxmox api call` line per request (`observe.go`), V(2) parameters, config summaries and addresses; no wire tier |
|
|
| `child-spans` | transport-wrapper: `tracing.RestConfigWrapper()` on its own `rest.Config`, parent-gated | sdk-instrumentation: the SDK adds `otelhttp` itself; not parent-gated, but every call site has a parent | **none**: `trace.go` is a log decorator, not spans | span-decorator: `kvm2.<Method>` per RPC (`observe`), plus `kvm2.dial` (`libvirtDialer`) and `kvm2.download` (`fetchImage`) | transport-wrapper: `tracing.RestConfigWrapper()` on the client's transport, parent-gated, one `HTTP <METHOD>` span per request (`newHTTPAPI`) |
|
|
| `backend-test` | cluster-e2e: kind (`test/e2e`) | manual-runbook: [gcp-vm-validation.md](gcp-vm-validation.md) | integration-tag: `kvm_integration_test.go`, `make test-integration-kvm` | integration-tag: `kvm2_integration_test.go`, `make test-integration-kvm2` | integration-tag: `proxmox_integration_test.go`, `make test-integration-proxmox`; passed on a PVE 9.2.20 host |
|
|
|
|
## 2. Other differences
|
|
|
|
Rows that are not profile axes but that a new provider author compares
|
|
against.
|
|
|
|
| Feature | kubernetes | gcp | kvm | kvm2 | proxmox |
|
|
|---|---|---|---|---|---|
|
|
| Create verifies the adopted instance's UID | no | no | no | **yes**: a domain without the kvm2 mark or with a different non-empty UID is Permanent; a marked one with a matching or empty UID is adopted (`create`) | **yes**: a VM with the proxy's name and no mark, or with a different non-empty UID, is Permanent; a marked one with a matching or empty UID is adopted (`inspect`) |
|
|
| Placement used | none | Zone, MachineType, Image | Zone (host), MachineType, Image | Zone (host), MachineType, Image | Zone (node), MachineType, Image (a template key) |
|
|
| CloudInit delivered | **no**: ignored | yes: `user-data` metadata | yes: NoCloud seed ISO built in-process (`buildSeedISO`) | yes: NoCloud seed ISO built in-process (`nocloud.BuildSeedISO`) | yes: NoCloud seed ISO built in-process (`nocloud.BuildSeedISO`) and uploaded through the API (`uploadSeed`) |
|
|
| Port used | yes: Squid config and containerPort (`squidConf`) | no: firewall is manual | no | no | no |
|
|
| States produced | Provisioning, Running, Terminated | all four | Provisioning, Running, Stopped | Provisioning, Running, Stopped | Provisioning, Running, Stopped |
|
|
| Deleting instance still reports its state | yes: a Pod being deleted reports its phase | n/a | n/a | n/a: Delete waits, and a concurrent Get sees the domain until it is undefined | n/a: Delete waits, and a concurrent Get sees the VM (Stopped once stopped) until the destroy removes its config |
|
|
| Error.Provider is the configured name | **no**: hardcoded `"kubernetes"` | yes | yes | yes (`wrapErr`) | yes (`wrapErr`) |
|
|
| Checker target | `apiserver` | the project | each host, concurrently | each host, concurrently, on a fresh connection | each node, concurrently, on the shared HTTP client |
|
|
| Checker Details | none | none | `user`, `pool`, `hostname`, `libvirtVersion`, `poolState`, `poolCapacity`, `poolAvailable` | `user`, `pool`, `hostname`, `libvirtVersion`, `poolState`, `poolCapacityGiB`, `poolAvailableGiB`, filled in as far as the check got | `isoStorage`, `tokenID`, `pveVersion`, `kernel`, `isoStorageAvailGiB`, `templates`, filled in as far as the check got |
|
|
| Credential rotation without restart | n/a | via ADC | no: cached after first successful load | yes: both files read on every dial (`loadCredentials`) | the token yes, read on every request (`readToken`); the CA no, cached after first use |
|
|
| Connection handling | client-go | SDK client, never closed | one pooled SSH plus libvirt connection per host, 15s keepalive (`hostPool`) | one pooled SSH plus libvirt connection per host, 15s keepalive (`hostTable`); dedicated connections for the image upload and the check (`dialFresh`) | one `net/http` client, 4 idle connections per host, HTTP/1.1 only; a cancelled request ends only itself |
|
|
| Unit-test double | controller-runtime fake client | `fakeAPI` behind `instancesAPI`, plus `httptest` for the real SDK client in `check_test.go` | `fakeAPI` behind `libvirtAPI`, plus an in-process `x/crypto/ssh` server in `conn_test.go` | `fakeBackend` behind `backend` (`fake_test.go`), plus an in-process `x/crypto/ssh` server (`sshserver_test.go`) and go-libvirt's own mock for the adapter | `fakeAPI` behind `api` (`fake_test.go`), plus `httptest.NewTLSServer` replaying the Phase 0 captures for the real client (`client_test.go`) |
|
|
| Proxy software | generated Squid config (`pod.go`) | from `cloudInit`; the image must run cloud-init | from `cloudInit`, plus a vendor-data script that installs the guest agent (`seed.go`) | from `cloudInit`, plus a vendor-data script that installs the guest agent (`nocloud.GuestAgentScript`) | from `cloudInit`, plus a vendor-data script that installs the guest agent (`nocloud.GuestAgentScript`) |
|
|
| IP reachable from the operator | Pod network, so in-cluster only (not from `make run-dev` on a laptop) | external IP, once a firewall rule allows the cluster's egress IP | LAN address on the host's bridge; the operator needs a route to that LAN | LAN address on the host's bridge; the operator needs a route to that LAN | address on the proxy network (bridge or VLAN from the config); the operator needs a route to it |
|
|
| Distinct egress IP per proxy | no: the cluster's egress | yes: ephemeral external IP | whatever the host's LAN routes through; not per VM by itself | whatever the host's LAN routes through; not per VM by itself | whatever the proxy network routes through; not per VM by itself |
|
|
|
|
## 3. Observability status
|
|
|
|
| Layer | kubernetes | gcp | kvm | kvm2 | proxmox |
|
|
|---|---|---|---|---|---|
|
|
| 1. Operation spans and metrics | yes: decorators | yes: decorators | yes: decorators | yes: decorators | yes: decorators |
|
|
| 2. Child spans per backend call | yes: `tracing.RestConfigWrapper()`, parent-gated (`cmd/main.go`, `NewWithTransportWrapper`) | yes: the SDK's own `otelhttp`; not parent-gated | **no** | yes: `kvm2.<Method>` from the `observe` decorator, `kvm2.dial`, `kvm2.download`; always under the caller's span, since every call carries the caller's ctx | yes: `tracing.RestConfigWrapper()` on the transport, parent-gated, one `HTTP <METHOD>` per request |
|
|
| 3. Logs carry `traceID` and `spanID` | n/a: the package does not log | yes, including the V(5) wire log | yes: every `libvirt call` line at V(1) | yes: every `libvirt call` line at V(1), written inside its RPC's span | yes: every `proxmox api call` line at V(1), with the operation span's `spanID` |
|
|
| Tested against a real collector | yes: the kind e2e checks `provider.create` and `provider.delete` in Tempo (`test/e2e/tracing_test.go`), operation spans only | no | no | no; the integration test asserts the parent-child topology in an in-memory recorder (`assertLifecycleSpans`) | no; `TestTracing_httpSpansUnderOperationSpan` asserts the parent-child topology in an in-memory recorder |
|
|
|
|
## 4. kubernetes (`internal/provider/kubernetes`)
|
|
|
|
- **Package and seam.** `kubernetes.go`, `pod.go`, `check.go`. There is no
|
|
seam interface: `newWithClient` injects a controller-runtime
|
|
`client.Client`, and tests use the fake client. `New` calls
|
|
`NewWithTransportWrapper`, which builds its own uncached client from
|
|
`ctrl.GetConfig()` with an optional `transport.WrapperFunc` for tracing.
|
|
- **Config.** `KubernetesConfig.Image` only, defaulting to the Squid image
|
|
constant in `kubernetes.go`.
|
|
- **ID.** `providerID(namespace, name)` gives `namespace/name`;
|
|
`parseProviderID` uses `cache.SplitMetaNamespaceKey` and rejects an empty
|
|
namespace with `ErrPermanent`. An empty name part is not rejected.
|
|
- **Create.** `buildPod` (pure) then `client.Create`; `IsAlreadyExists` is
|
|
success. Only `Name`, `Namespace`, `UID` and `Port` are used. The Squid
|
|
config is generated by `squidConf` from the port and passed through the
|
|
`SQUID_CONF` environment variable, so there is exactly one object per
|
|
proxy.
|
|
- **Get and state.** `stateFromPod`:
|
|
|
|
| Pod phase | PodIP | Provider state |
|
|
|---|---|---|
|
|
| Pending | any | Provisioning |
|
|
| Running | empty | Provisioning |
|
|
| Running | set | Running |
|
|
| Succeeded, Failed, Unknown | any | Terminated |
|
|
|
|
`instanceFromPod` sets `IP` only when the state is Running. A Pod with a
|
|
deletion timestamp still reports its phase. `UID` from the `LabelUID`
|
|
label, `CreatedAt` from `CreationTimestamp`.
|
|
- **Delete.** `client.Delete`, `IsNotFound` ignored.
|
|
- **ListByTag.** One `client.List` across all namespaces with
|
|
`MatchingLabels{LabelManaged: LabelManagedYes}`; this is what the
|
|
cluster-wide Pod RBAC is for.
|
|
- **Errors.** `classify`: `IsNotFound` NotFound; `IsTooManyRequests`,
|
|
`IsServerTimeout`, `IsTimeout` Transient; `IsForbidden`, `IsInvalid`,
|
|
`IsBadRequest`, `IsUnauthorized` Permanent; default Transient. Nothing is
|
|
Quota, because a quota 403 and an RBAC 403 cannot be told apart.
|
|
- **Check.** Lists at most one managed Pod cluster-wide (`client.Limit(1)`),
|
|
proving read and list access, not create. Target `apiserver`, address the
|
|
rest config host. `checkReason`: context errors and timeouts
|
|
`ReasonTimeout`; `IsUnauthorized`; `IsForbidden`; any `APIStatus`
|
|
`ReasonAPIError`; else `ReasonUnreachable`.
|
|
- **Logging and tracing.** No log lines in the package. Child spans come
|
|
from the parent-gated transport wrapper.
|
|
- **Tests.** `kubernetes_test.go`, `pod_test.go`, `check_test.go` over the
|
|
fake client; the kind e2e (`test/e2e`) is the real-backend run.
|
|
- **Deployment.** RBAC markers for Pods live next to the reconciler's in
|
|
`internal/controller/proxy_controller.go`. `hack/providers-dev.yaml` holds
|
|
only this provider, for `make run-dev`.
|
|
- **Known gaps.**
|
|
- `Error.Provider` is hardcoded to `"kubernetes"`, so errors and logs show
|
|
the wrong name when the configured name differs.
|
|
- `CloudInit` and all of `Placement` are ignored.
|
|
- No quota class: a quota 403 is Permanent.
|
|
- A Pod being deleted still reports its phase, so it can read as Running.
|
|
- No securityContext, no resources, and `http_access allow all` in
|
|
`pod.go`.
|
|
- No logging in the package, and no Checker `Details`.
|
|
- No `OwnerReference` on Pods: a bypassed finalizer leaves the Pod behind.
|
|
- The Pod RBAC is always installed, even when no kubernetes provider is
|
|
configured.
|
|
|
|
## 5. gcp (`internal/provider/gcp`)
|
|
|
|
- **Package and seam.** `gcp.go`, `insert.go`, `errors.go`, `check.go`,
|
|
`wirelog.go`. The seam is `instancesAPI` (`Insert`, `Get`, `Delete`,
|
|
`AggregatedList`, `Probe`); `realInstances` adapts the SDK client and
|
|
`newWithAPI` is the test constructor. `New` calls `NewWithWireOptions`,
|
|
which takes the `--gcp-wire-log-full-payloads` flag from `cmd/main.go`.
|
|
- **Config.** `GCPConfig`: `project` required; `network`, `networkTag`,
|
|
`diskSizeGb` defaulted in `withDefaults`.
|
|
- **ID.** `formatProviderID(zone, name)` gives
|
|
`zones/<zone>/instances/<name>`; `parseProviderID` rejects anything else
|
|
with `ErrPermanent`. The project comes from the config.
|
|
- **Create.** Validates `Zone`, `MachineType` and `Image` up front and
|
|
returns `ErrPermanent` naming the missing field. `buildInsertRequest`
|
|
(pure) builds one NIC with an ephemeral external IP (`ONE_TO_ONE_NAT`) and
|
|
a boot disk with `AutoDelete`, labelled with both marks. `Insert` is
|
|
submitted and never awaited; 409 (`isAlreadyExists`) is success.
|
|
- **Get and state.** `mapState(status, ip)`:
|
|
|
|
| Instance status | NAT IP | Provider state |
|
|
|---|---|---|
|
|
| PROVISIONING, STAGING, REPAIRING | any | Provisioning |
|
|
| RUNNING | empty | Provisioning |
|
|
| RUNNING | set | Running |
|
|
| STOPPING, STOPPED, SUSPENDING, SUSPENDED | any | Stopped |
|
|
| TERMINATED | any | Terminated |
|
|
| anything else | any | Stopped |
|
|
|
|
`toInstance` reads the NAT IP of the first access config of the first
|
|
NIC, `UID` from the label, and `CreatedAt` from `CreationTimestamp` with
|
|
the parse error dropped (zero time on failure).
|
|
- **Delete.** Submits a delete operation and returns; 404 (`isNotFound`) is
|
|
success. Every deletion poll submits a new operation.
|
|
- **ListByTag.** `AggregatedList` with `managedFilter` on the managed label
|
|
only and `ReturnPartialSuccess`, so an unreachable zone is left out
|
|
silently; the zone is recovered from the URL by `lastPathSegment`.
|
|
- **Errors.** `classify`: non-`googleapi` errors Transient; 404 NotFound;
|
|
429 Quota; 403 with reason `quotaExceeded` or `rateLimitExceeded`
|
|
(`hasReason`) Quota; 400, 401 and other 403s Permanent; other codes
|
|
Transient. `logAPIError` records the HTTP status and reasons at V(1)
|
|
because `classify` collapses them.
|
|
- **Check.** `Probe` makes one `AggregatedList` page with `maxResults=1`
|
|
and the managed filter: credentials, the Compute API and
|
|
`compute.instances.list` in one call. Target is the project, address
|
|
`compute.googleapis.com`. `checkReason`: context errors `ReasonTimeout`;
|
|
non-`googleapi` errors `ReasonUnreachable`; 401; 403; else
|
|
`ReasonAPIError`.
|
|
- **Logging.** `logger(ctx)` from the context, named `gcp` with
|
|
`provider`. V(2) for the curated insert request (never the user-data),
|
|
V(1) per operation outcome, V(2) per listed instance. The wire log in
|
|
`wirelog.go` runs at V(5) with payloads elided unless the flag is set.
|
|
- **Tracing.** The SDK adds `otelhttp` itself unless telemetry is disabled;
|
|
not parent-gated.
|
|
- **Tests.** `insert_test.go` (pure builder), `errors_test.go` (code
|
|
table), `gcp_test.go` (IDs, 409, state mapping, list filter and partial
|
|
success) over `fakeAPI`; `check_test.go` also drives the real SDK client
|
|
against `httptest` to prove exactly one request with `maxResults=1`.
|
|
Real backend: the manual runbook in [gcp-vm-validation.md](gcp-vm-validation.md).
|
|
- **Deployment.** Credentials by ADC; the runbook mounts a service-account
|
|
key and points `GOOGLE_APPLICATION_CREDENTIALS` at it. A commented entry
|
|
in both providers configs, `config/samples/proxy_gcp.yaml`, and demo
|
|
scripts under `docs/demo/`.
|
|
- **Known gaps.**
|
|
- Asynchronous insert failures are never read: an insert the API accepts
|
|
but that fails later (quota, bad image) is seen only as NotFound on the
|
|
next `Get`, and the reconciler creates again, so the reason never
|
|
reaches status. [gcp-vm-validation.md](gcp-vm-validation.md#gotchas)
|
|
still says such errors do reach status.
|
|
- External IP only: a VM without an external NAT IP stays Provisioning
|
|
forever.
|
|
- No firewall management: the rule for the proxy port is a manual
|
|
prerequisite, and `req.Port` is unused.
|
|
- The SDK client is never closed, and `Placement.Region` is unused.
|
|
- Wrong label value in the docs: `gcp-vm-validation.md` filters on
|
|
`labels.proxy-operator-managed=yes`; the value is `true`.
|
|
- The sample cannot work: `config/samples/proxy_gcp.yaml` uses a
|
|
`debian-cloud` image, which has no cloud-init, so its Squid install never
|
|
runs ([gcp-in-specific-project.md](gcp-in-specific-project.md) uses
|
|
Ubuntu for that reason).
|
|
|
|
## 6. kvm (`internal/provider/kvm`)
|
|
|
|
The full user guide is [kvm-provider.md](kvm-provider.md); this section is
|
|
the author's view.
|
|
|
|
- **Package and seam.** `kvm.go` (the four methods), `conn.go` (SSH dial,
|
|
`hostPool`, `libvirtAPI`), `image.go` (`imageCache`), `seed.go` (NoCloud
|
|
ISO and vendor-data), `domainxml.go`, `volume.go`, `metadata.go`, `id.go`,
|
|
`errors.go`, `check.go`, `trace.go`. The seam is `libvirtAPI`, the twenty
|
|
go-libvirt calls the provider uses; `fake_test.go` implements it in memory
|
|
and `newWithDialer` injects the dialer and the HTTP client.
|
|
- **Config.** `KVMConfig` in `internal/provider/config_kvm.go`:
|
|
`sshKeyFile`, `knownHostsFile` or `insecureIgnoreHostKey`, `diskSizeGb`,
|
|
`vendorData`, `installGuestAgent`, `images` (`url`, `sha256`, `format`,
|
|
`firmware`), `machineTypes` (`vcpus`, `memoryMiB`), `hosts` (`name`,
|
|
`address`, `user`, `socket`, `pool`, `bridge`). `validateKVM` restricts
|
|
image keys and host names with `kvmIdentifier` because they become a volume
|
|
name and an ID segment.
|
|
- **ID.** `formatProviderID(host, name)` gives `hosts/<host>/domains/<name>`.
|
|
- **Create.** `host` resolves `Placement.Zone` (optional with one host),
|
|
the machine type must be a catalogue key, `resolveImage` accepts a
|
|
catalogue key or an http(s) URL. Then, inside one `hostPool.call`:
|
|
`DomainLookupByName`, and if found `ensureRunning`; otherwise
|
|
`imageCache.ensure` (download on first use, streamed through the operator
|
|
into the pool with a sha256 marker volume), `ensureOverlay` (reused if it
|
|
exists), `writeSeed` (always rebuilt: a cut-off upload must not be
|
|
trusted), `defineDomain` with the metadata element, `ensureRunning`.
|
|
`Create` returns only when the domain is running.
|
|
- **Get and state.** `mapState(state, ip)`:
|
|
|
|
| libvirt state | IP | Provider state |
|
|
|---|---|---|
|
|
| nostate | any | Provisioning |
|
|
| running, blocked | empty | Provisioning |
|
|
| running, blocked | set | Running |
|
|
| paused, shutdown, shutoff, crashed, pmsuspended | any | Stopped |
|
|
|
|
`address` asks the ARP table first, then the guest agent, and
|
|
`firstIPv4` drops loopback, link-local and unspecified addresses. Metadata
|
|
is read back by `metadata` and parsed by `parseInstanceMetadata`; a domain
|
|
without the element reports an empty UID.
|
|
- **Delete.** `removeDomain` (destroy, tolerating not running or gone; then
|
|
`DomainUndefineFlags` with NVRAM, managed save and snapshot metadata) and
|
|
`removeVolumes` (overlay and seed by name, a missing pool is a no-op). A
|
|
host no longer in the config is a logged no-op success.
|
|
- **ListByTag.** `ConnectListAllDomains` on every host; `managedInstance`
|
|
keeps domains whose metadata says managed and skips a domain whose
|
|
metadata or state cannot be read. One unreachable host fails the whole
|
|
list.
|
|
- **Errors.** `classify`: `isClassified` errors keep their class;
|
|
`ErrNoDomain`, `ErrNoStorageVol` NotFound; XML, unsupported,
|
|
invalid-argument, auth and access errors, and a missing pool, Permanent;
|
|
everything else Transient, including SSH handshake and auth failures.
|
|
Nothing is Quota. `logAPIError` adds `libvirtErrorFields`.
|
|
- **Check.** `checkHost` per host concurrently through `hostPool.probe`,
|
|
not `call`, so a check that runs out of time abandons its RPCs instead of
|
|
closing the connection a reconcile may be using. Two cheap RPCs
|
|
(`ConnectGetLibVersion`, `ConnectGetHostname`) then the pool must exist
|
|
and be running. Reasons: the shared ones plus `SSHMaterialUnavailable`,
|
|
`SSHHandshakeFailed`, `LibvirtConnectFailed`, `LibvirtError`,
|
|
`PoolNotFound`, `PoolNotActive`, chosen by `checkReason` from the
|
|
`dialError` stage. Details: `user`, `pool`, `hostname`, `libvirtVersion`,
|
|
`poolState`, `poolCapacity`, `poolAvailable`.
|
|
- **Connections.** `hostPool` keeps one SSH plus libvirt connection per
|
|
host, dialled lazily (`lazySSH.dial`, `sshAuth.dial`, `dialSSH`,
|
|
`connectLibvirt`), with a 15s `keepalive`. `hostEntry.acquire` is a
|
|
one-slot semaphore so waiting can end with the caller's context. A
|
|
cancelled `call` closes the shared connection to unblock the RPC;
|
|
`probe` abandons it and leaves the connection open. The SSH material is
|
|
read once by `lazySSH.load` and cached.
|
|
- **Logging.** Logger from the context, named `kvm`, with `provider`,
|
|
`host` and `name`. V(1): one line per operation outcome and one
|
|
`libvirt call` line per RPC from the `tracingAPI` decorator in `trace.go`
|
|
(rpc name, identifiers, duration, error fields). V(2): domain and volume
|
|
XML, metadata, addresses. Upload and download bodies are never logged.
|
|
- **Tracing.** No child spans; `trace.go` is a log decorator.
|
|
- **Tests.** Pure builders (`domainxml_test.go`, `volume_test.go`,
|
|
`seed_test.go`, `id_test.go`, `metadata_test.go`); `errors_test.go`,
|
|
`conn_test.go` (pool behaviour and a real in-process SSH server);
|
|
`image_test.go`, `kvm_test.go` (Create resumes after every partial step,
|
|
every Delete already-gone case, ListByTag policies, logging tiers, a
|
|
cloud-init sentinel never logged); `trace_test.go`; `check_test.go`. Real
|
|
backend: `kvm_integration_test.go` behind `//go:build integration`, driven
|
|
by `KVM_TEST_*` variables and `make test-integration-kvm`, described in
|
|
[testing.md](testing.md#the-kvm-integration-test-real-hypervisor).
|
|
- **Deployment.** The optional `kvm-ssh` Secret mounted at
|
|
`/etc/proxy-operator/kvm` (`config/manager/manager.yaml`),
|
|
`make kvm-ssh-secret`, a commented entry in both providers configs,
|
|
`config/samples/proxy_kvm.yaml`.
|
|
- **Known gaps** (see also [kvm-provider.md § Known gaps](kvm-provider.md#known-gaps)).
|
|
- No quota or capacity detection.
|
|
- `Create` blocks, including the first image download on each host.
|
|
- GC blind spots: volumes without a domain are invisible to GC, and so are
|
|
VMs on a host removed from the config, which `Delete` treats as done.
|
|
- SSH material is cached after the first successful load, so rotating
|
|
keys needs a restart.
|
|
- Cancelling one operation cuts off the others on that host: a cancelled
|
|
`call` closes the shared per-host connection.
|
|
- SSH auth failures retry forever: SSH authentication and host-key
|
|
failures are Transient, while libvirt auth errors are Permanent.
|
|
- `Create` can return `ErrNotFound` when a volume or domain disappears
|
|
during the call.
|
|
- No reused-overlay check: an existing overlay is reused without checking
|
|
its backing image or size.
|
|
- Sample mismatch: `config/samples/proxy_kvm.yaml` places on host `nas1`,
|
|
while both providers configs define only `beelink`.
|
|
- Doc drift in kvm-provider.md: it says ARP is the fallback, but the code
|
|
tries ARP first; it says `host` and `name` are on every log line, but
|
|
the check and list paths lack them.
|
|
|
|
## 7. kvm2 (`internal/provider/kvm2`)
|
|
|
|
The full user guide is [kvm2-provider.md](kvm2-provider.md) and the design
|
|
it was built from is [kvm2-provider-spec.md](kvm2-provider-spec.md); this
|
|
section is the author's view. Nothing in the package was taken from
|
|
`internal/provider/kvm`; where the two agree it is because both follow the
|
|
same libvirt.
|
|
|
|
- **Package and seam.** `kvm2.go` (`Provider`, `New`, `withDefaults`,
|
|
`host`, `wrapErr`, the per-name lock), `create.go`, `get.go`,
|
|
`delete.go`, `list.go` (the four methods), `backend.go` (the seam, the
|
|
production adapter and the budgets), `ssh.go` (credentials, the forwarded
|
|
socket, keepalive), `hosts.go` (the pooled table and the production
|
|
dialer), `observe.go` (the span and log decorator), `image.go` (cache,
|
|
download, quota precheck), `xml.go`, `volume.go`, `metadata.go`,
|
|
`id.go`, `state.go`, `errors.go`, `check.go`; the seed ISO comes from
|
|
the shared `internal/provider/nocloud` and both lock sets are
|
|
`internal/provider/keyedlock`. The seam is
|
|
`backend`: the twenty go-libvirt calls the provider uses, each with `ctx`
|
|
first (go-libvirt's own methods take none and block), plus `Close`.
|
|
`libvirtBackend` adapts `*libvirt.Libvirt` and runs every call through
|
|
`call`, which applies the budget and the abandon policy. The `dialer`
|
|
seam separates the SSH transport: `newProvider` injects one for tests,
|
|
`newLibvirtDialer` is production. The `observe` decorator is applied
|
|
once, in `libvirtDialer.dial`, so provider code never holds an
|
|
undecorated backend.
|
|
- **Config.** `ProviderConfig.KVM2`, the same `KVMConfig` type as kvm,
|
|
validated by `validateKVMBlock` with block name `kvm2` so messages read
|
|
`kvm2.hosts`. Defaults (`diskSizeGb` 10, socket, pool, image format and
|
|
firmware) are filled by `withDefaults`. `New` never dials; it checks that
|
|
the credential files exist and warns when one does not, or when
|
|
`insecureIgnoreHostKey` is on.
|
|
- **ID.** `formatProviderID(host, name)` gives `hosts/<host>/domains/<name>`;
|
|
`parseProviderID` accepts exactly four segments with the literal
|
|
`hosts` and `domains` and non-empty host and name, else `ErrPermanent`.
|
|
- **Create.** `Create` validates `Zone` (`host`; optional with one host),
|
|
`MachineType` (a catalogue key) and `Image` (`resolveImage`: a catalogue
|
|
key or an http(s) URL with qcow2 and uefi defaults) before any dial, and
|
|
logs the resolved parameters with `cloudInitBytes` at V(2). Then, under
|
|
the per-name lock (`lockName`) and on the pooled connection
|
|
(`hostTable.get`), `create`: `DomainLookupByName`; found means the
|
|
adoption guard (no kvm2 mark, or a non-empty metadata UID that differs
|
|
from `req.UID`, is Permanent) and `ensureRunning`. Otherwise
|
|
`StoragePoolLookupByName`, `imageSizeIfUncached` (marker lookup, then
|
|
`HEAD`, or a closed `GET` when `HEAD` is not 2xx) and
|
|
`checkPoolSpace` with `neededBytes` (image if uncached, plus
|
|
`diskSizeGb`, plus 16 MiB), then `ensureImage` (`cachedImage` on the
|
|
pooled connection, `fetchImage` streaming into `uploadImage` on a
|
|
`dialFresh` connection, `StoragePoolRefresh`, `writeMarker` last),
|
|
`ensureOverlay` (`ErrStorageVolExist` means reuse), `writeSeed` (delete,
|
|
build, upload; always rebuilt), two `StorageVolGetPath`, `buildDomainXML`
|
|
with file-type disks and the metadata element, `DomainDefineXML`,
|
|
`ensureRunning` (`DomainGetState`, where paused or pmsuspended is
|
|
Permanent; `DomainCreate` with `ErrOperationInvalid` as already running;
|
|
`waitRunning` polls every 500 ms for up to 15 seconds, Transient on
|
|
expiry and at once for any inactive state other than `nostate`).
|
|
`Create` returns only when the domain is running.
|
|
- **Get and state.** `mapState(state, ip)`:
|
|
|
|
| libvirt state | IP | Provider state |
|
|
|---|---|---|
|
|
| nostate | any | Provisioning |
|
|
| running, blocked | empty | Provisioning |
|
|
| running, blocked | set | Running |
|
|
| paused, shutdown, shutoff, crashed, pmsuspended, anything else | any | Stopped |
|
|
|
|
`get`: `DomainLookupByName` (only `ErrNoDomain` reaches NotFound),
|
|
`readMetadata` (`DomainGetMetadata` with `metadataURI`;
|
|
`ErrNoDomainMetadata` means unmanaged with an empty UID; an element that
|
|
does not parse is Permanent), then `DomainGetState` and, for an active
|
|
domain, `address`. `addressSources` asks the guest agent first,
|
|
then ARP; a source that fails is skipped at V(2), only ctx ending is an
|
|
error, so a cancelled Get is Transient and not a false Provisioning.
|
|
`firstIPv4` drops loopback, link-local and unspecified addresses and an
|
|
address whose type tag and literal disagree. `UID` and `CreatedAt` are
|
|
read back on every call; an unparsable `createdAt` gives the zero time.
|
|
- **Delete.** Under the per-name lock: `removeDomain` (`DomainDestroy`,
|
|
tolerating not running and gone; `DomainUndefineFlags` with NVRAM,
|
|
managed save and snapshot metadata) and `removeVolumes` (overlay and seed
|
|
by name through `deleteVolumeIfPresent`; a missing pool is a no-op). A
|
|
host no longer in the config is a logged no-op success. Nothing is left
|
|
when nil is returned.
|
|
- **ListByTag.** `ConnectListAllDomains` (active and inactive) on every
|
|
host in config order; `readMetadata` and `managed` keep kvm2's own
|
|
domains, and the `Instance` is `ID`, `UID` and `CreatedAt` from the name
|
|
and the element, with `State` and `IP` empty: GC reads nothing else
|
|
(contract §2.4), so no state or address RPC is made. A domain whose
|
|
metadata cannot be read is skipped at V(1), unless ctx ended, which
|
|
fails the call. A host that cannot be dialled or listed fails the whole
|
|
call.
|
|
- **Errors.** `classify(op, err)`: an error already carrying a class keeps
|
|
it; libvirt codes through `classifyLibvirt` (`ErrNoDomain` and
|
|
`ErrNoStorageVol` NotFound, Transient when `op == "create"`;
|
|
`ErrNoStoragePool`, `ErrXMLError`, `ErrNoSupport`, `ErrInvalidArg`,
|
|
`ErrAuthFailed`, `ErrAccessDenied` Permanent; the rest Transient);
|
|
`knownhosts.KeyError` and the `unable to authenticate` text from
|
|
`x/crypto/ssh` Permanent; everything else Transient, including a missing
|
|
credential file. `permanentf` and `transientf` classify at the source:
|
|
placement, XML builders, a parse failure of the key or known_hosts, an
|
|
image 4xx, a missing or zero `Content-Length`, a sha256 mismatch, a pool
|
|
that is not running (Permanent); an image 429 or 5xx, a short body, the
|
|
running wait expiring (Transient). `checkPoolSpace` is the only source of
|
|
`ErrQuotaExceeded`. `logAPIError` writes `kvm2 backend call failed` with
|
|
`libvirtErrorFields`.
|
|
- **Check.** `Check` runs `checkHost` per host concurrently, each through
|
|
`probeHost` on a `dialFresh` connection that is closed when the probe
|
|
ends; the pooled table is never consulted. `ConnectGetLibVersion`,
|
|
`ConnectGetHostname`, `StoragePoolLookupByName`, `StoragePoolGetInfo`;
|
|
a pool that is not running is `poolNotActiveError`. Details are filled
|
|
as facts arrive (`user` and `pool` always; `libvirtVersion`, `hostname`,
|
|
`poolState`, `poolCapacityGiB`, `poolAvailableGiB`). `checkHost` answers
|
|
`Timeout` whenever the round's ctx ended, whatever stage was running and
|
|
whatever shape the error has; otherwise `checkReason`: a `dialError` is
|
|
judged by its stage (`credentials` gives `CredentialsUnavailable`; `ssh`
|
|
with a `knownhosts.KeyError` or the auth text gives `Unauthorized`; any
|
|
other stage, including a dial that ran out of its own budget, gives
|
|
`Unreachable`); then `context.DeadlineExceeded` or `Canceled` `Timeout`,
|
|
`ErrAccessDenied` or `ErrAuthFailed` `Forbidden`, `ErrNoStoragePool`
|
|
`PoolNotFound`, any other libvirt code `APIError`, anything else
|
|
`Unreachable`; `PoolNotActive` from the pool state.
|
|
- **Connections.** `hostTable` keeps one pooled connection per host,
|
|
dialled lazily under a per-host one-slot semaphore so waiting ends with
|
|
the caller's ctx; a dead backend (`liveness`) is closed and redialled on
|
|
the next `get`. `dialSSHSocket` runs credentials, TCP, the SSH handshake
|
|
and the `direct-streamlocal` channel to the socket, each failure a
|
|
`dialError` with its stage; `libvirtDialer.connect` bounds the whole
|
|
sequence plus the libvirt `Connect` with the dial budget and starts the
|
|
`keepalive` (15 s, `keepalive@openssh.com`) and a watcher on go-libvirt's
|
|
`Disconnected`. `call` runs each blocking go-libvirt call in a goroutine
|
|
and waits on the result, ctx and a budget timer; abandoning a call
|
|
(`link.markDead`) closes the socket and the SSH client, returns Transient
|
|
wrapping the ctx or deadline error, and every other call waiting on that
|
|
link fails once with Transient. `loadCredentials` runs on every dial.
|
|
- **Logging.** Logger from the context, named `kvm2`, with `provider`,
|
|
and `host` and `name` on lines about an instance. V(1): one line per
|
|
operation outcome (`kvm2 domain ensured running`, `kvm2 domain fetched`,
|
|
`kvm2 domain deleted`, `kvm2 domains listed`, `kvm2 host checked`), one
|
|
`libvirt call` line per RPC from `observe` (rpc name, identifiers,
|
|
duration, error fields), the connection events `libvirt host connected`
|
|
and `libvirt host connect failed` with `stage`, and `kvm2 backend call
|
|
failed`. V(2): XML, metadata, addresses, listed names, pool sizes, the
|
|
resolved Create parameters. Upload and download bodies, cloud-init and
|
|
key material are never logged.
|
|
- **Tracing.** `observe.call` starts `kvm2.<Method>` per RPC with
|
|
`kvm2.host` and the identifiers (`kvm2.domain`, `kvm2.pool`,
|
|
`kvm2.volume`), sets `kvm2.libvirt_code` on failure and writes the
|
|
`libvirt call` line inside the span; `libvirtDialer.dial` starts
|
|
`kvm2.dial` (`kvm2.address`, `kvm2.user`); `fetchImage` starts
|
|
`kvm2.download` (`kvm2.url_host`, `kvm2.sha256_configured`,
|
|
`kvm2.bytes`). Both the decorator and the dialer take `tracing.Option`
|
|
so tests inject a recorder; `New` leaves them on the global provider.
|
|
- **Tests.** Pure builders (`xml_test.go`, `volume_test.go`,
|
|
`seed_test.go`, `id_test.go`, `metadata_test.go`, `state_test.go`);
|
|
`errors_test.go` (the classify table with real SSH error strings and a
|
|
real `knownhosts.KeyError`); `backend_test.go` (the adapter over
|
|
go-libvirt's own mock through a `net.Pipe`: deadline kills the
|
|
connection, cancel unblocks, killed by another caller); `hosts_test.go`;
|
|
`ssh_test.go` over `sshserver_test.go` (every dial stage, host-key
|
|
policies, key material never in errors or logs); `image_test.go`
|
|
(marker rules, streaming asserted by counting bytes, dedicated upload
|
|
connection); `create_test.go`, `get_test.go`, `delete_test.go`,
|
|
`list_test.go`, `kvm2_test.go`, `provider_test.go` (the contract §11
|
|
list, named test by test in the spec's entry 21); `observe_test.go`
|
|
(every seam method spanned and logged exactly once, by reflection);
|
|
`check_test.go` (the reason table, never touches the pool). The double is
|
|
`fakeBackend` in `fake_test.go`, whose `twin` shares state under its own
|
|
call log so a test can tell the pooled connection from a dedicated one.
|
|
Real backend: `kvm2_integration_test.go` behind `//go:build integration`,
|
|
driven by `KVM2_TEST_*` variables and `make test-integration-kvm2`,
|
|
described in
|
|
[testing.md](testing.md#the-kvm2-integration-test-real-hypervisor).
|
|
- **Deployment.** The `kvm-ssh` Secret and its mount are shared with kvm
|
|
(`config/manager/manager.yaml` says so), `make kvm-ssh-secret` is reused,
|
|
a commented `kvm2-home` entry in both providers configs,
|
|
`config/samples/proxy_kvm2.yaml` (zone `beelink`, matching the shipped
|
|
config), the `kvm2` type in the `status.type` comment of
|
|
`api/v1alpha1/providerhealth_types.go`. No RBAC.
|
|
- **Known gaps** (see also [kvm2-provider.md § Known gaps](kvm2-provider.md#known-gaps)).
|
|
- `Create` blocks, including the first image download on each host.
|
|
- GC blind spots: volumes without a domain are invisible to GC, and so are
|
|
VMs on a host removed from the config, which `Delete` treats as done.
|
|
- No adoption of kvm's domains: a switched entry needs its Proxies
|
|
deleted first.
|
|
- Cancelling one operation cuts off the others on that host once: a
|
|
cancelled `call` kills the pooled connection.
|
|
- `Content-Length` is required for image downloads.
|
|
- A paused or pmsuspended domain is not started by a repeat `Create`:
|
|
`ensureRunning` returns Permanent, so the Proxy latches until its spec
|
|
changes; the admin resumes the VM on the host.
|
|
- Credential parse errors carry the parser's own text, which can quote a
|
|
PEM header type or a malformed known_hosts token (never key bytes).
|
|
- No reused-overlay check: an existing overlay is reused without checking
|
|
its backing image or size.
|
|
- The sample cloud-init installs Squid with `http_access allow all`.
|
|
|
|
## 8. proxmox (`internal/provider/proxmox`)
|
|
|
|
The full user guide is [proxmox-provider.md](proxmox-provider.md) and the
|
|
design it was built from is
|
|
[proxmox-provider-spec.md](proxmox-provider-spec.md); this section is the
|
|
author's view. It is the second provider built from the contract, and the
|
|
first on a REST API of the hypervisor's own: no SSH, no libvirt. Every
|
|
error text it matches was captured from a PVE 9.2.20 host before the code
|
|
was written.
|
|
|
|
- **Package and seam.** `proxmox.go` (`Provider`, `New`, `withDefaults`,
|
|
`node`, `wrapErr`, the per-name lock, the budgets), `create.go`,
|
|
`get.go`, `delete.go`, `list.go` (the four methods), `api.go` (the
|
|
seam), `client.go` (the production adapter `httpAPI`, the token and CA
|
|
loading), `observe.go` (the logging decorator), `task.go` (task, lock
|
|
and running waits), `seed.go`, `mark.go`, `id.go`, `state.go`,
|
|
`errors.go`, `check.go`; the seed ISO comes from the shared
|
|
`internal/provider/nocloud` and the name lock from
|
|
`internal/provider/keyedlock`. The seam is `api`: one method per
|
|
endpoint, sixteen in all, each with `ctx` first. `New` wraps `httpAPI` in
|
|
`observe` once, so provider code never holds an undecorated seam;
|
|
`newProvider` is the test constructor.
|
|
- **Config.** `ProviderConfig.Proxmox`, the `ProxmoxConfig` type in
|
|
`internal/provider/config_proxmox.go`, validated by `validateProxmox`
|
|
(the URL shape, node names and storage ids by regexp because they end
|
|
up in request paths, `vlanTag`, `vmidRange`, the two catalogues).
|
|
`isoStorage` has no default on purpose: the token needs
|
|
`Datastore.Allocate` there. Defaults (`vmidRange` 100000 to 199999,
|
|
`requestTimeoutSeconds` 30, `installGuestAgent` true) are filled by
|
|
`withDefaults`. `New` never makes a request; it warns when the token
|
|
file does not exist, or when `insecureSkipTLSVerify` is on.
|
|
- **ID.** `formatProviderID(vmid, name)` gives `vms/<vmid>/<name>`, with no
|
|
node, because VMs migrate; the node is resolved from
|
|
`/cluster/resources` on every call (`resolve`). `parseProviderID`
|
|
accepts exactly three segments, the literal `vms`, a decimal VMID
|
|
without leading zeros between 100 and 999999999 and a non-empty name,
|
|
else `ErrPermanent`. The name is there so `Get` and `Delete` can tell
|
|
our VM from another that later got the same VMID.
|
|
- **Create.** `placement` checks `Zone` (`node`; optional with one node),
|
|
`MachineType` and `Image` (both catalogue keys) before any request, and
|
|
logs the resolved parameters with `cloudInitBytes` at V(2). Then, under
|
|
the per-name lock, `create`: one `ClusterResources` read;
|
|
`adoptByName` (two VMs with the name are Permanent, one goes to
|
|
`inspect`); with none, `template` reads the template's config and
|
|
refuses a non-template, a cloud-init drive, a missing `net0` when the
|
|
config rewrites it, and an unsized boot disk when `diskGiB` is set, all
|
|
before any clone; `allocate` probes `vmidFor(uid, attempt)` up to 32
|
|
times, cloning onto a free VMID and running `inspect` on a taken one or
|
|
on the clone's `config file already exists` answer. `inspect` waits out
|
|
a lock (`waitUnlocked`; a clone in progress is also nameless), then
|
|
adopts our name with our mark and a matching or empty UID, probes past
|
|
another name, and is Permanent for our name without the mark, with
|
|
another UID, or hibernated. An adopted VM gets one `VMStatus`: running
|
|
returns at once. Otherwise `provision`: `uploadSeed` (rebuilt and
|
|
overwritten every attempt), `SetVMConfig` (`configParams`: cores,
|
|
memory, `agent=1`, the tag, `ide2`, and `net0` through `rewriteNet0`
|
|
when a bridge or VLAN is configured), the resize task when
|
|
`resizeTarget` says the boot disk is smaller than `diskGiB`, the start
|
|
task, and `waitRunning` (every second up to 60 s, Transient on expiry
|
|
or on a stop). `Create` returns only when the VM runs.
|
|
- **Get and state.** `get` checks in order: absent from the resources is
|
|
NotFound; a lock other than `suspended` is Transient (checked before the
|
|
name, because a mid-clone entry has none); `status: unknown` is
|
|
Transient; the config's `does not exist` is NotFound, a config lock
|
|
Transient; another name or no mark is NotFound; hibernated is Stopped
|
|
without further calls. Then `VMStatus` and `mapState(status,
|
|
qmpstatus, ip)`:
|
|
|
|
| `status` | `qmpstatus` | agent | Provider state |
|
|
|---|---|---|---|
|
|
| running | running | an IPv4 | Running |
|
|
| running | running | 200 without an IPv4, or `QEMU guest agent is not running` | Provisioning |
|
|
| running | running | `No QEMU guest agent configured` | Stopped |
|
|
| running | prelaunch | not asked | Provisioning |
|
|
| running | paused, suspended, io-error, anything else | not asked | Stopped |
|
|
| stopped, anything else | any | not asked | Stopped |
|
|
|
|
Any other agent answer, including `VM <vmid> is not running`, is
|
|
Transient. `UID` and `CreatedAt` are read from the description on every
|
|
call. A running `Get` is four calls: resources, config, status, agent.
|
|
- **Delete.** Under the per-name lock, `destroy`: `resolve`; a lock other
|
|
than `suspended` or `status: unknown` is Transient; the config's `does
|
|
not exist`, another name or no mark leave the VM alone; our VM
|
|
hibernated is destroyed like a stopped one, since the destroy takes it.
|
|
Otherwise stop (unless stopped), `deleteSeed` on the
|
|
VM's node, where a failure fails the call and leaves the VM for the
|
|
next Delete (a 403 for `Datastore.Allocate` is Permanent), then destroy
|
|
with `purge=1&destroy-unreferenced-disks=1`. Stop and destroy are tasks
|
|
waited on through `runTask`, where `does not exist` means gone. The
|
|
seed goes first because a VM already destroyed is NotFound to `Get`,
|
|
and nothing would call Delete again. With no VM of ours,
|
|
`deleteSeedEverywhere` tries every configured node, where a node that
|
|
fails is logged and skipped unless ctx ended or the failure is
|
|
Permanent. A missing volume still ends in a successful task, so no
|
|
message is matched. Nothing is left when nil is returned.
|
|
- **ListByTag.** One `ClusterResources` covers every node, unreachable
|
|
ones included (their guests are listed with `status: unknown`).
|
|
`isCandidate` keeps QEMU VMs that are not templates and carry the tag
|
|
`proxy_operator` or a name starting `proxy-`; a nameless entry (a clone
|
|
in progress) is not a candidate. One `VMConfig` per candidate; the mark
|
|
in the description decides, and the `Instance` is `ID`, `UID` and
|
|
`CreatedAt` only. A candidate whose config cannot be read is skipped at
|
|
V(1), unless ctx ended, which fails the call; the resources call failing
|
|
fails the whole call.
|
|
- **Errors.** `classify(op, err)`: an error already carrying a class keeps
|
|
it; then `apiError`s by `classifyAPI` (every 4xx Permanent; the 500
|
|
`does not exist` NotFound, Transient when `op == "create"`; every other
|
|
status Transient, including the lock, the taken VMID and pveproxy's 595
|
|
and 596); TLS verification failures (`tls.CertificateVerificationError`
|
|
and the three `x509` types) Permanent; everything else Transient. The
|
|
500 matchers (`isNotExist`, `isAlreadyExists`, the two agent ones)
|
|
are anchored to the 500 status and each is backed by a captured
|
|
response. `permanentf` and `transientf` classify at the
|
|
source: placement, the template checks, the ownership check, a
|
|
malformed token file or CA file (Permanent); a missing token or CA file,
|
|
an expired wait, a failed task (Transient). Nothing is Quota.
|
|
- **Check.** `Check` runs `checkNode` per node concurrently on the shared
|
|
client. `probeNode` reads the token file for `tokenID` (no request when
|
|
it is unusable), then `Version`, `NodeStatus`, `checkStorage` (a 500
|
|
whatever its text, inactive, or no `iso` content is `StorageNotFound`)
|
|
and `checkTemplates` (each catalogue VMID exists and is a template,
|
|
else `TemplateNotFound`). `checkNode` answers `Timeout` whenever the
|
|
round's ctx ended; otherwise `checkReason`: the two failures above,
|
|
`errCredentials` `CredentialsUnavailable`, `errCAFile` or a TLS
|
|
verification failure `CertificateInvalid`, 401 `Unauthorized`, 403
|
|
`Forbidden`, 595 and 596 `Unreachable`, any other status `APIError`, a
|
|
`net.OpError` `Unreachable` (before the deadline, so a dial out of its
|
|
own budget is a node that did not answer), a deadline or cancellation
|
|
`Timeout`, a cut connection `Unreachable`.
|
|
- **Connections.** One `http.Client`, built on first use once the CA
|
|
loads (`httpClient`), shared by every call and the Checker: dial and
|
|
TLS handshake 10 s each, `Client.Timeout` `requestTimeoutSeconds`, 4
|
|
idle connections per host kept 90 s, HTTP/2 disabled. A cancelled
|
|
request ends only itself. The token file is read by `readToken` on
|
|
every request.
|
|
- **Logging.** Logger from the context, named `proxmox`, with `provider`,
|
|
and `vmid`, `name` and `node` on lines about an instance. V(1): one line
|
|
per operation outcome (`proxmox vm ensured running`, `proxmox vm
|
|
fetched`, `proxmox vm deleted` with `destroyed`, `proxmox vms listed`,
|
|
`proxmox node checked`), one `proxmox api call` line per seam call from
|
|
`observe` (call, identifiers, UPID, duration, `httpStatus` and `error`),
|
|
`proxmox task failed`, and the no-op lines. V(2): request parameters,
|
|
config summaries, agent addresses, the resolved Create parameters.
|
|
Cloud-init, the token, the CA and the upload body are never logged.
|
|
- **Tracing.** No span of its own: `newHTTPAPI` wraps the transport in
|
|
`tracing.RestConfigWrapper(opts...)`, so each request is an
|
|
`HTTP <METHOD>` client span under the caller's span, and none without
|
|
one. `observe` only logs, so nothing is counted twice.
|
|
- **Tests.** Pure pieces (`id_test.go`, `mark_test.go`, `state_test.go`,
|
|
`seed_test.go`, `errors_test.go` with the classify table over the
|
|
captures, `api_test.go`); `client_test.go` (the real client over
|
|
`httptest.NewTLSServer`: the token never in errors or logs, x509
|
|
failures Permanent, cancellation, the child span); `create_test.go`,
|
|
`get_test.go`, `delete_test.go`, `list_test.go`, `task_test.go`,
|
|
`proxmox_test.go`, `check_test.go`, `observe_test.go` (every seam
|
|
method logs exactly one line, by reflection) and `provider_test.go`
|
|
(the contract §11 sweep, named test by test in the spec's entry 21).
|
|
The double is `fakeAPI` in `fake_test.go`: VMs, tasks and ISOs in
|
|
memory, a clone that leaves a nameless locked entry until its task
|
|
ends, per-method errors and blocking, a call log; its error texts are
|
|
checked against the captures. Real backend:
|
|
`proxmox_integration_test.go` behind `//go:build integration`, driven
|
|
by `PROXMOX_TEST_*` variables and `make test-integration-proxmox`,
|
|
described in
|
|
[testing.md](testing.md#the-proxmox-integration-test-real-proxmox-ve-host);
|
|
it passed against PVE 9.2.20 on 2026-09-24.
|
|
- **Deployment.** The optional `proxmox-token` Secret (keys `token` and
|
|
`ca.pem`) mounted at `/etc/proxy-operator/proxmox`
|
|
(`config/manager/manager.yaml`), `make proxmox-token-secret`, a
|
|
commented `proxmox-home` entry in both providers configs (loaded by
|
|
`TestLoadConfig_proxmoxSampleEntry`), `config/samples/proxy_proxmox.yaml`,
|
|
the `proxmox` type in the `status.type` comment of
|
|
`api/v1alpha1/providerhealth_types.go`. No RBAC, and no
|
|
`hack/providers-dev.yaml` entry, since it needs credentials.
|
|
- **Known gaps** (see also [proxmox-provider.md § Known gaps](proxmox-provider.md#known-gaps)).
|
|
- `Create` blocks until the VM runs.
|
|
- A clone lock that never clears makes every Create for that Proxy wait
|
|
out the 10-minute clone budget; `qm unlock` by hand.
|
|
- GC blind spot: a seed ISO whose VM was destroyed outside the operator
|
|
is invisible to GC, and a seed on node-local storage stays behind
|
|
when its VM migrated.
|
|
- The VMID range is shared with humans; a hand-made VM with a proxy's
|
|
name is Permanent.
|
|
- Cross-node clones (`target`) need shared storage and are untested.
|
|
- A hibernated VM of ours latches its Proxy when Create's adoption
|
|
finds it; a recorded one is replaced.
|
|
- The CA is cached after first use; the token is not.
|
|
|
|
## 9. Gaps shared by the existing providers
|
|
|
|
- **Adoption is not verified in three of the five.** kubernetes, gcp and
|
|
kvm adopt any existing instance with the right name without checking
|
|
`LabelUID`; kvm2 and proxmox return `ErrPermanent` when the UID differs.
|
|
- **The Create comment does not match kvm, kvm2 and proxmox.** The
|
|
interface comment in `internal/provider/provider.go` says `Create`
|
|
returns as soon as the request is submitted; the three VM providers on
|
|
your own hypervisors block until the VM has started.
|
|
- **`Stopped` and `Terminated` are produced inconsistently.** kubernetes
|
|
never produces Stopped, kvm, kvm2 and proxmox never produce Terminated,
|
|
and the reconciler treats them the same.
|
|
- **Different `ListByTag` partial-failure policies.** gcp leaves failed
|
|
parts out, kvm and kvm2 fail the whole list, and proxmox fails it when
|
|
its one cluster-wide listing fails. Both are safe.
|
|
- **Stale `mock` comments** in `internal/provider/provider.go`,
|
|
`internal/provider/registry/registry.go` and `api/v1alpha1/proxy_types.go`
|
|
(and so the generated CRD's `spec.provider` description).
|
|
- **GC does not know about installations.** Two clusters sharing one project
|
|
or hypervisor delete each other's instances. This was a merge-blocker in
|
|
the 2026-08-10 PR review
|
|
([docs/reviews](reviews/2026-08-10-1134-proxy-operator-pr-review.md)) and
|
|
is still open.
|
|
- **Tracing.** kvm has no child spans, so a kvm trace shows `provider.create`
|
|
and nothing under it; correlation works only through `traceID` on its V(1)
|
|
log lines. kvm2 has them, but only an in-memory recorder checks their
|
|
topology. Only kubernetes is tested against a real collector, and that
|
|
test checks only the operation spans; nothing checks that gcp's SDK spans,
|
|
the kubernetes API spans, kvm2's spans or proxmox's HTTP spans nest under
|
|
`provider.*` in a real collector.
|
|
|
|
## 10. Proposed features
|
|
|
|
Listed only. Nothing below is designed or implemented, except where a
|
|
bullet says kvm2 already does it.
|
|
|
|
- **Child spans in kvm:** extend the `tracingAPI` decorator in
|
|
`kvm/trace.go`, which already wraps every libvirt call, to also start a
|
|
span per RPC, and add spans for the SSH dial and the image download. The
|
|
seam's methods take no `ctx` today (`hostPool.call` hands `fn` only the
|
|
API), so the span context has to be plumbed in. kvm2's `observe.go` over
|
|
its ctx-first seam is the shape this would take.
|
|
- **Tempo e2e checks for child spans**, and for gcp, kvm and kvm2 through
|
|
their real backend runbooks.
|
|
- **A shared conformance suite:** for example `internal/provider/providertest`,
|
|
which runs the contract §11 tests against any provider plus a fake
|
|
backend, so a new provider gets the contract tests for free.
|
|
- **Verify the UID when `Create` adopts an instance.** Return `ErrPermanent`
|
|
on a mismatch instead of adopting an instance that belongs to someone else.
|
|
kvm2 and proxmox do this; the other three do not.
|
|
- **Report asynchronous create failures:** gcp keeps the operation name, and
|
|
`Get` reports the failure instead of NotFound.
|
|
- **Capacity or quota signals** for kvm, such as pool free space and host
|
|
memory, mapped to `ErrQuotaExceeded`. kvm2 has the pool-space precheck;
|
|
host memory is still unchecked in both.
|
|
- **GC for secondary resources:** mark volumes and similar resources, and
|
|
let `ListByTag` or a second listing method cover them.
|
|
- **Reload credentials and config without a restart.** kvm2 re-reads its
|
|
SSH material on every dial; the providers config is still read once per
|
|
process for every type.
|
|
- **Checker `Details`** for kubernetes (server version) and gcp (quota
|
|
headroom).
|
|
- **Internal-IP mode** for gcp, and a firewall rule managed per provider.
|
|
- **An installation-identity mark**, set on create and filtered on in
|
|
`ListByTag`, so installations sharing a backend do not collect each
|
|
other's instances.
|
|
- **A permanent-failure latch that sees provider availability**, so adding
|
|
a missing provider releases the Proxies waiting on it.
|
|
- **Validate provider names at load**, and derive the config type
|
|
allow-list from the constructor map.
|
|
- **Per-provider RBAC** as an optional kustomize component, and an
|
|
`OwnerReference` on kubernetes Pods.
|