Skip to content

Conversation

rfranzke
Copy link
Member

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 the core.gardener.cloud/v1beta1.Backup{Bucket,Entry} resources. This results in a backup bucket + folder that can later be used by the etcd-backup-restore sidecar once etcd-druid manages the etcd instances. Like in the regular Gardener setup with make 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:

`BackupBucket` extension controllers: Instead of always creating the `Secret` referenced in `.status.generatedSecretRef` in the `garden` namespace, the controller should read the annotation `backupbucket.extensions.gardener.cloud/generated-secret-ref-namespace` and use its value.

@gardener-prow gardener-prow bot requested review from ScheererJ and timebertt May 16, 2025 10:31
@gardener-prow gardener-prow bot added area/ipcei IPCEI (Important Project of Common European Interest) kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. labels May 16, 2025
@gardener-prow gardener-prow bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 16, 2025
@ScheererJ
Copy link
Member

/assign

@rfranzke
Copy link
Member Author

/retest

@rfranzke rfranzke force-pushed the gep28/backup2 branch 2 times, most recently from 98e39e6 to 23f02f2 Compare May 16, 2025 12:04
Copy link
Member

@ScheererJ ScheererJ left a 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.

rfranzke added 16 commits May 16, 2025 16:18
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 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`.
@rfranzke rfranzke requested a review from ScheererJ May 16, 2025 14:19
Copy link
Member

@ScheererJ ScheererJ left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label May 19, 2025
Copy link
Contributor

gardener-prow bot commented May 19, 2025

LGTM label has been added.

Git tree hash: 8ecf47bbc7eb453ea5c083bd9b48c43acd225570

Copy link
Contributor

gardener-prow bot commented May 19, 2025

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 19, 2025
@gardener-prow gardener-prow bot merged commit 7e4fae7 into gardener:master May 19, 2025
19 checks passed
@rfranzke rfranzke deleted the gep28/backup2 branch May 19, 2025 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/ipcei IPCEI (Important Project of Common European Interest) cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants