feat: add last refresh status for application
Fixes #20591
Checklist:
- [x] Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
- [x] The title of the PR states what changed and the related issues number (used for the release note).
- [x] The title of the PR conforms to the Toolchain Guide
- [x] I've included "Fixes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
- [x] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
- [ ] Does this PR require documentation updates?
- [ ] I've updated documentation as required by this PR.
- [x] I have signed off all my commits as required by DCO
- [x] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
- [x] My build is green (troubleshooting builds).
- [x] My new feature complies with the feature status guidelines.
- [x] I have added a brief description of why this PR is necessary and/or what this PR solves.
- [x] Optional. My organization is added to USERS.md.
- [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).
:x: Preview Environment undeployed from Bunnyshell
Available commands (reply to this comment):
- :rocket:
/bns:deployto deploy the environment
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Please upload report for BASE (
master@0484f9f). Learn more about missing BASE report. Report is 155 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #23116 +/- ##
=========================================
Coverage ? 60.03%
=========================================
Files ? 343
Lines ? 57901
Branches ? 0
=========================================
Hits ? 34760
Misses ? 20358
Partials ? 2783
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
We should design this feature very carefully. A lot of refreshes are no-ops. Updating a CR field for refreshes could quickly result in a lot of meaningless updates.
We use four different "comparison levels" when reconciling:
const (
// Compare live application state against state defined in latest git revision with no resolved revision caching.
CompareWithLatestForceResolve CompareWith = 3
// Compare live application state against state defined in latest git revision.
CompareWithLatest CompareWith = 2
// Compare live application state against state defined using revision of most recent comparison.
CompareWithRecent CompareWith = 1
// Skip comparison and only refresh application resources tree
ComparisonWithNothing CompareWith = 0
)
Level 0 happens a lot (potentially many times a second). We shouldn't update a field in the CR every time we update the resource tree.
We should design this feature very carefully. A lot of refreshes are no-ops. Updating a CR field for refreshes could quickly result in a lot of meaningless updates.
We use four different "comparison levels" when reconciling:
const ( // Compare live application state against state defined in latest git revision with no resolved revision caching. CompareWithLatestForceResolve CompareWith = 3 // Compare live application state against state defined in latest git revision. CompareWithLatest CompareWith = 2 // Compare live application state against state defined using revision of most recent comparison. CompareWithRecent CompareWith = 1 // Skip comparison and only refresh application resources tree ComparisonWithNothing CompareWith = 0 )Level 0 happens a lot (potentially many times a second). We shouldn't update a field in the CR every time we update the resource tree. Thank you for your review. You raised a very good point about the frequency of refresh status updates. I completely agree that we need to carefully design this feature to avoid unnecessary CR field updates.
I suggest the following modifications:
-
Update refresh status only in these cases:
- When executing CompareWithLatestForceResolve (level 3)
- When executing CompareWithLatest (level 2)
-
For ComparisonWithNothing (level 0) and CompareWithRecent (level 1) refresh operations, we should not update the status field as these operations are very frequent and typically don't result in actual changes.
This design will:
- Avoid frequent meaningless updates
- Only record refresh status when truly needed
- Maintain system performance
What do you think about this approach? We can further adjust the design based on your feedback.
@yafeiaa, please attach a screenshot showing this indicator in action.
@yafeiaa, please attach a screenshot showing this indicator in action.
done