Fix ci issue #111

zhan9san · 2023-03-14T02:23:06Z

No description provided.

zhan9san · 2023-03-14T09:51:18Z

Could you kindly review this PR?

The ansible-galaxy is the worst package management tool. 🤪

apatard · 2023-03-15T13:50:48Z

src/molecule_plugins/podman/playbooks/create.yml

@@ -39,6 +39,7 @@
        - item.registry is defined
        - item.registry.credentials is defined
        - item.registry.credentials.username is defined
+      changed_when: false


are you sure it's the proper way to do that ? this command (and the next one) are clearly doing something on the system. Unless podman login or build are doing nothing ?

Thanks for pointing it out.

IMO, if the login fails, the playbook would fail.
Instead, if the login run successfully, there would be a bit noise if the status is changed. So the changed_when is set to false.

Does that make sense?

apatard · 2023-03-15T13:52:33Z

.ansible-lint

@@ -2,11 +2,13 @@
 exclude_paths:
  - .cache/ # implicit unless exclude_paths is defined in config
  - .github/
+  - requirements.yml


why ? what's the issue ?

The ansible-lint will try to check the yaml format file. But this file does not need linting.

apatard · 2023-03-15T13:53:59Z

test/gce/scenarios/linux/requirements.yml

@@ -1,3 +1,3 @@
 collections:
  - name: google.cloud
-    source: https://galaxy.ansible.com
+    source: https://github.com/ansible-collections/google.cloud


this should have been mentioned at least in the commit message, with explanations. It's easy to miss
such important change in the other changes...

apatard · 2023-03-15T13:55:58Z

tox.ini

@@ -30,6 +30,7 @@ extras =
 deps =
    py-{devel}: git+https://github.com/ansible-community/molecule.git@main#egg=molecule[test]
 commands =
+    ansible-galaxy install -r requirements.yml


My knowledge is quite limited in this part of the repository. Why it is needed now and it was not before ?
(side node: if someone doesn't understand a change in a commit, it's usually means that the commit message has to be improved/fixed)

@apatard

Thanks for pointing it out.

The collection is required in

molecule-plugins/test/podman/test_func.py

Line 81 in 8867bd3

["ansible-playbook", "-i", "localhost,", "playbooks/validate-dockerfile.yml"],

The difference

This issue is introduced in this commit, ansible/molecule@4dd1294

Before the commit

The ansible is installed by package management tool(apt-get on ubuntu), and some collections will be installed automatically in /usr/lib/python3/dist-packages.
When pytest runs, it will use /usr/bin/ansible installed by apt-get, so the collection can be found. That's why it worked before.

After the commit

The ansible-core is added in molecule's requirements, and molecule-plugin requires molecule, so tox will install Ansible in tox virtual enviroment, e.g. /home/runner/work/molecule-plugins/molecule-plugins/.tox/py310/bin/ansible.

Why

Collections are installed in /usr/lib/python3/dist-packages before and after.

Besides the default collection path like ~/.ansible/collections and /usr/share/ansible/collections, Ansible will try to search collection from sys.path, but in non virtual environment, '/usr/lib/python3/dist-packages' is in sys.path while in virtual environment, it's not. The test is as below. The source code can be find in Reference.

In non virtual environment

vagrant@vagrant:~$ /usr/bin/python3 -c "import sys; print (sys.path)" ['', '/usr/lib/python310.zip', '/usr/lib/python3.10', '/usr/lib/python3.10/lib-dynload', '/home/vagrant/.local/lib/python3.10/site-packages', '/usr/local/lib/python3.10/dist-packages', '/usr/lib/python3/dist-packages']

In virtual environment

vagrant@vagrant:~$ python3 -m venv venv vagrant@vagrant:~$ ./venv/bin/python3 -c "import sys; print (sys.path)" ['', '/usr/lib/python310.zip', '/usr/lib/python3.10', '/usr/lib/python3.10/lib-dynload', '/home/vagrant/venv/lib/python3.10/site-packages']

Solution

We can install collections explicitly in default collection path(~/.ansible/collections) where is respected in both non virtual environment and virtual environment.

Reference:

https://github.com/ansible/ansible/blob/0937cc486219663b6b6e6a178ef40798217864fa/lib/ansible/utils/collection_loader/_collection_finder.py#L243-L257

/cc @ssbarnea

apatard · 2023-03-15T13:57:24Z

test/vagrant/scenarios/molecule/default-compat/destroy.yml

@@ -36,5 +36,5 @@
          # Molecule managed
          {{ instance_conf | to_json | from_json | to_yaml }}
        dest: "{{ molecule_instance_config }}"
-        mode: 0600
+        mode: ":0600"


typo ? should have been "0600" ? Has this pull request been really tested ?

You are right. Thanks for your careful review.

It's a typo. I'll fix that in a new PR.

* Fix ci issue * fix ci

* 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>

zhan9san added the bug Something isn't working label Mar 14, 2023

zhan9san marked this pull request as draft March 14, 2023 03:36

Fix ci issue

fe5cc12

zhan9san force-pushed the feature/fix-ci branch from 84b798d to fe5cc12 Compare March 14, 2023 08:31

fix ci

a3f0594

zhan9san marked this pull request as ready for review March 14, 2023 09:49

zhan9san added the skip-changelog label Mar 14, 2023

zhan9san requested review from apatard and ssbarnea March 14, 2023 09:52

ssbarnea approved these changes Mar 15, 2023

View reviewed changes

ssbarnea merged commit 8867bd3 into ansible-community:main Mar 15, 2023

zhan9san deleted the feature/fix-ci branch March 15, 2023 13:18

apatard reviewed Mar 15, 2023

View reviewed changes

zhan9san mentioned this pull request Mar 17, 2023

Optimize CI #112

Merged

dankow pushed a commit to dankow/molecule-plugins that referenced this pull request Mar 31, 2023

Fix ci issue (ansible-community#111)

c92caf0

* Fix ci issue * fix ci

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ci issue #111

Fix ci issue #111

zhan9san commented Mar 14, 2023

zhan9san commented Mar 14, 2023

apatard Mar 15, 2023

zhan9san Mar 15, 2023

apatard Mar 15, 2023

zhan9san Mar 15, 2023

apatard Mar 15, 2023

apatard Mar 15, 2023

zhan9san Mar 17, 2023

apatard Mar 15, 2023

zhan9san Mar 15, 2023

Fix ci issue #111

Fix ci issue #111

Conversation

zhan9san commented Mar 14, 2023

zhan9san commented Mar 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

The difference

Before the commit

After the commit

Why

In non virtual environment

In virtual environment

Solution

Choose a reason for hiding this comment

Choose a reason for hiding this comment