-
Notifications
You must be signed in to change notification settings - Fork 5k
iso: Fix minikube stop
with vfkit and krunkit drivers
#21089
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
Conversation
Create default arm64 config and disable stuff that we cannot use in a VM. This chagne was generated by: 1. Create defualt arm64 config cd out/buildroot/output-aarch64/build/linux-6.6.95 make ARCH=arm64 defconfig make ARCH=arm64 menuconfig (exit saving changes) 2. Disable features that we don't need in the minikube VM: - Platform suppport - all platforms - Device drivers - Multimedia support - Sound support 3. Updated our linux defconfig cd out/buildroot/output-aarch64 make linux-update-defconfig 4. Normalize the config make linux-menuconfig-aarch64 (exit saving changes) With this config qemu, vfkit, and krunkit boot with --no-kubernetes, and graceful shutdown works in vfkit and krunkit (using --restful-uri). We cannot start kubernetes yet since some features are not available in the default architecture config.
This restores the configs removed by updating from the default architecture config. These configs are required for kubernetes support. After adding the removed configs, run `make linux-menuconfig-aarch64` to normalize the config and remove multimedia and sound card support again.
Hi @nirs. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
ok-to-build-iso |
This sounds like it would be related to ACPI, which is what is shutting the machines down. |
I agree, but when I tried to add only the default CONFIG_ACPI_* configs to our image it did not fix the issue. So there must be something else missing. Copying all the default configs is ugly but it fixes the issue. The interesting point is that shutting down qemu do work with the current image. Maybe qemu terminate the guest using another way, or maybe it does not do graceful shutdown. |
iso build logs: https://storage.googleapis.com/minikube-builds/logs/21089/d297c3e/iso_build.txt podman build failed. The iso I built was created from the previous build directory, only the kernel was rebuilt.
|
Reproduced the error locally by:
|
Adding go.work seems to break podman build. The workspace is needed only for running the update commands so let's disable it when building the iso. We may need much bigger change to ensur that the workspace is used only when running the update go commands, or remove it. This change fixes only the iso build.
ok-to-build-iso |
Hi @nirs, we have updated your PR with the reference to newly built ISO. Pull the changes locally if you want to test with them or update your PR further. |
Timing minikube start with the new kernelTested with #20826 rebased on top of this PR. no kubernetes
with kubernetes
|
/ok-to-test |
kvm2 driver with docker runtime
Times for minikube start: 51.5s 48.6s 50.7s 48.8s 50.0s Times for minikube (PR 21089) ingress: 15.5s 15.0s 15.0s 15.0s 15.0s docker driver with docker runtime
Times for minikube start: 22.0s 23.0s 23.1s 26.3s 23.0s Times for minikube (PR 21089) ingress: 13.3s 13.3s 12.3s 12.3s 13.8s docker driver with containerd runtime
Times for minikube start: 25.0s 22.8s 21.3s 21.8s 23.3s Times for minikube (PR 21089) ingress: 22.8s 22.8s 23.3s 39.8s 22.8s |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh, nirs 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 |
With current minikube kernel stopping vfkit or krunkit is never complete:
The vfkit (and krunkit) drivers stop by sending POST request to change the VM state. vfkit and krunkit trigger a graceful shutdown in the guest. Something is missing in our current kernel, making the shutdown request to be ignored, and the vfkit (or krunkit) child process never terminate.
minikube should handle this issue by falling back to SIGTERM after a reasonable timeout, and finally using SIGKILL. This should be implemented by the code running the drivers for all drivers, but we don't have such code.
The issue is fixed by creating default kernel config for arm64, and adding the missing configs to our kernel configs.
This was done in 2 steps:
The first commit created a default kernel config without unneeded parts (like specific arm platform support, multimedia support, and sounds card support), and adding the missing configs to our kernel config. With this kernel we can start and stop successfuly with qemu, vfkit, and krunkit with --no-kubernetes.
The second commit add back the configs removed by the first commit. With this we can start clusters normally. This commit will be helpful to understand which configs we need to add to a default kernel config when we upgrade the kernel again.
Build the iso is currently broken by the go.work file. This change also disable the workspace to unbreak the iso build.
Testing
Tested with #20826 and kernel built locally.
Testing basic life cycle: start, stop, start, delete
Integration tests: