kubecost / cost-analyzer-helm-chart

Kubecost helm chart
http://kubecost.com/install
Apache License 2.0
458 stars 410 forks source link

[CI] Bump/add cluster versions #3510

Closed chipzoller closed 1 week ago

chipzoller commented 1 week ago

Signed-off-by: chipzoller chipzoller@gmail.com

What does this PR change?

Does this PR rely on any other PRs?

No

How does this PR impact users? (This is the kind of thing that goes in release notes!)

None, CI only.

Links to Issues or tickets this PR addresses or fixes

Closes #3493

What risks are associated with merging this PR? What is required to fully test this PR?

None, CI only.

How was this PR tested?

Will be tested manually after PR is merged. Testing of this change will not be reflected in the Actions run as part of this PR (known limitation).

Have you made an update to documentation? If so, please provide the corresponding PR.

N/A

chipzoller commented 1 week ago

An open question here: Should we add into the EKS values file the equivalent value to set gp2 as the StorageClass? Pros: it will "just work" for users of EKS 1.30+. Cons: it's a default change which may not work for users who have removed this StorageClass.

thomasvn commented 1 week ago

I like the idea of defaulting to a storageClass that "just works" if one is not already defined. As you've mentioned I think it can get quite complicated for existing users if they are already on a different storageClass. Maybe for now it's best to doc as a troubleshooting step?

chipzoller commented 1 week ago

I did at least document it here: https://docs.kubecost.com/install-and-configure/install/provider-installations/aws-eks-cost-monitoring#prerequisites-1

thomasvn commented 1 week ago

Perfect! I'd be open to attempting a gp2 default. My biggest concern would b that swapping storageClasses may be a destructive operation on some people's PVs? Not 100% sure.

chipzoller commented 1 week ago

Adding a default value sounds good to me. Does that work for you, @jessegoodier ?