Skip to content

fix: k8s client setup if agent service account auth is not used #731

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ipochi
Copy link
Contributor

@ipochi ipochi commented Apr 15, 2025

Currently the setting up of k8s client is broken if service account
authentication is not used between server and agent.

This condition if o.AgentNamespace != "" { acts as a gatekeeper for
setting the k8s client which worked fine previously as server never
needed to talk to apiserver apart from authenticating agents using
service account token.

However when lease controller logic was added, it meant that setting up
k8s client was required if lease controller was enabled but
authentication was done using mTLS instead of service account
authentication.

This fixes that.

Closing #728 in favour of this.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 15, 2025
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 15, 2025
@ipochi ipochi force-pushed the imran/fix-k8sclient-for-mtls branch 2 times, most recently from c4a2ca5 to 0b9115e Compare April 15, 2025 22:31
@ipochi
Copy link
Contributor Author

ipochi commented Apr 15, 2025

/retest

@ipochi
Copy link
Contributor Author

ipochi commented Apr 16, 2025

e2e test failure is related to actions/runner-images#11985

/cc @cheftako

@k8s-ci-robot k8s-ci-robot requested a review from cheftako April 16, 2025 00:07
@ipochi ipochi changed the title fix: k8s client setup if mtls used fix: k8s client setup if agent service account auth is not used Apr 16, 2025
Currently the setting up of k8s client is broken if service account
authentication is not used between server and agent.

This condition  `if o.AgentNamespace != "" {` acts as a gatekeeper for
setting the k8s client which worked fine previously as server never
needed to talk to apiserver apart from authenticating agents using
service account token.

However when lease controller logic was added, it meant that setting up
k8s client was required if lease controller was enabled but
authentication was done using mTLS instead of service account
authentication.

This fixes that.

Closing kubernetes-sigs#728  in favour of this.

Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
@ipochi ipochi force-pushed the imran/fix-k8sclient-for-mtls branch from 0b9115e to 3e4b939 Compare April 16, 2025 05:37
@ipochi
Copy link
Contributor Author

ipochi commented Apr 16, 2025

/retest

2 similar comments
@ipochi
Copy link
Contributor Author

ipochi commented Apr 18, 2025

/retest

@cheftako
Copy link
Contributor

/retest

@cheftako
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 22, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheftako, ipochi

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

@cheftako
Copy link
Contributor

Change looks good. Lets see what we need to do to get the tests healthy.

@ipochi
Copy link
Contributor Author

ipochi commented Apr 23, 2025

/retest

@liangyuanpeng
Copy link
Contributor

ipv6 runners have a kernel bug actions/runner-images#11985 , this is the reason of CI failed.

There is an error in github actions runner that runs on 22.04. This
issue hasn't been rectified yet but the issue is not reported on 24.04

issue details: actions/runner-images#11985

So this bumps the action to run on ubuntu 24.04

Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 29, 2025
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

switching to 24.04 github action runner introduces this error chmod:
cannot operate on dangling symlink '/usr/local/bin/now'

hence instead of chmod all binaries present in /usr/local/bin, we only
chmod the binaries that we copy.

Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
@ipochi
Copy link
Contributor Author

ipochi commented Apr 29, 2025

@cheftako @jkh52

after updating the github actions to run on 24.04, the issue involved with runners is resolved.
PR to update the runner: #734

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants