Skip to content

Conversation

@leo-amd
Copy link
Collaborator

@leo-amd leo-amd commented Nov 25, 2025

Description:
This PR addresses an issue where the TransformerEngine CI workflow fails when triggered from forked repositories. GitHub Actions security policies prevent forked PRs from accessing repository variables (vars), causing the Docker image name to resolve to an empty string during docker pull.

Changes:

  1. Introduced ci/ci_config.json:

    • Moves Docker image tags out of repository variables and into a version-controlled JSON file.
    • Could be used for other configurations as well.
  2. Updated Select Docker Image Tag step in CI workflow:

    • Removed dependency on ${{ vars }} context.
    • Now fetches ci/docker_config.json dynamically from the upstream dev branch via curl.
    • Uses jq to parse the JSON and populate the necessary environment variables.

Benefits:

  • Fixes CI for external contributors (forks).
  • Centralizes configuration: Updating the Docker image in the dev branch instantly propagates the new image to all release branches (since they fetch the config dynamically), eliminating the need to cherry-pick image tag updates across multiple branches.

@matthiasdiener
Copy link
Contributor

If you like, I can test this on #369 when ready.

CALC_LEVEL="3"
elif [[ "${{ github.event_name }}" == "pull_request" && "${{ github.base_ref }}" == "dev" ]]; then
echo "::notice::PR targeting dev detected. Forcing Test Level to 3."
CALC_LEVEL="3"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is 1 to match Jenkins logic.
Actually, desired logic is to distinct 3 situations:

  • level 3 for push to any dev
  • level 3 (may change independently from push to dev) for push to any monitored branch.
  • level 1 for PR to any monitored branch
    but it may be further development

@leo-amd leo-amd merged commit 610173a into dev Nov 28, 2025
4 of 5 checks passed
@matthiasdiener
Copy link
Contributor

As far as I can tell, the docker image is not set correctly yet for forked PRs, even with this PR merged. See e.g. https://github.com/ROCm/TransformerEngine/actions/runs/19716553519/job/56491482864?pr=369:

BRANCH_NAME="dev"
  echo "Determining image for branch: $BRANCH_NAME"
  DEV_DOCKER_IMAGE="$DEV_IMAGE"
  REL613_DOCKER_IMAGE="$REL_IMAGE"
  IMAGE_TO_USE="$DEV_DOCKER_IMAGE"
  if [[ $BRANCH_NAME =~ ^release_v([0-9]+)\.([0-9]+)_rocm$ ]]; then
    MAJOR_VERSION=${BASH_REMATCH[1]}
    MINOR_VERSION=${BASH_REMATCH[2]}
    if (( MAJOR_VERSION == 1 )); then
      if (( MINOR_VERSION == 13 || MINOR_VERSION == 14 )); then IMAGE_TO_USE="$REL613_DOCKER_IMAGE"; fi
    fi
  fi
  echo "Selected image: $IMAGE_TO_USE"
  echo "image-tag=$IMAGE_TO_USE" >> $GITHUB_OUTPUT
  shell: /usr/bin/bash -e {0}
  env:
    DEV_IMAGE: 
    REL_IMAGE: 
Determining image for branch: dev
Selected image: 
0s
Run docker pull 
  docker pull 
  shell: /usr/bin/bash -e {0}
docker: 'docker pull' requires 1 argument

Usage:  docker pull [OPTIONS] NAME[:TAG|@DIGEST]

Run 'docker pull --help' for more information
Error: Process completed with exit code 1.

@ipanfilo
Copy link
Collaborator

ipanfilo commented Dec 1, 2025

You have to merge dev branch yaml to your fork

@matthiasdiener
Copy link
Contributor

It's working now - I merged the dev branch locally, but then pushed to the wrong repo 🤦 . Sorry for the confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants