[CI][REFACTOR] Decouple data.py from Jenkins script and docker images#19445
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request centralizes Docker image configurations into a new ci/docker-images.ini file and updates the CI pipeline to use this as the single source of truth. It also introduces a bundle-based artifact resolution system for S3 uploads, replacing hardcoded file lists in Jenkinsfiles with logical bundle names defined in ci/jenkins/data.py. Feedback focuses on improving the robustness of the INI file parsing in both the Python data loader and the shell-based image lookup script to handle potential formatting variations or missing keys.
tqchen
force-pushed
the
jenkins-s3-bundle-refactor
branch
from
685ebef to
29ffe67
Compare
Member
Author
|
cc @yongwww |
…tags
`ci/jenkins/data.py` had grown into a grab-bag of two unrelated CI concerns
(s3 stash file lists + docker image tag registry) plus a custom CLI for
``docker/dev_common.sh``. This PR splits those apart and pushes each piece
to its rightful owner so the generated `.groovy` files stop carrying
build-path or tag literals that belong elsewhere.
## s3 stash → bundle resolution at runtime
* `ci/scripts/jenkins/s3.py` learns ``--bundle NAME`` (repeatable). It loads
``ci/jenkins/data.py::files_to_stash`` via ``importlib.util`` and resolves
the named bundles to a flat file list at Jenkins runtime. ``--items`` is
preserved for ad-hoc literal-path uploads.
* ``upload_artifacts`` (``ci/jenkins/templates/utils/macros.j2``) emits one
``--bundle NAME`` flag per entry instead of expanding the path list inline.
* ``data.py`` itself becomes the bundle-resolver CLI:
``python3 ci/jenkins/data.py tvm_lib cpptest`` → one path per line.
* ``tvm_allvisible`` is folded into ``tvm_lib`` — every consumer of the
standard libs needs the all-visible variant for cpptest, so a separate
bundle was just two-name ceremony.
After this change ``grep -rn 'build/lib\|build/cpptest' ci/jenkins/generated/``
returns nothing — paths live exclusively in ``data.py``.
## Docker image registry → live ini, no duplicate
The repo already has ``ci/jenkins/docker-images.ini`` (the file the nightly
docker-update workflow writes to; latest tag `20260301-134651-63f099ad`).
``data.py`` was carrying an inline ``docker_images`` dict that had drifted
out of sync (`20251130-`). Drop it:
* ``data.py`` no longer touches the ini at all. Replaced with a flat
``images = [{name, platform}, ...]`` list — what templates actually
iterate over (tags themselves are resolved at Jenkins runtime by
``determine_docker_images.py``).
* ``determine_docker_images.py`` gains ``--lookup-only NAME`` (pure ini
read, no Docker Hub query, no tlcpackstaging fallback).
* ``docker/dev_common.sh::lookup_image_spec`` shells out to
``determine_docker_images.py --lookup-only`` instead of duplicating ini
parsing in awk. Single Python module owns the ini schema (configparser
interpolation for ``%(ci_tag)s``); bash no longer needs to know about it.
## Verification
* ``python3 ci/jenkins/generate.py`` re-runs cleanly; the only diff in the
generated ``.groovy`` files is ``--items <paths>`` → ``--bundle NAME`` and
the merged ``tvm_allvisible`` bundle.
* ``python3 ci/jenkins/data.py tvm_lib cpptest`` resolves to the same file
list ``files_to_stash["tvm_lib"] + files_to_stash["cpptest"]`` produced.
* ``bash -c 'source docker/dev_common.sh && lookup_image_spec ci_cpu'``
returns the same live tag string as
``python3 ci/scripts/jenkins/determine_docker_images.py --lookup-only ci_cpu``.
* All 5 ``ci_*`` images resolve correctly through both paths.
tqchen
force-pushed
the
jenkins-s3-bundle-refactor
branch
from
29ffe67 to
eebcf23
Compare
tlopex
approved these changes
Apr 25, 2026
…es.ini
The squashed CI refactor moved docker image tags from
``ci/jenkins/data.py`` to ``ci/jenkins/docker-images.ini`` (which already
existed and is the live source the nightly bot updates). The bot script
``open_docker_update_pr.py`` was still pointing at ``data.py`` and
regex-matching the ``"tag": "..."`` JSON-style format that no longer
exists there, so it became a silent no-op (zero replacements, no
"Tag names were the same" log emitted) and broke
``test_open_docker_update_pr``.
Rewrite the script to:
- Read ``ci/jenkins/docker-images.ini`` via configparser. The ini has
one ``[jenkins]`` section with a shared ``ci_tag`` key plus per-image
``ci_<name>: tlcpack/ci-<name>:%(ci_tag)s`` entries.
- Iterate every key except ``ci_tag``, resolve each to its full spec
(with interpolation applied), and check Docker Hub for newer tags via
the existing ``latest_tlcpackstaging_image`` helper.
- When a newer tag is found, rewrite the corresponding ``ci_<name>:``
line with the resolved tag (breaking the ``%(ci_tag)s`` interpolation
for that single entry) so the update is unambiguous and doesn't
disturb other images that share the old shared tag.
Also fix a long-standing typo in the ini: ``ci_cpu: tlcpack/ci_cpu``
(underscore) → ``tlcpack/ci-cpu`` (dash) — every other image uses the
dash form, and the test mocks already assume the dash convention
(``image.replace('_', '-')`` for the tlcpack repo half of the URL).
Update ``test_open_docker_update_pr[tlcpack_update]``'s
``expected_images`` from the old ``"tag": "tlcpack/ci-arm:234-234-abc",``
JSON-style snippet to the new ini-style line
``ci_arm: tlcpack/ci-arm:234-234-abc``.
Verified locally: ``--dry-run`` against same-tag mock data emits
"Tag names were the same, no update needed", and against new-tag mock
data emits "Found newer image, using: tlcpack/ci-X:234-234-abc" plus
the rewritten line in the printed "Updated to:" output for all 5 images.
Previous fix updated only the [tlcpack_update] expected_images snippet
to the new ini-line format. The [staging_update] parametrization had
the same JSON-style snippet (legacy data.py format) and still failed.
'"tag": "tlcpack/ci-arm:456-456-abc"'
→
"ci_arm: tlcpack/ci-arm:456-456-abc"
Verified locally by simulating both [same_tags], [tlcpack_update], and
[staging_update] cases with the test's mock data: all three now pass
the expected-string assertions against the script's stdout.
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
2 participants
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
ci/jenkins/data.pywas carrying two unrelated concerns: artifact bundle definitions for s3 staging, and the docker image tag registry. This PR decouples both from the Jenkinsfile-generated code, making each concern standalone and machine-readable without Python.Changes
Part 1 — Bundle refactor (
10950ac192)ci/scripts/jenkins/s3.pygains--bundle <name>(repeatable) that resolves bundle names fromci/jenkins/data.py::files_to_stashat runtime;--itemspreserved for back-compatci/jenkins/templates/utils/macros.j2upload_artifactsmacro emits--bundle <name>flags instead of inlining the file list as--itemsbundles=[...]names.groovyfiles regenerated — nobuild/paths appear in artifact upload blocks; only bundle namesPart 2 — Docker image registry extraction (
6df22c09b9)ci/docker-images.ini(one[ci_*]section per image) is the single source of truth for image tagsdocker/dev_common.sh::lookup_image_specreads the ini directly withawk— no Python invocationci/jenkins/data.pydrops the inlinedocker_imagesdict; loads it viaconfigparserfrom the ini, preserving the same nested-dict shape so Jinja templates and the currents3.pymodule import keep working unchangeddata.py __main__is now the bundle-resolver CLI (python3 ci/jenkins/data.py <bundle> [...]→ file paths, one per line) — no more dual-purpose image-name lookup branchci/scripts/jenkins/open_docker_update_pr.pyupdated to read/writeci/docker-images.iniinstead ofdata.pyAcceptance