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.

Sign up for GitHub

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

Fix ci issue #111

Merged
merged 2 commits into from
Mar 15, 2023
Merged

Fix ci issue #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe5cc12
Fix ci issue
zhan9san Mar 14, 2023
a3f0594
fix ci
zhan9san Mar 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .ansible-lint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
exclude_paths:
- .cache/ # implicit unless exclude_paths is defined in config
- .github/
- requirements.yml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why ? what's the issue ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

- src/**/cookiecutter/{{*
# - src/molecule_gce/cookiecutter/{{cookiecutter.molecule_directory}}

skip_list:
# Temporary skips made during migration
- args[module]
- fqcn-builtins
- yaml[line-length]
- var-spacing
Expand Down
6 changes: 3 additions & 3 deletions molecule/test-podman/verify.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
- name: Verify
hosts: all
tasks:
- name: Example assertion
ansible.builtin.assert:
that: true
- name: Example assertion
ansible.builtin.assert:
that: true
4 changes: 2 additions & 2 deletions src/molecule_plugins/docker/playbooks/validate-dockerfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
ansible.builtin.template:
src: Dockerfile.j2
dest: "{{ temp_dockerfiles.results[index].path }}"
mode: 0600
mode: "0600"
register: result
with_items: "{{ platforms }}"
loop_control:
Expand Down Expand Up @@ -59,7 +59,7 @@
ansible.builtin.file:
path: "{{ item }}"
state: absent
mode: 0600
mode: "0600"
loop: "{{ temp_dockerfiles.results | map(attribute='path') | list }}"

- name: Display results
Expand Down
4 changes: 2 additions & 2 deletions src/molecule_plugins/gce/playbooks/tasks/create_linux_instance.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
- name: create ssh keypair
- name: Create ssh keypair
community.crypto.openssh_keypair:
comment: "{{ lookup('env','USER') }} user for Molecule"
path: "{{ ssh_identity_file }}"
register: keypair

- name: create molecule Linux instance(s)
- name: Create molecule Linux instance(s)
google.cloud.gcp_compute_instance:
state: present
name: "{{ item.name }}"
Expand Down
2 changes: 1 addition & 1 deletion src/molecule_plugins/gce/playbooks/tasks/create_windows_instance.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
- name: create molecule Windows instance(s)
- name: Create molecule Windows instance(s)
google.cloud.gcp_compute_instance:
state: present
name: "{{ item.name }}"
Expand Down
1 change: 0 additions & 1 deletion src/molecule_plugins/podman/driver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ def sanity_checks(self):
# fully adopts ansible-compat
runtime = Runtime()
if runtime.version < Version("2.10.0"):

if runtime.config.ansible_pipelining:
sysexit_with_message(
"Podman connections do not work with Ansible "
Expand Down
2 changes: 2 additions & 0 deletions src/molecule_plugins/podman/playbooks/create.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
- item.registry is defined
- item.registry.credentials is defined
- item.registry.credentials.username is defined
changed_when: false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


- name: Check presence of custom Dockerfiles
ansible.builtin.stat:
Expand Down Expand Up @@ -97,6 +98,7 @@
retries: 3
delay: 30
no_log: false
changed_when: false

- name: Determine the CMD directives
ansible.builtin.set_fact:
Expand Down
4 changes: 1 addition & 3 deletions src/molecule_plugins/podman/playbooks/validate-dockerfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
hosts: localhost
connection: local
gather_facts: false
collections:
- containers.podman
vars:
platforms:
# platforms supported as being managed by molecule/ansible, this does
Expand All @@ -30,7 +28,7 @@
ansible.builtin.template:
src: Dockerfile.j2
dest: "{{ temp_image_dirs.results[index].path }}/Dockerfile"
mode: 0600
mode: "0600"
register: result
with_items: "{{ platforms }}"
loop_control:
Expand Down
1 change: 0 additions & 1 deletion src/molecule_plugins/vagrant/modules/vagrant.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,6 @@ def _get_vagrant(self):
return v

def _get_instance_vagrant_config_dict(self, instance):

checksum = instance.get("box_download_checksum")
checksum_type = instance.get("box_download_checksum_type")
if bool(checksum) ^ bool(checksum_type):
Expand Down
2 changes: 1 addition & 1 deletion src/molecule_plugins/vagrant/playbooks/create.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,4 @@
ansible.builtin.copy:
content: "{{ instance_conf | to_json | from_json | to_yaml }}"
dest: "{{ molecule_instance_config }}"
mode: 0600
mode: "0600"
2 changes: 1 addition & 1 deletion src/molecule_plugins/vagrant/playbooks/destroy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,5 @@
# Molecule managed
{{ instance_conf | to_json | from_json | to_yaml }}
dest: "{{ molecule_instance_config }}"
mode: 0600
mode: "0600"
when: server.changed | bool
1 change: 0 additions & 1 deletion test/docker/test_func.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ def test_command_init_and_test_scenario(tmp_path: pathlib.Path, DRIVER: str) ->
scenario_name = "default"

with change_dir_to(tmp_path):

scenario_directory = tmp_path / "molecule" / scenario_name
cmd = [
"molecule",
Expand Down
6 changes: 3 additions & 3 deletions test/gce/scenarios/linux/create.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
molecule_yml.driver.instance_os_type | lower == "windows"
fail_msg: "instance_os_type is possible only to specify linux or windows either"

- name: get network info
- name: Get network info
google.cloud.gcp_compute_network_info:
filters:
- name = "{{ molecule_yml.driver.network_name | default('default') }}"
Expand All @@ -26,7 +26,7 @@
auth_kind: "{{ molecule_yml.driver.auth_kind | default(omit, true) }}"
register: my_network

- name: get subnetwork info
- name: Get subnetwork info
google.cloud.gcp_compute_subnetwork_info:
filters:
- name = "{{ molecule_yml.driver.subnetwork_name | default('default') }}"
Expand All @@ -37,7 +37,7 @@
auth_kind: "{{ molecule_yml.driver.auth_kind | default(omit, true) }}"
register: my_subnetwork

- name: set external access config
- name: Set external access config
set_fact:
external_access_config:
- access_configs:
Expand Down
2 changes: 1 addition & 1 deletion test/gce/scenarios/linux/requirements.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
collections:
- name: google.cloud
source: https://galaxy.ansible.com
source: https://github.com/ansible-collections/google.cloud
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

6 changes: 3 additions & 3 deletions test/gce/scenarios/linux/tasks/create_linux_instance.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
- name: create ssh keypair
openssh_keypair:
- name: Create ssh keypair
community.crypto.openssh_keypair:
comment: "{{ lookup('env','USER') }} user for Molecule"
path: "{{ ssh_identity_file }}"
register: keypair

- name: create molecule Linux instance(s)
- name: Create molecule Linux instance(s)
google.cloud.gcp_compute_instance:
state: present
name: "{{ item.name }}"
Expand Down
2 changes: 1 addition & 1 deletion test/gce/scenarios/linux/tasks/create_windows_instance.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
- name: create molecule Windows instance(s)
- name: Create molecule Windows instance(s)
google.cloud.gcp_compute_instance:
state: present
name: "{{ item.name }}"
Expand Down
6 changes: 3 additions & 3 deletions test/gce/scenarios/linux/verify.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
hosts: all
gather_facts: false
tasks:
- name: Example assertion
assert:
that: true
- name: Example assertion
assert:
that: true
6 changes: 3 additions & 3 deletions test/gce/scenarios/windows/create.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
molecule_yml.driver.instance_os_type | lower == "windows"
fail_msg: "instance_os_type is possible only to specify linux or windows either"

- name: get network info
- name: Get network info
google.cloud.gcp_compute_network_info:
filters:
- name = "{{ molecule_yml.driver.network_name | default('default') }}"
Expand All @@ -26,7 +26,7 @@
auth_kind: "{{ molecule_yml.driver.auth_kind | default(omit, true) }}"
register: my_network

- name: get subnetwork info
- name: Get subnetwork info
google.cloud.gcp_compute_subnetwork_info:
filters:
- name = "{{ molecule_yml.driver.subnetwork_name | default('default') }}"
Expand All @@ -37,7 +37,7 @@
auth_kind: "{{ molecule_yml.driver.auth_kind | default(omit, true) }}"
register: my_subnetwork

- name: set external access config
- name: Set external access config
set_fact:
external_access_config:
- access_configs:
Expand Down
6 changes: 3 additions & 3 deletions test/gce/scenarios/windows/tasks/create_linux_instance.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
- name: create ssh keypair
openssh_keypair:
- name: Create ssh keypair
community.crypto.openssh_keypair:
comment: "{{ lookup('env','USER') }} user for Molecule"
path: "{{ ssh_identity_file }}"
register: keypair

- name: create molecule Linux instance(s)
- name: Create molecule Linux instance(s)
google.cloud.gcp_compute_instance:
state: present
name: "{{ item.name }}"
Expand Down
2 changes: 1 addition & 1 deletion test/gce/scenarios/windows/tasks/create_windows_instance.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
- name: create molecule Windows instance(s)
- name: Create molecule Windows instance(s)
google.cloud.gcp_compute_instance:
state: present
name: "{{ item.name }}"
Expand Down
6 changes: 3 additions & 3 deletions test/gce/scenarios/windows/verify.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
hosts: all
gather_facts: false
tasks:
- name: Example assertion
assert:
that: true
- name: Example assertion
assert:
that: true
3 changes: 0 additions & 3 deletions test/vagrant/functional/test_func.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ def test_command_init_scenario(temp_dir):
not is_vagrant_supported(), reason="vagrant not supported on this machine"
)
def test_invalid_settings(temp_dir):

scenario_directory = os.path.join(
os.path.dirname(util.abs_path(__file__)), os.path.pardir, "scenarios"
)
Expand Down Expand Up @@ -111,7 +110,6 @@ def test_invalid_settings(temp_dir):
],
)
def test_vagrant_root(temp_dir, scenario):

scenario_directory = os.path.join(
os.path.dirname(util.abs_path(__file__)), os.path.pardir, "scenarios"
)
Expand All @@ -126,7 +124,6 @@ def test_vagrant_root(temp_dir, scenario):
not is_vagrant_supported(), reason="vagrant not supported on this machine"
)
def test_multi_node(temp_dir):

scenario_directory = os.path.join(
os.path.dirname(util.abs_path(__file__)), os.path.pardir, "scenarios"
)
Expand Down
2 changes: 1 addition & 1 deletion test/vagrant/scenarios/molecule/default-compat/create.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,4 @@
ansible.builtin.copy:
content: "{{ instance_conf | to_json | from_json | to_yaml }}"
dest: "{{ molecule_instance_config }}"
mode: 0600
mode: "0600"
2 changes: 1 addition & 1 deletion test/vagrant/scenarios/molecule/default-compat/destroy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@
# Molecule managed
{{ instance_conf | to_json | from_json | to_yaml }}
dest: "{{ molecule_instance_config }}"
mode: 0600
mode: ":0600"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. Thanks for your careful review.

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

when: server.changed | bool
1 change: 1 addition & 0 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

pytest --collect-only
pytest --color=yes {tty:-s}
setenv =
Expand Down