Drop "interface" mode in linkerd-cni

[alpeb]

"interface" mode was ill-conceived, in that it wasn't providing full network capabilities as supposed, and so pods provisioned with linkerd-cni in this mode weren't having their network properly set up. This change removes that mode, leaving only the "chained" mode, where linkerd-cni waits for another CNI plugin to drop its config in the target directory, over which linkerd-cni will append its config. It's important to emphasize that no config will be generated until another CNI plugin adds its own, as to not trigger pod scheduling while there is no full CNI network config available.

Also added proper logging timestamps.

Tests

This was successfully tested in k3d, AKS, EKS and GKE (with Calico). GKE with its default CNI still has issues as pointed in the Followup below.

The following log is from having installed linkerd-cni in an node that already a CNI plugin running:

$ k -n linkerd-cni logs -f linkerd-cni-hznp2 install-cni
[2023-05-17 08:50:46] Wrote linkerd CNI binaries to /host/home/kubernetes/bin
[2023-05-17 08:50:46] Installing CNI configuration for /host/etc/cni/net.d/10-calico.conflist
[2023-05-17 08:50:46] Using CNI config template from CNI_NETWORK_CONFIG environment variable.
      "k8s_api_root": "https://__KUBERNETES_SERVICE_HOST__:__KUBERNETES_SERVICE_PORT__",
      "k8s_api_root": "https://10.60.0.1:__KUBERNETES_SERVICE_PORT__",
[2023-05-17 08:50:46] CNI config: {
  "name": "linkerd-cni",
  "type": "linkerd-cni",
  "log_level": "info",
  "policy": {
      "type": "k8s",
      "k8s_api_root": "https://10.60.0.1:443",
      "k8s_auth_token": "__SERVICEACCOUNT_TOKEN__"
  },
  "kubernetes": {
      "kubeconfig": "/etc/cni/net.d/ZZZ-linkerd-cni-kubeconfig"
  },
  "linkerd": {
    "incoming-proxy-port": 4143,
    "outgoing-proxy-port": 4140,
    "proxy-uid": 2102,
    "ports-to-redirect": [],
    "inbound-ports-to-ignore": ["4191","4190"],
    "simulate": false,
    "use-wait-flag": false
  }
}
[2023-05-17 08:50:47] Created CNI config /host/etc/cni/net.d/10-calico.conflist
Setting up watches.
Watches established.

After adding an extra node, the linkerd-cni DaemonSet starts and we can see it waits until another CNI plugin drops its config:

$ k -n linkerd-cni logs -f linkerd-cni-tvv6r
[2023-05-17 08:58:12] Wrote linkerd CNI binaries to /host/home/kubernetes/bin
[2023-05-17 08:58:12] No active CNI configuration files found
Setting up watches.
Watches established.
[2023-05-17 08:58:22] Detected change in /host/etc/cni/net.d/: CREATE 10-calico.conflist
[2023-05-17 08:58:22] New file [10-calico.conflist] detected; re-installing
[2023-05-17 08:58:22] Using CNI config template from CNI_NETWORK_CONFIG environment variable.
      "k8s_api_root": "https://__KUBERNETES_SERVICE_HOST__:__KUBERNETES_SERVICE_PORT__",
      "k8s_api_root": "https://10.60.0.1:__KUBERNETES_SERVICE_PORT__",
[2023-05-17 08:58:22] CNI config: {
  "name": "linkerd-cni",
  "type": "linkerd-cni",
  "log_level": "info",
  "policy": {
      "type": "k8s",
      "k8s_api_root": "https://10.60.0.1:443",
      "k8s_auth_token": "__SERVICEACCOUNT_TOKEN__"
  },
  "kubernetes": {
      "kubeconfig": "/etc/cni/net.d/ZZZ-linkerd-cni-kubeconfig"
  },
  "linkerd": {
    "incoming-proxy-port": 4143,
    "outgoing-proxy-port": 4140,
    "proxy-uid": 2102,
    "ports-to-redirect": [],
    "inbound-ports-to-ignore": ["4191","4190"],
    "simulate": false,
    "use-wait-flag": false
  }
}
[2023-05-17 08:58:22] Created CNI config /host/etc/cni/net.d/10-calico.conflist
[2023-05-17 08:58:22] Detected change in /host/etc/cni/net.d/: DELETE 10-calico.conflist
[2023-05-17 08:58:22] Detected change in /host/etc/cni/net.d/: CREATE 10-calico.conflist
[2023-05-17 08:58:22] Ignoring event: CREATE /host/etc/cni/net.d/10-calico.conflist; no real changes detected

Followup

This doesn't fix another class of problem where pods start to get scheduled after the first CNI plugin config is available, but before the linkerd-cni DaemonSet gets a chance to append its config. This results in the Pod not getting the iptables rules set in time. The injected linkerd-network-validator will catch that and fail the pod. To acquire proper network config, the pod needs to be externally bounced (manually or through an operator).

This happens in GKE with its default CNI plugin when the node pool is scaled.

[mateiidavid]

For some reason when I try it out without any CNI installed, it errors out... not sure what's happening since there are no logs. Think this might all be my fault but I want to dig into it so I can make sure my tests didn't uncover anything.

For the record, I'd expect that if no CNI plugin exists it will simply exist in a vacuum until one appears.

NAME                READY   STATUS             RESTARTS      AGE
linkerd-cni-gmgf7   0/1     CrashLoopBackOff   5 (48s ago)   3m56s


:; k logs -n linkerd-cni linkerd-cni-gmgf7
E0519 15:59:46.376582 1086953 memcache.go:255] couldn't get resource list for metrics.k8s.io/v1beta1: the server is currently unable to handle the request
E0519 15:59:46.381207 1086953 memcache.go:106] couldn't get resource list for metrics.k8s.io/v1beta1: the server is currently unable to handle the request
E0519 15:59:46.382144 1086953 memcache.go:106] couldn't get resource list for metrics.k8s.io/v1beta1: the server is currently unable to handle the request

[mateiidavid]

hmm why not just delete DEFAULT_CNI_CONF_PATH?

[mateiidavid]

Is this for backwards compatibility? i.e. to clean up unused files? if so, it should automatically be cleaned up when the pod is restarted so it might not be necessary?

[alpeb]

Yes I had BC in mind. Isn't this the host filesystem, so the pod restart won't touch that? Or you mean it should be cleaned up by this plugin's logic?

[mateiidavid]

Hmmm, here's how I initially thought this through:

linkerd2-proxy-init/cni-plugin/deployment/scripts/install-cni.sh

Lines 91 to 95 in 15512bc

	# and if so, rm it.
	if [ -e "${DEFAULT_CNI_CONF_PATH}" ]; then
	echo "Cleaning up ${DEFAULT_CNI_CONF_PATH}"
	rm -f "${DEFAULT_CNI_CONF_PATH}"
	fi

the logic above will clean-up 01-linkerd.conf.

The trap call should trigger the clean-up function whenever a signal is received

linkerd2-proxy-init/cni-plugin/deployment/scripts/install-cni.sh

Lines 111 to 114 in 15512bc

	# Capture the usual signals and exit from the script
	trap 'echo "SIGINT received, simply exiting..."; cleanup' INT
	trap 'echo "SIGTERM received, simply exiting..."; cleanup' TERM
	trap 'echo "SIGHUP received, simply exiting..."; cleanup' HUP

When doing an upgrade to a new cni version:

1. We update the manifest
2. Triggers a re-deploy
3. Old pod will begin shutting down, since it's on the old version, the script should correctly remove 01-linkerd.conf as part of the clean-up function
4. New pod will be created and it will block until a file is found.

In my mind it should work, however I wonder if we're still liable to get a race condition... perhaps we should have a TODO instead to remove it after one release (we guarantee BC for successive upgrades, should be fine to remove at that point).

wdyt?

[alpeb]

If the pod shuts down uncleanly that file will be left around, so we'd still have to keep some logic around to clean it up at startup, right? I'll add a comment to remember to remove that rm call at some point 👍

[alpeb]

For some reason when I try it out without any CNI installed, it errors out... not sure what's happening since there are no logs. Think this might all be my fault but I want to dig into it so I can make sure my tests didn't uncover anything.

Hmmm I haven't been able to repro that..

$ k3d cluster create --k3s-arg --flannel-backend=none@server:0
...
$ linkerd install-cni --cni-image ghcr.io/alpeb/cni-plugin --cni-image-version v1.2.8 |k apply -f -
...
$ k -n linkerd-cni logs -f linkerd-cni-b57l2
[2023-05-19 21:52:24] Wrote linkerd CNI binaries to /host/opt/cni/bin
[2023-05-19 21:52:24] No active CNI configuration files found
Setting up watches.
Watches established.

[mateiidavid]

Gotcha:

[2023-05-24 13:58:49] Wrote linkerd CNI binaries to /host/opt/cni/bin
[2023-05-24 13:58:49] No active CNI configuration files found

seems to be fine.

[mateiidavid]

Great work! Still a nitpick left with the default path TODO, I'll leave that to you and will defer to your judgement. Everything else looks good to me 👍🏻