Do not reconcile egressIPPod objects that are being deleted when the namespace no longer exists

[kyrtapz]

When a namespace with pods gets removed and the pod removal event is handled after the namespace is already gone, we should ignore the NotFound error in reconcileEgressIPPod. Any potential configuration will get removed in reconcileEgressIPNamespace.

Found in downstream CI, tracked in https://issues.redhat.com/browse/OCPBUGS-15804
Example error:

I0707 01:15:21.862524       1 namespace.go:260] [e2e-netpol-z-1913] deleting namespace
...
E0707 01:15:21.894356       1 obj_retry.go:680] Failed to delete *factory.egressIPPod e2e-netpol-z-1913/b, error: namespace "e2e-netpol-z-1913" not found
I0707 01:15:21.894452       1 pods.go:100] Deleting pod: e2e-netpol-z-1913/b
E0707 01:15:21.904170       1 obj_retry.go:680] Failed to delete *factory.egressIPPod e2e-netpol-z-1913/c, error: namespace "e2e-netpol-z-1913" not found
I0707 01:15:21.904206       1 pods.go:100] Deleting pod: e2e-netpol-z-1913/c

/cc @tssurya @jcaamano

[tssurya]

hey @flavio-fernandes : dualstack is failing again on IC: https://github.com/ovn-org/ovn-kubernetes/actions/runs/5483719976/jobs/9990837646?pr=3751 PTAL! I really don't want to reopen our tracker card again HAHA
@kyrtapz you are rebased on #3724 i guess

[tssurya]

hope we have coverage for this somewhere in our UTs? I am afraid each if condition we are adding is now for a bug fix (another sample: #3692 ; these should have tests...) and don't want someone in future (like i did for IC ROFL) to come and change these :D - won't hold the PR though for missing UT.

[tssurya]

@kyrtapz: what about the other paths? reconcileEgressIP might get called and let's say pod is not found or namespace is not found.. are there other places where this protection needs to be added? (Having said that each if in egressIP code at this point is a specific bug fix so its like walking on shells if we ignore something we shouldn't be..at long as at least one path handles the deletion we should be safe).

[kyrtapz]

reconcileEgressIPPod doesn't fetch a pod so it won't be directly affected by pod not existing anymore.
As for namespace not found I think we should still return an error on update since this seems like a messed up setup if there are pods running in non-existing namespaces.

[kyrtapz]

hope we have coverage for this somewhere in our UTs? I am afraid each if condition we are adding is now for a bug fix (another sample: #3692 ; these should have tests...) and don't want someone in future (like i did for IC ROFL) to come and changes these :D - won't hold the PR though for missing UT.

I can try think of how to trigger this specific issue in our tests but I think we might need to get that PR in first as it is affecting CI.

[dcbw]

So we don't forget... test could Namespace("foo").Delete() via the fake client which shouldn't trigger any additional events because it's not a real apiserver.

Then use an Eventually() calling oc.getNamespaceLocked("foo", false) and wait for a nil return to indicate the namespace is gone from our cache.

Then delete the pod and verify that it doesn't get retried.

[flavio-fernandes]

hey @flavio-fernandes : dualstack is failing again on IC: https://github.com/ovn-org/ovn-kubernetes/actions/runs/5483719976/jobs/9990837646?pr=3751 PTAL! I really don't want to reopen our tracker card again HAHA @kyrtapz you are rebased on #3724 i guess

@tssurya I think the test is not being patient enough. Check this out:

...
+ kubectl wait --for=condition=ready pods --all --timeout=100s
pod/httpd-deployment-6b68fbf665-r8fzm condition met
pod/httpd-deployment-6b68fbf665-vfwd9 condition met
pod/server-deployment-8468968cd5-c5q8j condition met
pod/server-deployment-8468968cd5-vmznw condition met
+ IPS=()
++ kubectl get services my-service-v4 -o 'jsonpath={.spec.clusterIPs[0]}'
+ CLUSTER_IPV4=10.96.222.173
+ IPS+=("${CLUSTER_IPV4}:8081")
++ kubectl get services my-service-v6 -o 'jsonpath={.spec.clusterIPs[0]}'
+ CLUSTER_IPV6=fd00:10:96::57f5
+ IPS+=("\[${CLUSTER_IPV6}\]:8080")
+ for n in $NODES
+ for ip in ${IPS[@]}
+ docker exec ovn-worker2 curl --connect-timeout 5 10.96.222.173:8081
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    45  100    45    0     0     42 <html><body><h1>It works!</h1></body></html>
     0  0:00:01  0:00:01 --:--:--    42
+ for ip in ${IPS[@]}
+ docker exec ovn-worker2 curl --connect-timeout 5 '\[fd00:10:96::57f5\]:8080'
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:--  0:00:05 --:--:--     0
curl: (28) Connection timeout after 5000 ms


❯ docker exec ovn-worker2 curl --connect-timeout 5 '\[fd00:10:96::57f5\]:8080'
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    45  100    45    0     0   6695      0 --:--:-- --:--:-- --:--:--  7500
<html><body><h1>It works!</h1></body></html>

The kubectl wait --for=condition=ready pods --all --timeout=100s is not taking into account the mili-seconds it will take ovnk to setup the static routes across the nodes; I think. I will tweak the test to accommodate that.