Skip to content

feat(purchases): make GCP Compute commitment creation idempotent#666

Merged
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
fix/654-gcp-compute-idempotency
May 22, 2026
Merged

feat(purchases): make GCP Compute commitment creation idempotent#666
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
fix/654-gcp-compute-idempotency

Conversation

@cristim
Copy link
Copy Markdown
Member

@cristim cristim commented May 22, 2026

Closes #654. Refs #641 (final executor slice, after AWS #652 and Azure #653).

GCP Compute CUD creation previously named commitments cud- and ignored opts.IdempotencyToken, so a re-drive could create a second committed-use discount (financial double-purchase). Now both GCP idempotency levers are derived deterministically from the token:

  • RequestId on RegionCommitments.Insert (GCP native server-side dedupe) via common.IdempotencyGUID, the same canonical-UUID derivation Azure feat(purchases): make Azure reservation purchases idempotent (refs #641) #653 uses for reservationOrderID. Same token -> same RequestId -> second Insert is a server-side no-op.
  • Deterministic RFC1035-valid Name (cud-) as defense-in-depth: a re-drive that reaches Insert collides on the name and GCP returns ALREADY_EXISTS.

Empty token preserves the prior timestamp-based behaviour (CLI path). Token masked in logs via common.MaskToken (matching #652). Scope: computeengine only; CloudSQL/Storage/Memorystore stay advisory-only per #649.

Regression tests: same token on a 2nd call yields identical RequestId + name (no double-buy); empty-token CLI path stays non-idempotent. Full computeengine suite green (39 tests). A small adjacent refactor extracted buildInsertRequest to keep PurchaseCommitment under the gocyclo-10 pre-commit limit.

Once merged, #641 closes and #639 (idempotent auto-re-drive) unblocks.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved idempotent request handling for commitment purchases to ensure duplicate requests with the same idempotency token are properly deduplicated and do not result in duplicate commitments.
  • Tests

    • Added regression test to verify correct idempotent behavior for commitment purchase operations.

Review Change Stack

GCP Compute CUD creation named commitments cud-<unix-second> and ignored
opts.IdempotencyToken, so a re-drive of the same execution (issue #639's
recovery sweep) would create a SECOND committed-use discount: a financial
double-purchase.

Thread opts.IdempotencyToken into RegionCommitments.Insert via two
deterministic mechanisms so the same token can never buy twice:

- RequestId: GCP's native server-side idempotency key on Insert, which the
  API documents as preventing duplicate commitments. It must be a valid
  non-zero UUID, so we format the SHA-256 token into a canonical UUID with
  the existing common.IdempotencyGUID helper (the same derivation PR #653
  used for the Azure reservationOrderID). The same token always yields the
  same RequestId, so the second Insert is a server-side no-op.
- Name: also derived from the token (cud-<first-32-hex>, RFC1035-valid) as
  defense in depth. Commitment names are unique per project+region, so a
  re-drive that somehow reached Insert collides on the name and GCP rejects
  it with ALREADY_EXISTS rather than creating a duplicate.

An empty token preserves the prior non-idempotent timestamp-based name (the
CLI path, which has no owning execution). The token is masked in logs via
common.MaskToken (never logged verbatim), matching PR #652.

Adds a re-drive regression test mirroring the AWS/Azure ones: the same token
on a second PurchaseCommitment yields an identical RequestId and commitment
name, plus an empty-token test confirming the CLI path stays non-idempotent.

Scope is computeengine only; CloudSQL/Storage/Memorystore were made
advisory-only in #649 and are untouched.

Closes #654
refs #641
@cristim cristim added bug Something isn't working triaged Item has been triaged priority/p1 Next up; this sprint severity/high Significant harm urgency/this-sprint Within the current sprint impact/few Limited audience effort/m Days type/bug Defect labels May 22, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e383e574-1126-4d3d-9095-428163386c40

📥 Commits

Reviewing files that changed from the base of the PR and between 12aba9d and 039ae96.

📒 Files selected for processing (2)
  • providers/gcp/services/computeengine/client.go
  • providers/gcp/services/computeengine/client_test.go

📝 Walkthrough

Walkthrough

GCP Compute commitment creation is refactored to use the provided IdempotencyToken to derive a deterministic commitment name and set the GCP Insert RequestId, enabling idempotent re-drive behavior. Helper functions generate RFC1035-valid names from token digests with timestamp fallback. Tests verify that repeated calls with identical tokens produce matching request IDs and commitment IDs.

Changes

Idempotent GCP Compute Commitment Creation

Layer / File(s) Summary
Deterministic Name and RequestId Generation
providers/gcp/services/computeengine/client.go
Adds log import and introduces idempotentNameTokenLen and idempotentCommitmentName helpers to derive deterministic RFC1035-valid commitment names from IdempotencyToken (with timestamp fallback). New buildInsertRequest helper constructs the Insert request, derives the commitment name, conditionally sets GCP's RequestId idempotency key when token yields a non-empty GUID, and logs the masked token and commitment name.
PurchaseCommitment Integration
providers/gcp/services/computeengine/client.go
PurchaseCommitment refactored to delegate Insert request construction to buildInsertRequest, obtaining both the request and deterministic commitment name. Commitment ID is now read from the returned name instead of the commitment payload.
Test Infrastructure and Idempotent Re-Drive Test
providers/gcp/services/computeengine/client_test.go
MockCommitmentsService enhanced with insertReqs slice to record all Insert requests, enabling multi-call assertions. New TestComputeEngineClient_PurchaseCommitment_IdempotentReDrive regression test verifies two calls with the same IdempotencyToken produce identical RequestId and commitment name (non-zero UUID) and matching commitment IDs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • LeanerCloud/CUDly#641: This PR completes the GCP Compute slice of issue #641 by implementing idempotent commitment creation via deterministic name derivation and RequestId setting from IdempotencyToken, directly unblocking auto-re-drive.

Possibly related PRs

  • LeanerCloud/CUDly#638: Both PRs implement idempotent commitment creation by using PurchaseOptions.IdempotencyToken to ensure repeated PurchaseCommitment calls deduplicate consistently, addressing the same root issue across different provider implementations.

Suggested labels

enhancement, priority/p2, severity/medium, urgency/this-quarter, impact/all-users, type/feat

Poem

🐰 A token transforms to a name,
Deterministic, RFC-tame,
GCP's RequestId now stable and sound,
Re-drives duplicate-free, no second found!
The commitment persists, idempotent and true,
One purchase, one promise—our rabbit breaks through! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: implementing idempotent GCP Compute commitment creation, which is the primary objective of the PR.
Linked Issues check ✅ Passed The PR fully implements all coding requirements from #654: idempotent commitment naming via IdempotencyToken, RequestId setting via common.IdempotencyGUID, regression test for re-drive behavior, and scoping to computeengine only.
Out of Scope Changes check ✅ Passed All changes are within scope: computeengine client refactoring and tests, with no modifications to CloudSQL/Storage/Memorystore as required.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/654-gcp-compute-idempotency

Comment @coderabbitai help to get the list of available commands and usage tips.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 22, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 22, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 22, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working effort/m Days impact/few Limited audience priority/p1 Next up; this sprint severity/high Significant harm triaged Item has been triaged type/bug Defect urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant