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

check for completed backups and update compliant messages (https://issues.redhat.com/browse/ACM-14460) #502

Merged

Conversation

birsanv
Copy link
Contributor

@birsanv birsanv commented Sep 25, 2024

https://issues.redhat.com/browse/ACM-14460

Changes:

  • Updated the check-backup-completed policy template to look for any backup in completed state, not only for the latest backup.
  • The other templates are still validating the latest backup phase ( will show violation if the Backup matching the Schedule.status.lastBackup has a status of Error, FailedValidation or '')
  • Updated compliance messages with a more readable content, see attached image
    The reason for the change : sometimes - and it seems that it usually happens first time the Schedule gets created but cannot always be reproduced - the Schedule.status.lastBackup timestamp if off and does not match the generated Backup startTimestamp.
    Because of this we show a violation when we check if the latest backup is completed. The violation is because the Backup was not found based on the Schedule.status.lastBackup timestamp rule, but the Backup exists and is Completed.
    To avoid this false positive, I am just checking if there is a Backup generated by this Schedule, with a status of Completed. This check will show compliant though even if the Completed backup was generated long ago. This should be ok though because we have other templates checking that the latest backup must not have a status of ( Error, FailedValidation or '' ). So if those are compliant, the latest backup should be completed
Screenshot 2024-09-25 at 11 17 12 AM

@birsanv
Copy link
Contributor Author

birsanv commented Sep 25, 2024

/cc @JustinKuli

@birsanv
Copy link
Contributor Author

birsanv commented Sep 25, 2024

/cc @sahare

Copy link

openshift-ci bot commented Sep 25, 2024

@birsanv: GitHub didn't allow me to request PR reviews from the following users: sahare.

Note that only open-cluster-management-io members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @sahare

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link
Contributor

@sahare sahare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, Thanks!

Copy link

openshift-ci bot commented Sep 25, 2024

@sahare: changing LGTM is restricted to collaborators

In response to this:

Looks good, Thanks!

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@birsanv
Copy link
Contributor Author

birsanv commented Sep 25, 2024

/cc @mprahl

@openshift-ci openshift-ci bot requested a review from mprahl September 25, 2024 21:39
Copy link

openshift-ci bot commented Sep 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: birsanv, gparvin, sahare

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

@openshift-merge-bot openshift-merge-bot bot merged commit be0ebf2 into open-cluster-management-io:main Sep 26, 2024
3 checks passed
@JustinKuli
Copy link
Member

Very clever use of hub-templates to get lookup calls in the customMessage fields!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants