Containerized contoller cleanup when raised exception

[bkopilov]

Current code depends on _is_running for cleanup fixture. In case this param is False , remove function skipp cleanup

Moving the _is_running to the top of the run function to trigger deletion .

[openshift-ci]

Hi @bkopilov. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[bkopilov]

@eliorerz , @adriengentil , @lalon4

[adriengentil]

instead of maintaining a boolean, can't we just ask podman if the container is running?

[bkopilov]

podman if the container is running?

Yes we can but there is another problem that we should address on self._on_container_start(**kwargs)
In this code user is free to override on create and cleanup ... so you may get into issue that controller is "down" but user allocates resources (podman-remote command) on _on_container_start

[adriengentil]

we could define is_running as a property, and the user would have to override it as well

[adriengentil]

approving to solve the issue, we could think at something better later

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adriengentil, bkopilov

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 [adriengentil]

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

[bkopilov]

we could define is_running as a property, and the user would have to override it as well

Yes , also no need boolean at all ... we just need to verify on run if the container already up and on remove we always run delete...

maybe decorate the function that test if running before .... something like:

def is_running_container(func):

    def inner(self, **kwargs):
        if container_is_running:
            return func(self, **kwargs)
        return None
    return inner

class Test:
    @is_running_container
    def run(self, **kwargs):
        print(f"run {kwargs}")
```I did not try it but the idea is that we wont call to run if container is already running
if you agree i will work on it.