-
Notifications
You must be signed in to change notification settings - Fork 885
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
support auto delete WorkloadRebalancer when time up #4894
Conversation
9b1e1a5
to
b016c25
Compare
4606fb1
to
7b02e4e
Compare
55d9849
to
5ca7500
Compare
/hold cancel |
5ca7500
to
1d5ecba
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #4894 +/- ##
=======================================
Coverage 53.34% 53.34%
=======================================
Files 252 252
Lines 20481 20531 +50
=======================================
+ Hits 10926 10953 +27
- Misses 8834 8853 +19
- Partials 721 725 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
47b6e50
to
28b3332
Compare
CC @RainbowMango can this begin to review? |
/assign By the way.
|
pkg/controllers/workloadrebalancer/workloadrebalancer_controller.go
Outdated
Show resolved
Hide resolved
b1dce42
to
c2f38ca
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango 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 |
/hold |
@XiShanYongYe-Chang not only e2e, but also ut |
e405822
to
f5076cd
Compare
@RainbowMango sorry, I did a updation after you send lgtm: https://github.com/karmada-io/karmada/compare/e405822231187e59ef62505a349aea75b8411ed2..f5076cdbeeb796d7df6426beb7ad98a1e790ceac |
Ok~ |
pkg/controllers/workloadrebalancer/workloadrebalancer_controller.go
Outdated
Show resolved
Hide resolved
pkg/controllers/workloadrebalancer/workloadrebalancer_controller.go
Outdated
Show resolved
Hide resolved
f5076cd
to
bbfa542
Compare
pkg/controllers/workloadrebalancer/workloadrebalancer_controller.go
Outdated
Show resolved
Hide resolved
67968da
to
eb84176
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks~
Generally OK.
262b69c
to
015a571
Compare
It seems related, see the logs here.
|
015a571
to
129c42e
Compare
Signed-off-by: chaosi-zju <[email protected]>
129c42e
to
a8b4050
Compare
CI problem fixed, is there any further comments? |
/lgtm |
/hold cancel |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Support auto delete WorkloadRebalancer when time up:
referring to Automatic Cleanup for Finished Jobs.
Introduces field
ttlSecondsAfterFinished
which limits the lifetime of a WorkloadRebalancer that has finished execution(finished execution means each target workload is finished with result of
Successful
orFailed
).ttlSecondsAfterFinished
after the WorkloadRebalancer finishes, it is eligible to be automatically deleted.Considering several corner cases:
WorkloadRebalancer
beforettlSecondsAfterFinished
expired,which means the finish time of the
WorkloadRebalancer
is refreshed, so thedelete
action is deferred since expire time is refreshed too.ttlSecondsAfterFinished
is modified beforettlSecondsAfterFinished
expired,delete
action should be performed according to latestttlSecondsAfterFinished
.WorkloadRebalancer
object and try to delete it,if a modification to
WorkloadRebalancer
occurred just right between the two time point, the previousdelete
action should be Interrupted.Several key implementation:
WorkloadRebalancer
is judged as finished should meet two requirements:Successful
orFailed
.ObservedGeneration
toStatus
of WorkloadRebalancer, and it should be equal to.metadata.Generation
,to prevent that the WorkloadRebalancer is updated but controller hasn't in time refreshed its
Status
.WorkloadRebalancer
isCreated
orUpdated
, add it to the workqueue and calculate its expiring time, andcall
workqueue.AddAfter()
function to re-enqueue it once more if it hasn't expired.WorkloadRebalancer
, do a final sanity check. Use the latestWorkloadRebalancer
directlyfetched from api server to see if the TTL truly expires, rather than object from lister cache.
WorkloadRebalancer
, it is needed to confirm that theresourceVersion
of the deleted object is as expected,to prevent from above corner case 3.
Which issue(s) this PR fixes:
Fixes part of #4840
Special notes for your reviewer:
Does this PR introduce a user-facing change?: