karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.12k stars 807 forks source link

add: Use opts.ResyncPeriod as controlPlaneInformerManager and sharedF… #4825

Closed wangxf1987 closed 6 days ago

wangxf1987 commented 2 weeks ago

…actory

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes: Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

karmada-bot commented 2 weeks ago

Welcome @wangxf1987! It looks like this is your first PR to karmada-io/karmada 🎉

RainbowMango commented 2 weeks ago

Is this related to #4823 ?

wangxf1987 commented 2 weeks ago

Is this related to #4823 ?

not related

wangxf1987 commented 2 weeks ago

@RainbowMango I want to contribute our resource interperter, it name is fed.jd.com/v1/KService, which is equivalent to apps.kruise.io. It it possible?

RainbowMango commented 2 weeks ago

Do you mean to put fed.jd.com/v1/KService under the thirdparty resourcecustomizations?

As far as I know, the third-party customizations are designed for those well-known open-sourced resource types. Is the fed.jd.com/v1/KService open-sourced?

wangxf1987 commented 2 weeks ago

Do you mean to put fed.jd.com/v1/KService under the thirdparty resourcecustomizations?

As far as I know, the third-party customizations are designed for those well-known open-sourced resource types. Is the fed.jd.com/v1/KService open-sourced?

open will be soon

wangxf1987 commented 2 weeks ago

Do you mean to put fed.jd.com/v1/KService under the thirdparty resourcecustomizations?

As far as I know, the third-party customizations are designed for those well-known open-sourced resource types. Is the fed.jd.com/v1/KService open-sourced?

we have an plan in the jdcloud, and open the kservice project

RainbowMango commented 2 weeks ago

we have an plan in the jdcloud, and open the kservice project

Nice. Please talk to me then.

wangxf1987 commented 2 weeks ago

add the opts.ResyncPeriod in the controller-manager, I not found the NotSync issue. it's should be consider to accpet the PR?

wangxf1987 commented 2 weeks ago

add the opts.ResyncPeriod in the controller-manager, I not found the NotSync issue. it's should be consider to accpet the PR? @RainbowMango

RainbowMango commented 2 weeks ago

/assign

RainbowMango commented 2 weeks ago

I'll look at it. But I'm curious why you need this. Using resync to solve problems might not be a recommended pattern.

codecov-commenter commented 6 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 53.05%. Comparing base (40d65f9) to head (3232c52).

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

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #4825 +/- ## ======================================= Coverage 53.04% 53.05% ======================================= Files 250 250 Lines 20396 20396 ======================================= + Hits 10820 10822 +2 + Misses 8857 8855 -2 Partials 719 719 ``` | [Flag](https://app.codecov.io/gh/karmada-io/karmada/pull/4825/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=karmada-io) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/karmada-io/karmada/pull/4825/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=karmada-io) | `53.05% <ø> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=karmada-io#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

RainbowMango commented 6 days ago

@wangxf1987 Xiaofei, this PR contains a merge commit that is not what we want. Could you please rebase your PR, you can find the guidelines at https://karmada.io/docs/next/contributor/github-workflow.

karmada-bot commented 6 days ago

[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: - ~~[cmd/OWNERS](https://github.com/karmada-io/karmada/blob/master/cmd/OWNERS)~~ [RainbowMango] - ~~[pkg/resourceinterpreter/OWNERS](https://github.com/karmada-io/karmada/blob/master/pkg/resourceinterpreter/OWNERS)~~ [RainbowMango] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
XiShanYongYe-Chang commented 6 days ago

Thanks~ /lgtm

RainbowMango commented 6 days ago

I talked to @wangxf1987, this PR isn't for fixing anything, just a cleanup. /kind cleanup