clustermesh: Ignore symlink files on fsnotify events

[tgraf]

Kubernetes secrets are mapped into the pod using symlinks. The initial
scan was already correctly ignoring symlinks but the fsnotify events
have not been. This has resulted in invalid cluster configurations being
added:

ClusterMesh:            0/3 clusters ready, 0 global-services
   cluster2: not-ready, 0 nodes, 0 identities, 0 services, 0 failures (last: never)
   └  Waiting for initial connection to be established
   ..2021_01_08_21_11_57.892158678: not-ready, 0 nodes, 0 identities, 0 services, 0 failures (last: never)
   └  Waiting for initial connection to be established
   ..data: not-ready, 0 nodes, 0 identities, 0 services, 0 failures (last: never)
   └  Waiting for initial connection to be established

Fixes: 076b018 ("Inter cluster connectivity (ClusterMesh)")

[aanm]

@kaworu can we reuse the same code done for hubble-relay certificates in here? It looks to me the code use in both places have the same functionalities

[tgraf]

test-me-please

[tgraf]

@kaworu can we reuse the same code done for hubble-relay certificates in here? It looks to me the code use in both places have the same functionalities

I agree that's a good refactoring but we should fix the existing code first so we back backport the minimal fix.

[tgraf]

retest-gke

[kaworu]

@kaworu can we reuse the same code done for hubble-relay certificates in here? It looks to me the code use in both places have the same functionalities

If I understand correctly, the use case is a little different between clustermesh and hubble mTLS. In the clustermesh case we don't know what paths are going to show up, whereas in the Hubble mTLS case we know all the paths we should watch at startup. Although this should not be blocking (if correct) the change is probably not trivial and separate warrant a refactor as @tgraf suggested.

Question: here we are watching the symlinks under /var/lib/cilium/clustermesh and avoiding the files pointed to in the /var/lib/cilium/clustermesh/..2021_01_11_12_28_37.877026628/ directory correct? If yes, do we get update events when the backing files change? I believe we don't, and that is why pkg/crypto/certloader/fswatcher is complex. cc @gandro

[gandro]

Question: here we are watching the symlinks under /var/lib/cilium/clustermesh and avoiding the files pointed to in the /var/lib/cilium/clustermesh/..2021_01_11_12_28_37.877026628/ directory correct? If yes, do we get update events when the backing files change? I believe we don't, and that is why pkg/crypto/certloader/fswatcher is complex. cc @gandro

Yes, I agree with your statement: Because K8s uses indirect symlinks (see example below), the code here will observe cluster file being added and clusters being removed, but not cluster files being modified. Not sure how much of an issue that is, I lack a bit conext when it comes to clustermesh.

As a reminder, the symlink structure of K8s is as follows:

/var/lib/cilium/clustermesh/..data -> ..2021_01_01_01_01_01.11111111
/var/lib/cilium/clustermesh/cluster1 -> ..data/cluster1

Thus /var/lib/cilium/clustermesh/cluster1 is an indirect symlink that resolves to /var/lib/cilium/clustermesh/..data1/cluster1 which resolves to /var/lib/cilium/clustermesh/..2021_01_01_01_01_01.11111111/cluster1

When K8s updates the secret mount (e.g. when a cluster file is modified), the new version of each file is copied into a new directory called ..2021_02_02_02_02_02.22222222 and the ..data symlink is redirected:

/var/lib/cilium/clustermesh/..data -> ..2021_02_02_02_02_02.22222222

Now the /var/lib/cilium/clustermesh/cluster1 symlink now resolves to /var/lib/cilium/clustermesh/..2021_02_02_02_02_02.22222222/cluster1 even though the cluster1 symlink itself has not been touched at all. This update is missed by the fsnotify code here.

The old and new code here will pick up the creation and deletion of the /var/lib/cilium/clustermesh/cluster1 symlink, but not the rewrite of the ..data symlink.

However: I believe the fact that cluster file modifications are not tracked is already present in the old version of the code (because the ..foo directories are not watched in either version), so in that sense this PR does not change the behavior in this regard

[tgraf]

Yes, the lack of notification is potentially an issue although the primary use case is to detect new files and deletions as modifications will be extremely rare. The cluster name to IP mapping is not in the secret but in the hostAliases of the DaemonSet itself. A change in the secrets would only occur if a cluster changes its TLS certificates.

[gandro]

GKE scaling failure https://jenkins.cilium.io/job/Cilium-PR-K8s-GKE/3892/

retest-gke