ansible-community / molecule-plugins

Collection on molecule plugins
MIT License
109 stars 73 forks source link

install python3 only when ansible_system == Linux #110

Closed konstruktoid closed 1 year ago

konstruktoid commented 1 year ago

This PR:

apatard commented 1 year ago

thanks for doing it. I'm wondering : Are there some other connection type needing the same "fix" ?

konstruktoid commented 1 year ago

I guess everything that installs things using Linux package managers. I know dont Windows enough regarding e.g ansible package:

zhan9san commented 1 year ago

Are there some other connection type needing the same "fix" ?

Like @konstruktoid mentioned, I suggest to use ansible.builtin.package, and add a condition ansible_system == Linux

It needs enable gather_facts.

konstruktoid commented 1 year ago

replaced raw: with package:

apatard commented 1 year ago

replaced raw: with package:

why ? it's the bootstrap phase which means that on systems python may even not be installed, that's why it's using raw.

konstruktoid commented 1 year ago

why ? it's the bootstrap phase which means that on systems python may even not be installed, that's why it's using raw.

but they all do? all checks passed, and i assumed this was to ensure that python3 was installed. but no worries, if it's an issue then i'll revert it back

apatard commented 1 year ago

why ? it's the bootstrap phase which means that on systems python may even not be installed, that's why it's using raw.

but they all do? all checks passed, and i assumed this was to ensure that python3 was installed. but no worries, if it's an issue then i'll revert it back

Just take the alpine box we're using in CI:

$ cat Vagrantfile
Vagrant.configure("2") do |config|
  config.vm.box = "generic/alpine316"
end
$ vagrant up
$ vagrant ssh
alpine316:~$ python
-bash: python: command not found
alpine316:~$ python3
-bash: python3: command not found
konstruktoid commented 1 year ago

reverted back to raw:

but shouldn’t the tests had failed due to the alpine boxes and package: then?

apatard commented 1 year ago

vagrant tests are not run. I've some code to fix that but I've yet some issues to fix with it. I've opened an issue for that: https://github.com/ansible-community/molecule-plugins/issues/88

apatard commented 1 year ago

btw, if we're enabling gather_facts, should we use gather_subset too ?

konstruktoid commented 1 year ago

don't merge, if we assume a system doesn't have python installed, gather_facts wont work

konstruktoid commented 1 year ago

it may not be pretty, but it works

$ vagrant up
Bringing machine 'default' up with 'virtualbox' provider...
==> default: Importing base box 'generic/alpine316'...
[...]
$ vagrant ssh -c 'python3 --version || echo "no python"'
bash: line 1: python3: command not found
no python
Connection to 127.0.0.1 closed.
$ ansible-playbook -i inventory -c ssh subset.yml

PLAY [all] ******************************************************************************************************

TASK [Gather system info] ****************************************************************************************************************
The authenticity of host '[127.0.0.1]:2222 ([127.0.0.1]:2222)' can't be established.
ED25519 key fingerprint is SHA256:lgbEKJGReOkWt6m6RqSvyv6cWnLln3AOLSwq2VWLwbU.
This key is not known by any other names
Are you sure you want to continue connecting (yes/no/[fingerprint])? yes
ok: [alpine]

TASK [Bootstrap python for Ansible] *****************************************************************************************************************
ok: [alpine]

PLAY RECAP *****************************************************************************************************************
alpine                     : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

$ vagrant ssh -c 'python3 --version || echo "no python"'
Python 3.10.10
Connection to 127.0.0.1 closed.
zhan9san commented 1 year ago

LGTM

Thanks for your patience@konstruktoid

If you don't have concerns, we can merge it.@apatard

apatard commented 1 year ago

@konstruktoid thanks for bearing with me. Merging it now.