[gardenlet] Switch BackupEntry controller to controller-runtime

[shafeeqes]

How to categorize this PR?

/area dev-productivity scalability
/kind enhancement

What this PR does / why we need it:
Refactor the BackupEntry controller to controller-runtime.

Which issue(s) this PR fixes:
Part of #4251

Special notes for your reviewer:
Generally, we want to follow this cookbook while refactoring existing controllers:

Add documentation
Add integration test based on envtest (if not already present)
Switch controller to controller-runtime

Depends on #6837, Hence in draft state.
Also I will eliminate the wait for extension in favour of adding watches, once the approach in BackupBucket controller is reviewed and merged.

Release note:

NONE

[gardener-prow]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[rfranzke]

Only nits

[rfranzke]

/lgtm
/approve

[gardener-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rfranzke

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:

• OWNERS [rfranzke]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rfranzke]

/lgtm

[shafeeqes]

@shafeeqes: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-gardener-e2e-kind-migration b49c65e link true /test pull-gardener-e2e-kind-migration
Full PR test history. Your PR dashboard. Command help for this repository. Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

Looks like the Cluster resource is gone before the backupEntry is deleted,

2022-11-23T07:30:07.590367904Z stderr F {"level":"info","ts":"2022-11-23T07:30:07.590Z","msg":"Extension BackupEntry not yet deleted","controller":"backupentry","object":{"name":"shoot--local--e2e-migrate--7afb4ec6-3318-49a4-9562-ee070f464933","namespace":"garden-local"},"namespace":"garden-local","name":"shoot--local--e2e-migrate--7afb4ec6-3318-49a4-9562-ee070f464933","reconcileID":"6e61b81b-661a-43f1-8384-46ca59244fd2","extensionBackupEntry":{"name":"shoot--local--e2e-migrate--7afb4ec6-3318-49a4-9562-ee070f464933"}}
2022-11-23T07:30:07.603772924Z stderr F {"level":"error","ts":"2022-11-23T07:30:07.603Z","msg":"Failed to get shoot from cluster","controller":"backupentry","shootTechnicalID":"shoot--local--e2e-migrate","error":"Cluster.extensions.gardener.cloud \"shoot--local--e2e-migrate\" not found","stacktrace":"github.com/gardener/gardener/pkg/gardenlet/controller/backupentry/backupentry.(*Reconciler).MapExtensionBackupEntryToCoreBackupEntry

Fixing it.
Update: This is just an expected log, real issue was #6926 (comment).

[shafeeqes]

Had a combined look with @plkokanov,
We found that this is caused because of a race condition.

The BackupEntry is deleted if the shoot is deleted, and in this deletion flow, it checks for a successfully reconciled BackupBucket. If the BackupBucket is deleted in this time, the BackupBucket status is updated to Processing, and only after this it checks if there are any associated BackupEntrys.
This causes a deadlock, because the BackupEntry needs a Succeeded BackupBucket to proceed with deletion and the BackupBucket requires all the BackupEntrys to be gone.
This happened because just after the Shoot deletion test is successful, we delete the Seed in the script, and the seed deletion flow deletes the BackupBucket.
6367d91 moves the check for associated BackupEntrys before accepting deletion, so that we don't set the BackupBucket status to Processing.

Also, if there is a deletion graceperiod, the BackupEntry will not be deleted even if the Shoot is gone, In this case, the cluster resource won't be present anymore. So we shouldn't try to fetch the cluster in that case. Anyway, for deletion of BackupEntry extension, we have a RequeueAfter, so we don't need to map the extension to core BackupEntry if there is a deletionTimestamp.

[rfranzke]

Awesome @shafeeqes and @plkokanov, this sounds very reasonable. Nice finding and thanks for fixing it! 👏🏻
/lgtm