karmada icon indicating copy to clipboard operation
karmada copied to clipboard

Fix the issue of get empty EndpointSlice from cluster informer when report EndpointSlice

Open XiShanYongYe-Chang opened this issue 6 months ago • 5 comments

What type of PR is this?

/kind bug

What this PR does / why we need it:

When the karmada-controller-manager is restarted, the servcie-export-controller will requeue the ServiceExport associated with the collected EndpointSlice resources for processing:

https://github.com/karmada-io/karmada/blob/bd5692f08a259c1a959dc3fd1944071be9fda729/pkg/controllers/mcs/service_export_controller.go#L149-L159

Then, during the processing, the EndpointSlice resource is obtained from the cluster cache:

https://github.com/karmada-io/karmada/blob/bd5692f08a259c1a959dc3fd1944071be9fda729/pkg/controllers/mcs/service_export_controller.go#L399-L421

The cache may not have been synchronized at this time, and an empty EndpointSlice list will be obtained, which will be deleted in the removeOrphanWork function.

Which issue(s) this PR fixes: Fixes #6433

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

karmada-controller-manager: fix the issue of get empty EndpointSlice from cluster informer when report EndpointSlice

XiShanYongYe-Chang avatar Jun 09 '25 09:06 XiShanYongYe-Chang

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 45.33%. Comparing base (5f4bd5e) to head (b0560d6).

Files with missing lines Patch % Lines
pkg/controllers/mcs/service_export_controller.go 0.00% 8 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6434      +/-   ##
==========================================
- Coverage   45.35%   45.33%   -0.03%     
==========================================
  Files         687      687              
  Lines       56384    56393       +9     
==========================================
- Hits        25572    25563       -9     
- Misses      29217    29233      +16     
- Partials     1595     1597       +2     
Flag Coverage Δ
unittests 45.33% <0.00%> (-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.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov-commenter avatar Jun 09 '25 09:06 codecov-commenter

/retest

XiShanYongYe-Chang avatar Jun 09 '25 13:06 XiShanYongYe-Chang

Please cc me again once you figure out a better solution.

RainbowMango avatar Jun 18 '25 10:06 RainbowMango

Please cc me again once you figure out a better solution.

Another modification is to perform a check when using the cluster Informer, which introduces the cost of an additional cache check.

Apart from this, no better solution has been thought of for now.

XiShanYongYe-Chang avatar Jun 19 '25 09:06 XiShanYongYe-Chang

/retitle Ensure EndpointSlice informer cache is synced before reporting EndpointSlice

RainbowMango avatar Jul 24 '25 16:07 RainbowMango

[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 /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

karmada-bot avatar Jul 26 '25 07:07 karmada-bot

@zhzhuang-zju Just a reminder, this fix should be included in the next patch release.

RainbowMango avatar Jul 28 '25 06:07 RainbowMango