fix: split pod controller from workflow controller

[Joibel]

Superceeds #14088

Motivation

Split the pod (formerly pod GC) controller from the main workflow controller to make it easier to reason about.

Attempt to handle various conditions whereby pods can become orphaned when pod finalizers are in use.

Modifications

Split the pod controller components and its queue into the workflows/controller/pod subdirectory.

Callbacks into the workflow controller are provided so that pod events can enqueue workflows for reconciliation.

The pod controller can also now make decisions on it's own. This is especially important for orphaned pods where the workflow has undergone deletion without the pod finalizers being removed. The most obvious case here is a PodGC strategy with delete delay duration. In this case the only place the existence of the pod enqueued for deletion is in the pod GC delayed queue, which is in memory and subject to a workflow-controller restart losing track of it.

Added a number of testing hooks so that the workflow/controller tests didn't need rewriting where they need to inspect the internals of the pod-controller.

Verification

Existing E2E and unit testing, and a few extensions.

Documentation

This is mostly a refactor and amendment of code to accommodate more configurations.

[isubasinghe]

I presume this was done to ensure that the pods don't get processed by the workers, so that you may reliably inspect the queue?

[Joibel]

By "this" I assume you mean the 0 for number of workers. Yes.

In the same way the old code created a queue but never actually ran any workers, this means the tests have to manually do processNextItem so they can evaluate the result and inspect.

I'll add a comment.

[isubasinghe]

Looks good but I have some questions/issues

[isubasinghe]

Should this not move to the Run method?

[Joibel]

Good point, I'll move it.

[isubasinghe]

I don't know how I feel about this code and queuePodsForCleanup being relatively similar.
More in the sense that they both drive the reconciliation of pods but live in different packages.

[Joibel]

We discussed this on a call.

queuePodsForCleanup is the call into the pod-controller to reduce the kubernetes load and make life saner.
Thoughts:

• We must await the node status update to be successfully written to the workflow. This eliminates pure pod observation to know when we can do anything to them, unless we add a lot of extra load to k8s by writing something to the pods so the pod-controller can see the change.
• An option: The workflow informer is also observed by the pod-controller and it then unwraps the workflows to observe node statuses. This is costly in terms of CPU, and may also involve getting this information from the status offload database. Avoided for the double dip into workflow update events as we already know at this point we need to do a thing, and it's unclear that this is much less evil.
• queuePodsForCleanup know which pods are in need of cleanup (by doing the naughty list operation) because they are still in the informer (e.g. not marked complete or deleted) thus minimizing the number needing checking. If we sent this event via a queue into the pod-controller we would have to send the entire set of node statuses and teach the pod-informer to understand node to pod mapping.

[isubasinghe]

I think we should remove this personally, the controllers job is to manage all pods created by argo.
People upgrading into this version might have trouble with pods not being handled with this remaining the same.

[Joibel]

There is no path whereby the pods have the finalizer and are not labelled as completed. In this state they drop out of view of the informer, yes. Monitoring pods that are marked as completed is a waste of resources, owner reference to the workflow does all the necessary work.

[isubasinghe]

Just a small comment, good to go after thats been addressed.

[isubasinghe]

This needs to be removed.

[Joibel]

Done