kewlfft / ansible-aur

Ansible module to manage packages from the AUR
GNU General Public License v3.0
294 stars 46 forks source link

Migrate `aur` module from legacy role to collection #58

Closed gotmax23 closed 3 years ago

gotmax23 commented 3 years ago

This pull request creates the necessary directory structure and metadata for an Ansible Galaxy collection. It provides necessary documentation for the new collection, as well.

I created a Github Actions workflow that will automatically push the collection to Ansible Galaxy when you create a Github Release. In order to use it, you will have to add your Galaxy API key in a Github secret named GALAXY_API_KEY. To release a new version of the collection, all you will need to do is update the version: tag in galaxy.yml (the collection metadata file) and create a Github release; Github Actions will handle the rest. Please note Ansible requires collections to use Semantic Versioning, which this repo does not currently use.

Right now, the only thing that we need to address is maintaining compatibility for current users. I will follow up in a separate response about this.

Commits:

Fixes #57

gotmax23 commented 3 years ago

Since you said you weren't familiar with collections, here are some links to Ansible's collections documentation:

  1. Using collections
  2. Developing collections
  3. Migrating Roles to Roles in Collections on Galaxy
hummeltech commented 3 years ago

Great job, seems like this could eventually be merged into community.general at this rate!

I.E. https://github.com/ansible-collections/community.general/blob/c0740ca3985d827e309d9bff868374a5fb86bce7/plugins/modules/packaging/os/pacman.py#L50-L56

gotmax23 commented 3 years ago

For the role on Ansible Galaxy, we will have to delete the ansible.aur role so we can replace it with the ansible.aur collection. Changing the new collection's name would not work either, as we still have to change this repository's structure to migrate to a collection.

It's a different story for the AUR package because the AUR package does not actually contain the ansible.aur role. Instead, it just copies aur.py to Ansible's local custom modules path. This PR moves aur.py from library/aur.py to plugins/modules/aur.py in accordance with Ansible's collection structure. Therefore, we only need to change the directory from where the PKGBUILD installs aur.py to maintain current functionality. (See the diff below) /cc AUR package maintainer @panchoh

However, in the long run, I recommend we deprecate that package and replace it with an ansible-collection-kewlfft-aur package that contains the new collection. This way, the AUR package will be versioned and have the exact same content that is found on Ansible Galaxy.

As you'll notice after reading my changes to the README, after we migrate to a collection it will be required to use the module's FQCN in playbooks (both my changes to the README and the documentation I linked above explain what an FQCN is). But, when the aur module is installed using the ansible-aur-git AUR package, it does not have the same FQCN due to the nature of local custom modules. This is the other reason I think we should deprecate the current aur package.

In the PR, I updated the README with instructions to install and use the collection and completely removed the old installation instructions. When we decide how to move forward, I will fix the README.


diff --git a/PKGBUILD b/PKGBUILD
index 2baef95..ee188a7 100644
--- a/PKGBUILD
+++ b/PKGBUILD
@@ -24,6 +24,6 @@ pkgver() {

 package() {
    cd "${_pkgname}"
-   install -Dm644 README.md    "$pkgdir/usr/share/doc/${_pkgname}/README.md"
-   install -Dm644 library/aur.py   "$pkgdir/usr/share/ansible/plugins/modules/aur.py"
+   install -Dm644 README.md        "$pkgdir/usr/share/doc/${_pkgname}/README.md"
+   install -Dm644 plguins/modules/aur.py   "$pkgdir/usr/share/ansible/plugins/modules/aur.py"
 }
panchoh commented 3 years ago

Great job, @gotmax23! Just say the word, and I'll add you as a comaintainer of the AUR package.

Happy hacking,

gotmax23 commented 3 years ago

Fyi, I just rebased my branch and force pushed to fix an error in the Create workflow to push collection to Galaxy commit.

igas commented 3 years ago

Hi @kewlfft is there any other blockers? I'm looking forward to use it with awx in my homelab 🙏🏼

kewlfft commented 3 years ago

Hi @kewlfft is there any other blockers? I'm looking forward to use it with awx in my homelab 🙏🏼

Sure I’ll review within a few days

kewlfft commented 3 years ago

@gotmax23 ready for release then. As is, it will release 1.0.0 as a collection and I will let GitHub operate. I have set my secret. Anything else?

gotmax23 commented 3 years ago

@gotmax23 ready for release then.

Yay!

As is, it will release 1.0.0 as a collection and I will let GitHub operate. I have set my secret. Anything else?

Yes, you will have to delete the existing kewlfft.aur role from Ansible Galaxy before pushing the kewlfft.aur collection. You can do so from the command line with the ansible-galaxy role delete command or from the Ansible Galaxy webui. We probably should add a notice to the top of the README to explain that we migrated the role to a collection and provide brief migration instructions. After that, the Github Actions workfow should work as expected.

Also, we need to figure out what to do with the ansible-aur-git package. @panchoh already updated ansible-aur-git to work with the new structure. However, that AUR package is no longer mentioned in the README. As I explained in previous comments. I created a new AUR package ansible-collection-kewlfft-aur that installs the new collection, instead. We have three options:

  1. Leave the ansible-aur-git package as it is but don't mention it in the README
  2. Mention the ansible-aur-git package in the ### Install theaurmodule as a local custom module section of the README.
  3. Completely deprecate ansible-aur-git in favor of ansible-collection-kewlfft-aur.

2 is probably the best option. @panchoh and @kewlfft, what do you think?

Thanks, Maxwell

kewlfft commented 3 years ago

I have deleted the previous aur role in Ansible Galaxy, numbered the version 0.9.0 as I suppose it will need a bit of testing in this new environment before considering it stable. For the AUR package I'm fine with 2) or 3), we can go with 2). If you have time to push the README changes you're welcome. Let me know if anything didn't work as expected.

panchoh commented 3 years ago

Hi, @gotmax23 and @kewlfft.

We have three options:

  1. Leave the ansible-aur-git package as it is but don't mention it in the README
  2. Mention the ansible-aur-git package in the ### Install theaurmodule as a local custom module section of the README.
  3. Completely deprecate ansible-aur-git in favor of ansible-collection-kewlfft-aur.

2 is probably the best option. @panchoh and @kewlfft, what do you think? 2 is OK, but so is 3, IMHO.

Seems to me that upholding the full namespace thingy is the right way to go about this. You have my blessings if you decide to deprecate ye'ole ansible-aur-git. Good riddance! ;-)

gotmax23 commented 3 years ago

Seems to me that upholding the full namespace thingy is the right way to go about this. 

I agree with @panchoh; I think it makes sense to focus on the collection. This is why I wanted to deprecate installing aur into the custom module directories (/usr/share/ansible/plugins/ modules and ~/.ansible/plugins/modules), entirely. Since @kewlfft expressed hesitation about doing that, I figured option 2 would be the best.

However, I found another problem after I thoguht about it more: the two different installation methods are not intercompatible. Here is an example scenario that explains this issue: Let's say user A installed the module in the local custom module directory and user B installed it through the kewlfft.aur collection. As I explained in the README, user A would have to use the short name aur, but user B would have to use the FQCN kewlfft.aur.aur. If user A shared a playbook that uses this module with user B (or vice-versa) and the second user runs it, the playbook would fail with a "Module not found" error because of the different module names.

Therefore, I recommend that you reconsider removing the instructions to install the module in the custom module directory from README. For the same reason, I now think we should go with option 3.

I don't mean to be pushy; I just don't want my changes to cause problems!

Thanks, Maxwell

kewlfft commented 3 years ago

Then we go with 3) and focus on the collection installation rather than 2 packages for such a small tool. Then Arch is about flexibility and simplicity, consistent with the wiki we present different ways and also simpler manual methods.