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

Document how to migrate a standalone role to a collection #68687

Merged
merged 13 commits into from
May 4, 2020

Conversation

samccann
Copy link
Contributor

@samccann samccann commented Apr 3, 2020

SUMMARY

Document how to migrate a standalone role to a collection.

Fixes #68473

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

docs.ansible.com

ADDITIONAL INFORMATION

@samccann samccann added docs This issue/PR relates to or includes documentation. affects_2.10 This issue/PR affects Ansible v2.10 labels Apr 3, 2020
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. docsite This issue/PR relates to the documentation website. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Apr 3, 2020
@samccann
Copy link
Contributor Author

samccann commented Apr 6, 2020

@geerlingguy - would love feedback on this when you get a chance.

Copy link
Contributor

@acozine acozine left a comment

Choose a reason for hiding this comment

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

This PR has a lot of good detail. I've suggested some ways of streamlining it a bit . . .

docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved
| :file:`library/` with custom modules | - Move to :file:`plugins/modules/` |
| | - Use :abbr:`FQCN (Fully Qualified Collection Name)` external to collection |
+----------------------------------------+------------------------------------------------------------------------------+
|``ansible.module_utils`` in Python + Use :abbr:`FQCN (Fully Qualified Collection Name)` for Python imports |
Copy link
Contributor

Choose a reason for hiding this comment

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

Also since we're talking about roles still (a standalone role vs a role in a collection), what other content within the role directory besides plugins (which I believe have to actually move out of the role and into the collection proper) would need to use ansible.module_utils?

If this is more of a guide for "how to move plugins out of roles into collection plugins directories", then maybe this section/table should be re-introduced as

"roles can have the same structure inside collections as when they're standalone, but you have to move all the plugins and supporting Python code out of the role and into the collection, renaming and modifying the Python code like so:"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

snipped the table entirely. I think this is covered in the steps but let me know if anything is still missing.

docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved
@geerlingguy
Copy link
Contributor

(Forgot to leave comment for my review, so here it is)

One thing I think is a little difficult with this doc is the fact that it's written mostly for the perspective of someone maintaining a role on Galaxy that is intended to become a collection on Galaxy—and even in that case there are a few parts that I think could be improved (see review comments).

But from the perspective of converting an existing standalone role that is not on Galaxy, there are a few mines that are waiting for the unsuspecting user to step on, with regard to namespacing (not relevant for standalone roles, but it looks like we'll be required to make up our own namespace and have another three levels of empty directory heirarchy to navigate when using collections, so it's in ansible_collections/empty_namespace_dir/collection_name_dir/roles/[the role] instead of roles/[the role]).

Additionally, and I've opened an issue for this, the given ansible-galaxy collection init command doesn't behave like I would expect and resulted in me having to manually create an ansible_collections directory and copy the generated collection and namespace dirs into that path so my playbook could pick it up. See issue: #68915

@geerlingguy
Copy link
Contributor

(Noting that even prominent examples of Red Hat-maintained playbooks use standalone roles like the ones I mention in the comment above—see https://github.com/ansible/tower/tree/devel/installer/roles)

Also, I didn't review the RPM packaging notes at the end of this PR as I don't have any experience there.

@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 Apr 13, 2020
@samccann
Copy link
Contributor Author

@geerlingguy - hmm github doesn't let me directly reply to some of your final comments. WRT being Galaxy-specific... I think this is in general a statement for the collection docs overall, correct? I'm not sure how many people create collections with no plan to upload them for others to use (in Galaxy/AH, etc). I'm reluctant to sprinkle in 'if you aren't ever uploading your collections... do blablabla'. Maybe what we need is a separate docs page that lists the exceptions for those who plan on 'standalone' private collections to cover this?

@geerlingguy
Copy link
Contributor

@samccann - I think the problem is there are thousands (likely hundreds of thousands) of roles out there that have never been intended to be stored on galaxy (or in many cases even used in more than one playbook), and that use case is often neglected in collections docs since the primary driver behind most things collections—so far—has been module and plugin extraction from core and distribution.

That use case is vastly different than what roles are/have been used for, so focusing on Galaxy and abstracted roles exclusively has blinded a lot of people to the problems and difficulties end users face when trying to work with roles in collections.

@samccann
Copy link
Contributor Author

the latest commit clarifies this is the instructions for shared roles in a collection on Galaxy. Also, separated the sections for simple roles vs roles with plugins. The TOC will look like this now:
image

@ansibot ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels May 4, 2020
docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved

1. Create a local :file:`ansible_collections` directory and ``cd`` to this new directory.

2. Create a collection. You need a `Galaxy namespace <https://galaxy.ansible.com/docs/contributing/namespaces.html>`_ to import this collection to Galaxy.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we talked about this previously—but as this guide seems to be for any role (and not just roles that are already on Galaxy)—should this be a top level requirement, or can we move it to an aside? If an aside, I'd suggest:

If you wish to share your collection on Ansible Galaxy, you will need a `Galaxy namespace <https://galaxy.ansible.com/docs/contributing/namespaces.html>` to import the collection. Make sure you use the same namespace when creating the collection.

Something along those lines.

Copy link
Contributor

@acozine acozine left a comment

Choose a reason for hiding this comment

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

Overall looks good. I added a few edits.

docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved

.. note::

In standalone roles, some of the plugin directories referenced their plugin types in the plural sense; this is not the case in collections.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this true? In the example above, we have modules/ in the plugins/ directory - that's plural.

docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved

.. note::

If you want to import your collection to Galaxy, you need a `Galaxy namespace <https://galaxy.ansible.com/docs/contributing/namespaces.html>`_.
Copy link
Contributor

Choose a reason for hiding this comment

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

I might move this to a "Prerequisites" section right above the migration instructions.

docs/docsite/rst/dev_guide/migrating_roles.rst Outdated Show resolved Hide resolved
Co-authored-by: Alicia Cozine <879121+acozine@users.noreply.github.com>
@acozine acozine merged commit caa263e into ansible:devel May 4, 2020
@samccann samccann deleted the role_migrate branch May 18, 2020 18:28
@ansible ansible locked and limited conversation to collaborators Jun 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.10 This issue/PR affects Ansible v2.10 docs This issue/PR relates to or includes documentation. docsite This issue/PR relates to the documentation website. has_issue needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document how to convert legacy roles to collections
6 participants