-
Notifications
You must be signed in to change notification settings - Fork 9
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
jwhb
wants to merge
2
commits into
chasinglogic:master
Choose a base branch
from
jwhb:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
--- | ||
|
||
- name: Determine list of required pods and ports | ||
when: service.pod is defined | ||
loop: "{{ podman_services }}" | ||
loop_control: | ||
loop_var: service | ||
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 | ||
loop: "{{ podman_pods | subelements('ports', skip_missing=yes) }}" | ||
loop_control: | ||
loop_var: pod | ||
when: pod.0.ports is defined | ||
shell: | | ||
podman pod exists {{pod.0.name}} \ | ||
&& (podman pod inspect {{pod.0.name}} \ | ||
| jq -e ".Config.infraConfig.infraPortBindings[] | select(.hostPort=={{ lookup('dict', pod.1).key }} and .containerPort=={{ lookup('dict', pod.1).value }})" \ | ||
|| podman pod rm -f {{pod.0.name}}) || true | ||
register: podman_pod_inspect | ||
|
||
- name: Create required pods | ||
loop: "{{podman_pods | dict2items }}" | ||
loop_control: | ||
loop_var: pod | ||
shell: | | ||
podman pod exists "{{pod.key}}" \ | ||
|| podman pod create --name "{{pod.key}}" \ | ||
{% if pod.value.ports|default([])|length > 0 %}\ | ||
--publish {% for port in pod.value.ports %}{{lookup('dict', port).key}}:{{lookup('dict', port).value}}{%- if not loop.last -%},{%- endif -%}{% endfor %}\ | ||
{% endif %} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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:
What does
--pod
provide over--network
?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 reportchanged
. 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:To make it "idempotent"-ish and I think I'd be willing to accept that but this is already a pretty gnarly script.
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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