search

evrardjp / ansible-keepalived

Keepalived role for ansible deployment
Apache License 2.0
98 stars 98 forks source link

Add installation via snap #161

Closed devster31 closed 1 year ago

devster31 commented 3 years ago

I find snap a useful installation method for some binaries as it allows consistent, up-to-date versions across distribution and it's a bit easier to use than a container, hence this proposal. I added this option to the role and a simple molecule scenario which should behave exactly like the default installation but use the snap binary. I see two possible pending points:

evrardjp commented 3 years ago

I see two possible pending points:

* the inclusion of a separate role for snap setup and installation (e.g. `don_rumata.ansible_role_install_snap`) instead of including tasks in this role

* the addition of `community.general` to `meta/requirements.yml`; the role now uses `community.general.snap` and the molecule tests use `community.general.docker_network`.

To be honest, I would prefer NOT include the snap setup in the role itself (it's fine in the molecule testing though). This is similar to the experience we have with selinux or many other necessary tools. What I mean by this, is that we should document that for snap usage, the user should ensure all the snap requirements are setup, if necessary highlight how we've done it in our molecule testing. However, the role should not require other roles to be used.

I am fine to introduce a community general in meta requirements, as it seems it's the general tendency anyway. This role was refreshed recently, and we removed some old backwards compatibility, so i think it's a good time to do so now.

devster31 commented 3 years ago

I moved most of the snap setuo in the converge.yml playbook. I also tried something different with https://github.com/evrardjp/ansible-keepalived/pull/161/commits/ff2d15d9f9a8002be04fea57c7f75826ab67e0a5 but it introduces dependencies on community.general.json_query and the pip package jmespath instead of community.general.snap.

evrardjp commented 3 years ago

I think ff2d15d is fine, but I want to really document why we do this before merging.

evrardjp commented 3 years ago

Do you want me to add the "why" into this PR, so we can merge it?

devster31 commented 3 years ago

Sorry, missed the last message. Sure, I commented above on the why I used the command, some snaps have "services" which are difficult to customize and not managed by a systemd file. I haven't found an alternative to stop/disable these kind of daemons provided by snap besides my last attempt.