ci: Optimize docker layer for caching and add remote cache by thomasdhc · Pull Request #1437 · NVIDIA-NeMo/Curator

thomasdhc · 2026-01-28T18:39:48Z

Description

Improve docker layer caching and add container build stage that push and pulls to remote cache

Usage

# Add snippet demonstrating usage

Checklist

I am familiar with the Contributing Guide.
New or Existing tests cover these changes.
The documentation is up to date with these changes.

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps

_{3 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-28T18:50:25Z

.github/actions/build-container/action.yml

+        client-id: ${{ inputs.AZURE_CLIENT_ID }}
+        tenant-id: ${{ inputs.AZURE_TENANT_ID }}
+        subscription-id: ${{ inputs.AZURE_SUBSCRIPTION_ID }}


Input reference using uppercase instead of lowercase will fail

Suggested change

client-id: ${{ inputs.AZURE_CLIENT_ID }}

tenant-id: ${{ inputs.AZURE_TENANT_ID }}

subscription-id: ${{ inputs.AZURE_SUBSCRIPTION_ID }}

client-id: ${{ inputs.azure-client-id }}

tenant-id: ${{ inputs.azure-tenant-id }}

subscription-id: ${{ inputs.azure-subscription-id }}

.github/actions/build-container/action.yml

greptile-apps · 2026-01-28T18:50:33Z

.github/actions/build-container/action.yml

+          ${{ env.container-registry }}/${{ env.REPO_LOWER }}:${{ fromJSON(steps.get-pr-info.outputs.pr-info || '{}').number || 0 }}
+          ${{ env.container-registry }}/${{ env.REPO_LOWER }}:${{ github.sha }}
+        secrets: |
+          GH_TOKEN=${{ secrets.PAT }}


Should reference input parameter not secret in composite action - change secrets.PAT to inputs.PAT

greptile-apps · 2026-01-28T18:50:36Z

Additional Comments (2)

.github/actions/test-template/action.yml
Still building container locally instead of using inputs.container-image passed from workflow - defeats the purpose of the remote cache optimization

    - name: Pull container
      shell: bash
      run: |
        docker pull ${{ inputs.container-image }}
        docker tag ${{ inputs.container-image }} ray-curator

.github/actions/test-template/action.yml
Checkout path NeMo-Curator doesn't match repo name Curator

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps

_{2 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-28T19:02:30Z

.github/actions/build-container/action.yml

+    - name: Checkout
+      uses: actions/checkout@v4
+      with:


Empty with block will cause the checkout action to fail or behave unexpectedly

Suggested change

- name: Checkout

uses: actions/checkout@v4

with:

- name: Checkout

uses: actions/checkout@v4

greptile-apps · 2026-01-28T19:02:38Z

Additional Comments (2)

.github/actions/test-template/action.yml
The container-image input parameter is defined but not used - this step still builds the container locally instead of pulling the pre-built image from the registry, negating the caching benefits

    - name: Pull container
      shell: bash
      env:
        GH_TOKEN: ${{ inputs.PAT }}
      run: |
        docker pull ${{ inputs.container-image }}
        docker tag ${{ inputs.container-image }} ray-curator

.github/actions/test-template/action.yml
Missing sudo - apt-get commands will fail without elevated privileges in GitHub Actions runners

      run: |
        sudo apt-get update
        sudo apt-get install -y uuid-runtime

greptile-apps

_{3 files reviewed, 6 comments}

_{Edit Code Review Agent Settings | Greptile}

.github/actions/build-container/action.yml

.github/workflows/cicd-main.yml

.github/actions/build-container/action.yml

greptile-apps · 2026-01-28T19:06:31Z

Additional Comments (2)

.github/actions/test-template/action.yml
Container is being built locally instead of using the pre-built image from cicd-container-build job. The container-image input is defined but never used, wasting the remote cache benefits.

    - name: Pull container image
      shell: bash
      env:
        GH_TOKEN: ${{ inputs.PAT }}
      run: |
        docker pull ${{ inputs.container-image }}
        docker tag ${{ inputs.container-image }} ray-curator

.github/workflows/cicd-main.yml
Tags use PR number 0 when not in a PR context (e.g., main branch pushes), which will cause tag collisions and cache conflicts

          ${{ env.container-registry }}/${{ env.REPO_LOWER }}:${{ github.event.pull_request.number || github.run_id }}
          ${{ env.container-registry }}/${{ env.REPO_LOWER }}:${{ github.sha }}

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps

_{4 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-28T19:58:11Z

.github/actions/build-container/action.yml

+                }
+              }
+            }' | jq -r '.data.repository.pullRequests.nodes[].number' | while read -r number; do
+            echo "type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:$number-buildcache,mode=max"


${{ env.REPO_LOWER }} is empty here - it's set in step "Normalize repo name to lowercase" which runs after this step. Use ${{ inputs.repo-name }} with bash lowercasing in the same script instead.

Suggested change

echo "type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:$number-buildcache,mode=max"

echo "type=registry,ref=${{ env.container-registry }}/$(echo ${{ inputs.repo-name }} | tr '[:upper:]' '[:lower:]'):$number-buildcache,mode=max"

greptile-apps · 2026-01-28T19:58:12Z

.github/actions/build-container/action.yml

+        apt-get update
+        apt-get install -y gh


apt-get requires sudo in GitHub Actions runners

Suggested change

apt-get update

apt-get install -y gh

sudo apt-get update

sudo apt-get install -y gh

.github/actions/test-template/action.yml

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-28T20:59:34Z

.github/actions/build-container/action.yml

+        apt-get update
+        apt-get install -y gh


Missing sudo - inconsistent with Azure CLI installation at line 66 which uses sudo bash. Will fail if runner doesn't have root privileges.

Suggested change

apt-get update

apt-get install -y gh

sudo apt-get update

sudo apt-get install -y gh

greptile-apps · 2026-01-28T20:59:35Z

.github/actions/test-template/action.yml

        apt-get update
        apt-get install -y uuid-runtime


Missing sudo - inconsistent with Azure CLI installation at line 76 which uses sudo bash. Will fail if runner doesn't have root privileges.

Suggested change

apt-get update

apt-get install -y uuid-runtime

sudo apt-get update

sudo apt-get install -y uuid-runtime

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps

_{3 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps

_{2 files reviewed, 6 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-28T22:33:09Z

.github/actions/build-container/action.yml

+        apt-get update
+        apt-get install -y gh


apt-get requires sudo - inconsistent with Azure CLI installation which uses sudo bash

Suggested change

apt-get update

apt-get install -y gh

sudo apt-get update

sudo apt-get install -y gh

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

greptile-apps · 2026-01-28T22:33:10Z

.github/actions/test-template/action.yml

        apt-get update
        apt-get install -y uuid-runtime


apt-get requires sudo - inconsistent with Azure CLI installation which uses sudo bash

Suggested change

apt-get update

apt-get install -y uuid-runtime

sudo apt-get update

sudo apt-get install -y uuid-runtime

greptile-apps · 2026-01-28T22:33:12Z

.github/actions/build-container/action.yml

+                }
+              }
+            }' | jq -r '.data.repository.pullRequests.nodes[].number' | while read -r number; do
+            echo "type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:$number-buildcache,mode=max"


${{ env.REPO_LOWER }} empty here - the step "Normalize repo name to lowercase" runs after this step (line 87). The variable is set at line 92 but not available until subsequent steps. Will produce malformed cache references like nemoci.azurecr.io/:123-buildcache

greptile-apps · 2026-01-28T22:33:12Z

.github/actions/build-container/action.yml

+          type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:${{ fromJSON(steps.get-pr-info.outputs.pr-info || '{}').number || 0 }}-buildcache,mode=max
+          type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:main-buildcache,mode=max
+          ${{ steps.cache_from.outputs.LAST_PRS }}


${{ env.REPO_LOWER }} empty here - it's set in the step at line 87-92 which runs before this, but env variables aren't available in subsequent steps within the same job in composite actions. Will produce malformed image references

greptile-apps · 2026-01-28T22:33:13Z

.github/actions/test-template/action.yml

+    - name: Checkout repository
+      uses: actions/checkout@v2
+      with:
+        path: NeMo-Curator


Checkout path NeMo-Curator doesn't match repository name Curator

greptile-apps · 2026-01-28T22:33:14Z

docker/Dockerfile

+RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
+    --mount=type=cache,target=/var/lib/apt,sharing=locked \
+    --mount=type=cache,target=/root/.cache/uv \
+    uv sync --link-mode copy --locked --extra all --all-groups --no-cache && \


Using --no-cache flag negates the --mount=type=cache,target=/root/.cache/uv above it, preventing uv from utilizing its cache during sync

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps

_{3 files reviewed, 7 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-28T22:38:48Z

.github/actions/build-container/action.yml

+        apt-get update
+        apt-get install -y gh


apt-get requires sudo in GitHub Actions runners

Suggested change

apt-get update

apt-get install -y gh

sudo apt-get update

sudo apt-get install -y gh

greptile-apps · 2026-01-28T22:38:49Z

.github/actions/test-template/action.yml

        apt-get update
        apt-get install -y uuid-runtime


apt-get requires sudo in GitHub Actions runners

Suggested change

apt-get update

apt-get install -y uuid-runtime

sudo apt-get update

sudo apt-get install -y uuid-runtime

greptile-apps · 2026-01-28T22:38:50Z

.github/actions/build-container/action.yml

+            }' | jq -r '.data.repository.pullRequests.nodes[].number' | while read -r number; do
+            echo "type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:$number-buildcache,mode=max"


${{ env.REPO_LOWER }} is empty here - it's set in the next step at line 92, but environment variables aren't available until steps after they're set. This produces malformed cache references like nemoci.azurecr.io/:123-buildcache

Use inline bash lowercasing instead:

echo "type=registry,ref=${{ env.container-registry }}/${REPO,,}:$number-buildcache,mode=max"

greptile-apps · 2026-01-28T22:38:52Z

.github/actions/build-container/action.yml

+          type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:${{ fromJSON(steps.get-pr-info.outputs.pr-info || '{}').number || 0 }}-buildcache,mode=max
+          type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:main-buildcache,mode=max
+          ${{ steps.cache_from.outputs.LAST_PRS }}
+        cache-to: |
+          type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:${{ fromJSON(steps.get-pr-info.outputs.pr-info || '{}').number || 0 }}-buildcache,mode=max
+        no-cache: false
+        tags: |
+          ${{ env.container-registry }}/${{ env.REPO_LOWER }}:${{ fromJSON(steps.get-pr-info.outputs.pr-info || '{}').number || 0 }}
+          ${{ env.container-registry }}/${{ env.REPO_LOWER }}:${{ github.sha }}


${{ env.REPO_LOWER }} is empty at line 130-131 - it's set at line 92 but composite actions don't make env variables available to subsequent steps. Cache and tag references will be malformed like nemoci.azurecr.io/:abc123

Set REPO_LOWER as output from the normalize step and reference ${{ steps.normalize.outputs.repo_lower }} instead

greptile-apps · 2026-01-28T22:38:52Z

.github/actions/test-template/action.yml

+    - name: Checkout repository
+      uses: actions/checkout@v2
+      with:
+        path: NeMo-Curator


Checkout path NeMo-Curator doesn't match repository name curator

Suggested change

path: NeMo-Curator

path: curator

greptile-apps · 2026-01-28T22:38:53Z

.github/actions/test-template/action.yml

@@ -116,7 +119,7 @@ runs:
          --volume $(pwd)/NeMo-Curator:/workspace \


Volume mount path references NeMo-Curator but should be curator to match the repository name

Suggested change

--volume $(pwd)/NeMo-Curator:/workspace \

--volume $(pwd)/curator:/workspace \

greptile-apps · 2026-01-28T22:38:55Z

docker/Dockerfile

 # Install Curator
+RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
+    --mount=type=cache,target=/var/lib/apt,sharing=locked \
+    uv sync --link-mode copy --locked --extra all --all-groups --no-cache && \


--no-cache flag negates the --mount=type=cache,target=/var/cache/apt above it, preventing the cache mount from functioning

Suggested change

uv sync --link-mode copy --locked --extra all --all-groups --no-cache && \

uv sync --link-mode copy --locked --extra all --all-groups && \

thomasdhc added 2 commits January 28, 2026 10:33

Optimize docker for layer caching

e68ee67

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

Add docker build stage and utilize remote caching

7489afb

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

copy-pr-bot bot temporarily deployed to test January 28, 2026 18:40 Inactive

copy-pr-bot bot had a problem deploying to nemo-ci January 28, 2026 18:40 Error

copy-pr-bot bot temporarily deployed to nemo-ci January 28, 2026 18:40 Inactive

copy-pr-bot bot had a problem deploying to nemo-ci January 28, 2026 18:40 Error

copy-pr-bot bot temporarily deployed to nemo-ci January 28, 2026 18:40 Inactive

copy-pr-bot bot had a problem deploying to nemo-ci January 28, 2026 18:40 Error

copy-pr-bot bot temporarily deployed to nemo-ci January 28, 2026 18:40 Inactive

copy-pr-bot bot had a problem deploying to nemo-ci January 28, 2026 18:40 Error

copy-pr-bot bot temporarily deployed to nemo-ci January 28, 2026 18:40 Inactive

copy-pr-bot bot had a problem deploying to nemo-ci January 28, 2026 18:40 Error

copy-pr-bot bot had a problem deploying to nemo-ci January 28, 2026 18:48 Error

copy-pr-bot bot temporarily deployed to nemo-ci January 28, 2026 18:48 Inactive

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Further address feedback

717480c

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

copy-pr-bot bot temporarily deployed to test January 28, 2026 19:00 Inactive

copy-pr-bot bot had a problem deploying to nemo-ci January 28, 2026 19:00 Error

copy-pr-bot bot temporarily deployed to nemo-ci January 28, 2026 19:00 Inactive

copy-pr-bot bot had a problem deploying to nemo-ci January 28, 2026 19:00 Error

Fix syntax error

0ab9d30

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

.github/actions/build-container/action.yml Show resolved Hide resolved

.github/workflows/cicd-main.yml Show resolved Hide resolved

.github/actions/build-container/action.yml Show resolved Hide resolved

.github/actions/build-container/action.yml Show resolved Hide resolved

Remove building container in test template

271af09

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Merge branch 'main' into donghyukc/remote_cache

6d4c704

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

ko3n1g approved these changes Jan 28, 2026

View reviewed changes

Cache prune ci for a lighter image

f458aa5

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

No cache for uv sync

d6350cf

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Skip mounting cache

2de1463

Signed-off-by: Dong Hyuk Chang <donghyukc@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

	echo "type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:$number-buildcache,mode=max"
	echo "type=registry,ref=${{ env.container-registry }}/$(echo ${{ inputs.repo-name }} \| tr '[:upper:]' '[:lower:]'):$number-buildcache,mode=max"

		}' \| jq -r '.data.repository.pullRequests.nodes[].number' \| while read -r number; do
		echo "type=registry,ref=${{ env.container-registry }}/${{ env.REPO_LOWER }}:$number-buildcache,mode=max"

		@@ -116,7 +119,7 @@ runs:
		--volume $(pwd)/NeMo-Curator:/workspace \

	--volume $(pwd)/NeMo-Curator:/workspace \
	--volume $(pwd)/curator:/workspace \

	uv sync --link-mode copy --locked --extra all --all-groups --no-cache && \
	uv sync --link-mode copy --locked --extra all --all-groups && \

Conversation

thomasdhc commented Jan 28, 2026

Description

Usage

Checklist

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Jan 28, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Jan 28, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot commented Jan 28, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!