Skip to content

fix: preserve Docker feature flags when enabling CDI - #2049

Merged
cdesiniotis merged 1 commit into
NVIDIA:mainfrom
git-jxj:fix/preserve-docker-features-when-enabling-cdi
Sep 14, 2026
Merged

cdesiniotis merged 1 commit into
NVIDIA:mainfrom
git-jxj:fix/preserve-docker-features-when-enabling-cdi

Conversation

@git-jxj

@git-jxj git-jxj commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

Description

Running nvidia-ctk runtime configure --runtime=docker --enable-cdi against a daemon.json with existing feature flags replaces them with only cdi: true. For example, containerd-snapshotter: true and buildkit: false disappear.

Docker configuration is decoded into map[string]any, but EnableCDI asserts the features entry as map[string]bool. Use the decoded map type so existing settings survive while CDI is enabled.

Add file-based tests for missing/empty features, existing true and false flags, CDI already enabled or disabled, and repeated load/update/save operations.

Checklist

  • No secrets, sensitive information, or unrelated changes
  • Unit tests passing (make test) — the full suite has the baseline failures described below.
  • Lint checks passing (make lint)
  • Test cases are added for new code paths
  • Commits are signed-off and cryptographically signed.

Testing

  • Three preservation cases fail before the fix and pass afterward.
  • go test -race ./pkg/config/... ./cmd/nvidia-ctk/runtime/... -count=1 passed.
  • make lint: 0 issues.
  • The real CLI preserves existing features and unrelated settings in a temporary daemon.json.

make test has the same four failing packages and nine failing subtests on unmodified upstream and this branch: installer, installer/toolkit, modifier, and nvcdi. These involve missing test driver libraries and CSV hook expectation differences; the full suite is not reported as passing.

@copy-pr-bot

copy-pr-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown

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.

@git-jxj
git-jxj force-pushed the fix/preserve-docker-features-when-enabling-cdi branch from 1f25bcd to e3184b6 Compare September 8, 2026 06:15
@henry118

henry118 commented Sep 9, 2026

Copy link
Copy Markdown
Member

/ok to test e3184b6

Comment thread pkg/config/engine/docker/docker_test.go Outdated
@git-jxj

git-jxj commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

The GitHub Actions runs for current head 545b7d3d5afa are waiting for maintainer approval (action_required), including CodeQL, Golang. Could a maintainer approve the pending workflow runs so CI can validate this head? The review explanation is in this head, and go test ./pkg/config/engine/docker -count=1 passes locally.

@henry118

Copy link
Copy Markdown
Member

@git-jxj Please sign commit 545b7d3 with a GPG/SSH key. or squash two commits.

@git-jxj
git-jxj force-pushed the fix/preserve-docker-features-when-enabling-cdi branch from 545b7d3 to b0ee6cd Compare September 12, 2026 00:32
@git-jxj

git-jxj commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Signed the previously unsigned commit with my SSH key and pushed it as b0ee6cde. GitHub now reports it as Verified. The code tree, parent commit, author and commit message are unchanged; go test ./pkg/config/engine/docker -count=1 passed.

The new head still needs CI validation through the repository's vetting/approval process. Could you authorize b0ee6cde58d87cdcfa347316e0c0bd15461da899 when convenient?

@cdesiniotis

Copy link
Copy Markdown
Contributor

@git-jxj could you squash your commits to make a more readable git history? Thanks.

@git-jxj
git-jxj force-pushed the fix/preserve-docker-features-when-enabling-cdi branch from b0ee6cd to 54f6ad1 Compare September 12, 2026 01:30
@git-jxj

git-jxj commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Squashed the two commits into 54f6ad1f. The PR now contains one SSH-signed commit, which GitHub reports as Verified, with both DCO sign-offs preserved. The code tree is unchanged, including the explanation of the repeated load/enable/save test cycle.

go test ./pkg/config/engine/docker -count=1 passes, and DCO passes. The new head’s CodeQL and Golang runs are awaiting maintainer approval (action_required); could you approve CI for 54f6ad1f4f962c1be54c11878fffe0f725bff098?

@cdesiniotis

Copy link
Copy Markdown
Contributor

/ok to test 54f6ad1

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 34665054753

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.3%) to 44.322%

Details

  • Coverage increased (+0.3%) from the base build.
  • Patch coverage: 2 of 2 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 13517
Covered Lines: 5991
Line Coverage: 44.32%
Coverage Strength: 0.44 hits per line

💛 - Coveralls

Use the decoded map type when enabling CDI so existing Docker feature
flags survive. Cover file-based configuration updates and repeated
load/enable/save cycles to verify idempotency.

Signed-off-by: xinjun.jiang <xinjun.jiang@daocloud.io>
Signed-off-by: git-jxj <65210887+git-jxj@users.noreply.github.com>
Co-authored-by: Christopher Desiniotis <cdesiniotis@nvidia.com>
@cdesiniotis
cdesiniotis force-pushed the fix/preserve-docker-features-when-enabling-cdi branch from 54f6ad1 to 205c389 Compare September 14, 2026 16:58
@cdesiniotis

Copy link
Copy Markdown
Contributor

/ok to test 205c389

@cdesiniotis

Copy link
Copy Markdown
Contributor

/cherry-pick release-1.20

@cdesiniotis
cdesiniotis merged commit bd3398d into NVIDIA:main Sep 14, 2026
21 checks passed
@github-actions

Copy link
Copy Markdown

🤖 Backport PR created for release-1.20: #2094 ✅

@git-jxj
git-jxj deleted the fix/preserve-docker-features-when-enabling-cdi branch September 22, 2026 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants