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.

Sign up for GitHub

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

Jump to bottom

Adjust action groups to moved modules #72428

Merged
merged 7 commits into from
Nov 5, 2020
Merged

Adjust action groups to moved modules #72428

merged 7 commits into from
Nov 5, 2020

Conversation

felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Nov 1, 2020 •
edited
Loading

SUMMARY

The docker modules, kubevirt modules and the k8s module are moving to other collections:

  1. The docker modules are moving from community.general to community.docker (already in progress, new collection already released).
  2. The kubevirt modules are moving from community.general to community.kubevirt (in preparation, collection not yet published).
  3. The k8s module is moving from community.kubernetes to community.okd (already in progress, collection with module already released).
  4. The community.k8s collection will be renamed to kubernetes.core (already in progress, new collection already released).

Right now, support for the docker and k8s action groups / module default groups does need to know in which collection the affected modules / action plugins are contained. Therefore, lib/ansible/executor/module_common.py needs to be updated (and this change backported to stable-2.10) for the moves to work flawlessly, and to avoid breaking playbooks and roles.

The old names must not be removed either to avoid breakage with older (current) versions of the collections.

CC @s-hertel @gundalow @geerlingguy @tima

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/executor/module_common.py

@felixfontein
Copy link
Contributor Author

@geerlingguy @tima similar PRs like ansible-collections/community.docker#17 will be needed in your collections as well for this to work.

@felixfontein felixfontein mentioned this pull request Nov 1, 2020
Merged
@ansibot ansibot added affects_2.11 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Nov 1, 2020
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Nov 1, 2020
@felixfontein
Copy link
Contributor Author

/rebuild_failed

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. labels Nov 1, 2020
@felixfontein
Copy link
Contributor Author

/rebuild_failed

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. labels Nov 1, 2020
@gundalow
Copy link
Contributor

gundalow commented Nov 1, 2020

/rebuild_failed

@felixfontein
Copy link
Contributor Author

@gundalow I think the tests are broken (maybe just temporarily), unrelated to this PR.

@geerlingguy
Copy link
Contributor

@felixfontein the k8s module is not moving to the okd collection; that collection just has an alias/override of the k8s module customized for OpenShift.

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 1, 2020
@felixfontein
Copy link
Contributor Author

@geerlingguy thanks! I've removed community.okd from this PR then.

@geerlingguy
Copy link
Contributor

@felixfontein though the oc plugin is moving from community.general to community.okd, and the k8s_auth plugin moved from community.kubernetes to community.okd.

@felixfontein
Copy link
Contributor Author

@geerlingguy ok, in that case I reverted the last commit :) It's then needed for the k8s_auth module. (Module defaults are not used for non-action plugins, so the oc connection plugin doesn't matter.)

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Nov 1, 2020
bcoca
bcoca reviewed Nov 2, 2020
View reviewed changes
lib/ansible/executor/module_common.py
@@ -1357,9 +1357,9 @@ def get_action_args_with_defaults(action, args, defaults, templar, redirected_na
'aws': ['amazon.aws', 'community.aws'],
'azure': ['azure.azcollection'],
'cpm': ['wti.remote'],
'docker': ['community.general'],
'docker': ['community.general', 'community.docker'],
Copy link
Member

Choose a reason for hiding this comment

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

not a blocker to this PR, but this should be moved to config (really should not be a thing in core at all, but setlingling for moving into config).

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 was assuming that this is a placeholder anyway until the action groups feature is fully designed and implemented.

Copy link
Member

Choose a reason for hiding this comment

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

I believe that is tentatively slated for 2.11

@felixfontein
Copy link
Contributor Author

/rebuild_failed

@felixfontein
Copy link
Contributor Author

/rebuild

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 3, 2020
@felixfontein felixfontein mentioned this pull request Nov 4, 2020
Closed
gundalow
gundalow approved these changes Nov 5, 2020
View reviewed changes
Copy link
Contributor

@gundalow gundalow left a comment

Choose a reason for hiding this comment

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

Looks OK to me

@gundalow gundalow merged commit c7a4b39 into ansible:devel Nov 5, 2020
@felixfontein felixfontein deleted the action-groups branch November 5, 2020 10:21
@felixfontein
Copy link
Contributor Author

@geerlingguy @bcoca @sivel @gundalow thanks for reviewing!

felixfontein added a commit to felixfontein/ansible that referenced this pull request Nov 5, 2020
@felixfontein
Adjust action groups to moved modules (ansible#72428)
c251f6d 
* Support docker and k8s action groups for moved modules in community.docker and community.kubevirt.

* Also support k8s action group for community.okd.

* Also add kubernetes.core.

* Adjust PR #.

* Fix changelog fragment.

* Remove community.okd.

* Revert "Remove community.okd."

This reverts commit 812b5aa.

(cherry picked from commit c7a4b39)
@felixfontein felixfontein mentioned this pull request Nov 5, 2020
Merged
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Nov 5, 2020
@ansible ansible locked and limited conversation to collaborators Dec 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sivel sivel sivel left review comments

@bcoca bcoca bcoca left review comments

@gundalow gundalow gundalow approved these changes

@s-hertel s-hertel Awaiting requested review from s-hertel

Assignees
No one assigned
Labels
affects_2.11 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@felixfontein @gundalow @geerlingguy @sivel @bcoca @ansibot