Ensure container start command has the attach flag

[rabi]

When starting an already created (not running) container detach=False is not honored. This results container failures not being reported as failure.

[sshnaidm]

@rabi Can you please paste a task that you're running? It's not clear from the description what is the problem.

[rabi]

@rabi Can you please paste a task that you're running? It's not clear from the description what is the problem.

Sorry I'll try and explain here.. When running containers for which we want to wait for it to finish, when the container already exists and there is no diff, we run podman container start <container-name> and it returns immediately with rc code 0 (without --attach) in case of failure. For non-exsiting cntainer we run podman run and pass --detach=False and it works fine.

Jan 11 15:39:09 controller-0 python3[9955]: ansible-tripleo_container_manage PODMAN-CONTAINER-DEBUG: podman start container-puppet-rabbitmq

[rabi]

@rabi Can you please paste a task that you're running? It's not clear from the description what is the problem.

Below is simple reproducer for podman start.

[root@controller-0 ~]# podman run --name test --detach=False --entrypoint=sh docker.io/library/alpine:3.12 -c 'exit 1'
[root@controller-0 ~]# echo $?
1
[root@controller-0 ~]# podman rm test
test
[root@controller-0 ~]# podman create --name test --attach=True --entrypoint=sh docker.io/library/alpine:3.12 -c 'exit 1'
b5f88e1a6fb66dcced4e112e9b3a2457fcb984e2026b487d436de516d0c8ff37
[root@controller-0 ~]# podman start test
test
[root@controller-0 ~]# echo $?
0
[root@controller-0 ~]# podman start --attach test
[root@controller-0 ~]# echo $?
1

[sshnaidm]

I see, now it's clear what is the problem. Can you please paste an Ansible task with podman_container you use? I'm not sure forcing --attach for all is the best idea and won't break backward compatibility. Let's see what is the use case and maybe we'll find out more smooth solution

[rabi]

I see, now it's clear what is the problem. Can you please paste an Ansible task with podman_container you use? I'm not sure forcing --attach for all is the best idea and won't break backward compatibility.

Why do you think we're forcing --attach and would break compatibility? In the proposed change it's done only when the module is called with detach:false i.e if action == 'start' and not self.module_params['detach'] or I am missing something?

[rabi]

Just for context, where this is coming from, all puppet containers[1] in TripeO, don't wait for failure when redeployed (existing containers that needs to be started).

[1] https://github.com/openstack/tripleo-heat-templates/blob/stable/wallaby/common/host-container-puppet-tasks.yaml#L27

https://github.com/openstack/tripleo-ansible/blob/stable/wallaby/tripleo_ansible/ansible_plugins/modules/container_puppet_config.py#L271-L272

[xbezdick]

+1 now it works as expected

[sshnaidm]

I see, now it's clear what is the problem. Can you please paste an Ansible task with podman_container you use? I'm not sure forcing --attach for all is the best idea and won't break backward compatibility.

Why do you think we're forcing --attach and would break compatibility? In the proposed change it's done only when the module is called with detach:false i.e if action == 'start' and not self.module_params['detach'] or I am missing something?

It breaks compatibility because --attach is always False by default and it doesn't depend on --detach as it seems... It behaves just independently. All containers which were sent to background will be attached now and stuck.
I'd suggest a better approach with additional parameters that you can use when setting containers in specific statuses. For example in your case to add attach: true, please tell me what you think - #703 and if it helps you.

[rabi]

It breaks compatibility because --attach is always False by default and it doesn't depend on --detach as it seems.

It does depend on --detach when using podman run (which is used for non-existing/changed containers). The other issue is podman commands have different options to do the same thing.

a. podman run --detach
b. podman start --attach

IMO having two arguments detach and attach for this module is not a good idea and would create more issues. From backward compatibility pov, I don't think anyone wants the container to start detached when they provide detach=False when calling the module. So this should be fixed purely as a bug. Nothing changes when you start a new/changed container with podman run.

[sshnaidm]

@rabi I think #703 is ready, can you please ensure it solves the problem?
thanks

[sshnaidm]

Fixed in #703