Fix unhandled exception when a running a command that does not exist (#83)

fgiorgetti · web-flow · commit b251368e03c4 · 2026-02-19T10:52:12.000-03:00
* Fix unhandled exception when a running a command that does not exist

* Fixed lint issues

* Python 3.13 now works with docker

* Fixed lint issues

* Fix to allow unit tests to run with docker

* Updated version of coverage based on latest ansible requirements

* Fixed unit test and lint issues

* Refactored unit tests to comply with ansible 2.19 changes
diff --git a/Makefile b/Makefile
@@ -1,7 +1,7 @@
 VERSION := 2.1.2-preview
 TARBALL := skupper-v2-$(VERSION).tar.gz
 
-IMAGES = default fedora40 ubuntu2404
+IMAGES = default ubuntu2404
 PYTHON ?= `python -c 'import platform; print(".".join(platform.python_version_tuple()[0:2]))'`
 PYTHON_DOCKER = 3.12
 
@@ -51,7 +51,7 @@ sanity:
 .PHONY: unit
 unit:
 	pip install -r ./tests/unit/requirements.txt -U
-	ansible-test units --color --coverage --python $(PYTHON) -v
+	ansible-test units --color --coverage --venv --python $(PYTHON) -v
 
 .PHONY: unit-docker
 unit-docker: $(foreach img,$(IMAGES),unit-docker-$(img))
diff --git a/plugins/module_utils/command.py b/plugins/module_utils/command.py
@@ -7,9 +7,13 @@
 
 
 def run_command(module: AnsibleModule, command: list) -> t.Tuple[int, str, str]:
-    module.debug("running command: %s" % (command))
-    code, out, err = module.run_command(command)
-    module.debug("code: %d" % (code))
-    module.debug("stdout: %s" % (out))
-    module.debug("stderr: %s" % (err))
-    return code, out, err
+    try:
+        module.debug("running command: %s" % (command))
+        code, out, err = module.run_command(command, handle_exceptions=False)
+        module.debug("code: %d" % (code))
+        module.debug("stdout: %s" % (out))
+        module.debug("stderr: %s" % (err))
+        return code, out, err
+    except Exception as ex:
+        module.debug("exception running command %s: %s" % (command, str(ex)))
+        return 1, "", str(ex)
diff --git a/tests/integration/targets/delete_nonkube_namespace/tasks/main.yml b/tests/integration/targets/delete_nonkube_namespace/tasks/main.yml
@@ -1,11 +1,11 @@
 ---
 - name: Setting namespace home directory (rootless)
   ansible.builtin.set_fact:
-    _namespace_home: "{{ lookup('env', 'HOME') + '/.local/share/skupper/namespaces/' + namespace + '/' }}"
+    _namespace_home: "{{ lookup('env', 'HOME') + '/.local/share/skupper/namespaces/' + namespace | string + '/' }}"
   when: ansible_real_user_id != 0
 - name: Setting namespace home directory (rootful)
   ansible.builtin.set_fact:
-    _namespace_home: "{{ '/var/lib/skupper/namespaces/' + namespace + '/' }}"
+    _namespace_home: "{{ '/var/lib/skupper/namespaces/' + namespace | string + '/' }}"
   when: ansible_real_user_id == 0
 - name: Removing namespace directory
   ansible.builtin.file:
diff --git a/tests/integration/targets/resource/tasks/kube/resource.yml b/tests/integration/targets/resource/tasks/kube/resource.yml
@@ -2,7 +2,7 @@
 - name: Create Site A resources based on definition
   skupper.v2.resource:
     def: "{{ lookup('ansible.builtin.file', 'site_a.yml') }}"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     kubeconfig: "{{ kubeconfig }}"
   register: _result
 - name: Assert that resources have been created
@@ -19,7 +19,7 @@
 - name: Create Site A resources based on definition (no changes expected)
   skupper.v2.resource:
     def: "{{ lookup('ansible.builtin.file', 'site_a.yml') }}"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     kubeconfig: "{{ kubeconfig }}"
   register: _result
 - name: Assert that resources have not been changed
@@ -30,7 +30,7 @@
 - name: Create Site A resources based on local path (no changes expected)
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     kubeconfig: "{{ kubeconfig }}"
   register: _result
 - name: Assert that resources have not been changed
@@ -41,7 +41,7 @@
 - name: Create Site A resources based on remote path (no changes expected)
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     remote: true
     kubeconfig: "{{ kubeconfig }}"
   register: _result
@@ -53,7 +53,7 @@
 - name: Override Site A resources based on local path
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     state: latest
     kubeconfig: "{{ kubeconfig }}"
   register: _result
@@ -65,7 +65,7 @@
 - name: Delete Site A resources based on remote path
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     state: absent
     kubeconfig: "{{ kubeconfig }}"
   register: _result
diff --git a/tests/integration/targets/resource/tasks/kube/resource_created.yml b/tests/integration/targets/resource/tasks/kube/resource_created.yml
@@ -4,7 +4,7 @@
     api_version: skupper.io/v2alpha1
     kind: "{{ item.kind }}"
     name: "{{ item.name }}"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     kubeconfig: "{{ kubeconfig }}"
   register: my_resource
 - name: Assert that the resource was successfully retrieved
diff --git a/tests/integration/targets/resource/tasks/kube/resource_deleted.yml b/tests/integration/targets/resource/tasks/kube/resource_deleted.yml
@@ -4,7 +4,7 @@
     api_version: skupper.io/v2alpha1
     kind: "{{ item.kind }}"
     name: "{{ item.name }}"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     kubeconfig: "{{ kubeconfig }}"
   register: _resource
   until: _resource.resources | length == 0
diff --git a/tests/integration/targets/resource/tasks/nonkube/invalid_resource.yml b/tests/integration/targets/resource/tasks/nonkube/invalid_resource.yml
@@ -2,7 +2,7 @@
 - name: Create Site A resources based on definition
   skupper.v2.resource:
     def: "{{ lookup('ansible.builtin.file', 'site_a.yml') }}"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     platform: podman
   register: _result
 - name: Assert that resources have been created
@@ -19,7 +19,7 @@
 - name: Delete Site A resources based on remote path with invalid resources
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_invalid.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     state: absent
     platform: podman
   register: _result
diff --git a/tests/integration/targets/resource/tasks/nonkube/resource.yml b/tests/integration/targets/resource/tasks/nonkube/resource.yml
@@ -2,7 +2,7 @@
 - name: Create Site A resources based on definition
   skupper.v2.resource:
     def: "{{ lookup('ansible.builtin.file', 'site_a.yml') }}"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     platform: podman
   register: _result
 - name: Assert that resources have been created
@@ -19,7 +19,7 @@
 - name: Create Site A resources based on definition (no changes expected)
   skupper.v2.resource:
     def: "{{ lookup('ansible.builtin.file', 'site_a.yml') }}"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     platform: podman
   register: _result
 - name: Assert that resources have not been changed
@@ -30,7 +30,7 @@
 - name: Create Site A resources based on local path (no changes expected)
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     platform: podman
   register: _result
 - name: Assert that resources have not been changed
@@ -41,7 +41,7 @@
 - name: Create Site A resources based on remote path (no changes expected)
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     remote: true
     platform: podman
   register: _result
@@ -53,7 +53,7 @@
 - name: Override Site A resources based on local path
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     state: latest
     platform: podman
   register: _result
@@ -65,7 +65,7 @@
 - name: Delete Site A resources based on remote path
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     state: absent
     platform: podman
   register: _result
diff --git a/tests/integration/targets/resource/tasks/nonkube/resource_deleted.yml b/tests/integration/targets/resource/tasks/nonkube/resource_deleted.yml
@@ -5,7 +5,7 @@
   when: ansible_real_user_id != 0
 - name: Set the filename to be verified (rootful)
   ansible.builtin.set_fact:
-    _filename: "{{ '/var/lib/skupper/namespaces/' + namespace + '/input/resources/' + item.file }}"
+    _filename: "{{ '/var/lib/skupper/namespaces/' + namespace | string + '/input/resources/' + item.file }}"
   when: ansible_real_user_id == 0
 - name: Retrieve expected resource from the filesystem
   ansible.builtin.stat:
diff --git a/tests/integration/targets/system/tasks/action-reload.yml b/tests/integration/targets/system/tasks/action-reload.yml
@@ -2,7 +2,7 @@
 - name: Create Site A resources
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     platform: podman
 - name: Get skupper-site-ca key file hash
   ansible.builtin.set_fact:
@@ -11,7 +11,7 @@
 - name: Run the reload action
   skupper.v2.system:
     action: reload
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     image: "{{ system_image }}"
   register: _system
   environment:
diff --git a/tests/integration/targets/system/tasks/action-shell-script.yml b/tests/integration/targets/system/tasks/action-shell-script.yml
@@ -2,12 +2,12 @@
 - name: Create Site A resources
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     platform: podman
 - name: Run the bundle action
   skupper.v2.system:
     action: shell-script
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     image: "{{ system_image }}"
   register: _system
   environment:
diff --git a/tests/integration/targets/system/tasks/action-start.yml b/tests/integration/targets/system/tasks/action-start.yml
@@ -2,12 +2,12 @@
 - name: Create Site A resources
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     platform: podman
 - name: Run the start action
   skupper.v2.system:
     action: start
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     image: "{{ system_image }}"
   register: _system
   environment:
@@ -37,7 +37,7 @@
 - name: Run the start action (no change)
   skupper.v2.system:
     action: start
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     image: "{{ system_image }}"
   register: _system
   environment:
diff --git a/tests/integration/targets/system/tasks/action-stop.yml b/tests/integration/targets/system/tasks/action-stop.yml
@@ -2,7 +2,7 @@
 - name: Run the stop action
   skupper.v2.system:
     action: stop
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     image: "{{ system_image }}"
   register: _system
 - name: Assert stop action performed successfully
diff --git a/tests/integration/targets/system/tasks/action-tarball.yml b/tests/integration/targets/system/tasks/action-tarball.yml
@@ -2,12 +2,12 @@
 - name: Create Site A resources
   skupper.v2.resource:
     path: "{{ role_path }}/files/site_a.yml"
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     platform: podman
 - name: Run the tarball action
   skupper.v2.system:
     action: tarball
-    namespace: "{{ namespace }}"
+    namespace: "{{ namespace | string }}"
     image: "{{ system_image }}"
   register: _system
   environment:
@@ -42,15 +42,15 @@
     remote_src: true
 - name: Installing the tarball bundle
   ansible.builtin.command:
-    cmd: "{{ './install.sh -n ' + namespace }}"
+    cmd: "{{ './install.sh -n ' + namespace | string }}"
     chdir: "{{ '/tmp/' + site_name + '-bundle' }}"
   register: _bundle
   changed_when: _bundle.rc == 0
 - name: Validate namespace is running
   ansible.builtin.include_tasks: validate-namespace-running.yml
 - name: Deleting namespace using the bundle script
   ansible.builtin.command:
-    cmd: "{{ './install.sh -x -n ' + namespace }}"
+    cmd: "{{ './install.sh -x -n ' + namespace | string }}"
     chdir: "{{ '/tmp/' + site_name + '-bundle' }}"
   register: _bundle
   changed_when: _bundle.rc == 0
diff --git a/tests/integration/targets/system/tasks/main.yml b/tests/integration/targets/system/tasks/main.yml
@@ -7,7 +7,7 @@
   when: ansible_real_user_id != 0
 - name: Set facts for system module tests (rootful)
   ansible.builtin.set_fact:
-    namespace_home: "{{ '/var/lib/skupper/namespaces/' + namespace }}"
+    namespace_home: "{{ '/var/lib/skupper/namespaces/' + namespace | string }}"
     bundle_path: "{{ '/var/lib/skupper/bundles/skupper-install-' + site_name + '.sh' }}"
     tarball_path: "{{ '/var/lib/skupper/bundles/skupper-install-' + site_name + '.tar.gz' }}"
   when: ansible_real_user_id == 0
@@ -30,7 +30,7 @@
     - name: Run the stop action
       skupper.v2.system:
         action: stop
-        namespace: "{{ namespace }}"
+        namespace: "{{ namespace | string }}"
       failed_when: false
     - name: Remove namespace files
       ansible.builtin.include_role:
diff --git a/tests/integration/targets/system/tasks/validate-namespace-deleted.yml b/tests/integration/targets/system/tasks/validate-namespace-deleted.yml
@@ -2,12 +2,12 @@
 - name: Set expected values for rootless execution
   ansible.builtin.set_fact:
     runtime_dir: "{{ lookup('env', 'HOME') + '/.local/share/skupper/namespaces/' + namespace + '/runtime' }}"
-    systemctl_cmd: "{{ 'systemctl --user is-active skupper-' + namespace }}"
+    systemctl_cmd: "{{ 'systemctl --user is-active skupper-' + namespace | string }}"
   when: ansible_real_user_id != 0
 - name: Set expected values for rootful execution
   ansible.builtin.set_fact:
-    runtime_dir: "{{ '/var/lib/skupper/namespaces/' + namespace + '/runtime' }}"
-    systemctl_cmd: "{{ 'systemctl is-active skupper-' + namespace }}"
+    runtime_dir: "{{ '/var/lib/skupper/namespaces/' + namespace | string + '/runtime' }}"
+    systemctl_cmd: "{{ 'systemctl is-active skupper-' + namespace | string }}"
   when: ansible_real_user_id == 0
 - name: Stats for the namespace runtime directory
   register: _dir
@@ -27,7 +27,7 @@
     - "{{ 'inactive' not in _systemctl.stdout }}"
 - name: Assert that the skupper-router container does not exist
   containers.podman.podman_container_info:
-    name: "{{ namespace + '-skupper-router' }}"
+    name: "{{ namespace | string + '-skupper-router' }}"
   register: _container
 - name: Assert that no containers returned
   ansible.builtin.assert:
diff --git a/tests/integration/targets/system/tasks/validate-namespace-running.yml b/tests/integration/targets/system/tasks/validate-namespace-running.yml
@@ -2,12 +2,12 @@
 - name: Set expected values for rootless execution
   ansible.builtin.set_fact:
     runtime_dir: "{{ lookup('env', 'HOME') + '/.local/share/skupper/namespaces/' + namespace + '/runtime' }}"
-    systemctl_cmd: "{{ 'systemctl --user is-active skupper-' + namespace }}"
+    systemctl_cmd: "{{ 'systemctl --user is-active skupper-' + namespace | string }}"
   when: ansible_real_user_id != 0
 - name: Set expected values for rootful execution
   ansible.builtin.set_fact:
-    runtime_dir: "{{ '/var/lib/skupper/namespaces/' + namespace + '/runtime' }}"
-    systemctl_cmd: "{{ 'systemctl is-active skupper-' + namespace }}"
+    runtime_dir: "{{ '/var/lib/skupper/namespaces/' + namespace | string + '/runtime' }}"
+    systemctl_cmd: "{{ 'systemctl is-active skupper-' + namespace | string }}"
   when: ansible_real_user_id == 0
 - name: Stats for the namespace runtime directory
   register: _dir
@@ -26,7 +26,7 @@
     - _systemctl.rc != 0
 - name: Assert that the skupper-router container exists
   containers.podman.podman_container_info:
-    name: "{{ namespace + '-skupper-router' }}"
+    name: "{{ namespace | string + '-skupper-router' }}"
   register: _container
 - name: Assert that one container has been found
   ansible.builtin.assert:
diff --git a/tests/integration/targets/system/tasks/validate-namespace-stopped.yml b/tests/integration/targets/system/tasks/validate-namespace-stopped.yml
@@ -1,11 +1,11 @@
 ---
 - name: Set expected values for rootless execution
   ansible.builtin.set_fact:
-    systemctl_cmd: "{{ 'systemctl --user is-active skupper-' + namespace }}"
+    systemctl_cmd: "{{ 'systemctl --user is-active skupper-' + namespace | string }}"
   when: ansible_real_user_id != 0
 - name: Set expected values for rootful execution
   ansible.builtin.set_fact:
-    systemctl_cmd: "{{ 'systemctl is-active skupper-' + namespace }}"
+    systemctl_cmd: "{{ 'systemctl is-active skupper-' + namespace | string }}"
   when: ansible_real_user_id == 0
 - name: Assert that the systemd service exists
   ansible.builtin.command: "{{ systemctl_cmd }}"
@@ -16,7 +16,7 @@
     - "{{ 'inactive' not in _systemctl.stdout }}"
 - name: Assert that the skupper-router container exists
   containers.podman.podman_container_info:
-    name: "{{ namespace + '-skupper-router' }}"
+    name: "{{ namespace | string + '-skupper-router' }}"
   register: _container
 - name: Assert that one container has been found and it is not running
   ansible.builtin.assert:
diff --git a/tests/unit/plugins/modules/test_controller.py b/tests/unit/plugins/modules/test_controller.py
diff --git a/tests/unit/plugins/modules/test_resource.py b/tests/unit/plugins/modules/test_resource.py
diff --git a/tests/unit/plugins/modules/test_system.py b/tests/unit/plugins/modules/test_system.py
diff --git a/tests/unit/plugins/modules/test_token.py b/tests/unit/plugins/modules/test_token.py
diff --git a/tests/unit/requirements.txt b/tests/unit/requirements.txt
diff --git a/tests/unit/utils/ansible_module_mock.py b/tests/unit/utils/ansible_module_mock.py
