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 scheduleReconciler log info #6524

Merged

Conversation

yanggangtony
Copy link
Contributor

@yanggangtony yanggangtony commented Jul 19, 2023

Thank you for contributing to Velero!

Please add a summary of your change

The log info is wrong , we should move it to the above code .
/kind cleanup

Does your change fix a particular issue?

Fixes #(issue)

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@yanggangtony
Copy link
Contributor Author

/kind changelog-not-required

@github-actions github-actions bot added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Jul 19, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2023

Codecov Report

Merging #6524 (c5af315) into main (d2b5e90) will decrease coverage by 0.03%.
The diff coverage is 7.69%.

@@            Coverage Diff             @@
##             main    #6524      +/-   ##
==========================================
- Coverage   60.28%   60.25%   -0.03%     
==========================================
  Files         238      238              
  Lines       25256    25268      +12     
==========================================
  Hits        15226    15226              
- Misses       8976     8988      +12     
  Partials     1054     1054              
Impacted Files Coverage Δ
pkg/cmd/cli/nodeagent/server.go 9.03% <0.00%> (-0.37%) ⬇️
pkg/controller/schedule_controller.go 72.54% <100.00%> (ø)

@@ -199,11 +199,10 @@ func (c *scheduleReconciler) checkIfBackupInNewOrProgress(schedule *velerov1.Sch

for _, backup := range backupList.Items {
if backup.Status.Phase == velerov1.BackupPhaseNew || backup.Status.Phase == velerov1.BackupPhaseInProgress {
log.Debugf("Schedule %s/%s still has backups are in InProgress or New state, skip submitting backup to avoid overlap.", schedule.Namespace, schedule.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT should it be "... still has backups that are in InProgress..." or "... still has backups in InProgress..."?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the quick review.
I am not a english speaker.
Your suggestion is make sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rebased.

@@ -199,11 +199,10 @@ func (c *scheduleReconciler) checkIfBackupInNewOrProgress(schedule *velerov1.Sch

for _, backup := range backupList.Items {
if backup.Status.Phase == velerov1.BackupPhaseNew || backup.Status.Phase == velerov1.BackupPhaseInProgress {
log.Debugf("%s/%s still has backups that are in InProgress...", schedule.Namespace, schedule.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Change it to in InProgress or New? That's confuse if only contains the InProgress state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rebased, Thanks review.

@shubham-pampattiwar shubham-pampattiwar merged commit eb35f12 into vmware-tanzu:main Jul 24, 2023
22 checks passed
@yanggangtony
Copy link
Contributor Author

Thanks

@yanggangtony yanggangtony deleted the fix-log-scheduler branch July 24, 2023 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants