Skip to content
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

Fix the backup.gardener.cloud/created-by annotation not being added on existing etcd-backup Secrets #9613

Merged

Conversation

ialidzhikov
Copy link
Member

@ialidzhikov ialidzhikov commented Apr 18, 2024

How to categorize this PR?

/area quality
/kind bug

What this PR does / why we need it:
In

etcdSecret := emptyEtcdBackupSecret(be.Name)
metav1.SetMetaDataAnnotation(&etcdSecret.ObjectMeta, AnnotationKeyCreatedByBackupEntry, be.Name)
_, err = controllerutils.GetAndCreateOrMergePatch(ctx, a.client, etcdSecret, func() error {
etcdSecret.Data = etcdSecretData
return nil
})
the corresponding annotation will be set for new etcd-backup Secret but it won't be if the Secret already exists in the system.

We also added logs around the etcd-backup Secret deletion for debugging purposes as we faced another occurrences of #8675.

Which issue(s) this PR fixes:
Follow-up on #8675

Special notes for your reviewer:
N/A

Release note:

extension library: An issue causing the `backup.gardener.cloud/created-by` annotation not being added on existing `etcd-backup` Secrets is now fixed.

Copy link
Contributor

gardener-prow bot commented Apr 18, 2024

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

@gardener-prow gardener-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/bug Bug size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 18, 2024
@gardener-prow gardener-prow bot added the cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. label Apr 18, 2024
@ialidzhikov ialidzhikov force-pushed the fix/etcd-backup-secret-annotation branch from a833af7 to bff8a2e Compare April 19, 2024 06:36
@ialidzhikov ialidzhikov marked this pull request as ready for review April 19, 2024 06:36
@gardener-prow gardener-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 19, 2024
@gardener-prow gardener-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 19, 2024
@ialidzhikov
Copy link
Member Author

/cc @Kostov6

@gardener-prow gardener-prow bot requested a review from Kostov6 April 19, 2024 06:38
@rfranzke
Copy link
Member

/lgtm
/approve
/retest

Do we need to cherry-pick this? 👀

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

gardener-prow bot commented Apr 19, 2024

LGTM label has been added.

Git tree hash: d2ee1a31e84031eb18c51ee61b67248fc582143e

Copy link
Contributor

gardener-prow bot commented Apr 19, 2024

[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:

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 Apr 19, 2024
@gardener-prow gardener-prow bot merged commit c910a5a into gardener:master Apr 19, 2024
17 checks passed
@ialidzhikov
Copy link
Member Author

Do we need to cherry-pick this? 👀

IMO, it is not needed.
Context: we looked into a new occurrence of #8675. It was a new Shoot creation failed due to a missing etcd-backup Secret. On theory, the fix from #8709 should be good enough to prevent new occurrences because the annotation is added a new etcd-backup Secrets. But we were not able to find how it happened.
While looking into it, we noticed that the annotation is not added on existing etcd-backup Secrets (created prior to #8709). We also added logs for the etcd-backup Secret deletion so that we have more info if/when it happens again. It was only one occurrence of #8675 in the last 5months so it is not that pressing IMO.

@ialidzhikov ialidzhikov deleted the fix/etcd-backup-secret-annotation branch April 19, 2024 09:21
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/quality Output qualification (tests, checks, scans, automation in general, etc.) related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/bug Bug lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants