Remove CiliumNode deletion logic from CiliumNode watcher and guarantee CiliumNode's OwnerReference is always set

[christarazi]

First commit:

We don't need to implement this logic for two reasons:

1. We rely on CiliumNode resources to be deleted / cleaned up by
   attaching the corresponding K8s Node as an ownerReference in the
   CiliumNode.
2. It is redundant to delete the CiliumNode in response to an
   event...of the CiliumNode deletion itself.

In very rare cases, this logic can actually delete a newly created
CiliumNode by accident (see example below). Instead, keep all deletion
logic besides the actual K8s API calls (DELETE) and perform a Get() to
ensure that it's been deleted. Otherwise, log to the user that the
resource may still exist.

Example:
Say an existing node was deleted and then recreated in
quick succession with the same name. When the node is recreated, the
agent will be scheduled on it. During bootstrap it'll create a
corresponding CiliumNode resource. Given that only one Operator is
operational at any time in a cluster, it is already running on another
node in the cluster. The node-delete event will first delete the K8s
node and then trigger a CN-delete via reason 1 from above. It is
possible for the CN-delete event to be delayed such that it is received
after the node-create event (the recreate). When the CN-delete event is
received by the already-running Operator, the CiliumNode watcher logic
will then trigger (erroneously) another CN-delete, thereby deleting the
CiliumNode resource while the K8s node is still alive.

Second commit:

It is impossible to set the OwnerReference if we fail to fetch the
corresponding Kubernetes Node and the existing CiliumNode resource
doesn't already have it set. We can rely the OwnerReference to be set
because this logic was added in v1.6, which is sufficiently earlier
version of Cilium. 1

The reason for doing this is to ensure that the OwnerReference can
always be set. If we cannot, this should be treated as an error and we
shouldn't proceed. Cilium should not run in an environment where the
Kubernetes Node resource is missing.

[christarazi]

test-me-please

[pchaigno]

The Travis test failure seems legitimate... and not very reassuring. There's actually a test (TestNewNodesPodCIDRManager) that expects two calls to k8sNode's OnDelete when receiving a k8sOpDelete. Makes me wonder if all this is on purpose for some reason :-/

[christarazi]

test-me-please

[pchaigno]

Filed #17336 for that flake:
ci-l4lb

[pchaigno]

Filed #17337 for that flake:
ci-eks

[tgraf]

So far, we are not guaranteeing the that ownerReference is set:

        // Tie the CiliumNode custom resource lifecycle to the lifecycle of the
        // Kubernetes node
        if k8sNode, err := n.k8sNodeGetter.GetK8sNode(context.TODO(), nodeTypes.GetName()); err != nil {
                log.WithError(err).Warning("Kubernetes node resource representing own node is not available, cannot set OwnerReference")
        } else {

We would have to fail bootstrapping of the node in this case, otherwise we might have a silent failure to set the ownerReference and a leak of the CiliumNode.

I can't think of a reason to node fail on failure to retrieve the k8s node. I would propose the following logic:
In mutateNodeResource():

• Fail if n.k8sNodeGetter.GetK8sNode() fails and nodeResource.ObjectMeta.OwnerReferences is not already set.

[christarazi]

Thanks @tgraf, good suggestion. I added the commit to guarantee we always have an ownerReference.