Flaky unit test TestExpectedPodDeletions_DeletionsSatisfied

[thbkrkr]

The unit test TestExpectedPodDeletions_DeletionsSatisfied is flaky since this morning.

=== RUN   TestExpectedPodDeletions_DeletionsSatisfied/two_deletions_expected,_only_one_satisfied
    --- FAIL: TestExpectedPodDeletions_DeletionsSatisfied/two_deletions_expected,_only_one_satisfied (0.00s)
        deletions_test.go:100: 
            	Error Trace:	deletions_test.go:100
            	Error:      	Not equal: 
            	            	expected: map[types.NamespacedName]types.UID{types.NamespacedName{Namespace:"ns", Name:"pod2"}:"41ca8fb2-718a-44c1-b26f-862e303baedd"}
            	            	actual  : map[types.NamespacedName]types.UID{types.NamespacedName{Namespace:"ns", Name:"pod1"}:"17578556-9e86-42ba-b1bd-50a23bffb356", types.NamespacedName{Namespace:"ns", Name:"pod2"}:"41ca8fb2-718a-44c1-b26f-862e303baedd"}
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1,2 +1,3 @@
            	            	-(map[types.NamespacedName]types.UID) (len=1) {
            	            	+(map[types.NamespacedName]types.UID) (len=2) {
            	            	+ (types.NamespacedName) ns/pod1: (types.UID) (len=36) "17578556-9e86-42ba-b1bd-50a23bffb356",
            	            	  (types.NamespacedName) ns/pod2: (types.UID) (len=36) "41ca8fb2-718a-44c1-b26f-862e303baedd"
            	Test:       	TestExpectedPodDeletions_DeletionsSatisfied/two_deletions_expected,_only_one_satisfied


Expected :map[types.NamespacedName]types.UID{types.NamespacedName{Namespace:"ns", Name:"pod2"}:"41ca8fb2-718a-44c1-b26f-862e303baedd"}
Actual   :map[types.NamespacedName]types.UID{types.NamespacedName{Namespace:"ns", Name:"pod1"}:"17578556-9e86-42ba-b1bd-50a23bffb356", types.NamespacedName{Namespace:"ns", Name:"pod2"}:"41ca8fb2-718a-44c1-b26f-862e303baedd"}

Full logs of two failed builds:
https://devops-ci.elastic.co/job/cloud-on-k8s-pr/1915/consoleText
https://devops-ci.elastic.co/job/cloud-on-k8s-pr/1916/consoleText

[thbkrkr]

Bug spotted in ExpectedPodDeletions#DeletionsSatisfied due to three things:

• map order iteration is random (e.podDeletions is a map[types.NamespacedName]types.UID ; that explains the flakiness)
• the method has a side-effect, it can mutate the map it is iterating (L48)
• the method should not return until the deletion of all pods has been verified and the map may have been mutated accordingly (L50)

cloud-on-k8s/pkg/controller/common/expectations/deletions.go

Lines 40 to 54 in b6f86c6

	func (e *ExpectedPodDeletions) DeletionsSatisfied() (bool, error) {
	for pod, uid := range e.podDeletions {
	isDeleted, err := podDeleted(e.client, pod, uid)
	if err != nil {
	return false, err
	}
	if isDeleted {
	// expectation fulfilled
	delete(e.podDeletions, pod)
	} else {
	return false, nil
	}
	}
	return len(e.podDeletions) == 0, nil
	}

In the above test, we start with 2 pods in the e.podDeletions map and expect terminate with 1 pod.
If the pod2 is checked for its deletion before the pod1, as the pod2 is not deleted, we return (L50) without checking for the pod1 deletion which should update the e.podDeletions map (L48).

[sebgl]

I realize I fixed almost the same problem for expectations on the StatefulSets updates: ded8e46, but forgot to look at the deletions side of things.