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

Inform synchronize module about community.docker collection #144

Conversation

tadeboro
Copy link
Contributor

SUMMARY

The synchronize action plugin has a built-in list of connection plugins that it knows how to handle.

One of those connection plugins is the docker connection plugin. And because the docker content has been moved around quite a lot, the docker connection plugin has quite a few names:

  • docker in Ansible 2.9,
  • community.general.docker for community.general < 2.0.0, and
  • community.docker.docker since a few months ago.

And while the synchronize module already knew about the first two names, the last one was still missing. This commit fixes that omission and adds a third name into the mix.

Fixes #132

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

synchronize

@Akasurde
Copy link
Member

Akasurde commented Mar 1, 2021

CI failures are known. PR ansible/ansible#73609 will resolve the error.

@Akasurde
Copy link
Member

Akasurde commented Mar 1, 2021

cc @maxamillion @felixfontein

@Akasurde Akasurde self-assigned this Mar 1, 2021
@felixfontein
Copy link
Contributor

👍 for this change!

@tadeboro tadeboro force-pushed the teach-synchronize-about-community-docker branch from e625fbf to 6cfde26 Compare March 1, 2021 08:28
@tadeboro
Copy link
Contributor Author

tadeboro commented Mar 1, 2021

I added a changelog fragment. My plan was to add it once I know things are green, but I guess this might take a while to become reality ;)

Comment on lines 3 to 4
- synchronize - add community.docker.docker to the list of supported
transports (https://github.com/ansible-collections/ansible.posix/issues/132)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- synchronize - add community.docker.docker to the list of supported
transports (https://github.com/ansible-collections/ansible.posix/issues/132)
- synchronize - add ``community.docker.docker`` to the list of supported
transports (https://github.com/ansible-collections/ansible.posix/issues/132).

@tadeboro tadeboro force-pushed the teach-synchronize-about-community-docker branch from 6cfde26 to a5e4601 Compare March 2, 2021 07:45
@Akasurde
Copy link
Member

Akasurde commented Mar 2, 2021

recheck

@Akasurde Akasurde self-requested a review March 2, 2021 15:31
@tadeboro tadeboro force-pushed the teach-synchronize-about-community-docker branch from a5e4601 to 5814ce1 Compare March 5, 2021 12:09
@Akasurde
Copy link
Member

Akasurde commented Mar 5, 2021

recheck

@Andersson007
Copy link
Contributor

@Akasurde i think we should wait until #147 merged

@Akasurde
Copy link
Member

Akasurde commented Mar 5, 2021

@Akasurde i think we should wait until #147 merged

👍🏼

The synchronize action plugin has a built-in list of connection
plugins that it knows how to handle.

One of those connection plugins is the docker connection plugin. And
because the docker content has been moved around quite a lot, the
docker connection plugin has quite a few names:

 - docker in Ansible 2.9,
 - community.general.docker for community.general < 2.0.0, and
 - community.docker.docker since a few months ago.

And while the synchronize module already knew about the first two
names, the last one was still missing. This commit fixes that omission
and adds a third name into the mix.
@tadeboro tadeboro force-pushed the teach-synchronize-about-community-docker branch from 5814ce1 to 98c86c5 Compare March 5, 2021 15:43
@Andersson007
Copy link
Contributor

the tests passed now

@Andersson007
Copy link
Contributor

after this merged, i can release the collection next week (if nobody disagrees)

@maxamillion maxamillion added the gate Gate PR in Zuul CI (Obsolete: Please set "mergeit" instead of "gate") label Mar 5, 2021
@ansible-zuul ansible-zuul bot merged commit 1747370 into ansible-collections:main Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gate Gate PR in Zuul CI (Obsolete: Please set "mergeit" instead of "gate")
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker action plugin moved but synchronize doesn't know that
5 participants