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

Migrate to dokku collection #148

Open
ltalirz opened this issue Oct 16, 2022 · 20 comments
Open

Migrate to dokku collection #148

ltalirz opened this issue Oct 16, 2022 · 20 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@ltalirz
Copy link
Member

ltalirz commented Oct 16, 2022

The ansible-dokku role bundles a number of ansible modules in the libraries folder - e.g. the dokku_app module.

Sometimes, it is useful to run these modules in isolation, without running the entire role. While this can currently be achieved via a hack, it would seem better to migrate ansible-dokku to a dokku ansible collection, which would, once installed, make the modules available via

- name: create my app
  dokku.dokku_app:
    app: hello-world
@ltalirz ltalirz added the enhancement New feature or request label Oct 16, 2022
@ltalirz
Copy link
Member Author

ltalirz commented Oct 16, 2022

See the migration guide. I currently don't have time to work in this - help welcome!

@ltalirz ltalirz added the help wanted Extra attention is needed label Oct 16, 2022
@tbho
Copy link

tbho commented Nov 29, 2022

Hey @ltalirz,
I gave the issue a try in the linked PR. Just opened it as a draft because of the incompleteness. Would love to hear your opinion.

@ltalirz
Copy link
Member Author

ltalirz commented Nov 29, 2022

Hi @tbho , thanks a lot for this contribution, this is very welcome.
I think this goes into the right direction.

One open question is which name the new collection should have.
After having a look around, it appears that converting a role into a collection of the same name is still not a supported scenario ansible/galaxy#2253

Perhaps instead of ansible_dokku, we should call it dokku_collection?

It also appears that there is currently no practical way of alerting current users of the role about the existence of the collection. For that reason, geerlingguy is still holding off with the transition to collections. ansible/galaxy#2253 (comment)

I personally suspect that most users of our role have visited the Github page before, and would eventually come back here to check.
I also think that the change in users' playbooks that this will necessitate is balanced out by the benefit of being able to use individual modules in isolation rather than just as part of running the entire role [1].

@josegonzalez Do you have any thoughts on this topic?

And @tbho: Just to make sure we're solving your problem, I'd be interested to understand what prohibits you from going down route [1]. In my personal daily use, I always had the ansible_dokku role added on the dokku host(s), and then had a couple more tasks/roles (which used modules like dokku_app) that I could run individually via ansible tags if desired.
The example in #137 (comment) from the test suite of the role is actually the first case where came across this problem (I guess we could also have used tags here to skip running the role, but it would have been a bit awkward).

[1] Although, in practice, you can easily achieve this with tags already with today's role.

@tbho
Copy link

tbho commented Nov 29, 2022

Hi @tbho , thanks a lot for this contribution, this is very welcome. I think this goes into the right direction.

One open question is which name the new collection should have. After having a look around, it appears that converting a role into a collection of the same name is still not a supported scenario ansible/galaxy#2253

Perhaps instead of ansible_dokku, we should call it dokku_collection?

I am not that deep into ansible, so didn't know about the double name problem yet. But dokku_collection sounds good.

It also appears that there is currently no practical way of alerting current users of the role about the existence of the collection. For that reason, geerlingguy is still holding off with the transition to collections. ansible/galaxy#2253 (comment)

I personally suspect that most users of our role have visited the Github page before, and would eventually come back here to check. I also think that the change in users' playbooks that this will necessitate is balanced out by the benefit of being able to use individual modules in isolation rather than just as part of running the entire role [1].

@josegonzalez Do you have any thoughts on this topic?

Is there some sort of deprecation notice we can add to the role? Then we could set up the collection in parallel and disable the role after some time to give time for migration of the users playbooks.

And @tbho: Just to make sure we're solving your problem, I'd be interested to understand what prohibits you from going down route [1]. In my personal daily use, I always had the ansible_dokku role added on the dokku host(s), and then had a couple more tasks/roles (which used modules like dokku_app) that I could run individually via ansible tags if desired. The example in #137 (comment) from the test suite of the role is actually the first case where came across this problem (I guess we could also have used tags here to skip running the role, but it would have been a bit awkward).

Did not know about tags. How did you tag the playbooks so only the app provisioning parts are run?

How we use dokku and the ansible-dokku package @ finstreet right now:
We built roles with the ansible modules which are based on yaml group variables which do look similar to docker-compose files. As an example, I put our template right here:

apps:
  - name: product-partner
    redis: product-partner
    postgres: product-partner
    domains:
      - test.domain.de
    environment:
      # specify a email for dokku-letsencrypt
      DOKKU_LETSENCRYPT_EMAIL: [email protected]
      # specify port so `domains` can setup the port mapping properly
      PORT: "5000"
    custom_nginx_conf:
      client-max-body-size: 10m
    docker_options:
      # build:
      # deploy:
      run:
        - --add-host=mail.server.tld:1.2.3.4
    acl:
      - karl-ranseier
    storage_mounts:
      - /mnt/path/subpath/log:/app/log
    proxy:
      - http:80:5000
      - https:443:5000
    letsencrypt: true

We can define multiple apps for a server, which are provisioned to that specific host then. We then define one server as a group and can limit the provisioning to that server. We are even thinking of a provisioning limitation of multiple apps which belong to a customer. It's a pretty crazy setup to be honest. Right now we manage about 60-80 dokku apps on about 10-15 servers (can get the exact numbers if you want to know 😉). We adapted this to not have to copy the dokku_app definitions for every app and so on. I think there are ways to do it more pretty, but that's what go us going in the first place. It's some sort of work in progress, as we are constantly updating our roles and stuff.

I hope you can understand what I am saying and what I mean. 😂

[1] Although, in practice, you can easily achieve this with tags already with today's role.

@ltalirz
Copy link
Member Author

ltalirz commented Nov 29, 2022

Did not know about tags. How did you tag the playbooks so only the app provisioning parts are run?

See e.g. https://github.com/ltalirz/ansible-playbook-dokku/blob/master/playbook.yml
You would then simply run ansible-playbook playbook.yml --tags my_dokku_users
It doesn't matter that the ansible_dokku role isn't actually run - it being listed under roles: is enough to make its modules available.

This reminds me that we have a blog post "almost ready" that goes through the steps of how to use the role to set up you dokku instance dokku/dokku.github.io#7

Perhaps I'll manage to get it done one of these days.

@tbho
Copy link

tbho commented Nov 29, 2022

Did not know about tags. How did you tag the playbooks so only the app provisioning parts are run?

See e.g. https://github.com/ltalirz/ansible-playbook-dokku/blob/master/playbook.yml You would then simply run ansible-playbook playbook.yml --tags my_dokku_users It doesn't matter that the ansible_dokku role isn't actually run - it being listed under roles: is enough to make its modules available.

Ah, see the solution.
The tags would not be practicable for us to be honest. As we would need to copy every 'dokku_' task for every new instances. With our solution, we added a layer of abstraction that enables us to define apps on the one side, but change every 'dokku_' style task at once.

@josegonzalez
Copy link
Member

@ltalirz I have no preference either way.

@tbho
Copy link

tbho commented Dec 28, 2022

I'm not quite sure how we should proceed. Tried to get the collection to work, including the dokku installation role. As of now, I tried to separate the two parts. The collection for the configuration of dokku and the role to install dokku onto a host.

In my mind, a clean solution would be the separation. What do you think about the topic @josegonzalez @ltalirz?

@ltalirz
Copy link
Member Author

ltalirz commented Jan 2, 2023

Thanks for this work @tbho !
Since the pull request involves a number of files and will take some time to review, would you mind adding a comment to it that documents the choices you made in separating the configuration and installation stage (and where you are looking for input specifically)?

@tbho
Copy link

tbho commented Jan 2, 2023

@ltalirz Yes of course I can provide some explanation. But in my mind we need to open a new repository where the collection can reside as the old tasks and the new collections have a different repository structure.

The created PR does reflect the collection repository as it should look like. If you like we can have a short meeting where I can give you some further information.

@ltalirz
Copy link
Member Author

ltalirz commented Jan 3, 2023

@tbho if possible, keeping the history of the repository alive makes sense to me.
In your PR, git recognizes the movement of files, i.e. a significant portion of the provenance will be retained.

Looking at the proposed directory layout, I seem to be missing the roles/ folder, which would contain the modified ansible-dokku role that uses the plugins from the collection (see e.g. this example).

If you prefer a chat over Github comments, we can arrange a short meeting late this evening. Feel free to contact me via any of the methods on https://ltalirz.github.io/

@tbho
Copy link

tbho commented Jan 3, 2023

It's possible to integrate the roles "back" into the collection. But somehow the molecule throws some weird errors.
I can revert one commit I made where I removed the roles. Then I need some help though to resolve the molecule errors.

I will prepare the commit and give some explanation so you can review the PR.

@ltalirz
Copy link
Member Author

ltalirz commented Jan 3, 2023

Thanks!

@tbho
Copy link

tbho commented Jan 3, 2023

Okay, remembered why I thought it might be an idea to split up installation and configuration parts. If I'm understanding the ansible documentation the right way, collections should only be dependent on other collections (https://docs.ansible.com/ansible/latest/dev_guide/developing_collections_shared.html#listing-collection-dependencies and https://docs.ansible.com/ansible/latest/dev_guide/collections_galaxy_meta.html#collections-galaxy-meta).

Including the role into the collection works though (tested this on my own server), but I'm not quite sure how to specify geerlingguy.docker and nginxinc.nginx as dependencies.

@ltalirz
Copy link
Member Author

ltalirz commented Jan 3, 2023

Okay, remembered why I thought it might be an idea to split up installation and configuration parts. If I'm understanding the ansible documentation the right way, collections should only be dependent on other collections (https://docs.ansible.com/ansible/latest/dev_guide/developing_collections_shared.html#listing-collection-dependencies and https://docs.ansible.com/ansible/latest/dev_guide/collections_galaxy_meta.html#collections-galaxy-meta).

Oh, I see. Thanks for alerting me to this, unfortunately I was not aware.

This is indeed quite worrying. Both the recommendation not to have dependencies at all (seriously?), and the decision to support only collection-collection dependencies seem quite unfriendly to existing developers.
Some reasoning behind this decision is provided in ansible/ansible#76030 (comment)

If we want to proceed along the recommended path, we would need to drop the dependency on the geerlingguy.docker and nginxinc.nginx roles, either by reimplementing what we need here, or by vendoring a copy of these roles in the roles/ directory [1].
However, that inevitably leads to code duplication and, in my view, would be a degradation of the status quo - with the current setup, we profit from developments in those widely used roles, such as #142 (comment) , and I would hate to have to constantly backport these changes into the dokku collection.

I understand your confusion as to how to proceed, at this point it is not clear to me either - my apologies.
I'm tempted to suggest putting a few months break on the pull request to see whether the ecosystem will evolve (e.g. whether the role dependencies we need will become available in the form of collections, or whether dependencies between collections and roles will be supported).
Let me know what you guys think.

I'd still be happy to have a chat some time concerning your particular use case, and why the current role cannot support it in a practical way.

[1] Both roles have permissive licenses (geerlingguy.docker has MIT, nginxinc.nginx has apache), so in principle we could vendor them.

@tbho
Copy link

tbho commented Jan 3, 2023

Okay, remembered why I thought it might be an idea to split up installation and configuration parts. If I'm understanding the ansible documentation the right way, collections should only be dependent on other collections (https://docs.ansible.com/ansible/latest/dev_guide/developing_collections_shared.html#listing-collection-dependencies and https://docs.ansible.com/ansible/latest/dev_guide/collections_galaxy_meta.html#collections-galaxy-meta).

Oh, I see. Thanks for alerting me to this, unfortunately I was not aware.

This is indeed quite worrying. Both the recommendation not to have dependencies at all (seriously?), and the decision to support only collection-collection dependencies seem quite unfriendly to existing developers. Some reasoning behind this decision is provided in ansible/ansible#76030 (comment)

Yeah, I see the same problems. I was perplexed as I found the documentation.

If we want to proceed along the recommended path, we would need to drop the dependency on the geerlingguy.docker and nginxinc.nginx roles, either by reimplementing what we need here, or by vendoring a copy of these roles in the roles/ directory [1]. However, that inevitably leads to code duplication and, in my view, would be a degradation of the status quo - with the current setup, we profit from developments in those widely used roles, such as #142 (comment) , and I would hate to have to constantly backport these changes into the dokku collection.

With this, I came up with the idea to isolate the plugins into an own collection and develop them separately from the installation role (which can further exist as a role and therefore can profit from the dependency). Users then could use something like this:

- name: Install dokku (using the role)
  include_role:
    name: dokku_bot.ansible_dokku
  vars:
    dokku_plugins:
    .....

- name: 'Create app some name'
  dokku_bot.dokku_collection.dokku_app:
    app: 'some_name'

If I'm correct, the users only would need to prepend the correct collection name in front of the configuration tasks. Bonus points: There could be a time horizon with both the role and the collection coexisting to give users the chance to migrate to the collection. Duplication of the roles is a no-go in my eyes.

I understand your confusion as to how to proceed, at this point it is not clear to me either - my apologies. I'm tempted to suggest putting a few months break on the pull request to see whether the ecosystem will evolve (e.g. whether the role dependencies we need will become available in the form of collections, or whether dependencies between collections and roles will be supported). Let me know what you guys think.

I'm also fine with putting the PR to a break. The -tags option can be a suitable solution, but we would need to change a lot of our playbooks and structures, I think. The collection solution would be more suitable to my company.

@ltalirz
Copy link
Member Author

ltalirz commented Jan 3, 2023

Thanks for your understanding!

I think your suggestion can be a path forward if no better solution is found in the coming months. Currently, your proposal would still be a degradation of the user experience, since (correct me if I'm wrong) there is no way to specify that dokku_bot.ansible_dokku depends on dokku_bot.dokku_collection. I.e. if users just ansible-galaxy install dokku_bot.ansible_dokku as before, they would end up with a broken role (and all scripts that install the latest version of the dokku_bot.ansible_dokku role will break).

It is true that the fix for users is rather simple (just install the collection alongside the role), but it still feels wrong to put extra burden on the user for a role that should work out of the box.

Let's wait a little bit for some positive signals from the ansible universe

@tbho
Copy link

tbho commented Jan 4, 2023

Ok, no problem.

@josegonzalez
Copy link
Member

Can we use tags to disable running the role and only have access to the modules?

@ltalirz
Copy link
Member Author

ltalirz commented Feb 8, 2023

In order to get access to the modules without running the role, you can use this trick:

- name: Run role without running any tasks
include_role:
name: dokku_bot.ansible_dokku
tasks_from: init.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants