Skip to content

feat(ci): enhance triage script to assign on-call assignee dynamically - #6469

Merged
balasaajay merged 4 commits into
NVIDIA:mainfrom
balasaajay:use-sso-users-list
Aug 12, 2026
Merged

balasaajay merged 4 commits into
NVIDIA:mainfrom
balasaajay:use-sso-users-list

Conversation

@balasaajay

@balasaajay balasaajay commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor
  • I, the PR author, have personally reviewed every line of this PR.

What does this PR do?

  • Updated the triage:linear_write job in .gitlab/stages/06.triage.yml to resolve the on-call assignee using a Python script.
  • Modified the Dockerfile.linting to update the CERNO_COMMIT argument.
  • Introduced a new script, resolve_oncall_assignee.py, to handle the logic for resolving the on-call assignee from the schedule.

⚠️ For major changes (either in lines of code or in its impact), please make sure to first share a design doc with the team. If you're unsure what's the best way to do so, contact @NVIDIA/mcore-oncall.

Issue tracking

For PRs from open-source community contributors:

  • New features: a linked issue is required. Please open a feature request and reference it here before submitting the PR.
  • Small updates (bug fixes, minor improvements): a linked issue is recommended and will accelerate the PR review process.

Linked issue:

Contribution process

Pre-checks

  • I have added relevant unit tests
  • I have added relevant functional tests
  • I have added proper typing to my code Typing guidelines
  • I have added relevant documentation
  • I have run the autoformatter.sh on my PR

Code review

Feel free to message or comment @NVIDIA/mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!

All PRs start as draft. If you open a non-draft PR, it will be automatically converted to draft.

Step 1: Mark PR as "Ready for Review"

  1. When your PR is ready, click Ready for Review.
  2. An oncall reviewer is auto-assigned and expert reviewers are notified based on your changes.
    • Some PRs may jump straight to step 2. This is determined by .github/CODEOWNERS.

⚠️ Only mark as ready once merge-conflicts are resolved and the CI is passing.
Final Review might get declined if these requirements are not fulfilled.

Step 2: Final Review

For PRs that change megatron/core, once all expert reviewers have approved, the Final Review label is applied automatically and final reviewers are assigned.

For PRs outside megatron/core, this step is skipped.

Step 3: Approved

Once all required reviewers have approved, the Approved label is applied automatically.

Merge

Any member of mcore-engineers will be able to merge your PR.

- Updated the triage:linear_write job in .gitlab/stages/06.triage.yml to resolve the on-call assignee using a Python script.
- Modified the Dockerfile.linting to update the CERNO_COMMIT argument.
- Added tests for the new on-call assignee resolution functionality in test_oncall_assignee.py and test_ci_triage.py.
- Introduced a new script, resolve_oncall_assignee.py, to handle the logic for resolving the on-call assignee from the schedule.

Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
@balasaajay
balasaajay requested a review from a team as a code owner August 11, 2026 23:05
@svcnvidia-nemo-ci
svcnvidia-nemo-ci marked this pull request as draft August 11, 2026 23:06
@github-actions

Copy link
Copy Markdown
Contributor

This PR has been automatically converted to draft because all PRs must start as drafts.

When you are ready for review, click Ready for Review to begin the review process. This will:

  1. Add the oncall reviewer (optional reviewer)
  2. Add required review teams based on your changes

See the contribution guide for more details.

@copy-pr-bot

copy-pr-bot Bot commented Aug 11, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

- The test file for on-call assignee resolution has been deleted as part of a cleanup process. This file contained various test cases for the functionality related to resolving on-call assignees and handling user email resolutions.

Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
@balasaajay
balasaajay marked this pull request as ready for review August 11, 2026 23:12
@kajalj22

Copy link
Copy Markdown
Contributor

/claude review

@claude

claude Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Light review — three things worth addressing before merge.

1. test_cerno_hard_cutover_contract drops the CERNO_COMMIT pin assertion instead of updating it

tests/test_utils/test_ci_triage.py:132 — the PR deletes:

assert "ARG CERNO_COMMIT=5a5fb5360e67f8f09d189871bbc0d768c09c43fa" in dockerfile

That line was the only guard anywhere in the repo pinning the cerno revision (grep -rn CERNO_COMMIT now only hits docker/Dockerfile.linting). Since the new --assignee flag is a hard dependency on the bumped cerno, removing the assertion loses exactly the coverage that would catch a future unintended downgrade. Prefer updating the hash rather than deleting the check:

    assert "ARG CERNO_COMMIT=3219afcbfcdbddd1d55e013d94e54a15986e2b1f" in dockerfile

