[merged] add "atomic ps" to show containers

[giuseppe]

I'll probably need something like this for https://github.com/openshift/openshift-ansible/ to be able to query what containers are running. In any case, this is generally a good thing to have.

[cgwalters]

Ones that are running should showup in systemctl (which has a DBus API and one can extract bits from the command line too), right?

[giuseppe]

yes, running system containers showup in systemctl as well, but I plan to add more information to the atomic ps output, at least when --json is used, which cannot be easily retrieved from systemctl.

For example have all the information required for having an ansible task that "will recreate the container if a different configuration is used", so that means at least exporting all the set variables for the system container.

Another difference is that "atomic ps" unifies both Docker containers and system containers.

[rhatdan]

Should we list all container on the system? IE other containers started with runc ? runc list?
What about containers started with systemd-nspawn? Is this just listing running containers or all containers?

[rhatdan]

rkt containers?

machinectl list

[giuseppe]

what I would like is to have a way to show system containers and query their status at a higher level of what "runc list" does, as runc does not know how a system container was configured or what version of the image is used. I thought of Ansible as a consumer of this information (with --json output).
By default "atomic ps" lists only running containers, you can list all containers with --all.

I added the support for docker containers as well so that it works like "atomic images" where only system containers and docker images are known.

[rhatdan]

No I like the idea, but I still think we should provide less info about other containers like runc, rkt, systemd-nspawn ...
ps --all would only work with the containers we know about I guess.

[rhatdan]

Can you split this out into a separate file ps.py, rather then continuously growing atomic.py. I eventually would like to break each command into its own py file.

[giuseppe]

I have moved the new functionality to a separate ps.py file

[rhatdan]

@baude PTAL

[cgwalters]

I agree that systemctl not supporting JSON is an annoying gap, but OTOH this already exists: https://docs.ansible.com/ansible/systemd_module.html

Though probably even with that we do need the union of information with the container image data etc...so that argues for either having this or a dedicated ansible module for syscontainers.

So to be clear I'm not objecting to this, but I also see it as a key advantage of syscontainers that we're making use of systemd.

[baude]

Can you use get_containers OR get_active_containers here conditionally? Something like?

func = self.containers if self.args.all else self.get_active_containers

The do your list comprehension using in func()]:

[giuseppe]

I cannot use get_containers or get_active_containers as they already include the system containers while I want only the docker containers. get_(active_)containers must probably be refactored to use the ps command.

[rhatdan]

I did not know about this syntax &> /dev/null. Cool

[rhatdan]

rm -rf /etc/systemd/system/${NAME}.service || true

No need for || true. -f will not error out if the file does not exist. If you got a permission error, you would probably want to see that.

[rhatdan]

Can we split these patch sets in two, one for images and fixes for testing, the other for atomic ps.

[giuseppe]

I moved the other patches that are not part of "atomic ps" here: #439

[rhatdan]

rebase please.

[yuqi-zhang]

So I was trying this out on a Centos-CI vagrant vm, and found that "atomic ps" needs "runc state mycontainer" from runc 1.0 release. But when I built runc from upstream, trying to install giuseppe's etcd system container fails ("atomic install --system --name=etcd gscrivano/spc-etcd").

To be more exact, the command itself succeeds, but etcd fails to start. Looking into journalctl, it complains that "open /run/runc/etcd/state.json: no such file or directory" (obviously was never created). If I had to guess, this is due to runc 1.0 splitting the create/start process, and thus just running "atomic install --system" is no longer enough to set up the system container? (In previous versions of runc, this whole process worked fine without having to directly interact with runc)

I could be misunderstanding some of this. Any thoughts are appreciated.

[rhatdan]

@yuqi-zhang Please open a separate issue on the atomic install --system issue or better yet a bugzilla.

[rhatdan]

We need to fix the tests.

************* Module Atomic.ps
E: 10, 0: Unable to import 'dateutil.parser' (import-error)
Makefile:28: recipe for target 'pylint-check' failed
make: *** [pylint-check] Error 2

I would like to see us add some flexibility like @baude added for atomic top to show different fields from the ps command. But lets get the test fixed and we can merge.

[yuqi-zhang]

The install seems to be an issue with how images were set up, will look into. Since giuseppe isn't here, I'll also take a look at tests.

[jlebon]

bot, retest this please

[jlebon]

I installed python-dateutil and python3-dateutil on the tester to give the PR a try, but I think they should probably be added to requirements.txt instead.

[rhatdan]

Ok I am going to merge, but I would like to see this extended to allow users to specify which fields they would like to see in the ps output.

[rhatdan]

@rh-atomic-bot r+

[rh-atomic-bot]

📌 Commit 8c0d5cc has been approved by rhatdan

[rh-atomic-bot]

⌛ Testing commit 8c0d5cc with merge 061a907...

[rh-atomic-bot]

☀️ Test successful - status-atomicjenkins
Approved by: rhatdan
Pushing 061a907 to master...