Skip to content
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.

Sign up for GitHub

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

Jump to bottom

enhance: separate clients for yurt-manager #2069

Merged
merged 1 commit into from
Jun 17, 2024
Merged

enhance: separate clients for yurt-manager #2069

merged 1 commit into from
Jun 17, 2024

Conversation

luc99hen
Copy link
Member

@luc99hen luc99hen commented Jun 3, 2024

What type of PR is this?

/kind enhancement

What this PR does / why we need it:

Fix #2051

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

other Note

@luc99hen luc99hen force-pushed the separate_new branch 3 times, most recently from 19dd93a to 947e50d Compare June 5, 2024 03:01
@codecov Codecov
Copy link

codecov bot commented Jun 5, 2024 •
edited
Loading

Codecov Report

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

Project coverage is 56.09%. Comparing base (3713163) to head (eb6cc42).
Report is 3 commits behind head on master.

Files Patch % Lines
...roller/nodelifecycle/node_life_cycle_controller.go 0.00% 4 Missing ⚠️
...r/controller/yurtappset/yurt_app_set_controller.go 0.00% 4 Missing ⚠️
...roller/yurtappdaemon/yurt_app_daemon_controller.go 0.00% 3 Missing ⚠️
.../daemonpodupdater/daemon_pod_updater_controller.go 0.00% 2 Missing ⚠️
...et/loadbalancerset/load_balancer_set_controller.go 0.00% 2 Missing ⚠️
...r/raven/gatewaypickup/gateway_pickup_controller.go 0.00% 2 Missing ⚠️
...publicservice/gateway_public_service_controller.go 0.00% 2 Missing ⚠️
...dinator/delegatelease/delegate_lease_controller.go 0.00% 2 Missing ⚠️
.../controller/csrapprover/csr_approver_controller.go 0.00% 1 Missing ⚠️
...er/controller/nodebucket/node_bucket_controller.go 0.00% 1 Missing ⚠️
... and 11 more
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2069       /-   ##
==========================================
- Coverage   56.09%   56.09%   -0.01%     
==========================================
  Files         186      186              
  Lines       18092    18090       -2     
==========================================
- Hits        10149    10147       -2     
  Misses       6910     6910              
  Partials     1033     1033
Flag Coverage Δ
unittests 56.09% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@luc99hen luc99hen force-pushed the separate_new branch 3 times, most recently from 7b88938 to 6450a4b Compare June 11, 2024 07:24
@luc99hen luc99hen force-pushed the separate_new branch 11 times, most recently from c5a7b39 to b01a04e Compare June 15, 2024 09:22
@luc99hen
Copy link
Member Author

luc99hen commented Jun 16, 2024 •
edited
Loading

@rambohe-ch I have found the root cause of the blocking e2e tests are there are no permission for controller-manager's cache building process. As we know, Cache is shared among all controllers/clients in the Manager level. And the process of building Cache need the full permissions from all clients.

Therefore, in our case we can still separate the permissions for each individual client, but the base client used by yurt-manager need the full permission at the initialization stage.

@luc99hen
Copy link
Member Author

We can create a cache for each client used by individual controllers. However, I opt against this approach due to the escalating memory footprint.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
40 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@rambohe-ch
Copy link
Member

/lgtm

@rambohe-ch rambohe-ch added the approved approved label Jun 17, 2024
@rambohe-ch rambohe-ch added the lgtm lgtm label Jun 17, 2024
@rambohe-ch rambohe-ch merged commit 013700d into openyurtio:master Jun 17, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rambohe-ch rambohe-ch rambohe-ch approved these changes

Assignees
No one assigned
Labels
approved approved lgtm lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@luc99hen @rambohe-ch