Remove schedule-related metrics on schedule delete

[nilesh-akhade]

Thank you for contributing to Velero!

Summary of the change

Currently, when a schedule is deleted, the associated metrics are still exported, which leads to inconsistencies in the metrics data. With this PR, we ensure that when a schedule is deleted, the related metrics are correctly removed or no longer exported. This enhances the accuracy and consistency of the metrics data.

Example

If schedule myschedule exists with one successful backup and if we access the /metrics endpoint on the Velero server.

velero_backup_success_total{schedule="myschedule"} 1
# .. other metrics created for this schedule

After we delete the schedule myschedule, the above line is removed(not exported).

Does your change fix a particular issue?

Fixes #1333

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.

[kaovilai]

Do we want to consider the case where deletionTimestamp is not nil (user has initiated deletion request) but object is not yet not found (gone from cluster)?

[nilesh-akhade]

The scenario you described is when the metadata.deletionTimestamp is not nil, indicating that a user has initiated a deletion request for the schedule, but the schedule is not yet deleted.
Do you mean we should move the function call for removing schedule-related metrics in the if-block, as shown below?

if schedule.ObjectMeta.DeletionTimestamp != nil {
  log.Debug("Got a deletion request for the schedule")
  c.metrics.RemoveSchedule(req.Name)
}

[kaovilai]

That would remove metrics even if schedule is stuck/remain in cluster due to finalizers etc. but yes just questioning if that would be preferred or not?

[nilesh-akhade]

If the deletion of the schedule gets stuck, the metrics will still be exported. This makes sense because the schedule is not yet deleted. Additionally, the removal of metrics is quick and should never fail. Hence, I believe that removal in the reconciliation cycle after deletion should be preferred.

[kaovilai]

+1

[codecov]

Codecov Report

Merging #6715 (c7c4413) into main (f234dd6) will increase coverage by 0.07%.
Report is 168 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #6715      +/-   ##
==========================================
+ Coverage   60.28%   60.36%   +0.07%     
==========================================
  Files         238      242       +4     
  Lines       25256    25983     +727     
==========================================
+ Hits        15226    15684     +458     
- Misses       8976     9196     +220     
- Partials     1054     1103      +49     

Files Changed	Coverage Δ
pkg/controller/schedule_controller.go	72.72% <100.00%> (+0.17%)	⬆️

... and 41 files with indirect coverage changes

[shubham-pampattiwar]

Thank you @nilesh-akhade