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.

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

inject clusterrolebinding for apiserver in pool-coordinator to enable kubectl logs within nodepool scope #1384

Closed
wants to merge 1 commit into from

Conversation

Congrool
Copy link
Member

@Congrool Congrool commented Apr 18, 2023

What type of PR is this?

/kind enhancement

What this PR does / why we need it:

Currently, kubectl logs to poolcoordinator cannot work because the apiserver in pool-coordinator is not authorized to access the kubelet server, in other words it cannot get sub-resources proxy/logs. This pr will inject relative rbac rule to enable the apiserver to access the kubelet server.

It works in the following step:

  1. Using a initContainer, which is alpine:3.14, to get necessary tools that are used to inject rbac. Tools include busybox.static, which is a statically linked busybox that can run in kube-apiserver container without any dynamic link lib, and kubectl which is actually used to send request to apiserver to create rbac rules.
  2. A new configmap is added to provide rbac we need. It will be mounted by kube-apiserver container.
  3. kube-apiserver container will also mount the dir that contains tools we previously prepared. The command of this container is changed to run in a shell, which will attempt to use kubectl to create rbac and execute the kube-apiserver command.

why not use postStart hook of kube-apiserver container:

After adding postStart hook, the kube-apiserver is too slow to be ready(typically over 20s) and will exit when exceeding the internal dial timeout(20s used to dial to etcd and it's unconfigurable). It will cause the pool-coordinator cannot be ready and, of course, the postStart hook cannot get successfully executed.

I've not figured out why adding postStart hook has influence on kube-apiserver.

other Note

TODOs:

  • rbac rules can be injected into apiserver
  • kubeclt logs can work within nodepool

Currently the test kubeclt logs can work within nodepool has not got done yet, but I think it can work since we tested it through manually creating rbac into pool-coordinator before releasing openyurt v1.12.

… kubectl logs

Signed-off-by: Congrool <chpzhangyifei@zju.edu.cn>
@openyurt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Congrool
To complete the pull request process, please assign rambohe-ch
You can assign the PR to them by writing /assign @rambohe-ch in a comment when ready.

The full list of commands accepted by this bot can be found 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

@openyurt-bot openyurt-bot added the size/L size/L: 100-499 label Apr 18, 2023
@sonarcloud
Copy link

sonarcloud bot commented Apr 18, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
1.9% 1.9% Duplication

@codecov
Copy link

codecov bot commented Apr 21, 2023

Codecov Report

Merging #1384 (ee58384) into master (4c30cf2) will not change coverage.
Report is 103 commits behind head on master.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1384   +/-   ##
=======================================
  Coverage   51.48%   51.48%           
=======================================
  Files         125      125           
  Lines       15021    15021           
=======================================
  Hits         7733     7733           
  Misses       6589     6589           
  Partials      699      699           
Flag Coverage Δ
unittests 51.48% <ø> (ø)

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

@Congrool
Copy link
Member Author

/hold

Need to solve the situation that node is disconnected from network. The pool-coordinator may fail to restart.

@openyurt-bot openyurt-bot added the do-not-merge/hold do-not-merge/hold label Apr 21, 2023
@stale
Copy link

stale bot commented Jul 20, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 20, 2023
@stale stale bot closed this Jul 27, 2023
@rambohe-ch rambohe-ch reopened this Jul 27, 2023
@stale stale bot removed the wontfix label Jul 27, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jul 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 33 Code Smells

No Coverage information No Coverage information
1.2% 1.2% Duplication

@Congrool
Copy link
Member Author

Congrool commented Aug 6, 2023

New PR to solve this problem #1637 , close this one now.

@Congrool
Copy link
Member Author

Congrool commented Aug 6, 2023

/close

@openyurt-bot
Copy link
Collaborator

@Congrool: Closed this PR.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold do-not-merge/hold kind/enhancement kind/enhancement size/L size/L: 100-499
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants