-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Refactor Invoking Kubeadm command to prepare for debian 12 #21642
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
base: master
Are you sure you want to change the base?
Conversation
We want to use it to build kubeadm command in other packages.
Previously we ran: bash -c 'sudo env PATH="/path/to/minikube/binaries:$PATH" kubeadm init ...' The shell expanding $PATH is running as a normal user, which does not include /usr/sbin: $ out/minikube ssh -- printenv PATH /usr/local/bin:/usr/bin:/bin:/usr/games This breaks kubeadm since it expects to find losetup in the PATH[1]. Now we run: sudo bash -c 'env PATH="/path/to/minikube/binaries:$PATH" kubeadm init ...' The shell expanding $PATH is running as root, so it uses root PATH that includes /usr/sbin: $ out/minikube ssh -- sudo printenv PATH /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin With this change kubeadm init can find losetup in the PATH, and creating a clusters succeeds. More work is needed to fix other invocations of kubeadm. [1] kubernetes/kubernetes#129450
… the correct path
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh 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 |
/ok-to-test |
This comment has been minimized.
This comment has been minimized.
…util and remove from machine package to avoid "import cycle not allowed" lint error
@medyagh: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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-sigs/prow repository. I understand the commands that are listed here. |
kvm2 driver with docker runtime
Times for minikube ingress: 15.8s 15.8s 15.8s 15.8s 15.9s Times for minikube start: 42.3s 43.6s 45.0s 40.9s 45.3s docker driver with docker runtime
Times for minikube start: 22.8s 21.6s 21.4s 19.8s 21.8s Times for minikube ingress: 10.6s 10.6s 11.6s 10.6s 12.6s docker driver with containerd runtime
Times for minikube start: 20.9s 21.8s 21.2s 19.5s 20.1s Times for minikube (PR 21642) ingress: 23.1s 24.1s 23.1s 23.1s 39.1s |
This comment has been minimized.
This comment has been minimized.
Here are the number of top 10 failed tests in each environments with lowest flake rate.
Besides the following environments also have failed tests:
To see the flake rates of all tests by environment, click here. |
} | ||
|
||
if _, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", "sudo systemctl daemon-reload && sudo systemctl enable kubelet && sudo systemctl start kubelet")); err != nil { | ||
if _, err := k.c.RunCmd(exec.Command("sudo", "/bin/bash", "-c", "systemctl daemon-reload && systemctl enable kubelet && systemctl start kubelet")); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not needed since we don't use env PATH=
in the command.
kubeadmPath := path.Join(vmpath.GuestPersistentDir, "binaries", cc.KubernetesConfig.KubernetesVersion) | ||
bashCmd := fmt.Sprintf("sudo env PATH=\"%s:$PATH\" kubeadm certs renew all --config %s", kubeadmPath, constants.KubeadmYamlPath) | ||
bashCmd := fmt.Sprintf("%s certs renew all --config %s", bsutil.KubeadmCmdWithPath(cc.KubernetesConfig.KubernetesVersion), constants.KubeadmYamlPath) | ||
if _, err := cmd.RunCmd(exec.Command("/bin/bash", "-c", bashCmd)); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing sudo
defer func() { | ||
if err := f.Close(); err != nil { | ||
klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The asset is opened for reading, and close error is not very interesting. But the issue was not introduced by your change.
@medyagh can you squash the fixup commits so we can have clean git history? |
Based on this PR #21623