search

lae / ansible-role-travis-lxc

Ansible role that prepares a Travis CI environment and creates LXC containers for testing roles.
MIT License
4 stars 2 forks source link

Packages cleanup #15

Closed wilmardo closed 6 years ago

wilmardo commented 6 years ago

Removes a couple of packages which in my opinion shouldn't be installed. The OS'es should be as clean as possible to narrow the failure of a role. If a role needs a package out of the EPEL repo for example the test wouldn't fail on the test environment but on a clean installed CentOS it would.

Let me know what you think!

lae commented 6 years ago

curl is needed for several Ansible modules and I believe it is a sane expectation that most standard Linux installations will already have it installed (by this logic we should remove python from the Debian-based distributions too and expect users of this role to, well, install it within their containers). redhat-lsb-core is also standard in most installations and is used for some Ansible facts in CentOS.

The LXC templates in general are much more stripped down than their bare-metal counterparts, hence why these packages are installed in addition. (I didn't do a complete look through of the package differences between LXC and bare-metal installs so there are probably more missing packages)

Removing epel-release is fine, however.

wilmardo commented 6 years ago

Very reasonable explanation, did not know the LXC containers where more stripped down compared to their bare-metal counterparts. Thanks, I keep learning everyday 👍

To keep the commit history clean I did a force push with the EPEL remove commit.

Also I think that it might be a good idea to bump a major release after you merge this, removing the EPEL release seems a backwards compatibility change to me (might result in builds failing). Semver.org suggests a major version bump in that case (see point 8) What do you think about this?

wilmardo commented 6 years ago

Any update on this PR?

lae commented 6 years ago

v0.6.0 release cut with this change. I couldn't exactly merge this (deleted repo?) so had to recreate that commit.

Regarding semver, that only applies if major is > 0. This is specified in point 8, but see also point 4.