K8s cleanup

[ddgenome]

Various refactorings and consolidations.

[cdupuis]

I like the cleanup in this branch!

Issues that are actually not part of this PR but are broken nonetheless need to get addressed:

• https://github.com/atomist/sdm/blob/k8s-cleanup/lib/core/goal/container/container.ts#L277: the KubernetesJobDeletingGoalCompletionListener went missing so this require is broken. I'm also not sure why it was deleted as it was responsible for cleaning up completed k8s goal jobs after goal completion
• https://github.com/atomist/sdm/blob/k8s-cleanup/lib/core/goal/container/container.ts#L33-L36: there seems to be circular dependency between container.ts and KubernetesFulfillmentGoalScheduler.ts which is worth breaking.

[ddgenome]

Thanks for fixing those requires. What is strange is that the build was successful locally. Those requires are a pain when moving stuff around though.

[ddgenome]

That code is not hit when running tests locally, i.e., not in Kubernetes.

[ddgenome]

Regarding you review comments:

• I removed KubernetesJobDeletingGoalCompletionListener as it is less reliable and provided no additional benefit over the interval-based cleanup
• I kept the k8s container goal implementation as a single file because of the "lots of files are slow to load under gVisor" issue, but I can break the circularity

[ddgenome]

Actually, I'm not sure how to break the circular dependency. The implementations of the Container and KubernetesFulfillmentGoalScheduler classes use each other. I can create intermediate files to move the imports and implementations, but that does not break the circularity, it just adds a file between them. Any ideas @cdupuis ?

[ddgenome]

I've cleaned things up. I left the circular dependency for now. Thoughts @cdupuis ?

[atomist]

Pull request auto merged by Atomist.

• 1 approved review by @cdupuis
• 2 successful checks

[atomist:generated] [auto-merge:on-approve]