Repository navigation
fix(gcp): read the CUD commitment Type from the recommender payload instead of deriving it from the machine family #60
Description
Activity
- addedtriagedItem has been triagedItem has been triagedpriority/p2Backlog-worthyBacklog-worthyseverity/mediumModerate harmModerate harmurgency/this-quarterWithin the quarterWithin the quarterimpact/manyAffects most usersAffects most userseffort/lWeeksWeekstype/featNew capabilityNew capability
on Jul 28, 2026 Claimed by worker 4 (shard rank 28). Implementing: read the CUD commitment Type from the recommender operation payload (validated against computepb.Commitment_Type_value, fail-loud on unknown), carried via a new field on common.Recommendation/ComputeDetails, with the machine-family map kept as documented fallback. Acceptance per the issue.
Draft PR #275 adds only the shared ComputeDetails.GCPCommitmentType persistence prerequisite. Issue #60 remains open for reading and validating the recommender commitment type in the GCP provider and replacing machine-family derivation. No provider code or dependency pins change in this prerequisite. #275
- added a commit that references this issue
on Oct 7, 2026 Shared persistence prerequisite PR #275 merged normally at 07d8b1f. The remote shared types and codec tests match the verified files exactly; pkg resolves as v0.0.0-20261007231611-07d8b1fb3e3c. Issue #60 remains open for the recommender payload/provider validation implementation, pending its plan review. No provider changes or module pins were made, and no live GCP scenario or purchase was performed. #275
- added a commit that references this issue
on Oct 8, 2026 Issue #60 is closed by PR #281, merged normally as
5053efa7c1674cf7145eca4cc35a1c6cc2db3c3a, following shared prerequisite #275 and persisted-type consumer #280.Acceptance verification covers the public local path from root Recommender operations through conversion, persisted codec details, duplicate filtering, and the
InsertRegionCommitmentRequestpassed to the SDK boundary:- Supplied M4 and X4 commitment types override machine-family derivation and reach the captured Insert request with exact VCPU and MEMORY quantities.
- Unknown, non-concrete, malformed and unsupported supplied types refuse with zero Insert; absence preserves the documented legacy machine-family fallback.
- Direct values and codec-decoded pointers behave consistently. Matching existing commitment buckets suppress purchase, while different buckets remain eligible. Existing inventory carries commitment Type rather than its first VCPU or MEMORY resource kind, satisfying the related read-side audit requirement.
- Independent original-parent overlays fail all twelve public regression cases. Wrong-Type and compound-mutation probes fail; restored final source passes.
Fresh canonical fetch verifies the merged tree is byte-identical to reviewed
25519609df1bb97a1a6e91920b37fe57880a4264, including all five changed files. Exact CI lint, all-five-module build, full standalone GCP race-short suite, normal hooks and independent full-PR review passed. Both actual merge-commit workflows completed successfully, and all watchers are reaped.Evidence uses schema-based fixtures, local SDK/gRPC scenarios, codec roundtrips and mock typed requests. No captured production response, live GCP acceptance or cloud purchase was performed. Post-merge evidence records the actual CI runs and owner's CodeRabbit quota waiver. The retrospective review remains tracked when quota returns.
Follow-up issues filed: none after the exit audit. Existing #87 and #107 retain separate AWS inventory and Redshift tagging work.
Context
PR LeanerCloud/cloud-commitments-cli#1650 (issue LeanerCloud/cloud-commitments-cli#1538) fixed GCP CUDs being purchased as
GENERAL_PURPOSEregardless of the recommended machine family. It derives the commitment
Typefrom the machine-family segment of
rec.ResourceTypevia a hand-maintainedmachineFamilyCommitmentTypemap inproviders/gcp/services/computeengine/client.go.That map is a derivation, and therefore a guess that has to be kept in sync
with GCP:
purchases are refused (already true today for
m4,x4,t2a).machine-type name (
MEMORY_OPTIMIZED_M4vs_M4_6TB; the sevenMEMORY_OPTIMIZED_X4_*buckets) are not derivable at all from the machinetype, so they can never be supported this way.
What to do instead
The Commitment Recommender's own operation payload carries the commitment
resource it wants created, including its
typefield. GCP is telling us whichCommitment_Typeto buy; we do not have to infer it.Read the commitment
typefrom the recommender operation value (the operationwhose path targets the commitment resource root), validate it against
computepb.Commitment_Type_valueso an unknown member fails loud rather thanbeing passed through as a bare string, and use the machine-family map only as a
fallback for payloads that omit it.
This removes the guess entirely and makes the size-bucketed M4/X4 families
purchasable.
Why it was not done in LeanerCloud/cloud-commitments-cli#1650
buildInsertRequestreceives acommon.Recommendation, not the rawrecommenderpb.Recommendation. Carrying the commitment type from the converterto the purchase path requires a new field on
pkg/common.Recommendation(or oncommon.ComputeDetails), which lives in the separatepkgGo module sharedby the AWS, Azure and GCP providers, and which is persisted. That is a design
change touching all three providers plus storage, not a bug fix, and folding it
into a p0 money-path PR would have made that PR unreviewable.
Acceptance
Typecomes from the recommender payload when present.typevalue fails loud (no fallback to a guessed family type,no bare string literal on the wire).
Typecarried by theInsertRegionCommitmentRequesthanded to the SDK client, not on anintermediate struct.
Related: LeanerCloud/cloud-commitments-cli#1538, LeanerCloud/cloud-commitments-cli#1650, #14, #22.
Findings from the 2026-09-02 codebase audit
Added by an automated audit of
3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd(tip oforigin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report:docs/audits/codebase-audit-2026-09-02.md.A08b-029 (high)
Sibling defect on the read side of the same field, worth folding into whatever lands here.
convertCommitmentat providers/gcp/services/computeengine/client.go:524 setscom.ResourceType = commitment.Resources[0].Type, which the repo's own doc at :535-538 says is theVCPU/MEMORY/LOCAL_SSDenum, so every existing commitment reportsResourceType: "VCPU". Recommendations setResourceTypeto a machine type viaextractResourceTypeFromRecommendation(:1197-1222), so no existing commitment can ever match a recommendation by resource type and dedupe/coverage see zero overlap, re-recommending capacity the project already owns.commitment.Type(e.g.GENERAL_PURPOSE_N2), the field that does identify the covered family, is never read in the converter. If this issue lands a recommender-suppliedType, the same value should be carried here as the inverse ofmachineFamilyCommitmentType. Audit finding A08b-029.