Upgrade minikube to 1.33.0 #1375

nirs · 2024-05-06T21:42:53Z

Minikube 1.33.0 adds useful features, fixes, and performance
improvements, but we could not use it because of a regression in
systemd-resolved[1].

A critical change in 1.33.0 is upgrading the kernel to 5.10.207. This
version fixes bad bug with minikube 1.32.0, when qemu assertion fails
while starting a kubevirt VM[2] on newer Intel CPUs (i7-12700k).

This change:

Rework command line parsing so we can add global commands that do not use an environment file
Add setup command for configuring minikube for drenv
Add cleanup command to remove changes done in setup

drenv configuration includes:

Increase sysctl fs.ionotify limits to avoid random errors when starting kubevirt VMs
- ISO: Add 50-fs-inotify.conf to increase limits kubernetes/minikube#18832
Disable DNSSEC in systemd-resolved, which breaks pulling images from some registries
- ISO: Add 50-dnssec.conf to fix DNSSEC issue kubernetes/minikube#18830

With the new configuration we can upgrade to minikube 1.33.0, and Alex can run drenv
kubevirt environment without manual hacks.

When the configuration changes are released in minikube, we can remove the code in
drenv.minikube, but I would keep the infrastructure for adding and loading configuration
so in the next time we have an issue we will have easy way fix.

[1] kubernetes/minikube#18705
[2] https://gitlab.com/qemu-project/qemu/-/issues/237

nirs · 2024-05-06T21:45:55Z

@akalenyu do you want to review this?

akalenyu

Looks great! just something to consider about leaving an escape hatch for the user to inject files/scripts

akalenyu · 2024-05-07T08:46:29Z

test/drenv/minikube.py

+    Must be called after the cluster is started, before running any addon. Not
+    need when starting a stopped cluster.
+    """
+    _load_sysctl(profile)


So this works and is totally fine, however, it might be a tiny bit limiting;
I think it's very likely that new developers on this project will have varying hacking requirements to get minikube (or minikube+other operators in my case) working properly.

Would you consider executing user-placed scripts from ~/.drenv/minikube_setup?

Running same scripts for every cluster is too broad in scope. The way to run custom code is to add an addon in test/addons/myaddon/start and put your addon in the first of of one of the workers in the clusters you want to run on.

We don't have yet mechanism to run some addon before all other addons in a cluster, but we can add something like this, maybe:

profiles: - name: my-profile before: # Run before all other addons - name: my-before-addon run: - addons: # Run in parallel - addons: - name: kubevirt - addons: - name: cdi after: # Run after all other addons - name: my-after-addon

If you have a use case for running your own script for every drenv environment please open an issue describing the use case.

akalenyu · 2024-05-07T08:51:18Z

.github/workflows/e2e.yaml

+    - name: Setup drenv
+      working-directory: test
+      run: drenv setup envs/regional-dr.yaml
+


I see the CI is green, which means this was exercised, and 1.33 works?

No, since we do not install yet the tool dynamically in the CI, this just shows that the changes are compatible with 1.32.0 (version installed in the CI runner).

akalenyu · 2024-05-07T08:58:08Z

test/drenv/__main__.py

@@ -124,6 +124,12 @@ def handle_termination_signal(signo, frame):
    sys.exit(1)


+def cmd_setup(env, args):


Should we document this step in the user-quick-start.md?

Note that in this commit message you setup $ drenv setup envs/regional-dr.yaml
but end up starting $ drenv start envs/kubevirt.yaml -v

Should we document this step in the user-quick-start.md?

Yes we must, thanks!

Note that in this commit message you setup $ drenv setup envs/regional-dr.yaml but end up starting $ drenv start envs/kubevirt.yaml -v

The issue is that command line parsing is too simplistic, so the envfile is required for every command. Like "clear", the new "setup" command requires an enf file but does not use it.

I'll fix this in the next version.

Should we document this step in the user-quick-start.md?

Yes we must, thanks!

Current version setup minikube by default when creating virtual env (make venv) so this change is transparent for users and developers.

Looks like recent python does not install setuptools, used in drenv setup.py script. We probably need to switch to more modern library, but lets first fix the error by installing setuptools. We can look at modernizing the setup later. Signed-off-by: Nir Soffer <nsoffer@redhat.com>

Lets keep main() cleaner by moving out the details of command line parsing. Signed-off-by: Nir Soffer <nsoffer@redhat.com>

The previous command line parsing was too simplistic, resulting in several problems: - The filename argument is require for all commands, even the command does not use it. Because we validate it, you could not use a dummy string or /dev/null. - Some commands line options have no effect on some commands, but they were still documented (e.g. --skip-*) - Commands had no help This change fixes the issue by creating a command line parser for every command, and unifying the command interface to accept only args. Commands operating on an evnfile load it in the command. Example help with this change: $ drenv --help usage: drenv [-h] {start,stop,delete,suspend,resume,dump,fetch,clear} ... options: -h, --help show this help message and exit commands: {start,stop,delete,suspend,resume,dump,fetch,clear} start Start an environment stop Stop an environment delete Delete an environment suspend Suspend virtual machines resume Resume virtual machines dump Dump an environment yaml fetch Cache environment resources clear Cleared cached resources Help for specific commands: $ for cmd in start stop delete suspend resume dump fetch clear; do echo echo "\$ drenv $cmd --help"; drenv $cmd --help done $ drenv start --help usage: drenv start [-h] [-v] [--name-prefix PREFIX] [--max-workers N] [--skip-tests] [--skip-addons] filename positional arguments: filename path to environment file options: -h, --help show this help message and exit -v, --verbose be more verbose --name-prefix PREFIX prefix profile names --max-workers N maximum number of workers per profile --skip-tests Do not run addons 'test' hooks --skip-addons Do not run addons 'start' hooks $ drenv stop --help usage: drenv stop [-h] [-v] [--name-prefix PREFIX] [--max-workers N] [--skip-addons] filename positional arguments: filename path to environment file options: -h, --help show this help message and exit -v, --verbose be more verbose --name-prefix PREFIX prefix profile names --max-workers N maximum number of workers per profile --skip-addons Do not run addons 'stop' hooks $ drenv delete --help usage: drenv delete [-h] [-v] [--name-prefix PREFIX] [--max-workers N] filename positional arguments: filename path to environment file options: -h, --help show this help message and exit -v, --verbose be more verbose --name-prefix PREFIX prefix profile names --max-workers N maximum number of workers per profile $ drenv suspend --help usage: drenv suspend [-h] [-v] [--name-prefix PREFIX] [--max-workers N] filename positional arguments: filename path to environment file options: -h, --help show this help message and exit -v, --verbose be more verbose --name-prefix PREFIX prefix profile names --max-workers N maximum number of workers per profile $ drenv resume --help usage: drenv resume [-h] [-v] [--name-prefix PREFIX] [--max-workers N] filename positional arguments: filename path to environment file options: -h, --help show this help message and exit -v, --verbose be more verbose --name-prefix PREFIX prefix profile names --max-workers N maximum number of workers per profile $ drenv dump --help usage: drenv dump [-h] [-v] [--name-prefix PREFIX] [--max-workers N] filename positional arguments: filename path to environment file options: -h, --help show this help message and exit -v, --verbose be more verbose --name-prefix PREFIX prefix profile names --max-workers N maximum number of workers per profile $ drenv fetch --help usage: drenv fetch [-h] [-v] [--name-prefix PREFIX] [--max-workers N] filename positional arguments: filename path to environment file options: -h, --help show this help message and exit -v, --verbose be more verbose --name-prefix PREFIX prefix profile names --max-workers N maximum number of workers per profile $ drenv clear --help usage: drenv clear [-h] [-v] options: -h, --help show this help message and exit -v, --verbose be more verbose Signed-off-by: Nir Soffer <nsoffer@redhat.com>

Minikube supports injecting configuration files into the cluster nodes. This is very useful for solving 2 issues: - Regression in minikube 1.33 enabling DNSSEC by default. This breaks pulling images from some registries, breaking us badly. But minikube 1.33 have other improvements we would like to consume (such as faster cluster start) so this change work around this regression. - Random failures when starting kubevirt VM, failing to create inotify watch. This is caused by low fs.inotify limits, and can be fixed by increasing the limits. We use the default limits from OpenShift worker node. Creating the configuration files is not enough, since minikube inject the files to the VM too late. We need to reload daemons or run sysctl to apply the configuration changes when the cluster starts. This change adds new "setup" and "cleanup" commands. The "setup" command should run once when setting up a development system. It writes drenv configuration to $MINIKUBE_HOME/.minikube/files/, that will be used by future cluster. The "cleanup" command remove the changes made by the setup command, and will be used in CI to clean up after a build. Example usage: $ tree $MINIKUBE_HOME/.minikube/files /data/tmp/.minikube/files $ drenv setup -v 2024-05-07 17:49:11,444 INFO [main] Setting up minikube for drenv 2024-05-07 17:49:11,444 DEBUG [minikube] Writing drenv sysctl configuration /data/tmp/.minikube/files/etc/sysctl.d/99-drenv.conf 2024-05-07 17:49:11,445 DEBUG [minikube] Writing drenv systemd-resolved configuration /data/tmp/.minikube/files/etc/systemd/resolved.conf.d/99-drenv.conf ... $ tree $MINIKUBE_HOME/.minikube/files /data/tmp/.minikube/files └── etc ├── sysctl.d │ └── 99-drenv.conf └── systemd └── resolved.conf.d └── 99-drenv.conf $ drenv start envs/kubevirt.yaml -v ... 2024-05-07 17:55:04,568 DEBUG [kubevirt] Loading drenv sysctl configuration 2024-05-07 17:55:04,568 DEBUG [kubevirt] Running ['minikube', 'ssh', '--profile', 'kubevirt', 'sudo sysctl -p /etc/sysctl.d/99-drenv.conf'] 2024-05-07 17:55:04,783 DEBUG [kubevirt] fs.inotify.max_user_instances = 8192 2024-05-07 17:55:04,783 DEBUG [kubevirt] fs.inotify.max_user_watches = 65536 2024-05-07 17:55:04,789 DEBUG [kubevirt] Loading drenv systemd-resolved configuration 2024-05-07 17:55:04,790 DEBUG [kubevirt] Running ['minikube', 'ssh', '--profile', 'kubevirt', 'sudo systemctl restart systemd-resolved.service'] ... $ minikube ssh -p kubevirt 'cat /etc/systemd/resolved.conf.d/99-drenv.conf' # Added by drenv setup [Resolve] DNSSEC=no $ minikube ssh -p kubevirt 'cat /etc/sysctl.d/99-drenv.conf' # Added by drenv setup fs.inotify.max_user_instances = 8192 fs.inotify.max_user_watches = 65536 $ drenv cleanup -v 2024-05-07 17:50:38,608 INFO [main] Cleaning up minikube 2024-05-07 17:50:38,608 DEBUG [minikube] Removed file /data/tmp/.minikube/files/etc/systemd/resolved.conf.d/99-drenv.conf 2024-05-07 17:50:38,608 DEBUG [minikube] Removed empty directory /data/tmp/.minikube/files/etc/systemd/resolved.conf.d 2024-05-07 17:50:38,609 DEBUG [minikube] Removed empty directory /data/tmp/.minikube/files/etc/systemd 2024-05-07 17:50:38,609 DEBUG [minikube] Removed file /data/tmp/.minikube/files/etc/sysctl.d/99-drenv.conf 2024-05-07 17:50:38,609 DEBUG [minikube] Removed empty directory /data/tmp/.minikube/files/etc/sysctl.d 2024-05-07 17:50:38,609 DEBUG [minikube] Removed empty directory /data/tmp/.minikube/files/etc ... Signed-off-by: Nir Soffer <nsoffer@redhat.com>

Minikube 1.33.0 adds useful features, fixes, and performance improvements, but we could not use it because of a regression in systemd-resolved[1]. A critical change in 1.33.0 is upgrading the kernel to 5.10.207. This version fixes bad bug with minikube 1.32.0, when qemu assertion fails while starting a kubevirt VM[2] on newer Intel CPUs (i7-12700k). Now that we setup systemd-resolved configuration we can upgrade to minikube 1.33.0, and Alex can run drenv kubevirt environment without manual hacks. This change: - Updates the docs that latest minikube test is 1.33.0 - Setup minikube by default when creating the virtual environment, so minikube 1.33.0 support is added transparently for developers - Setup drenv in the e2e job to support minikube 1.33.0 - Cleanup drenv in the e2e job so setup from previous build will not leak into the next build [1] kubernetes/minikube#18705 [2] https://gitlab.com/qemu-project/qemu/-/issues/237 Thanks: Alex Kalenyuk <akalenyu@redhat.com> Signed-off-by: Nir Soffer <nsoffer@redhat.com>

nirs · 2024-05-07T20:08:37Z

test/drenv/minikube.py

+    path = _sysctl_drenv_conf()
+    data = """# Added by drenv setup
+fs.inotify.max_user_instances = 8192
+fs.inotify.max_user_watches = 65536


I opened minikube issue for this, maybe they will adopt this and we can remove this code soon:
kubernetes/minikube#18831

Posted a PR to minikube:
kubernetes/minikube#18832

nirs · 2024-05-07T20:09:42Z

test/drenv/minikube.py

+    data = """# Added by drenv setup
+[Resolve]
+DNSSEC=no
+"""


Minikube is adopting this workaround, so we can probably remove this very soon:
kubernetes/minikube#18830

nirs requested review from raghavendra-talur and ShyamsundarR May 6, 2024 21:42

nirs force-pushed the minikube-files branch from 29e7855 to a10c432 Compare May 6, 2024 21:53

akalenyu reviewed May 7, 2024

View reviewed changes

nirs marked this pull request as draft May 7, 2024 12:01

nirs added 2 commits May 7, 2024 16:57

Extract parse_args helper

5cb4a33

Lets keep main() cleaner by moving out the details of command line parsing. Signed-off-by: Nir Soffer <nsoffer@redhat.com>

nirs force-pushed the minikube-files branch 4 times, most recently from fa12baf to 8110e02 Compare May 7, 2024 15:38

nirs added 3 commits May 7, 2024 18:42

nirs force-pushed the minikube-files branch from 8110e02 to fae8de8 Compare May 7, 2024 15:42

nirs marked this pull request as ready for review May 7, 2024 15:46

nirs requested a review from akalenyu May 7, 2024 15:46

nirs commented May 7, 2024

View reviewed changes

raghavendra-talur approved these changes May 8, 2024

View reviewed changes

raghavendra-talur merged commit 449ec42 into RamenDR:main May 8, 2024
16 checks passed

nirs deleted the minikube-files branch May 20, 2024 21:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade minikube to 1.33.0 #1375

Upgrade minikube to 1.33.0 #1375

nirs commented May 6, 2024 •

edited

nirs commented May 6, 2024

akalenyu left a comment

akalenyu May 7, 2024

nirs May 7, 2024 •

edited

akalenyu May 7, 2024

nirs May 7, 2024

akalenyu May 7, 2024

akalenyu May 7, 2024

nirs May 7, 2024

nirs May 7, 2024

nirs May 7, 2024

nirs May 7, 2024

nirs May 7, 2024

nirs May 7, 2024

		@@ -124,6 +124,12 @@ def handle_termination_signal(signo, frame):
		sys.exit(1)


		def cmd_setup(env, args):

Upgrade minikube to 1.33.0 #1375

Upgrade minikube to 1.33.0 #1375

Conversation

nirs commented May 6, 2024 • edited

nirs commented May 6, 2024

akalenyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirs May 7, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirs commented May 6, 2024 •

edited

nirs May 7, 2024 •

edited