-
Notifications
You must be signed in to change notification settings - Fork 93
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
Change the default capacity of volumes for Etcd
from 80Gi
to 25Gi
.
#933
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/hold |
@shreyas-s-rao |
@renormalize thanks for making the change. I would suggest that you simply check whether the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@renormalize thanks for making the changes. Overall looks good. Just a few changes required in the tests. PTAL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@renormalize thanks for making the requested changes.
/lgtm
I've tested the changes on an extension-provider setup through @shreyas-s-rao's guidance. I've done the following to test the changes:
Thus:
/ping @gardener/gardener-extension-provider-aws-maintainers |
@gardener/gardener-extension-provider-aws-maintainers ℹ️ please take some time to help renormalize or redirect to someone else if you can't. |
/ping @gardener/gardener-extension-provider-aws-maintainers Could you take a look, since this PR directly affects storage costs? |
@gardener/gardener-extension-provider-aws-maintainers ℹ️ please take some time to help renormalize or redirect to someone else if you can't. |
* Change the default capacity from `80Gi` to `25Gi` in `charts/gardener-extension-provider-aws/values.yaml`. * Change the default capacity from `80Gi` to `25Gi` in `example/00-componentconfig.yaml`. * Change tests to check if `*etcd.Spec.StorageCapacity` equals `25Gi` instead of `80Gi`. * Run `make generate` to generate the new chart in `example/controller-registration.yaml`.
…ed on reconcile. * Add logic to `EnsureETCD()` to check if the old Etcd has a `80Gi` volume. If it does, the volumes are not resized. If it does not, regular reconiliation continues and whatever capacity specified by the ensurer is set. * Adapted tests to handle this case.
* Test spec names in `pkg/webhook/controlplaneexposure/ensurer_test.go` changed according to the suggestions in the pull request. * Introduce `checkOldETCDMain()` and `checkNewETCDMain()` in `pkg/webhook/controlplaneexposure/ensurer_test.go`. * Enhance `checkETCDEvents()` in `pkg/webhook/controlplaneexposure/ensurer_test.go` by adding a storage capacity check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
How to categorize this PR?
/area cost
/area storage
/kind technical-debt
/platform aws
What this PR does / why we need it:
This PR changes the capacity for volumes for
Etcd
s from 80Gi to 25Gi.Change the default capacity from
80Gi
to25Gi
incharts/gardener-extension-provider-aws/values.yaml
.Change the default capacity from
80Gi
to25Gi
inexample/00-componentconfig.yaml
.Change tests to check if
*etcd.Spec.StorageCapacity
equals25Gi
instead of80Gi
.Run
make generate
to generate the new chart inexample/controller-registration.yaml
.Volumes of size 80Gi are unnecessary since moving from
gp2
togp3
. See #932 for more.Which issue(s) this PR fixes:
Fixes #932
Special notes for your reviewer:
/invite @shreyas-s-rao
Release note: