-
Notifications
You must be signed in to change notification settings - Fork 45
Added dependency checker and unit test case #2375
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
base: dev
Are you sure you want to change the base?
Conversation
| # Compatibility Matrix | ||
| # ------------------------------ | ||
| COMPATIBILITY_MATRIX = [ | ||
| {"zoau_version": "1.3.5", "python_version": "3.12", "galaxy_core_version": "1.12.0", "zos_range": "2.5-3.1"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will be easier to use min_zos_version and max_zos_version instead of zos_range and then having to do a string manipulation, same for things that allow ranges like python.
Instead of galaxy_core_version I would use collection_version since we know this is our collection.
| {"zoau_version": "1.3.5", "python_version": "3.12", "galaxy_core_version": "1.12.0", "zos_range": "2.5-3.1"}, | |
| {"zoau_version": "1.3.5", "python_version": "3.12", "collection_version": "1.12.0", "min_zos_version": "2.5", "max_zos_version": "3.1"}, |
| # Version Fetchers | ||
| # ------------------------------ | ||
| def get_zoau_version(module): | ||
| output = run_command(module, "zoaversion") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to run zoaversion command, as we spoke before better to use the zoau_dependency_checker functionality.
try:
from zoautil_py import ZOAU_API_VERSION
except Exception:
ZOAU_API_VERSION = "1.4.0"| return f"{int(match_ver.group(1))}.{int(match_rel.group(1))}" | ||
| return None | ||
|
|
||
| def get_galaxy_core_version(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are in the managed node we cannot access the collection version using ansible-galaxy command, this is what we were talking about the other day, is it possible to get through an import by fetching the valye from meta/ibm_zos_core?
| def run_command(module, cmd): | ||
| rc, out, err = module.run_command(cmd) | ||
| if rc != 0: | ||
| module.fail_json(msg=f"Command failed: {cmd}", stdout=out, stderr=err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be problematic, rather than catching the error here I think we should do a try in the main dependency validation, and throw an Exception from there saying something like "The dependencies could not be fetched correctly. " and maybe add some details about the dependencias that were fetched successfully
| return f"{sys.version_info.major}.{sys.version_info.minor}.0" | ||
|
|
||
| def get_zos_version(module): | ||
| output = run_command(module, "zinfo -t sys -j") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, better to use ZOAU's zsystem python API https://www.ibm.com/docs/en/zoau/1.3.x?topic=apis-zsystem we try to avoid using commands when an API is available unless there is a bug in the python API
| except (ValueError, TypeError): | ||
| module.fail_json(msg=f"Unable to parse z/OS version: {zos_version}") | ||
|
|
||
| for row in COMPATIBILITY_MATRIX: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of iterating over the compatibility matrix we can use the core collection version as the key to find the correct row and only check the compatibility for the current version
| @@ -0,0 +1,43 @@ | |||
| import pytest | |||
| from ansible_collections.ibm.ibm_zos_core.plugins.module_utils import dependency_checker as dc | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For readability, would recommend this to be kept as dependency_checker
|
|
||
| def test_validate_dependencies_success(monkeypatch): | ||
| # Monkeypatch fetchers to match the compatibility matrix exactly | ||
| monkeypatch.setattr(dc, "get_zoau_version", lambda mod: "1.3.5") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know about this fixture, please add a comment explaining what this does so that next person who sees this doesn't have to spend much time researching why it was implemented this way and not with simple dictionaries as inputs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
fernandofloresg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requested few changes
| COMPATIBILITY_MATRIX = { | ||
| "2.0.0": { | ||
| "zoau_version": "1.4.0", | ||
| "min_python_version": "3.12", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can pick the python versions from this page:
https://access.redhat.com/support/policy/updates/ansible-automation-platform#phases
Right now we are officially supporting 2.15 trhough 2.18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return ZOAU_API_VERSION | ||
| except ImportError: | ||
| if module: | ||
| module.fail_json(msg=" ZOAU Python API not found.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change this error message.
| module.fail_json(msg=" ZOAU Python API not found.") | |
| module.fail_json(msg="Unable to import ZOAU. Please check PYTHONPATH, LIBPATH, ZOAU_HOME and PATH environment variables.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
| COMPATIBILITY_MATRIX = { | ||
| "2.0.0": [ | ||
| {"zoau_version": "1.4.0", "min_python_version": "3.12", "max_python_version": "3.13", "min_zos_version": 2.5, "max_zos_version": 3.1}, | ||
| {"zoau_version": "1.4.1", "min_python_version": "3.12", "max_python_version": "3.13", "min_zos_version": 2.5, "max_zos_version": 3.1}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there need to add different zoau_version objects here, we can have only min_zoau_version and validate from there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
like min_zoau_version: 1.4.0, if current zoau version is 1.4.1 then is ok
| {"zoau_version": "1.4.2", "min_python_version": "3.12", "max_python_version": "3.13", "min_zos_version": 2.5, "max_zos_version": 3.1}, | ||
| ], | ||
| "2.1.0": [ | ||
| {"zoau_version": "1.4.1", "min_python_version": "3.12", "max_python_version": "3.13", "min_zos_version": 2.5, "max_zos_version": 3.1}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry supported Python version in the managed node is indeed 3.12
| # Ensure all versions are available | ||
| if not all([zoau_version, zos_version_str, collection_version]): | ||
| module.fail_json( | ||
| msg=" Missing one or more required versions (ZOAU, Python, z/OS, Collection Version)." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mentions python but is not checked in line 100. Also, lets change the message:
| msg=" Missing one or more required versions (ZOAU, Python, z/OS, Collection Version)." | |
| msg="Unable to fetch one or more required dependencies. Depedencies checked are ZOAU, Python, z/OS." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
| """Fetch z/OS version using ZOAU Python API and return a string like '2.5'.""" | ||
| if zsystem is None: | ||
| if module: | ||
| module.fail_json(msg="ZOAU Python API not found.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| module.fail_json(msg="ZOAU Python API not found.") | |
| module.fail_json(msg="Unable to import ZOAU. Please check PYTHONPATH, LIBPATH, ZOAU_HOME and PATH environment variables.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
| {"zoau_version": "1.4.2", "min_python_version": "3.12", "max_python_version": "3.13", "min_zos_version": 2.5, "max_zos_version": 3.1}, | ||
| ], | ||
| "2.1.0": [ | ||
| {"zoau_version": "1.4.1", "min_python_version": "3.12", "max_python_version": "3.13", "min_zos_version": 2.5, "max_zos_version": 3.1}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry supported Python version in the managed node is indeed 3.12
fernandofloresg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partial Review, need to go back an check the messages



SUMMARY
validate_dependenciesto cover both success and failure scenarios.version.pyto define the collection version (__version__) for use in dependency checks.FakeModuleand monkey-patched version fetchers for deterministic, environment-independent testing.get_python_version,get_zos_version,get_zoau_version) are monkey patched to simulate different Python, z/OS, and ZOAU versions.Rationale:
validate_dependenciesacross supported and unsupported configurations.#2133
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION