Added install-ansible module - #158
Conversation
Ticket: none Changelog: none
…works with provides from cfbs.json
7800ff0 to
13b4ed6
Compare
13b4ed6 to
734b778
Compare
nickanderson
left a comment
nickanderson
left a comment
There was a problem hiding this comment.
There are a couple classes promises that I think should be guarded.
Did you try using packages promises for pipx and hit a wall?
| "_cache_ttl" string => "3600"; | ||
|
|
||
| # 1 hour | ||
| # 1 hour |
There was a problem hiding this comment.
Was there before ... Would have to research, was a formatting change needed
There was a problem hiding this comment.
1 hour is a comment that got formatted away from the cache_ttl 3600 above.
|
|
||
| # 1 hour | ||
| # 1 hour | ||
| linux._have_smartctl:: |
There was a problem hiding this comment.
Probably could just be _have_smartctl::
There was a problem hiding this comment.
Yeah, was preserving existing as much as possible.
There was a problem hiding this comment.
The rest of the policy uses the linux class guard as well, often. I think probably we should not change that.
There was a problem hiding this comment.
yeah, it also really wants to be able to have a default context.
| # | ||
| # host specific data supported, in the data bundle context | ||
| # | ||
| # ansible_installed: class, if defined, installed ansible |
There was a problem hiding this comment.
below you are keying off of data:install_ansible I would specify it the same here. When I see a class without a namespace, I assume the class is expected to be in the default namespace.
There was a problem hiding this comment.
Doc rot as usual, will fix.
| # ansible_minimal_install: class, if defined, minimal install aka ansible-core | ||
| # ansible_full_install: class, if defined, full install. This is the default if neither minimal or full classes are defined. |
There was a problem hiding this comment.
same, I would call out the namespace explicitly here.
| windows:: | ||
| "Use WSL to install Ansible on Windows. See https://blog.rolpdog.com/2020/03/why-no-ansible-controller-for-windows.html"; | ||
|
|
||
| !linux:: | ||
| "Installing Ansible on a non-Linux host is not currently supported"; |
There was a problem hiding this comment.
Maybe swap the order so that the reports come out saying installing ansible on non liux isnt supported and then emits the instruction for installing on windows.
There was a problem hiding this comment.
Will make them separate, windows and not windows or Linux aka other unix.
| pipx_installed.!ansible_installed:: | ||
| "${install_ansible_command}" | ||
| contain => in_shell_umask(022), | ||
| comment => "The default umask of 077 would create /opt/pipx/venvs/ansible which is inaccessible to everyone but root, so use 022 instead."; |
There was a problem hiding this comment.
I guess the old pip package methods don't work well for this?
There was a problem hiding this comment.
Not sure. I didn't want to implement a pipx package module so went this route instead.
| reports: | ||
| ansible_installed:: | ||
| "ansible_installed"; |
There was a problem hiding this comment.
I would delete this report before shipping.
| windows:: | ||
| "Use WSL to install software"; | ||
|
|
||
| !linux:: | ||
| "Installing Ansible on a non-Linux host is not currently supported"; |
There was a problem hiding this comment.
Maybe reverse the order of reports so it says not supported and then tells how to install on windows?
There was a problem hiding this comment.
Will make them windows and other unixes.
| "pipx_installed" | ||
| expression => returnszero("command -v pipx >/dev/null", "useshell"); |
There was a problem hiding this comment.
Guard this on linux:: to avoid execution on non-linux cause pre-eval
| "/opt/pipx/venvs" | ||
| perms => mog("755", "root", "root"), | ||
| depth_search => recurse_with_base("1"), | ||
| comment => "/opt/pipx/venvs in some cases will be 700 which prevents non-root users from running installed commands."; |
There was a problem hiding this comment.
Curious about those cases. Is that specific platform?
There was a problem hiding this comment.
Saw it on alpine, will test on debian.
There was a problem hiding this comment.
This was a side-effect of running pipx install with a commands promise with default umask. When changing the umask to something more "normal", 022, things are OK without this so will remove. I tested debian and alpine and neither creates /opt/pipx or /opt/pipx/venvs when just installing the pipx package, only when using pipx to install.
…t being installed
Added shell deploy test to ensure pipx and ansible are installed according to specific classes for minimal vs full install and specific version of ansible. Ticket: ENT-14328
3fb5173 to
c402ded
Compare
craigcomstock
changed the title
2 participants
Ticket: none
Changelog: none