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

k8s module: Add parameter to invoke the template module when given a path to k8s yaml files #60134

Closed
chris-short opened this issue Aug 6, 2019 · 16 comments
Labels
affects_2.9 This issue/PR affects Ansible v2.9 clustering Clustering category collection:community.kubernetes collection Related to Ansible Collections work feature This issue/PR relates to a feature request. k8s module This issue/PR relates to a module. needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md support:community This issue/PR relates to code supported by the Ansible community.

Comments

@chris-short
Copy link

chris-short commented Aug 6, 2019

SUMMARY

k8s module: Add parameter to invoke the template module when given a path to k8s yaml files

ISSUE TYPE
  • Feature Idea
COMPONENT NAME

k8s module template processor

ADDITIONAL INFORMATION

I think it'd be beneficial for the k8s module (and subsequently operator-sdk) to avoid using Jinja2 templating when possible. In a nutshell, I'd like a cleaner, simpler way to invoke templating of k8s YAML files as their deployed via Ansible. Example of the current state: https://github.com/geerlingguy/mcrouter-operator/blob/a3babecae85ece431f5cf316bd8a407ba60a5b42/roles/mcrouter/tasks/main.yml

It's 431 bytes in total, I get that. It's not a lot of Jinja2, I get that too. Jinja2 is another thing to add on to the pile of things to learn before you can harness it and removing that barrier will be beneficial. The effort of learning Jinja2 for this use case (globally) far outstrips our effort to help solve this problem on behalf of our users.

The overarching goal is to minimize the amount of Jinja2 needed in playbooks to make this work.

@ansibot
Copy link
Contributor

ansibot commented Aug 6, 2019

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Aug 6, 2019

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 clustering Clustering category feature This issue/PR relates to a feature request. k8s module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. labels Aug 6, 2019
@geerlingguy
Copy link
Contributor

I'm on board with this. I have basically had to add that boilerplate about 7 jillion times, and it would be nice to just have built-in functionality that would allow me to pass a .j2 file with multiple documents that's parsed into the individual docs without writing out an explicit lookup...

@tima
Copy link
Contributor

tima commented Aug 6, 2019

I also agree. Forcing a lookup template etc is unnecessary repetitive and annoying to keep doing especially given how common reading in a templatized YAML file will be.

@fabianvf
Copy link
Contributor

fabianvf commented Aug 6, 2019

big +1 from me, would make k8s targeted playbooks/roles much much cleaner

@willthames
Copy link
Contributor

willthames commented Aug 6, 2019

Lovely idea, not at all convinced that it's workable in practice.

How will the module get all the required variables to do the templating?

Write the logic once using e.g. kube-resource role and then move on with your lives. We literally have one kubernetes role that manages every kubernetes resource.

https://github.com/willthames/ansible-role-kube-resource/blob/master/tasks/main.yml

I never have to write that boilerplate any more. (edit: I still do in inventory to reference immutable configmaps and secrets: https://willthames.github.io/2019/01/28/immutable-kubernetes-configuration-with-ansible.html)

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Aug 6, 2019
@willthames
Copy link
Contributor

One other problem with this idea is that templating is done on the controller but the k8s module executes on the target. I'm not sure how many people actually have different controllers and targets for k8s.

I'd suggest this would be better as a proposal - it's not a k8s-specific concern, and suspect Ansible would need a bunch of changes to make this at all possible

@chris-short
Copy link
Author

@willthames At a cursory glance, this is marvelous. Will let the others look and dig in while I do the same this morning. But, regardless, thank you. cc @geerlingguy @tima @fabianvf

@geerlingguy
Copy link
Contributor

I'm looking through what it would take to make this possible in the k8s module—and a few notes:

  1. Regarding @willthames' concern with templating on the controller but k8s on target, there are two ways to overcome that problem:
    1. Executing against localhost (this seems to be the norm, in most of the real world usage I've seen, but it is not necessarily normative enough to require it to be done!).
    2. Copying the templates to the target first (I actually do this on a few projects for convenience).
  2. I initially explored writing my own module in my new k8s collection, but realized it would either require a pretty extensive amount of work to basically pull things from core into my own module to replicate what's in k8s, or it would require modifications to core, since bits would need changing in the k8s module_utils to support it.
  3. In [k8s] allow user to pass list of resources in to definition parameter #42377, it became possible to pass multiple YAML definitions in via resource_definition or src, so the splitting is already present, it seems it might just need to also run the split documents through Jinja in execute_module in raw.py?

I see the potential issues @willthames raises. I also know that those of us using this functionality in practice kind of wrap things up in roles to mask over the clunky inability to easily template, as roles like kube-resource and geerlingguy.k8s_manifests do.

But Jinja templating is probably the main reason I ever considered using Ansible for K8s resource management, and I think having this be available through the core k8s module (e.g. via a new template parameter which would work exactly like src but template the definition via Jinja when it is run through) would be a very useful improvement for most use cases.

At the very least, I wouldn't have to explain lookup plugins and Jinja filters just to demonstrate one simple templated K8s resource definition being applied to a K8s cluster through Ansible.

I the end, I find it much more comprehensible to write:

- name: Create Mcrouter resources.
  k8s:
    template: mcrouter.yaml.j2

instead of:

- name: Create Mcrouter resources.
  k8s:
    definition: "{{ lookup('template', 'mcrouter.yaml.j2') | from_yaml }}"

@geerlingguy
Copy link
Contributor

geerlingguy commented Aug 12, 2019

I have kind of a PoC in a branch of my k8s collection (see https://github.com/geerlingguy/ansible-collection-k8s/issues/2#issuecomment-520597539)—but I do see how on the module level, it may be impossible to get the templating done when run on a destination machine. (in response to "How will the module get all the required variables to do the templating?")

However, if using connection: local, I believe an action plugin could supply the vars for the templating...

It's definitely not as simple as "plug Templar into k8s module", but I still think if it's possible to have templating built in, it would be a huge UX/discovery improvement.

@willthames
Copy link
Contributor

I'd just like to note that there are also some very hard lessons (the kind that come out of Post Incident Reviews) now encoded in ansible-role-kube-resource that would be more difficult to embed in k8s_template.

This assumes you ever manage a bunch of resources in a single template (an incredibly common pattern)

Basically there's a reason I loop across an include_tasks, and that's that if you loop over the task, and one of the resources fail to take, the loop will not stop) - whereas looping over an include will fail immediately a resource fails to take

(the issue is that if we're updating a service because of a change in a deployment's pod's port or labels, that service might not have any targets if the pod changes fail but we carry on and update the service anyway)

@willthames
Copy link
Contributor

Thinking about this some more, with decent error handling and the loop inside k8s_template, we could be safer from the problem I identify above.

One thing I find hard with the "run one huge manifest in a single task" approach is the lack of error reporting - ok, so my manifest failed because of applying a single resource, but which one? With a looped include_task approach I can name the task appropriately (usually "Applying {{ resource.metadata.name }} {{ resource.kind }}" or similar) and it's obvious.

@geerlingguy
Copy link
Contributor

I've moved this feature request into the Kubernetes collection repository: https://github.com/ansible-collections/kubernetes/issues/37

This request should be closed and further discussion moved to that thread.

@ansibot
Copy link
Contributor

ansibot commented Mar 24, 2020

Files identified in the description:
None

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community. labels Mar 24, 2020
@ansibot
Copy link
Contributor

ansibot commented Mar 26, 2020

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Mar 26, 2020
@ansibot ansibot added collection Related to Ansible Collections work collection:community.kubernetes needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md labels Apr 29, 2020
@Akasurde
Copy link
Member

Akasurde commented Aug 4, 2020

Thank you very much for your interest in Ansible. This plugin/module is no longer maintained in this repository and has been migrated to https://github.com/ansible-collections/community.kubernetes

Migrated this issue in the above repository - https://github.com/ansible-collections/community.kubernetes/issues/176.

If you have further questions please stop by IRC or the mailing list:

@Akasurde Akasurde closed this as completed Aug 4, 2020
@ansible ansible locked and limited conversation to collaborators Sep 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 clustering Clustering category collection:community.kubernetes collection Related to Ansible Collections work feature This issue/PR relates to a feature request. k8s module This issue/PR relates to a module. needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

No branches or pull requests

7 participants