When using labels with PodGC, completion label is not added to the pod

[alexec]

No description provided.

[alexec]

@ziv-codefresh @roi-codefresh @pasha-codefresh @kostis-codefresh @plakyda-codefresh if you're looking for any issues to start work on, issues like this one, i.e. labelled with "good first issue" are the best places to start. When you want something more substantial look for "help wanted".

[alexec]

Cause: feat(controller): Support pod GC strategy based on label selector on pods (#5090)

@terrytangyuan you authored this feature in Feb 2021. It is minor (no reports).

[alexec]

Having analysed this, it is not a good first issue. There are at least two bugs in this code, one related to the matching, at least one other, that result in cases where completed pods are never labelled and sit consuming memory they do not need to.

I don't think this is a big problem, because no one has reported it yet.

I think the podGC code should be centralised and we can remove completePods and use woc.getAllWorkflowPods() to list the workflows pods. This func did not exist in 2018.

	// It is important that we *never* label pods as completed until we successfully updated the workflow
	// Failing to do so means we can have inconsistent state.
	// Pods may be be labeled multiple times.
	// We MUST not need to label the pod if we might delete it later for GC. 
	delay := woc.controller.Config.GetPodGCDeleteDelayDuration()
	podGC := woc.execWf.Spec.PodGC
	strategy := podGC.GetStrategy()
	pods, err := woc.getAllWorkflowPods()
	if err != nil {
		woc.log.WithError(err).Error("failed to get pods")
		return
	}

	for _, pod := range pods {
		podCompleted := pod.Status.Phase == apiv1.PodSucceeded || pod.Status.Phase == apiv1.PodFailed
		matches, _ := podGC.Matches(pod.Labels)
		if matches && (
			strategy == wfv1.PodGCOnPodCompletion && podCompleted ||
				strategy == wfv1.PodGCOnPodSuccess && pod.Status.Phase == apiv1.PodSucceeded ||
				strategy == wfv1.PodGCOnWorkflowCompletion && woc.wf.Status.Phase.Completed() ||
				strategy == wfv1.PodGCOnWorkflowSuccess && woc.wf.Status.Phase == wfv1.WorkflowSucceeded) {
			woc.controller.queuePodForCleanupAfter(woc.wf.Namespace, pod.Name, deletePod, delay)
		} else if woc.wf.Status.Fulfilled() || podCompleted {
			woc.controller.queuePodForCleanup(woc.wf.Namespace, pod.Name, labelPodCompleted)
		}
	}