CNI isn't required to be installed in kube-system on OpenShift

[jwendell]

No description provided.

[jwendell]

In theory we can deploy CNI in any namespace now.

I'm not sure if this is safe regarding upgrades though. If yes, I guess we need a release note for this?

If it will bring problems during upgrades, we can discard this PR and stick with kube-system forever.

@luksa @howardjohn thoughts?

[costinm]

Last time I tried - cni in kube-system and istio-system fought with each other to install as last component in the cni config. Not sure if it changed. Since all installs and tests are on istio-system - it is worth migrating or using that for new clusters. But you first must uninstall kube-system cni - if you can. The current ambient docs are written from fresh install on clean cluster perspective - I assume next release we'll focus on existing clusters and such cases. Will be a large problem in GKE where we have auto-managed CNI in kube-system for ASM users. Please keep me in the loop.

…

On Fri, May 10, 2024, 06:27 Jonh Wendell ***@***.***> wrote: In theory we can deploy CNI in any namespace now. I'm not sure if this is safe regarding upgrades though. If yes, I guess we need a release note for this? If it will bring problems during upgrades, we can discard this PR and stick with kube-system forever. @luksa <https://github.com/luksa> @howardjohn <https://github.com/howardjohn> thoughts? — Reply to this email directly, view it on GitHub <#50984 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAUR2QBJZWX6QMZKKTT2PDZBTDMTAVCNFSM6AAAAABHQUI5USVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBUGYYTAMJXGU> . You are receiving this because your review was requested.Message ID: ***@***.***>

[howardjohn]

This will break upgrades in a terrible way (constantly reconciling over each other). I don't think we should do this without more work somehow to make it safe

[costinm]

Fortunately helm will not install because the cluster resources are already present. The only safe upgrade I found so far is by using affinity and node labels - coincidentally this is also a good way to canary and upgrade ztunnel. But needs few changes to the charts - we should discuss in the WG. There are some ideas to make it more flexible - we do a lot more than we should in the node ( considering how much the pod can do), but it's a very risky are - code is complex and many things can break. IMO best choice is to consider splitting things up a bit, in particular the installed component which is the real issue.

…

On Fri, May 10, 2024, 07:17 John Howard ***@***.***> wrote: ***@***.**** commented on this pull request. This will break upgrades in a terrible way (constantly reconciling over each other). I don't think we should do this without more work somehow to make it safe — Reply to this email directly, view it on GitHub <#50984 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAUR2SSFDEUBMWJV7NA4XDZBTJFZAVCNFSM6AAAAABHQUI5USVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJQGE3TMNZSHA> . You are receiving this because your review was requested.Message ID: ***@***.***>

[bleggett]

It is not safe to install the same daemonset 2x in k8s as a general rule - that's why it's a daemonset (arguably).

Fortunately helm will not install because the cluster resources are already
present.

Yes, that's what I'd expect.

In theory we can deploy CNI in any namespace now.

We can, but we cannot deploy multiple CNI instances to different namespaces at the same time - that's both not allowed by Helm, and not really rational K8S practice - and not something we will ever support, I think.

See also #49009

[costinm]

On Tue, May 14, 2024, 10:09 Ben Leggett ***@***.***> wrote: It is not safe to install the same daemonset 2x in k8s as a general rule - that's why it's a daemonset.

Right. But you can have a daemon set that is only installed on select nodes, and another one selecting a different node pool. Unfortunately we have no way to detect another istio-derived cni in a different namespace, or another CNI doing 'stuff'. Fortunately helm will not install because the cluster resources are already

present. Yes, that's what I'd expect.

Istioctl or helm template doesn't have those problems... In theory we can deploy CNI in any namespace now.

We can, but we cannot deploy multiple CNIs to different namespaces at the same time - that's both not allowed by Helm, and not really rational K8S practice.

It's more complex - for example in GKE we auto install in kube-system, and with helm template. Others may do the same. Depending on order you may or may not be broken. Also it's not a problem if each CNI targets different pods. There is a project ( forgot the name) allowing pods to select one or more cni sets ( to enable multiple net interfaces in different protocols). We need to gradually improve our install story from 'hope only one cni is messing with network' —

…

Reply to this email directly, view it on GitHub <#50984 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAUR2T44EYYTSKRQJMPO2LZCJAM3AVCNFSM6AAAAABHQUI5USVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJQG4ZDQMRZGA> . You are receiving this because your review was requested.Message ID: ***@***.***>

[bleggett]

Istioctl or helm template doesn't have those problems...

Agreed, that's our special problem :D

It's more complex - for example in GKE we auto install in kube-system, and
with helm template. Others may do the same. Depending on order you may or may not be broken.

Auto-installing to a namespace in a cluster with an existing Istio install without checking if you should is probably a vendor caveat-emptor situation.

Also it's not a problem if each CNI targets different pods. There is a
project ( forgot the name) allowing pods to select one or more cni sets (
to enable multiple net interfaces in different protocols).

I've seen proposals like that, but in general I think the UX of selecting CNI at the pod level sucks, is not a user concern, and is a bad idea (it's convenient for some kinds of operators but it doesn't solve any real user-facing problems and creates quite a few user-facing footguns)

We need to gradually improve our install story from 'hope only one cni is
messing with network'

I don't think we need to improve it from 'only one istio-cni MAY mess with the network at a time', unless I'm missing something.

[costinm]

On Tue, May 14, 2024 at 11:11 AM Ben Leggett ***@***.***> wrote: Istioctl or helm template doesn't have those problems... Agreed, that's our special problem :D

I think our real problem is not 'helm template not accidentally protecting us' ( as helm install does ). The code in istio-cni dealing with install is ancient and over complicated - creating jwt tokens, not doing checks if other versions are installed, etc. I tried to do some changes but gave up - IMO best option is to rewrite the installer part and what runs on the host. One of the best things about the ambient changes in CNI is that the CNI pod can enter namespaces and fix, it doesn't depend on the host portion of the code doing 'stuff' - we can have a tiny program that just sends the params over UDS to one or more sockets. Then we can have as many CNI pods as we want - as long as they use different labels or check if someone else already did the setup, or apply incremental/independent changes. Unfortunately the code used for sidecars is even more complicated and slightly different from what ambient is using... So I'm still trying to find a simple solution - on some environments users can't just install their own CNI ( GKE autopilot for example, probably others that have strict restrictions on the host access ).

It's more complex - for example in GKE we auto install in kube-system, and with helm template. Others may do the same. Depending on order you may or may not be broken. Auto-installing to a namespace in a cluster with an existing Istio install without checking if you should is probably a vendor caveat-emptor situation.

The problem is users attempting to install while a vendor already did that. If the vendor doesn't block it - bad things happen. Perhaps we should add more warnings and checks to the docs.

Also it's not a problem if each CNI targets different pods. There is a project ( forgot the name) allowing pods to select one or more cni sets ( to enable multiple net interfaces in different protocols). I've seen proposals like that, but in general I think the UX of selecting CNI at the pod level sucks, is not a user concern, and is a bad idea (though it is convenient for some types of operators)

I don't disagree. Messing with CNI should not be possible for a user IMO, but be part of the platform and heavily locked.

We need to gradually improve our install story from 'hope only one cni is messing with network' I don't think we need to improve it from 'only one istio-cni MAY mess with the network at a time', unless I'm missing something

The definition of "one istio-cni" is a bit unclear. There are forks of Istio, other projects may reuse the code or model, vendors may have their own install ( GKE autopilot installs in kube-system in some cases - at least it also blocks any other install )