Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Podman pod feature and for RHEL/CentOS 8 #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jwhb
Copy link
Contributor

@jwhb jwhb commented Jul 29, 2020

The role currently uses dnf to install Podman tools. The packages are first available on Fedora 28 and dnf was introduced to RHEL/CentOS with major version 8. Therefore we check against Fedora 28 and RHEL/CentOS 8 when including the dnf tasks.

This PR also adds the ability to use the Podman Pod feature. The role user will want to put the containers created by this role (or rather systemd) into Podman Pods. These have to be created beforehand. I implemented task logic to first check for existing pods with the same name. If a pod matches and doesn't have the published ports as requested by the role variables the pod will be deleted.

The tasks will always try to create the pod with the specified ports, if it doesn't exist.

The containers will then be assigned to a pod using the --pod argument in the systemd unit.

I had to use a different format for the published port mapping (dict vs. string) to properly detected existing pods with "outdated" port mappings.

I tested the role against CentOS 8 with different pod configurations (no pod assigned, pod assigned with no published ports, pod assigned with multiple ports).

What do you think about this contribution?

Copy link
Owner

@chasinglogic chasinglogic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jwhb thanks for the pull request.

If you want to split out the RedHat family support into a separate PR I'd be happy to merge that right away. I think we're going to have to go back and forth a bit on the --pod support before I'd be happy merging it.

set_fact:
podman_pods: "{{ podman_pods | default({}) | combine({service.pod.name: {'name': service.pod.name, 'ports': service.pod.ports | default([]) + (podman_pods[service.pod.name]['ports'] | default([]))}}) }}"

- name: Delete pods with missing port bindings
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I never added pod support was because it would require some gnarly use of the shell module like this.

I keep waiting for upstream Ansible to a module like podman_pod support but that may never happen.

I have two real requests when it comes to this:

  1. What does --pod provide over --network?

  2. If this is to get merged it will need a way to be idempotent. As it stands if I utilize the pod option for any of my services this will always report changed. This breaks one of the core promises of Configuration Management.

Point number 2 is why I never did this, you can do some wicked things with args and shell like:

shell: echo "something" > this_file
args:
    creates: this_file

To make it "idempotent"-ish and I think I'd be willing to accept that but this is already a pretty gnarly script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I anticipated that it could be problematic to put both changes in one PR and to implement the pod feature in that way. For my use case (Apache Guacamole) I needed an automation to connect 3 containers and publish some of their ports. Neither using pods nor network was possible with the existing Ansible modules, so I picked the pod option.

I don't think we can change the pod feature to have the quality (i.e. idempotency) you require for this project. While we wait for a native Ansible module to cover this feature I'll keep the branch published in case somebody needs to configure pods.

I split the pod feature into #2. Also you can close this PR.

Thanks for your comments and excellent work!

Copy link
Owner

@chasinglogic chasinglogic Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a similar problem when trying to deploy Nextcloud without pods I eventually gave up in the end and just exposed all the ports for everything and networked them the old-school way.

I think it's a very valid use case and I would love to see this feature through. With the new Ansible split into modules I'm not even sure where to report this upstream.

I also do not know if we can ship custom modules with roles on Galaxy if so that might be an option?

Perhaps an alternative is to do a little more abusing of the shell module. I've also attempted to make pods be SystemD template units in the past which almost worked but I didn't need pods bad enough to push it over the line.

Copy link
Contributor Author

@jwhb jwhb Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipping custom modules on Galaxy is made possible by Collections.

I just stumbled upon this collection on Galaxy: https://galaxy.ansible.com/containers/podman. The project provides additional modules for pods and networks. However it seems that it doesn't let you create networks.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perusing the source code I found https://github.com/containers/ansible-podman-collections/blob/master/plugins/modules/podman_network.py

Which does appear to let us create networks. I bet we could pull this as a dependency and make it work.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @chasinglogic @jwhb ,

I'm really interested in this feature. Do you think this is something you can work on ?

Best,
Jerome

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jrevillard,

I'm not sure when I can get to working on this

chasinglogic pushed a commit that referenced this pull request Oct 9, 2020
* Add apt resource recomended bey podman dev
chasinglogic added a commit that referenced this pull request Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants