-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Removal of docker content for 2.0.0 #1304
Removal of docker content for 2.0.0 #1304
Conversation
@felixfontein This PR was evaluated as a potentially problematic PR for the following reasons:
Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: |
My idea is to merge this one once we're preparing this collection for the 2.0.0 release, i.e. after releasing 1.3.0 (the last planned minor 1.x.0 release). Let's polish this one so we can use it as a template for other content (community.routeros, community.postgresql, ...). |
d9adc56
to
0cf9fc4
Compare
0cf9fc4
to
7ea50dd
Compare
If you use Ansible 2.9 and the ``docker`` plugins or modules from this collections, community.general 2.0.0 results in errors when trying to use the docker content by FQCN, like ``community.general.docker_container``. | ||
Since Ansible 2.9 is not able to use redirections, you will have to adjust your playbooks and roles manually to use the new FQCNs (``community.docker.docker_container`` for the previous example) and to make sure that you have ``community.docker`` installed. | ||
|
||
If you use ansible-base 2.10 or newer and did not install Ansible 2.11, but installed (and/or upgraded) community.general manually, you need to make sure to also install ``community.docker`` if you are using any of the ``docker`` plugins or modules. |
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.
Did not install Ansible 2.11 ?? Did you mean 2.10?
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 first Ansible version this will be contained in is actually 2.11 :)
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.
(And now it's 3.0.0 and no longer 2.11...)
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.
Changelog looks great, thank you.
after this is done and merged, i'll do the same for postgres stuff |
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.
LGTM
b4fc5c3
to
1e1e62f
Compare
@acozine @samccann @gundalow @Andersson007 @dmsimard thanks a lot for reviewing and improving this PR! |
Is there no better way to handle the movement of plugins? This PR broke my AWX workflows as it could no longer find the docker_login plugin under general. Could the docker plugins not be marked as deprecated under general and modified to output a warning etc rather than breaking orchestration overnight? If not, could a warning not be added to the docs for 12 months and the plugin be available in both collections? I shall pin the version of the library in my requirements file but I find this a less than ideal solution... |
@SimonGurney with semantic versioning, you have two choices:
|
Your own documentation states: Deprecations are expected to have a deprecation cycle of at least 2 major versions (i.e. ~1 year). Maintainers can use a longer deprecation cycle if they want to support the old code for that long. And yet you deprecated docker out of general without any warning... I understand the benefit of semantic versioning, and use them when I feel I need to. I did not expect such poor disregard for all the organisations currently using this collection in their playbooks. Maybe stick to your own guidelines? |
We did not deprecate the docker content. We moved it to another collection. That's a big difference. The content is still there, it's not removed from the ecosystem, you only need to update FQCN (and install the new collection, in case you did not simply install Ansible 2.10) if you do not use a modern Ansible/ansible-base/ansible-core version. Also we did announce this change in time, and that actions are required for Ansible 2.9 users: https://github.com/ansible-collections/community.general/blob/stable-1/CHANGELOG.rst#major-changes-2 |
I disagree with this distinction. You removed it one from collection and put it in another, meaning the existing references are now broken in AWX / Tower playbooks. You say you gave adequate notice. Your guidelines say you will give ~ 12 months notice, you notification was issued on Nov 26 2020. Whether you like it or not, and I note you are keen to blame the end user here, there will be plenty of people realising they have broken implementations because you felt it was ok to do this. This damages the reputation of Ansible collections. You should have kept it in General for 12 months, with documentation advising users move immediately to the more specific collection. That's not rocket science, its a courtesy to your user base. |
The references are only broken when using Ansible 2.9. Upgrade to a newer version of Ansible / ansible-base / ansible-core if you want redirections to just work. I don't see why we should annoy all users of more modern versions of Ansible just so that 2.9 users are happy. Because that's the only other choice: annoy everyone by deprecating something that is not really deprecated, but just continues to work for everyone but Ansible 2.9 users. |
SUMMARY
Removes all docker content from community.general 2.0.0. Adds redirects to community.docker.
ISSUE TYPE
COMPONENT NAME
docker content