Skip to content
This repository has been archived by the owner on Jul 8, 2022. It is now read-only.

Add action groups for playbooks with module_defaults #107

Merged
merged 2 commits into from May 30, 2020
Merged

Add action groups for playbooks with module_defaults #107

merged 2 commits into from May 30, 2020

Conversation

s-hertel
Copy link
Contributor

SUMMARY

Keep backwards compatibility with playbooks using community.kubernetes content with module_defaults groups - for example

module_defaults:
  group/k8s:
    # set common options with default values
    option: value
tasks:
  # uses the module_default group value for the option
  - k8s_info:

  # should use the module_default group value for the option
  - community.kubernetes.k8s_info:

ansible/ansible#67291 adds support so the collection is no longer reliant on routing and the entries in core to keep things from breaking and now can define their own. This will also fix using group/k8s: with fully qualified module names.

ISSUE TYPE
  • Bugfix Pull Request

@codecov
Copy link

codecov bot commented May 28, 2020

Codecov Report

Merging #107 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #107   +/-   ##
=======================================
  Coverage   42.88%   42.88%           
=======================================
  Files           3        3           
  Lines         541      541           
  Branches      110      110           
=======================================
  Hits          232      232           
  Misses        266      266           
  Partials       43       43           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ed3b65...6d0218b. Read the comment docs.

@geerlingguy
Copy link
Collaborator

@s-hertel - A few quick questions:

  1. Should all the k8s_* modules be under this action group? (There's also k8s_log, k8s_exec now).
  2. Should we have a 2nd action_group for the helm_ modules (there are three now, helm, helm_info, and helm_repository).
  3. Should the merging of this wait until Allow collections to define their own action groups ansible/ansible#67291 is merged, or is that format pretty much set in stone/agreed upon? (I don't think it hurts anything to have it merged, I just don't want to merge this then have to re-format it again later.)
  4. Can you rebase the PR? There were some shaky tests that were recently fixed that should allow this to pass tests.

@s-hertel
Copy link
Contributor Author

s-hertel commented May 29, 2020

@geerlingguy

  1. It depends, it's really up to the collection maintainers. Action groups are just a convenience if there's a lot of potentially repetitive options across multiple modules/actions. My PR just keeps backwards compat for things that were in the hardcoded module_defaults.yml file in the ansible repo, and will keep it working if people update the playbook toe use the FQCN, but I didn't want to make assumptions about the management of the new content.
    My rationale for whether modules/action plugins would benefit from being in a group is if they take a common subset of options (doc fragments make that easy to spot - for example, all the AWS modules take a bunch of credential options).

  2. Maybe. I'm not familiar with those modules, but if it would support users who want to reduce boilerplate in playbooks using that content, it would make sense to me.

  3. The format seems to be agreed upon. I think merging before or after is fine at this point. If something does change I will feel responsible and open the PR to update the format as soon as I'm aware.

  4. Yes, just a minute, thanks. I hadn't dug into whether the failures were legitimate.

@willthames
Copy link
Collaborator

Tests are still failing - looks like molecule's yamllint checks don't like the new meta/runtime.yml file.

My view is that I'm totally grateful for @s-hertel maintaining the existing status quo regarding k8s module defaults - whether we add missing k8s modules or helm modules is down to us as collection maintainers.

@willthames willthames merged commit b46a5ee into ansible-collections:master May 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants