Revert "[ci][docker] Nightly Docker image update"#18723
Conversation
This reverts commit 176c773.
Summary of ChangesHello @tqchen, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request aims to restore stability to the CI system by reverting an earlier update to nightly Docker images. The change specifically rolls back the image tags in the configuration file to known stable versions, addressing reported issues with the recently updated images. This action also signals a potential shift towards a more controlled, manual process for managing Docker image updates to ensure reliability. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This PR correctly reverts the Docker image tags to a previous, stable version. My review includes a suggestion to refactor the image definitions to use a single source of truth, as you mentioned in the PR description. This will improve maintainability by eliminating the duplicate image definitions.
| docker_images = { | ||
| "ci_arm": { | ||
| "tag": "tlcpack/ci-arm:20251130-061900-c429a2b1", | ||
| "tag": "tlcpack/ci-arm:20221013-060115-61c9742ea", | ||
| "platform": "ARM", | ||
| }, | ||
| "ci_cpu": { | ||
| "tag": "tlcpack/ci-cpu:20251130-061900-c429a2b1", | ||
| "tag": "tlcpack/ci-cpu:20221013-060115-61c9742ea", | ||
| "platform": "CPU", | ||
| }, | ||
| "ci_gpu": { | ||
| "tag": "tlcpack/ci-gpu:20251130-061900-c429a2b1", | ||
| "tag": "tlcpack/ci-gpu:20221019-060125-0b4836739", | ||
| "platform": "GPU", | ||
| }, | ||
| "ci_hexagon": { | ||
| "tag": "tlcpack/ci-hexagon:20251130-061900-c429a2b1", | ||
| "tag": "tlcpack/ci-hexagon:20221013-060115-61c9742ea", | ||
| "platform": "CPU", | ||
| }, | ||
| "ci_i386": { | ||
| "tag": "tlcpack/ci-i386:20251130-061900-c429a2b1", | ||
| "tag": "tlcpack/ci-i386:20221013-060115-61c9742ea", | ||
| "platform": "CPU", | ||
| }, | ||
| "ci_lint": { | ||
| "tag": "tlcpack/ci-lint:20251130-061900-c429a2b1", | ||
| "tag": "tlcpack/ci-lint:20221013-060115-61c9742ea", | ||
| "platform": "CPU", | ||
| }, | ||
| "ci_wasm": { | ||
| "tag": "tlcpack/ci-wasm:20251130-061900-c429a2b1", | ||
| "tag": "tlcpack/ci-wasm:20221013-060115-61c9742ea", | ||
| "platform": "CPU", | ||
| }, | ||
| } |
There was a problem hiding this comment.
This revert is a good immediate fix. To address the underlying issue of duplicated image definitions that you mentioned in the PR description, you could refactor this to load the tags from ci/jenkins/docker-images.ini. This would make it the single source of truth and improve maintainability.
Here's a possible implementation for ci/jenkins/data.py:
import configparser
from pathlib import Path
# ... (other code)
# Docker Images
_DOCKER_IMAGE_PLATFORMS = {
"ci_arm": "ARM",
"ci_cpu": "CPU",
"ci_gpu": "GPU",
"ci_hexagon": "CPU",
"ci_i386": "CPU",
"ci_lint": "CPU",
"ci_wasm": "CPU",
}
def _get_docker_images():
# Assumes data.py and docker-images.ini are in the same directory
docker_images_ini = Path(__file__).parent / "docker-images.ini"
config = configparser.ConfigParser()
config.read(docker_images_ini)
docker_images = {}
for name, platform in _DOCKER_IMAGE_PLATFORMS.items():
tag = config.get("jenkins", name)
docker_images[name] = {
"tag": tag,
"platform": platform,
}
return docker_images
docker_images = _get_docker_images()This change would also require disabling or updating the open_docker_update_pr.py script to modify docker-images.ini instead of this file.
Note: There appears to be a typo in ci/jenkins/docker-images.ini where ci_cpu is listed as tlcpack/ci_cpu instead of tlcpack/ci-cpu.
|
cc @guan404ming @tlopex @yongwww . context, the data was used by docker/bash.sh command, but was not aligned with the ci behavior that uses https://github.com/apache/tvm/blob/main/ci/jenkins/docker-images.ini. We should temp disable it for now otherwise |
|
Thanks! |
|
perhaps we should followup by either only allow manual update, or make things consistent ideally we should make sure https://github.com/apache/tvm/blob/main/ci/jenkins/docker-images.ini is source of truth |
|
#18724 contains a PR to disable the PR. seems the current tag was not consistent but fine, |
Reverts #18710
Seems there is some issue in terms of the images, i think perhaps we should disable docker auto-updates, and instead relies on manual bump, we will need use https://github.com/apache/tvm/blob/main/ci/jenkins/docker-images.ini as ground truth