fix: modernize setup.sh and CI workflows for Python 3.12+#1335
Closed
dholt wants to merge 3 commits intoNVIDIA:masterfrom
Closed
fix: modernize setup.sh and CI workflows for Python 3.12+#1335dholt wants to merge 3 commits intoNVIDIA:masterfrom
dholt wants to merge 3 commits intoNVIDIA:masterfrom
Conversation
distutils.version.LooseVersion was removed from the Python stdlib in 3.12, breaking setup.sh on Ubuntu 24.04+ and any modern Python. Switch to packaging.version.Version (available via pip) and use the venv python3 instead of PYTHON_BIN so the import resolves correctly. Also bump jmespath 0.10.0 to 1.0.1 to match kubespray requirements, and add packaging to the explicit pip install list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Douglas Holt <dholt@nvidia.com>
- Update runners from ubuntu-20.04 (removed) to ubuntu-22.04 - Bump actions to current versions (checkout@v4, setup-python@v4, codeql-action@v3, stale@v9) - Update Python 3.9 to 3.12, Ansible 4.8.0 to 9.13.0 in CI - Add setup.yml workflow to test setup.sh on Ubuntu 22.04 and 24.04 - Use explicit venv python path in setup.sh version checks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Douglas Holt <dholt@nvidia.com>
- Keep ansible==4.8.0 for lint job (ansible-lint 5.4.0 is incompatible with ansible-core 2.16); use Python 3.10 for compatibility - Use molecule-plugins[docker] instead of molecule[docker] (driver moved to separate package in newer molecule versions) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Douglas Holt <dholt@nvidia.com>
There was a problem hiding this comment.
Pull request overview
This pull request modernizes the DeepOps setup infrastructure to support Python 3.12+ and current Ubuntu LTS releases. The changes address the removal of distutils from Python 3.12 stdlib and update CI infrastructure to use current GitHub Actions runner versions and action versions.
Changes:
- Replace deprecated
distutils.version.LooseVersionwithpackaging.version.Versionfor version comparisons in setup.sh - Update all GitHub Actions workflows to use ubuntu-22.04 runners and current action versions (v4/v3/v9)
- Add new setup.yml workflow to test setup.sh on Ubuntu 22.04 and 24.04
- Bump jmespath from 0.10.0 to 1.0.1 and add packaging as an explicit dependency
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/setup.sh | Migrates from distutils to packaging module for version comparisons; updates jmespath version; adds packaging dependency |
| .gitignore | Adds virtualenv and development tool artifacts to ignore list |
| .github/workflows/stale.yml | Updates actions/stale from v3 to v9 |
| .github/workflows/setup.yml | New workflow to test setup.sh on Ubuntu 22.04 and 24.04 |
| .github/workflows/molecule.yml | Updates runner to ubuntu-22.04, Python to 3.12, actions to v4, ansible to 9.13.0, and molecule plugin syntax |
| .github/workflows/codeql.yml | Updates actions to v4 and CodeQL actions to v3 |
| .github/workflows/ansible-lint-roles.yml | Updates runner to ubuntu-22.04, actions to v4, Python to 3.10 (intentional for ansible-lint compatibility) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+103
to
+108
| if "${VENV_DIR}/bin/python3" -c "from packaging.version import Version; print(Version('$current_version') >= Version('$ANSIBLE_TOO_NEW'))" | grep True 2>&1 >/dev/null; then | ||
| echo "Ansible version ${current_version} too new for DeepOps" | ||
| echo "Please uninstall any ansible, ansible-base, and ansible-core packages and re-run this script" | ||
| exit 1 | ||
| fi | ||
| if "${PYTHON_BIN}" -c "from distutils.version import LooseVersion; print(LooseVersion('$current_version') < LooseVersion('$ANSIBLE_VERSION'))" | grep True 2>&1 >/dev/null; then | ||
| if "${VENV_DIR}/bin/python3" -c "from packaging.version import Version; print(Version('$current_version') < Version('$ANSIBLE_VERSION'))" | grep True 2>&1 >/dev/null; then |
There was a problem hiding this comment.
The packaging module is imported on lines 103 and 108 to perform version comparisons, but it is not installed until line 118. This will cause the version checks to fail with an ImportError when ansible is already installed in the venv.
The packaging module should be installed before attempting to use it for version comparisons. Consider either:
- Installing packaging first (before the version checks), then installing the rest of the packages
- Installing packaging separately before line 100
- Using a different version comparison method that doesn't require the packaging module for the initial checks
Contributor
Author
|
Closing — will resubmit as a combined PR with Phase 1 (Ansible 9.x role compatibility fixes) so everything ships together. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
distutils.version.LooseVersionwithpackaging.version.Versioninsetup.sh—distutilswas removed from Python stdlib in 3.12, breaking setup on Ubuntu 24.04+${VENV_DIR}/bin/python3) for version checks instead of${PYTHON_BIN}(system Python doesn't havepackaging)jmespath0.10.0 → 1.0.1 to match kubespray requirementssetup.ymlworkflow that testssetup.shon Ubuntu 22.04 and 24.04Test plan
setup.shtested end-to-end on Ubuntu 24.04 VM (Python 3.12) via MAASsetup.ymlCI passes on both Ubuntu 22.04 and 24.04bash -n scripts/setup.shsyntax check passespackaging.version.Versioncomparisons verified correct on Python 3.12🤖 Generated with Claude Code