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

kubeadm init phase bootstrap-token ignores --kubeconfig in 1.29 #2992

Closed
avorima opened this issue Jan 12, 2024 · 6 comments · Fixed by kubernetes/kubernetes#122735
Closed
Assignees
Labels
area/cli area/UX kind/bug Categorizes issue or PR as related to a bug. kind/regression Categorizes issue or PR as related to a regression from a prior release. priority/backlog Higher priority than priority/awaiting-more-evidence.
Milestone

Comments

@avorima
Copy link

avorima commented Jan 12, 2024

What happened?

Using kubeadm init phase bootstrap-token no longer works stand-alone in 1.29. The --kubeconfig flag is ignored and it requires /etc/kubernetes/admin.conf or super-admin.conf to exist.

$ kubeadm init phase bootstrap-token --kubeconfig kubeconfig.yaml
error execution phase bootstrap-token: could not bootstrap the admin user in file admin.conf: failed to load admin kubeconfig: open /etc/kubernetes/admin.conf: no such file or directory
To see the stack trace of this error execute with --v=5 or higher

The release notes did not mention this behavior change.

What did you expect to happen?

In 1.28 it used to be possible to create a bootstrap-token using: kubeadm init phase bootstrap-token --kubeconfig someconfig.yaml.

For example using kubeadm 1.28.5 from https://dl.k8s.io/v1.28.5/kubernetes-node-linux-amd64.tar.gz:

./kubernetes/node/bin/kubeadm init phase bootstrap-token --kubeconfig kubeconfig.yaml
I0112 11:23:27.072367   27957 version.go:256] remote version is much newer: v1.29.0; falling back to: stable-1.28
[bootstrap-token] Using token: cmyl7h.z7egm8ygdbymguln
[bootstrap-token] Configuring bootstrap tokens, cluster-info ConfigMap, RBAC Roles
[bootstrap-token] Configured RBAC rules to allow Node Bootstrap tokens to get nodes
[bootstrap-token] Configured RBAC rules to allow Node Bootstrap tokens to post CSRs in order for nodes to get long term certificate credentials
[bootstrap-token] Configured RBAC rules to allow the csrapprover controller automatically approve CSRs from a Node Bootstrap Token
[bootstrap-token] Configured RBAC rules to allow certificate rotation for all node client certificates in the cluster
[bootstrap-token] Creating the "cluster-info" ConfigMap in the "kube-public" namespace

How can we reproduce it (as minimally and precisely as possible)?

Set up a kind cluster to get a working kubeconfig file:

kind create cluster --name kubeadm-test --kubeconfig kubeconfig.yaml

Run this command with kubeadm 1.28.5 and 1.29.0:

kubeadm init phase bootstrap-token --kubeconfig kubeconfig.yaml

Anything else we need to know?

To me it looks like the problem is that the codepath where --kubeconfig is used is unreachable. https://github.com/kubernetes/kubernetes/blob/v1.29.0/cmd/kubeadm/app/cmd/init.go#L529
The only place where adminKubeConfigBootstrapped is set is in the if block above, so the else is never taken.

This change was introduced in kubernetes/kubernetes#121305.

Kubernetes version

1.29.0

Cloud provider

OS version

# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here

Install tools

Container runtime (CRI) and version (if applicable)

Related plugins (CNI, CSI, ...) and versions (if applicable)

@avorima avorima added the kind/bug Categorizes issue or PR as related to a bug. label Jan 12, 2024
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 12, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@avorima
Copy link
Author

avorima commented Jan 12, 2024

/sig cluster-lifecycle

@k8s-ci-robot k8s-ci-robot added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 12, 2024
@neolit123
Copy link
Member

/transfer kubeadm

@k8s-ci-robot k8s-ci-robot transferred this issue from kubernetes/kubernetes Jan 12, 2024
@neolit123 neolit123 added priority/backlog Higher priority than priority/awaiting-more-evidence. area/UX area/cli and removed sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 12, 2024
@neolit123
Copy link
Member

neolit123 commented Jan 12, 2024

To me it looks like the problem is that the codepath where --kubeconfig is used is unreachable. https://github.com/kubernetes/kubernetes/blob/v1.29.0/cmd/kubeadm/app/cmd/init.go#L529
The only place where adminKubeConfigBootstrapped is set is in the if block above, so the else is never taken.

This change was introduced in kubernetes/kubernetes#121305.

you are correct and that's a regression. PR fixes are welcome, we also need to backport the fix to 1.29.

i think this is a fix:
(NOTE: i've updated multiple things, like comments in existing lines of code)

			isDefaultKubeConfigPath := d.KubeConfigPath() == kubeadmconstants.GetAdminKubeConfigPath()
			// Only bootstrap the admin.conf if it's used by the user (i.e. --kubeconfig has its default value)
			// and if the bootstrapping was not already done
			if !d.adminKubeConfigBootstrapped && isDefaultKubeConfigPath {
				// Call EnsureAdminClusterRoleBinding() to obtain a working client from admin.conf.
				d.client, err = kubeconfigphase.EnsureAdminClusterRoleBinding(kubeadmconstants.KubernetesDir, nil)
				if err != nil {
					return nil, errors.Wrapf(err, "could not bootstrap the admin user in file %s", kubeadmconstants.AdminKubeConfigFileName)
				}
				d.adminKubeConfigBootstrapped = true
			} else {
				// Alternatively, just load the config pointed at the --kubeconfig path
				d.client, err = kubeconfigutil.ClientSetFromFile(d.KubeConfigPath())
				if err != nil {
					return nil, err
				}
			}

other ideas are welcome too.

@neolit123 neolit123 added this to the v1.29 milestone Jan 12, 2024
@neolit123 neolit123 added the kind/regression Categorizes issue or PR as related to a regression from a prior release. label Jan 12, 2024
@avorima
Copy link
Author

avorima commented Jan 12, 2024

Thanks for the quick feedback @neolit123. I'd be happy to open a PR.

@neolit123
Copy link
Member

/assign avorima
sounds good, please @ me on it.
thanks

avorima added a commit to avorima/kubernetes that referenced this issue Jan 12, 2024
Don't create admin rolebindings when --kubeconfig is set to a
non-default value.

Fixes: kubernetes/kubeadm#2992

Signed-off-by: Mario Valderrama <[email protected]>
avorima added a commit to avorima/kubernetes that referenced this issue Jan 12, 2024
Don't create admin rolebindings when --kubeconfig is set to a
non-default value.

Fixes: kubernetes/kubeadm#2992

Signed-off-by: Mario Valderrama <[email protected]>
avorima added a commit to avorima/kubernetes that referenced this issue Jan 15, 2024
Don't create admin rolebindings when --kubeconfig is set to a
non-default value.

Fixes: kubernetes/kubeadm#2992

Signed-off-by: Mario Valderrama <[email protected]>
jiahuif pushed a commit to jiahuif-forks/kubernetes that referenced this issue Jan 23, 2024
Don't create admin rolebindings when --kubeconfig is set to a
non-default value.

Fixes: kubernetes/kubeadm#2992

Signed-off-by: Mario Valderrama <[email protected]>
leemingeer pushed a commit to leemingeer/kubernetes that referenced this issue Feb 2, 2024
Don't create admin rolebindings when --kubeconfig is set to a
non-default value.

Fixes: kubernetes/kubeadm#2992

Signed-off-by: Mario Valderrama <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli area/UX kind/bug Categorizes issue or PR as related to a bug. kind/regression Categorizes issue or PR as related to a regression from a prior release. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants