-
Notifications
You must be signed in to change notification settings - Fork 190
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
K8SPXC-1509: Fix reconciler error when PiTR enabled #1879
base: main
Are you sure you want to change the base?
Conversation
switch res { | ||
case controllerutil.OperationResultCreated: | ||
log.Info("Created binlog collector", "name", binlogCollector.Name) | ||
case controllerutil.OperationResultUpdated: | ||
log.Info("Updated binlog collector", "name", binlogCollector.Name) | ||
} |
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.
Why don't we log res
regardless of its status and we are switching only for these 2 cases? The status has an underlying type of string, which already has the information we want.
if err := r.reconcileBinlogCollector(ctx, cr); err != nil { | ||
return errors.Wrap(err, "reconcile binlog collector") |
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.
Discussed with @egegunes that if can also add a unit test for the controller if we have some time
commit: 571c7b9 |
CHANGE DESCRIPTION
Fix temporary
resource name may not be empty
error when PiTR enabled on a running cluster.CHECKLIST
Jira
Needs Doc
) and QA (Needs QA
)?Tests
compare/*-oc.yml
)?Config/Logging/Testability