Fix deletion of tunnel map entries when node has non-zero cluster ID.

[giorio94]

Given that tunnel mapping entries are currently inserted without specifying the cluster ID, we should do the same during the removal phase. Otherwise, we fail to remove the entries if the node is associated with a non-zero cluster ID. Let's additionally update the test so that it would catch future regressions.

Fix deletion of tunnel map entries when node has non-zero cluster ID.

[giorio94]

/test

[brb]

I wonder why we didn't see this in the logs earlier. From my reading, under the hood, the deleteTunnelMapping() invokes BPF_MAP_DELETE_ELEM which should return an error if the entry doesn't exist.

[giorio94]

I wonder why we didn't see this in the logs earlier. From my reading, under the hood, the deleteTunnelMapping() invokes BPF_MAP_DELETE_ELEM which should return an error if the entry doesn't exist.

My guess is that it is because we don't remove nodes during the clustermesh tests, and in all the other cases we are likely not setting a ClusterID different from zero (as for instance in the unit test that I modified). I had tested this bug in a local kind environment, and indeed the error log was output after deleting a CiliumNode resource.

[brb]

I had tested this bug in a local kind environment, and indeed the error log was output after deleting a CiliumNode resource.

👍

Do you see any risk with changing the approach to propagating the CID to the tunnel map when adding/deleting instead of passing it as zero?

[giorio94]

I had tested this bug in a local kind environment, and indeed the error log was output after deleting a CiliumNode resource.

+1

Do you see any risk with changing the approach to propagating the CID to the tunnel map when adding/deleting instead of passing it as zero?

My understanding is that the datapath is currently doing the lookups using CID=0, so that would not work.