search

geerlingguy / ansible-role-docker

Ansible Role - Docker
https://galaxy.ansible.com/geerlingguy/docker/
MIT License
1.84k stars 857 forks source link

Fix apt errors on Pop!_OS #396

Closed dale-c-anderson closed 1 year ago

dale-c-anderson commented 1 year ago

Tested on Pop!_OS 22.04, but this is a tiny fix which should work for any version (of pop or mint).

Closes #382 Closes #334

Guergeiro commented 1 year ago

This is somewhat relevant to #334 Maybe having a consistent way of targeting Ubuntu based distros instead of conditionally chaining all logic. For instance, I know that Linux Mint has a /etc/upstream-release/lsb-release.

dale-c-anderson commented 1 year ago

@Guergeiro This could potentially be done more cleanly with a lookup table, but the real issue here is that there is no way to get the string ubuntu out of ansible facts for these distributions, so there's going to be some conditional logic no matter how you slice it. If you can demonstrate how parsing config files (which may or may not be present) is going to be cleaner than that, I'll buy you a beer.

I added support for Linux Mint to my PR. So now this can close #334 as well.

stale[bot] commented 1 year ago

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark pull requests as stale.

github-actions[bot] commented 1 year ago

This pr has been closed due to inactivity. If you feel this is in error, please reopen the issue or file a new issue with the relevant details.

geerlingguy commented 1 year ago

The question I have is where is docker_apt_ansible_distribution defined?

dale-c-anderson commented 1 year ago

@geerlingguy The way github highlights the diff by default, it's not all that easy to spot.

Line 39: https://github.com/geerlingguy/ansible-role-docker/pull/396/files#diff-23e21b6c89468f7534697ad091835b3ee5e0213b37ace489488234e5a9548ec4R39

(edited)

dale-c-anderson commented 1 year ago

I see there's conflicts, which I'll get fixed up today.