Skip to content

Conversation

@Iroy30
Copy link
Member

@Iroy30 Iroy30 commented Dec 22, 2025

Description

Issue

Checklist

  • I am familiar with the Contributing Guidelines.
  • Testing
    • New or existing tests cover these changes
    • Added tests
    • Created an issue to follow-up
    • NA
  • Documentation
    • The documentation is up to date with these changes
    • Added new documentation
    • NA

Summary by CodeRabbit

  • Chores
    • Updated the build and deployment pipeline to capture and include additional Git metadata in artifacts, improving version tracking and traceability across releases.

✏️ Tip: You can customize this high-level summary in your review settings.

@Iroy30 Iroy30 requested a review from a team as a code owner December 22, 2025 18:44
@Iroy30 Iroy30 requested a review from AyodeAwe December 22, 2025 18:44
@copy-pr-bot
Copy link

copy-pr-bot bot commented Dec 22, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025

📝 Walkthrough

Walkthrough

Extends the CI/CD build process to capture additional Git metadata (commit timestamp and branch name) alongside the existing commit SHA, and integrates these new metadata fields into Docker image construction through corresponding Dockerfile modifications.

Changes

Cohort / File(s) Summary
Git Metadata Capture and Docker Integration
\.github/workflows/build_images.yaml, ci/docker/Dockerfile
Workflow step renamed and expanded to capture COMMIT_TIME (via git log -n1 --pretty='%ct') and BRANCH (via git rev-parse --abbrev-ref HEAD), writing both to dedicated files in ci/docker/context/. Dockerfile COPY instruction updated to transfer both new metadata files into /opt/cuopt alongside existing COMMIT_SHA.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Attention areas:
    • Verify file path consistency between workflow output locations (./ci/docker/context/COMMIT_TIME, ./ci/docker/context/BRANCH) and Dockerfile COPY source paths
    • Confirm git command syntax correctness, particularly the commit timestamp format (%ct) and branch reference retrieval
    • Ensure the renamed step name accurately reflects the expanded functionality

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'add branch and commit time info to image' accurately summarizes the main change: adding branch and commit time metadata to the Docker image.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
ci/docker/Dockerfile (1)

98-98: LGTM! Metadata files expanded appropriately.

The COPY command correctly includes the new COMMIT_TIME and BRANCH files alongside existing metadata. This maintains the single-layer approach for static files.

Note: Manual builds of this Dockerfile will fail unless COMMIT_TIME and BRANCH files exist in the build context. Consider documenting this requirement or providing a script to generate these files for local development.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3be4325 and 26f2315.

📒 Files selected for processing (2)
  • .github/workflows/build_images.yaml
  • ci/docker/Dockerfile
🔇 Additional comments (2)
.github/workflows/build_images.yaml (2)

58-58: LGTM! Step name accurately reflects expanded functionality.

The updated step name clearly indicates that commit SHA, commit time, and branch are all being captured.


61-61: LGTM! Commit time captured correctly.

Using %ct format captures the commit timestamp as a Unix epoch time, which is suitable for machine processing and avoids timezone ambiguities.

run: |
git rev-parse HEAD > ./ci/docker/context/COMMIT_SHA
git log -n1 --pretty='%ct' > ./ci/docker/context/COMMIT_TIME
git rev-parse --abbrev-ref HEAD > ./ci/docker/context/BRANCH
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and read the build_images.yaml file
fd "build_images.yaml" --type f

Repository: NVIDIA/cuopt

Length of output: 38


🏁 Script executed:

#!/bin/bash
# List all files in .github/workflows directory
find .github/workflows -type f -name "*.yaml" -o -name "*.yml" 2>/dev/null | head -20

Repository: NVIDIA/cuopt

Length of output: 505


🏁 Script executed:

#!/bin/bash
# Read the build_images.yaml file with line numbers, focusing on the relevant sections
cat -n .github/workflows/build_images.yaml | head -80

Repository: NVIDIA/cuopt

Length of output: 3036


🏁 Script executed:

#!/bin/bash
# Check if there are any other branch-related operations or context that affects BRANCH usage
grep -n "BRANCH" .github/workflows/build_images.yaml

Repository: NVIDIA/cuopt

Length of output: 129


🏁 Script executed:

#!/bin/bash
# Check if there are other workflows that call this workflow and potentially pass branch info
grep -r "build_images.yaml" .github/workflows/ --include="*.yaml" --include="*.yml"

Repository: NVIDIA/cuopt

Length of output: 152


🏁 Script executed:

#!/bin/bash
# Check how build_images.yaml is called from the parent workflow
grep -A 10 "build_images.yaml" .github/workflows/build_test_publish_images.yaml

Repository: NVIDIA/cuopt

Length of output: 473


Critical: git rev-parse --abbrev-ref HEAD will always capture "HEAD" instead of the actual branch name

Line 62 executes this command after checking out a specific commit SHA (line 47), which puts the repository in a detached HEAD state. In this state, the command always returns HEAD when you're in detached HEAD state, making the BRANCH file incorrect.

Recommended fix: Pass the branch name as an additional workflow input parameter and use it directly:

-          git rev-parse --abbrev-ref HEAD > ./ci/docker/context/BRANCH
+          echo "${{ inputs.branch_name }}" > ./ci/docker/context/BRANCH

Add branch_name to the workflow inputs (around line 8-30) and have the caller pass it.

Alternative: Use git branch -r --contains HEAD | grep -oP 'origin/\K.*' | head -n1 if you need to derive the branch from the commit, though this may return multiple branches if the commit exists on several.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
.github/workflows/build_images.yaml around line 62: the current command writes
the branch as `git rev-parse --abbrev-ref HEAD` but because the checkout uses a
specific commit SHA (detached HEAD) this will always write "HEAD"; change the
workflow to accept a branch_name input and use that input when creating
./ci/docker/context/BRANCH (add branch_name to the workflow inputs and ensure
callers pass it), or if you must derive the branch from the commit, replace the
current command with a remote-branch lookup such as using git branch -r
--contains HEAD piped to extract the origin/<branch> name and take the first
result (handle possible multiple matches).

@anandhkb anandhkb added this to the 26.02 milestone Dec 22, 2025
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.

3 participants