fix(compute,sql): normalize read-back so bootstrap reapply is drift-free#79
Merged
Conversation
Two createOnly fields read back in a different form than declared, forcing a spurious update/replace on reconcile: - Cloud SQL settings.ipConfiguration.privateNetwork: the API echoes a full 'https://www.googleapis.com/compute/v1/projects/...' URL; normalize to the 'projects/...' path a network resolvable reference resolves to. - Instance AttachedDisk.deviceName: was writeOnly + stripped on read, so a declared deviceName drifted (desired has it, read lacked it). Keep it in the disk read-back and mark it hasProviderDefault instead — a declared value now round-trips, and GCP's auto-assigned deviceName is suppressed for disks that do not set one. Unit tests for both.
RouterNat is a custom provisioner; hasProviderDefault is not applied to custom-provisioner resources at patch-gen, so type/autoNetworkTier/ enableEndpointIndependentMapping/endpointTypes (server defaults) read back as perpetual drift. Strip them in Read. Documented tradeoff: a user explicitly setting one loses drift detection until RouterNat is config-driven or core honors the hint for custom provisioners.
The forma declares db privateNetwork from a network resolvable's .selfLink (full compute URL), but Cloud SQL stores the bare projects/... path. The earlier normalize shortened the read-back instead, so it drifted the OTHER way (actual=short vs desired=full). Expand short->full on read so it round-trips against the declared selfLink.
1 req/s serialized every plugin API call; a stack destroy queued reads behind it long enough to blow the 20s operator timeout and cascade-fail. GCP quotas comfortably handle 10/s (matches azure/oci/k8s).
A Cloud SQL instance ships only the engine's default database; there was no
way to declare an application database in a forma (the bootstrap created it
imperatively via a startup-script curl to the Admin API). Add a config-driven
GCP::SQL::Database nested under its instance
(/projects/{p}/instances/{i}/databases/{name}), mirroring google_sql_database.
- schema: gcp.sql.sqlDatabase (name/instance/project createOnly, charset/
collation optional, instance accepts a DatabaseInstance resolvable)
- api: sqlPathBuilder + parseSQLNativeID/extractSQLNativeID handle the nested
parent path (4- and 6-segment native IDs)
- registered via ParentResourceConfig{ParentType: instances, PropertyName: instance}
StatusMessage renders as the CLI 'reason:' for a resource. Every provisioner
set it on in-progress/success too ('networks creation in progress'), so a clean
apply showed a spurious reason on each resource. Clear StatusMessage on any
non-failure result at the plugin's operation boundary (Create/Update/Delete/
Status in gcp.go) — one choke point covering base and all custom provisioners —
so a reason appears only when an operation actually failed.
Two read-back drifts made a fresh VM reconcile into a (failing) update:
- metadata: API returns items:[{key,value}]+fingerprint/kind; schema is
items:{key:value}. Convert back to the map form and drop the provider fields.
- deviceName: a disk that declares none gets GCP's auto 'persistent-disk-N';
the drift gate compares raw (ignores hasProviderDefault), so echoing it drifted.
Keep declared names, drop the auto form.
Without these the instance update also hit an empty-fingerprint error; with them
the VM no longer reconciles into an update at all.
The schema modeled metadata.items as Mapping<String,String> (a map) while the
GCE API uses items: [{key,value}]. The plugin bridged with map<->array
transforms, but that shape gap tripped core's desired/actual merge (object path
sjson.Set onto an array -> spurious "0": key -> invalid JSON -> persist crash,
PLA-276) whenever a stored baseline and a read disagreed on shape.
Model it faithfully instead: items is Listing<MetadataItem>{key,value} +
provider-default fingerprint. metadata now passes through on write (no map->
array build) and only drops the 'kind' discriminator on read. Desired array ==
actual array -> no drift, no shape-merge, no crash. Validated e2e: fresh VM,
reapply is a clean no-op, list_changes empty.
- schema: MetadataItem + Metadata.items Listing, fingerprint hasProviderDefault
- drop buildMetadata + normalizeInstanceMetadata; drop 'kind' on read
- remove obsolete metadata_test; update transformer test
Members are plain strings, not resolvable refs, so formae can't order a binding delete before its service account. When the SA is deleted first, the binding delete hit googleapi 400 'Service account ... does not exist' and Failed. Treat that as success — the binding is effectively gone. Reuses the same 400/does-not- exist predicate as the create-propagation retry.
Deleting a PSA connection right after its Cloud SQL instance is gone fails with 'Producer services ... are still using this connection' — Google releases the connection asynchronously (minutes). The delete op's Status mapped this to a terminal ServiceInternalError, failing the teardown (and blocking the range + VPC). Map it to NotStabilized instead so formae's operator retries the delete with backoff until the producer releases — no manual peering delete needed.
The NotStabilized retry couldn't help: the delete op is async and already DONE-failed, so re-polling just returns 'producer still using' until the operator times out, and the producer lag is many minutes regardless. Instead, when the connection delete fails with producer-still-using, fall back to removing the consumer VPC peering directly via the Compute API (Networks.RemovePeering servicenetworking-googleapis-com) — the documented recovery. It severs the connection immediately and unblocks the reserved range + VPC, so 'formae destroy' completes in one pass with no manual gcloud step.
A GCP::SQL::Database delete right after its client disconnects fails with 'database ... is being accessed by other users' — Cloud SQL reaps lingering Postgres sessions on a lag, and databases.delete (a server-side DROP) refuses while any session remains. The config-driven base mapped every async-op error to a terminal ServiceInternalError, so the teardown failed and needed a manual re-run once sessions cleared. Add an optional OperationConfig.RetryableError predicate: when it matches a completed-with-error operation, the base reports NotStabilized instead of a terminal failure, so formae core re-runs the delete until it converges. Wire it for Cloud SQL on the 'being accessed by other users' message.
…h too Cloud SQL returns 'database ... is being accessed by other users' directly on the DELETE request (not via the async operation), so it hit the terminal deleteFailureResult at base_resource.go:268 and never reached the op-status RetryableError hook. Apply the same RetryableError classification on the sync delete-error path → report NotStabilized so core retries while sessions drain, instead of failing the teardown.
The plugin canonicalizes settings.ipConfiguration.privateNetwork to the full compute selfLink on read (what the Cloud SQL API returns, and what the private- IP bootstrap declares via a network resolvable). The cloudsql-instance CRUD conformance testdata declared the short 'projects/.../networks/default' form, so after the read-back-expand change it mismatched (expected short, got full) and failed at create/extract/sync. Declare the full form for a clean round-trip.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Follow-up to 0.1.8. A
formae applyre-run of the GCP bootstrap reported drift/replace on resources that hadn't actually changed — twocreateOnlyfields read back in a different representation than declared:GCP::SQL::DatabaseInstancesettings.ipConfiguration.privateNetwork— Cloud SQL echoes it as a fullhttps://www.googleapis.com/compute/v1/projects/...URL, but a network resolvable reference (connection.res.network) resolves to theprojects/...path → spurious update every reconcile. The response transformer now normalizes it to theprojects/...form.GCP::Compute::InstanceAttachedDisk.deviceName— waswriteOnlyand stripped from the disk read-back, so a forma that setsdeviceNamedrifted (desired had it, read lacked it) → spurious update. Now kept in the read-back and markedhasProviderDefault(createOnly): a declared value round-trips, and GCP's auto-assigneddeviceName(persistent-disk-N) is suppressed as drift for disks that don't declare one.Why
These were the last two reapply drifts on the private-IP Cloud SQL bootstrap. With them fixed (plus the bootstrap wrapping
sourceImageinformae.value(...).setOnce— an image family resolves to a concrete image on read), a plainformae applyreconciles clean with no--force.Tests
Unit tests for both (
privatenetwork_test.go,instance_normalize_test.go). Full build + vet clean.