Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install python3 only when ansible_system == Linux #110

Merged
merged 8 commits into from
Mar 23, 2023

Conversation

konstruktoid
Copy link
Contributor

@konstruktoid konstruktoid commented Mar 13, 2023

This PR:

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
@apatard
Copy link
Member

apatard commented Mar 14, 2023

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

@apatard apatard added bug Something isn't working vagrant labels Mar 14, 2023
@konstruktoid
Copy link
Contributor Author

konstruktoid commented Mar 14, 2023

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

@zhan9san
Copy link
Member

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.

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
@konstruktoid
Copy link
Contributor Author

replaced raw: with package:

@apatard
Copy link
Member

apatard commented Mar 21, 2023

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
Copy link
Contributor Author

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

@konstruktoid konstruktoid changed the title dont prepare when ansible_connection is winrm install python3 only when ansible_system == Linux Mar 21, 2023
@apatard
Copy link
Member

apatard commented Mar 21, 2023

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

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
@konstruktoid
Copy link
Contributor Author

reverted back to raw:

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

@apatard
Copy link
Member

apatard commented Mar 21, 2023

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: #88

@apatard
Copy link
Member

apatard commented Mar 21, 2023

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

@konstruktoid
Copy link
Contributor Author

konstruktoid commented Mar 21, 2023

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

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
@konstruktoid
Copy link
Contributor Author

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
Copy link
Member

LGTM

Thanks for your patience@konstruktoid

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

@apatard
Copy link
Member

apatard commented Mar 23, 2023

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

@apatard apatard merged commit a3f3d2f into ansible-community:main Mar 23, 2023
@konstruktoid konstruktoid deleted the issue109 branch March 23, 2023 13:14
dankow pushed a commit to dankow/molecule-plugins that referenced this pull request Mar 31, 2023
)

* assume windows when ansible_connection is winrm

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* replace raw: with package:

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* revert back to raw

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* use uname to check if it's a Linux system

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

---------

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
ssbarnea added a commit that referenced this pull request Apr 5, 2023
* Docker: Add support for `platform` parameter

Pass `platform` value from `platforms.item` to `docker_container`. Example usage:
```
driver:
  name: docker
platforms:
  - name: test-01
    image: ...
    pre_build_image: true
    platform: linux/amd64
```

As suggested in the [docker_container documentation](https://docs.ansible.com/ansible/latest/collections/community/docker/docker_container_module.html#ansible-collections-community-docker-docker-container-module), ignore the `platform` parameter when comparing state.

Addresses issue #94.

* Trigger checks

* Fix ci issue (#111)

* Fix ci issue

* fix ci

* Optimize CI (#112)

Install collection from galaxy, fix typo, do not install ansible via apt-get

* README.md: Update plugin list (#91)

With newly added plugins, the list has to be updated.

Signed-off-by: Arnaud Patard <apatard@hupstream.com>

* Perform linting on Ansible playbooks (#101)

* skip experimental rules

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* Lint ansible files

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* fix missed alignment

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* Fix blk100 flake8 errors

Flake8 is giving errors about some empty lines in the code, so
remove the line to fix these errors.

Signed-off-by: Arnaud Patard <apatard@hupstream.com>

---------

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
Signed-off-by: Arnaud Patard <apatard@hupstream.com>
Co-authored-by: Arnaud Patard <apatard@hupstream.com>

* [pre-commit.ci] pre-commit autoupdate (#93)

updates:
- [github.com/psf/black: 22.12.0 → 23.1.0](psf/black@22.12.0...23.1.0)
- [github.com/ansible/ansible-lint.git: v6.10.2 → v6.14.2](https://github.com/ansible/ansible-lint.git/compare/v6.10.2...v6.14.2)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* install python3 only when ansible_system == Linux (#110)

* assume windows when ansible_connection is winrm

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* replace raw: with package:

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* revert back to raw

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* use uname to check if it's a Linux system

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

---------

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* Make selinux an optional extra (#89)

As our special selinux shim dependency also caused problems for
people that did not really need selinux, we now make it a simple
extra.

* Trigger checks

---------

Signed-off-by: Arnaud Patard <apatard@hupstream.com>
Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
Co-authored-by: Jack <jack4zhang@gmail.com>
Co-authored-by: Arnaud Patard <apatard@hupstream.com>
Co-authored-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Sorin Sbarnea <ssbarnea@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working vagrant
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not run Python bootstrap prepare tasks in vagrant plugin for WinRM connections
3 participants