Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gke): Handle ARM instance families #341

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Conversation

Pokom
Copy link
Contributor

@Pokom Pokom commented Oct 30, 2024

Now that GCP has publicly unveiled the C4A family, let's update the pricing map to handle both ARM and AMD chipsets.

Now that GCP has publicly unveiled the C4A family, let's update the
pricing map to handle both ARM and AMD chipsets.

- refs #337
@Pokom Pokom requested a review from a team as a code owner October 30, 2024 20:06
@@ -208,7 +208,9 @@ func (pm *PricingMap) Populate(ctx context.Context, billingService *billingv1.Cl

for _, sku := range skus {
rawData, err := getDataFromSku(sku)

if strings.Contains(sku.Description, "C4A") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove debugging line

},
},
// TODO: ADd spot
// Spot Preemptible C4A Arm Instance Ram running in Netherlands
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy+paste comment "Spot Preemptible C4A ..."?

Copy link
Member

@the-it the-it left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

legit ... save to rename the amd attribute, as it wasn't used

@Pokom Pokom merged commit 1bee25b into main Oct 31, 2024
2 checks passed
@Pokom Pokom deleted the feat/gke-arm-family-instances branch October 31, 2024 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants