Add service ctr cleanup to PlayKubeDown

[umohnani8]

Since we can't guarantee when the worker queue will come
and clean up the service container in the remote case when
podman kube play --wait is called, cleanup the service container
at the end of PlayKubeDown() to ensure that it is removed right
after all the containers, pods, volumes, etc are removed.

[NO NEW TESTS NEEDED]

Fixes #17803
Foxes #17820

Signed-off-by: Urvashi Mohnani [email protected]

Does this PR introduce a user-facing change?

None

[umohnani8]

@edsantiago I think this should fix the race. I will re-run the test a bunch of times in the f37 aarch64 root environment to verify.

[edsantiago]

This may be a stupid question, but what exactly is the purpose of --wait if it doesn't actually wait?

If --wait only guarantees that the containers are stopped, then maybe the fix is to remove -a from the podman ps -aq at the end? (Actually, removing the -q would be pretty helpful for future errors, too. --noheading would be better).

If --wait should guarantee that the containers are removed, then maybe these flakes are actually showing a real bug that needs to be fixed?

Either way, I think the documentation needs to be fixed to specify what --wait is intended to do.

[umohnani8]

--wait is supposed to clean up the resources created once the pods have exited, so the -a is needed to verify that. I have tested this countless times and the resources have always been removed. I have no way of really reproducing this flake given that is only happening in one environment. My guess it that remote might be a bit slow in this environment in finishing the removal by the time we do a ps -aq to verify it.
I can add some more debugs to the test to see what may be happening if this flake continues to happen after this patch.

[rhatdan]

Seems most likely that the containers were marked for removal but not fully removed. Is the remote side waiting for the content to be removed? Is there a way to tell containers to be removed and return without waiting for them to be removed?

[Luap99]

I think the problem is that the serviceContainer is removed via worker queue. This is not a problem for local podman because it waits for all queue jobs to be completed before it exits. However in the remote case the service will finish the API response but there is no way of controlling what jobs were done by the worker queue.

I think the client should make the equivalent to podman wait --condition removing serviceContianer in this case to ensure everything is cleaned up before it exits.

cc @vrothberg

[vrothberg]

I think the problem is that the serviceContainer is removed via worker queue. This is not a problem for local podman because it waits for all queue jobs to be completed before it exits. However in the remote case the service will finish the API response but there is no way of controlling what jobs were done by the worker queue.

Very thorough analysis, @Luap99!

I think the client should make the equivalent to podman wait --condition removing serviceContianer in this case to ensure everything is cleaned up before it exits.

Alternatively, the service container could be removed in PlayKubeDown() in the backend. Note that it must be removed last and after all containers, networks, volumes, etc.

[umohnani8]

Thanks @vrothberg and @Luap99 - added service container clean up to PlayKubeDown()

[Luap99]

LGTM

[vrothberg]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: umohnani8, vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [umohnani8,vrothberg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment