-
Notifications
You must be signed in to change notification settings - Fork 525
[GEP-28] Backup configuration, deployment of Backup{Bucket,Entry}
for etcd managed by etcd-druid
#12123
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
Conversation
/assign |
/retest |
98e39e6
to
23f02f2
Compare
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.
Thanks for this nicely-structured extension to prepare autonomous shoot clusters for real world scenarios with backups.
The annotation is immediately removed by `gardener-apiserver` via https://github.com/gardener/gardener/blob/df16d9b9a5b4603fe5f6f495bad260fc4f892688/pkg/apiserver/registry/core/backupbucket/strategy.go#L67-L70, so it doesn't make sense to check whether it is present or not. Let's remove this.
- the first parameter should be the control plane namespace - for regular shoots, the technical ID and the control plane namespace are equal, i.e., it doesn't make a difference - for autonomous shoots, they are not equal, and we actually want to use the control plane namespace here.
- earlier, the secret was just always put into the `garden` namespace (undocumented contract) - for autonomous shoots, this doesn't work since there is no `garden` namespace - now, the `BackupBucket` controller maintains an annotation for the extension controller to know in which namespace the generated secret should be put
- we need the UID to be able to create a `BackupEntry` (the UID is used there)
We will reuse this in the `Shoot` API, and there is no need for having the `Seed` prefix generally.
In the `controlPlane` section of the worker pool supposed to run the control plane components, it is now possible to configure the backup settings. For regular shoots, this does not work since the configuration is part of the `SeedSpec`.
for autonomous clusters, otherwise continue to use backup config from `Seed`
- configure backup in example `Shoot` manifest - define dummy backup secret - mount backup bucket directory into machine pods
- similar to extension resources
- improves readability
Similar to how `core.gardener.cloud/v1beta1.BackupEntry`s are handled, the logic is now put into a component package. Note for reviewers: It does not make much sense to write unit tests for `health.CheckBackup{Bucket,Entry}` as these functions mainly call `checkExtensionObject` which is already covered by the unit test of `CheckExtensionObject`.
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
/approve
LGTM label has been added. Git tree hash: 8ecf47bbc7eb453ea5c083bd9b48c43acd225570
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ScheererJ The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
How to categorize this PR?
/area ipcei
/kind enhancement
What this PR does / why we need it:
This PR augments the
gardenadm init
flow with the deployment of thecore.gardener.cloud/v1beta1.Backup{Bucket,Entry}
resources. This results in a backup bucket + folder that can later be used by theetcd-backup-restore
sidecar onceetcd-druid
manages theetcd
instances. Like in the regular Gardener setup withmake gardener-up
, the local backup bucket is a directory in./dev/local-backupbuckets
.Which issue(s) this PR fixes:
Part of #2906
Special notes for your reviewer:
/cc @ScheererJ @timebertt
Release note: