operator: Add cilium node garbage collector

[sayboras]

Description

In the normal scenario, CiliumNode is created by agent with owner
references attached all time time in below PR[0]. However, there could
be the case that CiliumNode is created by IPAM module[1], which
didn't have any ownerReferences at all. For this case, if the
corresponding node got terminated and never came back with same
name, the CiliumNode resource is still dangling, and needs to be
garbage collected.

This commit is to add garbage collector for CiliumNode, with below
logic:

• Gargage collector will run every predefined interval (e.g. specify
  by flag --nodes-gc-interval)
• Each run will check if CiliumNode is having a counterpart k8s node
  resource. Also, remove this node from GC candidate if required.
• If yes, CiliumNode is considered as valid, happy day.
• If no, check if ownerReferences are set.
  • If yes, let k8s perform garbage collection.
  • If no, mark the node as GC candidate. If in the next run, this
    node is still in GC candidate, remove it.

Testing

Testing was done locally with kind cluster

Create one dummy ciliumnode and check gc

$ ksyslo deployment/cilium-operator --timestamps | grep kind-worker-dummy
2022-04-26T10:59:22.106774489Z level=debug msg="UpdatedStatus Node" error="<nil>" node-name=kind-worker-dummy subsys=pod-cidr
2022-04-26T10:59:23.087007602Z level=debug msg="UpdatedStatus Node" error="<nil>" node-name=kind-worker-dummy subsys=pod-cidr
2022-04-26T10:59:51.977272944Z level=info msg="Add CiliumNode to garbage collector candidates" nodeName=kind-worker-dummy subsys=watchers
2022-04-26T11:00:21.978387751Z level=info msg="Perform GC for invalid CiliumNode" nodeName=kind-worker-dummy subsys=watchers

$ kg ciliumnodes
NAME                 AGE
kind-control-plane   156m
kind-worker          156m

[christarazi]

Changes look good, just a few minor comments

[kaworu]

Seeing that we are selectively adding RBAC permissions and passing the nodes-gc-interval flag through the ConfigMap, I'm assuming there is a use-case for disabling this feature. In what context users would want to disable it?

Follow-up question: how are users expected to effectively disable this feature through Helm? I think the combination of the flag being a duration, the default value, and different ways of checking it (hasKey in the ConfigMap, "emptiness" in the ClusterRole) might not work as expected:

% helm template cilium ./install/kubernetes/cilium --namespace kube-system | grep -E "nodes-gc-interval|# To perform CiliumNode garbage collector"
  nodes-gc-interval: "5m0s"
    # To perform CiliumNode garbage collector
% helm template cilium ./install/kubernetes/cilium --namespace kube-system --set operator.nodeGCInterval=1s | grep -E "nodes-gc-interval|# To perform CiliumNode garbage collector"
  nodes-gc-interval: "1s"
    # To perform CiliumNode garbage collector

Here the first grep expression is for the flag value, second expression for RBAC. Using the default and override will correctly set the flag and add the RBAC permission, happy day™.

% helm template cilium ./install/kubernetes/cilium --namespace kube-system --set operator.nodeGCInterval=null | grep -E "nodes-gc-interval|# To perform CiliumNode garbage collector"
% helm template cilium ./install/kubernetes/cilium --namespace kube-system --set operator.nodeGCInterval= | grep -E "nodes-gc-interval|# To perform CiliumNode garbage collector"
  nodes-gc-interval: ""
% helm template cilium ./install/kubernetes/cilium --namespace kube-system --set operator.nodeGCInterval=0s | grep -E "nodes-gc-interval|# To perform CiliumNode garbage collector"
  nodes-gc-interval: "0s"
    # To perform CiliumNode garbage collector

Settting to null shows nothing, meaning we don't pass the flag and we don't have the RBAC permission. The issue here is that we'll be using the non-zero CLI default (5m) and will fail because we don't have the permission.

Settting to empty will actually pass it as-is to the flag, which will fail duration parsing.

Settting to 0s will correctly disable the feature in the ConfigMap, but won't add the RBAC permission.

[sayboras]

Thanks for your input, indeed there are a few cases I didn't consider.

Seeing that we are selectively adding RBAC permissions and passing the nodes-gc-interval flag through the ConfigMap, I'm assuming there is a use-case for disabling this feature. In what context users would want to disable it?

Normally, we don't want to disable this GC, it's just an option for users in case of any issue later on.

Settting to null shows nothing, meaning we don't pass the flag and we don't have the RBAC permission. The issue here is that we'll be using the non-zero CLI default (5m) and will fail because we don't have the permission.
Settting to empty will actually pass it as-is to the flag, which will fail duration parsing.
Settting to 0s will correctly disable the feature in the ConfigMap, but won't add the RBAC permission.

Agreed on the above points, I have made below changes to rectify

• update default value in CLI to 0s i.e disabled by default, so that we don't couple with RBAC permission
• add default value in config map
• add check in RBAC to make sure that we only add permission if .Values.operator.nodeGCInterval is having some value, and it's not 0s. I don't think we need to check for correct data type (e.g. duration) in helm.

$ helm template cilium ./install/kubernetes/cilium --namespace kube-system --set operator.nodeGCInterval=null | grep -E "nodes-gc-interval|# To perform CiliumNode garbage collector"

$ helm template cilium ./install/kubernetes/cilium --namespace kube-system --set operator.nodeGCInterval= | grep -E "nodes-gc-interval|# To perform CiliumNode garbage collector" 
  nodes-gc-interval: "0s"
 
$ helm template cilium ./install/kubernetes/cilium --namespace kube-system --set operator.nodeGCInterval=0s | grep -E "nodes-gc-interval|# To perform CiliumNode garbage collector"
  nodes-gc-interval: "0s"
  
$ helm template cilium ./install/kubernetes/cilium --namespace kube-system --set operator.nodeGCInterval=1s | grep -E "nodes-gc-interval|# To perform CiliumNode garbage collector"
  nodes-gc-interval: "1s"
    # To perform CiliumNode garbage collector
  

[sayboras]

/test-1.16-4.9

[sayboras]

/test-1.22-4.19

[sayboras]

/test-1.23-net-next

[sayboras]

/test-1.21-5.4

[sayboras]

All the tests passed now, marking this ready for merge.