Skip to content

fix(gcp): read the CUD commitment Type from the recommender payload instead of deriving it from the machine family #1663

Description

@cristim

Context

PR #1650 (issue #1538) fixed GCP CUDs being purchased as GENERAL_PURPOSE
regardless of the recommended machine family. It derives the commitment Type
from the machine-family segment of rec.ResourceType via a hand-maintained
machineFamilyCommitmentType map in
providers/gcp/services/computeengine/client.go.

That map is a derivation, and therefore a guess that has to be kept in sync
with GCP:

  • New families need a map entry or they hit the fail-loud path and their
    purchases are refused (already true today for m4, x4, t2a).
  • Families whose commitment type splits into size buckets not encoded in the
    machine-type name (MEMORY_OPTIMIZED_M4 vs _M4_6TB; the seven
    MEMORY_OPTIMIZED_X4_* buckets) are not derivable at all from the machine
    type, 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 type field. GCP is telling us which
Commitment_Type to buy; we do not have to infer it.

Read the commitment type from the recommender operation value (the operation
whose path targets the commitment resource root), validate it against
computepb.Commitment_Type_value so an unknown member fails loud rather than
being 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 #1650

buildInsertRequest receives a common.Recommendation, not the raw
recommenderpb.Recommendation. Carrying the commitment type from the converter
to the purchase path requires a new field on pkg/common.Recommendation (or on
common.ComputeDetails), which lives in the separate pkg Go module shared
by 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

  • The purchased Type comes from the recommender payload when present.
  • An unrecognized type value fails loud (no fallback to a guessed family type,
    no bare string literal on the wire).
  • The machine-family map remains as the documented fallback.
  • Regression coverage asserts on the Type carried by the
    InsertRegionCommitmentRequest handed to the SDK client, not on an
    intermediate struct.
  • At least one M4 or X4 payload, currently refused, becomes purchasable.

Related: #1538, #1650, #1256, #1317.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions