-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Mark BackupRepository not ready when BSL changed #8975
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8975 +/- ##
==========================================
+ Coverage 59.57% 59.61% +0.04%
==========================================
Files 370 370
Lines 40278 40257 -21
==========================================
+ Hits 23995 24000 +5
+ Misses 14780 14754 -26
Partials 1503 1503 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
||
// When BSL is created, invalidate any backup repositories that reference it | ||
func (b bslPredicate) Create(event.CreateEvent) bool { | ||
return true |
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.
Should return false
for create, the creation of backupRepo is handled by another workflow, not by repo controller or on BSL creation.
return requests | ||
} | ||
|
||
type bslPredicate struct { |
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.
Let's try to reuse a create a generic predicate which could be shared with others.
for i := range list.Items { | ||
r.logger.WithField("BSL", bsl.Name).Infof("Invalidating Backup Repository %s", list.Items[i].Name) | ||
if err := r.patchBackupRepository(context.Background(), &list.Items[i], repoNotReady("re-establish on BSL change or create")); err != nil { | ||
if err := r.patchBackupRepository(context.Background(), &list.Items[i], repoNotReady("re-establish on BSL change, create or delete")); err != nil { |
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.
According to #8975 (comment), let's remove create
from the message.
for i := range list.Items { | ||
r.logger.WithField("BSL", bsl.Name).Infof("Invalidating Backup Repository %s", list.Items[i].Name) | ||
if err := r.patchBackupRepository(context.Background(), &list.Items[i], repoNotReady("re-establish on BSL change or create")); err != nil { | ||
if err := r.patchBackupRepository(context.Background(), &list.Items[i], repoNotReady("re-establish on BSL change, create or delete")); err != nil { |
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.
It is better that we can make a clear message for different actions, as this message is important for troubleshooting.
Mark BackupRepository not ready when BSL changed Fixes vmware-tanzu#8860 Signed-off-by: Wenkai Yin(尹文开) <yinw@vmware.com>
Mark BackupRepository not ready when BSL changed
Fixes #8860
Thank you for contributing to Velero!
Please add a summary of your change
Does your change fix a particular issue?
Fixes #(issue)
Please indicate you've done the following:
make new-changelog
) or comment/kind changelog-not-required
on this PR.site/content/docs/main
.