2. Assignee-resolution failure now takes down the whole Linear write

.gitlab/stages/06.triage.yml:54-63 puts the resolve script and cerno-linear write in one - | block. GitLab runs job scripts under set -eo pipefail, so any failure inside resolve_oncall_assignee.py aborts the block before cerno-linear write ever runs. Combined with allow_failure: true the effect is silent: Linear issues stop being created/updated at all, not merely left unassigned.

The script fails hard on several new external conditions — NVIDIA_MANAGEMENT_ORG_PAT missing from the GitLab CI variables (it currently exists as a GitHub Actions secret, per .github/workflows/cicd-main.yml:71), the private github-audits release being unreachable, or an on-call user with no nvidia_email SSO record.

Also worth confirming nvidia_email against the actual asset: it is a new field for this repo — every other consumer of users_sso.json reads org_roles or just the top-level keys (.github/actions/check-nvidia-sso-membership/action.yml, .github/workflows/cicd-approve-test-queue.yml:136).

Suggest degrading to an unassigned write instead:

  script:
    - |
      ASSIGNEE_ARGS=()
      if ONCALL_ASSIGNEE="$(
           python tests/test_utils/python_scripts/resolve_oncall_assignee.py \
             --schedule-file .github/oncall_schedule.json
         )" && [[ -n "${ONCALL_ASSIGNEE}" ]]; then
        ASSIGNEE_ARGS=(--assignee "${ONCALL_ASSIGNEE}")
      else
        echo "Warning: could not resolve on-call assignee; writing unassigned."
      fi
      cerno-linear write \
        --config "${CERNO_CONFIG}" \
        --plan linear_action_plan.json \
        --output linear_action_plan_post.json \
        "${ASSIGNEE_ARGS[@]}"

3. No unit tests for the 217 lines of new logic

tests/test_utils/python_scripts/resolve_oncall_assignee.py adds schedule parsing, bounded retries, release-asset selection/URL validation, and NVIDIA-email validation. The one test added (test_linear_write_assigns_new_issues_to_scheduled_oncall) only greps the YAML — none of the Python is exercised, so a regression in any of those branches surfaces only as a silently-failing scheduled pipeline.

The script is already written for testability (download_sso_users(token, http_get=...), _get_json(http_get, ...)), and there is close precedent in tests/test_utils/python_scripts/test_oncall_manager.py (FakeRequests) and tests/test_utils/test_community_request_assignee.py. Worth covering at least:

  • load_schedule / current_oncall: missing file, invalid JSON, empty list, non-dict entry, blank user, whitespace stripping.
  • download_sso_users: blank/whitespace token, zero and duplicate users_sso.json assets, a non-api.github.com asset URL (the SSRF guard), and a retry that succeeds on the second attempt after a 503.
  • resolve_nvidia_email: case-insensitive login match, ambiguous duplicate logins, missing nvidia_email, non-nvidia.com domain.

@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Approved All necessary approvals have been made label Aug 12, 2026
- Updated the triage:linear_write job in .gitlab/stages/06.triage.yml to clarify the use of the existing GH_TOKEN for email lookup.
- Refactored resolve_oncall_assignee.py to remove unnecessary dependencies and streamline the email resolution process.
- Adjusted test_ci_triage.py to reflect changes in the artifacts paths and ensure proper assertions.

Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
@balasaajay
balasaajay enabled auto-merge August 12, 2026 16:43
@balasaajay
balasaajay added this pull request to the merge queue Aug 12, 2026
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/31635806580

Merged via the queue into NVIDIA:main with commit b2f2888 Aug 12, 2026
91 checks passed
@balasaajay
balasaajay deleted the use-sso-users-list branch August 12, 2026 21:40
ksivaman pushed a commit to ksivaman/Megatron-LM that referenced this pull request Sep 1, 2026

This branch was previously deployed

2 inactive deployments
public — 5e2b86c1 Deployed Aug 12, 2026 by copy-pr-bot[bot] via finalize / publish-docs #5875
test — 5e2b86c1 Deployed Aug 12, 2026 by copy-pr-bot[bot] via cicd-wait-in-queue #18793
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved All necessary approvals have been made complexity: low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants