Use cni by default on Linux, too

[evol262]

Closes #104

[evol262]

@afbjorklund One quick turnaround to align Linux and WIndows. Doesn't look like this will have any negative effects on the minikube PR, but mind taking a look?

[afbjorklund]

I think the user needed to install the cni plugins, either way ? (for both "kubenet", and for "cni")

NetworkReady=false reason:NetworkPluginNotReady message:docker: network plugin is not ready: could not locate kubenet required CNI plugins [bridge host-local loopback] at [\"/opt/cni/bin\" \"/opt/cni/bin\"]

The main difference is that they would now also need to install something in /etc/cni/net.d ?

---

The code changes on the minikube end would be the same, as long as it is not "no-op".

There was an unfortunate shortcut, that avoided updating the config for "known" config...

	if networkPlugin == "" {
		// no-op plugin
		return nil
	}

It now needs to always update the cri-docker systemd unit file, with the users choice.

[afbjorklund]

@evol262 : I don't fully understand why changing to CNI involves updating to go1.19 ?

0470631

Seems like a separate change, one that quite recently hit Kubernetes (while still in RC)...

And it would still be crash-looping, unless providing some default "bridge" configuration

[adelton]

I just filed #104 where I describe that with 0.2.4, even explicitly setting cni (on Ubuntu 22.04) does not work. It seems like something else is needed, or something additional.

If it is now necessary to specify some options that were not needed with 0.2.3, it would be great if the .service file had support for EnvironmentFile so that people wouldn't need to fiddle with the systemd unit files directly.

But more important -- could the documentation get updated to describe how to achieve the exact 0.2.3 setup? I don't really care which plugin is used, I primary setup the K3s cluster to run GitHub Actions CI tests, so whichever was used and worked should likely be documented, if it cannot be the default anymore.

[evol262]

I just filed #104 where I describe that with 0.2.4, even explicitly setting cni (on Ubuntu 22.04) does not work. It seems like something else is needed, or something additional.

If it is now necessary to specify some options that were not needed with 0.2.3, it would be great if the .service file had support for EnvironmentFile so that people wouldn't need to fiddle with the systemd unit files directly.

I'm replying on the issue separately, but the change in 0.2.4 was actually to, essentially, swap --network-plugin=<none> to --network-plugin=kubenet (on Linux) or --network-plugin=cni on Windows. This patch is just to align them both, since kubenet is also going the way of the dinosaur.

Since, with 1.24, specifying a CNI is more or less required, the intention of the patch was actually to prevent needing to add a --network-plugin= flag, and with this one, to just to default to cni everywhere to match it up.

#102 was explicitly opened to make for a better configuration experience.

But more important -- could the documentation get updated to describe how to achieve the exact 0.2.3 setup? I don't really care which plugin is used, I primary setup the K3s cluster to run GitHub Actions CI tests, so whichever was used and worked should likely be documented, if it cannot be the default anymore.

Passing --network-plugin= with no arguments whatsoever will still go back to no-op. To make this easier, let me push a patch which adds no-op as a possibility, so --network-plugin=no-op will give the 0.2.3 behavior.

This would change your script to:

... sed 's/ExecStart=.*/& --network-plugin=no-op/;t;d' ...

@evol262 : I don't fully understand why changing to CNI involves updating to go1.19 ?

0470631

Seems like a separate change, one that quite recently hit Kubernetes (while still in RC)...

Surprisingly, considering when 1.19 released, the e2e tests from upstream don't build on 1.18.

I think the user needed to install the cni plugins, either way ? (for both "kubenet", and for "cni")

NetworkReady=false reason:NetworkPluginNotReady message:docker: network plugin is not ready: could not locate kubenet required CNI plugins [bridge host-local loopback] at [\"/opt/cni/bin\" \"/opt/cni/bin\"]

Yes. Well, since kubenet removal, anyway. containerd is also expecting to remove it (they also appear to be a little behind). With 1.24, it's pretty much "CNI or bust".

With 1.24 as the first upstream release where dockershim is really gone from upstream, it's a sane starting point for compatibility as the last "prerelease" version (if 0.1 was the handeover from upstream, and 0.2 was the rewrite/refactor/cleanup for 1.24) to expect that cri-dockerd users are likely to be on 1.24, and not have kubenet available out of the box (unless it was packaged here, which I'd be happy to avoid), and will install some CNI as a mandatory part of cluster bringup.

The main difference is that they would now also need to install something in /etc/cni/net.d ?

The code changes on the minikube end would be the same, as long as it is not "no-op".

There was an unfortunate shortcut, that avoided updating the config for "known" config...

	if networkPlugin == "" {
		// no-op plugin
		return nil
	}

It now needs to always update the cri-docker systemd unit file, with the users choice.

As above, the hope is that users will need to update only if they do not want cni for some reason, but that's the exception rather than the rule.

And it would still be crash-looping, unless providing some default "bridge" configuration

It doesn't quite crashloop. Well, cri-dockerd doesn't crashloop if the paths don't exist and it's cni. It waits for the CNI to be provisioned by the appropriate installer, then the cluster will become ready.

[adelton]

This would change your script to:
... sed 's/ExecStart=.*/& --network-plugin=no-op/;t;d' ...

Actually, I found out that plain

... sed 's/ExecStart=.*/& --network-plugin=/;t;d'  ...

with no value for the parameter seems to work as well on 0.2.4.

[afbjorklund]

With 1.24 as the first upstream release where dockershim is really gone from upstream, it's a sane starting point for compatibility as the last "prerelease" version (if 0.1 was the handeover from upstream, and 0.2 was the rewrite/refactor/cleanup for 1.24) to expect that cri-dockerd users are likely to be on 1.24, and not have kubenet available out of the box (unless it was packaged here, which I'd be happy to avoid), and will install some CNI as a mandatory part of cluster bringup.

It would have made sense, if 1.24 had made both CRI and CNI mandatory - but in the end, they did not.
In fact, this change is now not going to make it for 1.25 either - but for 1.26 it seems to be reasonable...

So I am not going to change anything wrt minikube, it would still run without CNI - unless requested to.

We offer --cni=bridge, for users that don't really care or just want something simple that works locally.
For multi-node clusters, I would still recommend flannel (with vxlan) - even if it is not part of the "plugins"

https://github.com/kubernetes/minikube/blob/master/pkg/minikube/cni/bridge.go

// bridge is what minikube defaulted to when `--enable-default-cni=true`

In fact, even previous releases were broken when using containerd - for precisely this issue (i.e. no CNI)

---

It is OK if cri-dockerd recommends using CNI, but it would have to be separately configured in minikube.

It doesn't quite crashloop

Right, I think the cluster will be up - it is the CoreDNS pod that will fail to appear. And it is documented...

I will revisit this during autumn, maybe as a part of upgrading kubernetes-cni from the current 0.8.7 🕸️

[afbjorklund]

will revisit this during autumn, maybe as a part of upgrading kubernetes-cni from the current 0.8.7

changed it so that minikube will use cni when it uses cri, i.e. when not using dockershim (< k8s 1.24)