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.

Sign up for GitHub

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

Jump to bottom

[ENG-2729] Savings Recommendations: Node group right-sizing & allow lists #1123

Merged
merged 13 commits into from
Sep 17, 2024
Merged

[ENG-2729] Savings Recommendations: Node group right-sizing & allow lists #1123

merged 13 commits into from
Sep 17, 2024

Conversation

biancaburtoiu
Copy link
Contributor

@biancaburtoiu biancaburtoiu commented Sep 4, 2024 •
edited
Loading

Related Issue

https://kubecost.atlassian.net/browse/ENG-2729

Proposed Changes

This PR extends the Cluster Right-Sizing Recommendations to include notes on the usage of the new node group right-sizing API, as well as a segment on support for instance type allow lists.

@biancaburtoiu biancaburtoiu changed the title [ENG-2729] Savings Recommendations: Allow lists [ENG-2729] Savings Recommendations: Node group right-sizing & allow lists Sep 4, 2024
@biancaburtoiu
Copy link
Contributor Author

@kwombach12 I have extended the public documentation on cluster right-sizing to include notes on the usage of the new node group right-sizing API. I've also added another segment on support for instance type allow lists. Please let me know your thoughts!

@biancaburtoiu biancaburtoiu marked this pull request as ready for review September 4, 2024 14:43
@biancaburtoiu biancaburtoiu requested a review from a team as a code owner September 4, 2024 14:43
@srpomeroy
Copy link
Contributor

@biancaburtoiu What version are these features targeting?
Can you please add the appropriate label? Thanks!

kwombach12
kwombach12 previously requested changes Sep 4, 2024
View reviewed changes
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
biancaburtoiu
biancaburtoiu commented Sep 4, 2024
View reviewed changes
Copy link
Contributor Author

@biancaburtoiu biancaburtoiu left a comment

Choose a reason for hiding this comment

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

Rewriting Kai's suggestions as suggested changes, to be able to batch commit.

using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
using-kubecost/navigating-the-kubecost-ui/savings/cluster-right-sizing-recommendations.md Outdated Show resolved Hide resolved
@kwombach12
Copy link
Contributor

Rewriting Kai's suggestions as suggested changes, to be able to batch commit.

ok wow really sorry to make extra work for you. I would love to learn how I can make these suggestions in the future so its easier to accept!

@srpomeroy
Copy link
Contributor

Rewriting Kai's suggestions as suggested changes, to be able to batch commit.

ok wow really sorry to make extra work for you. I would love to learn how I can make these suggestions in the future so its easier to accept!

It's super handy! https://haacked.com/archive/2019/06/03/suggested-changes/

@biancaburtoiu biancaburtoiu mentioned this pull request Sep 6, 2024
Merged
@biancaburtoiu biancaburtoiu mentioned this pull request Sep 10, 2024
Closed
@biancaburtoiu
Copy link
Contributor Author

Note for reviewers: This PR is ready, but should only be merged once v2.4 has been publicly released.
Reason: This feature is not available in versions < v2.4 and the way this repository is currently set up, the contents of every merged PR are immediately released on docs.kubecost.com.

Please note that the cost-analyzer-helm-chart references have already been merged and cherry-picked into v2.4.

@biancaburtoiu biancaburtoiu dismissed kwombach12’s stale review September 10, 2024 13:16

Changes already applied

kwombach12
kwombach12 previously approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@kwombach12 kwombach12 left a comment

Choose a reason for hiding this comment

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

LGTM. Great work @biancaburtoiu

@biancaburtoiu biancaburtoiu dismissed chipzoller’s stale review September 17, 2024 17:39

Changes already applied

@biancaburtoiu
Copy link
Contributor Author

@chipzoller @kwombach12 I'm missing an approval here :)

@chipzoller chipzoller merged commit 02e099e into main Sep 17, 2024
5 checks passed
@chipzoller chipzoller deleted the biancaburtoiu/eng-2729 branch September 17, 2024 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chipzoller chipzoller chipzoller approved these changes

@kwombach12 kwombach12 kwombach12 left review comments

@thomasvn thomasvn Awaiting requested review from thomasvn

@srpomeroy srpomeroy Awaiting requested review from srpomeroy

Assignees
No one assigned
Labels
2.4 next release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@biancaburtoiu @srpomeroy @kwombach12 @thomasvn @chipzoller