karmada
karmada copied to clipboard
karmada-io
reduce useless logic to improve dependencies distributor
What type of PR is this? /kind feature
What this PR does / why we need it: The dependent distributor does not pay attention to deleted or deleting attached resource templates, so optimize this part of the logic.
Which issue(s) this PR fixes: Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
NONE
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by:
To complete the pull request process, please assign kevin-wangzefeng after the PR has been reviewed.
You can assign the PR to them by writing /assign @kevin-wangzefeng in a comment when ready.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
I don't quite understand why dependencies distributor doesn't need to be concerned about deletion events. When the attached resource template is deleted, the corresponding rb should also need to be cleaned up.
I don't quite understand why dependencies distributor doesn't need to be concerned about deletion events. When the attached resource template is deleted, the corresponding rb should also need to be cleaned up.
Since the attached resource template is deleted, so it's resourceBinding is deleted, cleaning up makes no sense.
Since the attached resource template is deleted, so it's resourceBinding is deleted, cleaning up makes no sense.
Are you referring to the gc controller responsible for cleaning up the resource bindings of the resource template?
Are you referring to the gc controller responsible for cleaning up the resource bindings of the resource template?
sure
Thanks, I get it. Can you help add some comments on the event handler to explain why we don't need to care about this?
Can you help add some comments on the event handler to explain why we don't need to care about this?
okay
Codecov Report
Attention: Patch coverage is 26.66667% with 22 lines in your changes are missing coverage. Please review.
Project coverage is 53.26%. Comparing base (
81acb31) to head (b30e537).
| Files | Patch % | Lines |
|---|---|---|
| ...ependenciesdistributor/dependencies_distributor.go | 26.66% | 20 Missing and 2 partials :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## master #4532 +/- ##
==========================================
- Coverage 53.29% 53.26% -0.03%
==========================================
Files 252 252
Lines 20495 20508 +13
==========================================
+ Hits 10922 10924 +2
- Misses 8851 8861 +10
- Partials 722 723 +1
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 53.26% <26.66%> (-0.03%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Kind ping @chaunceyjiang @RainbowMango /cc @chaunceyjiang @RainbowMango
@chaunceyjiang Do you want to take another look? As you are the related code author.
Hi @whitewindmills, We can move forward with this PR. Can you rebase it and confirm if there are any required changes?
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from rainbowmango. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
