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

[Feature] Improve logging or condition messages when etcd-druid is not able to determine etcd backup status #732

Closed
plkokanov opened this issue Dec 7, 2023 · 1 comment
Labels
kind/enhancement Enhancement, improvement, extension status/closed Issue is closed (either delivered or triaged)

Comments

@plkokanov
Copy link
Contributor

Feature (What you would like to be added):
More details when etcd-druid is not able to determine an etcd's backup status should be reported either in the logs or in the conddtion.

Motivation (Why is this needed?):
Currently, the following could run into an error when trying to retrieve the full snapshot lease or the delta snapshot lease:

fullSnapErr = a.cl.Get(ctx, types.NamespacedName{Name: getFullSnapLeaseName(&etcd), Namespace: etcd.Namespace}, fullSnapLease)
deltaSnapErr = a.cl.Get(ctx, types.NamespacedName{Name: getDeltaSnapLeaseName(&etcd), Namespace: etcd.Namespace}, deltaSnapLease)
// Set status to Unknown if errors in fetching snapshot leases or lease never renewed
if fullSnapErr != nil || deltaSnapErr != nil || (fullSnapLease.Spec.RenewTime == nil && deltaSnapLease.Spec.RenewTime == nil) {
return result
}

However, the errors are not logged anywhere and no description is added to the BackupReady condition making the determining the reason for the Unkown status of the condition a bit hard to determine.
There are other cases where the condition could be Unknown and the only time the reason for that is logged is here:
logger.Error(err, "unable to compute full snapshot duration from full snapshot schedule", "fullSnapshotSchedule", *etcd.Spec.Backup.FullSnapshotSchedule)

I think that it would be useful to either log the reason for the Unknown condition in all cases or add it to the message of the BackupReady condition.

Approach/Hint to the implement solution (optional):

@plkokanov plkokanov added the kind/enhancement Enhancement, improvement, extension label Dec 7, 2023
@plkokanov plkokanov changed the title [Feature] Better log or condition messages when etcd-druid is not able to determine etcd backup status [Feature] Improve logging or condition messages when etcd-druid is not able to determine etcd backup status Dec 7, 2023
@plkokanov
Copy link
Contributor Author

Closing this as we agreed with @shreyas-s-rao that it will be tackled as part of #618 and #645

@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Enhancement, improvement, extension status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

No branches or pull requests

2 participants