Refactor Daemon.create - prep for call to NRI plugin#51526
Merged
robmry merged 8 commits intomoby:masterfrom Nov 25, 2025
Merged
Refactor Daemon.create - prep for call to NRI plugin#51526robmry merged 8 commits intomoby:masterfrom
robmry merged 8 commits intomoby:masterfrom
Conversation
robmry
added
area/daemon
robmry
force-pushed
the
refactor-create-mounts
branch
6 times, most recently
from
87bec98 to
1d3bf99
Compare
Signed-off-by: Rob Murray <[email protected]>
The call from Daemon.create -> Daemon.setHostConfig acquired container.Lock, but didn't need to because the container is newly created and solely owned by the caller. The call from Daemon.restore did not acquire the lock. Signed-off-by: Rob Murray <[email protected]>
It's set later in Daemon.create, setHostConfig's only caller. Signed-off-by: Rob Murray <[email protected]>
Daemon.createContainerOSSpecificSettings adds container config for the OS, and creates volumes. Split those two things. This will make it possible to call an NRI plugin after the config is complete, before volumes are created - so the NRI plugin can adjust a full set of config, including volumes. Signed-off-by: Rob Murray <[email protected]>
Call registerMountPoints after the rest of the container's configuration has been set up. This will make it possible to call an NRI plugin with the container's config, allowing it to adjust the mounts in that config, before it's used to find volumes etc. Signed-off-by: Rob Murray <[email protected]>
The container's constructor, Daemon.newContainer, already has hostConfig and can just assign it directly. Signed-off-by: Rob Murray <[email protected]>
Signed-off-by: Rob Murray <[email protected]>
Also: - remove the hostConfig param from Daemon.createContainerVolumesOS. - rename var container -> ctr Signed-off-by: Rob Murray <[email protected]>
robmry
force-pushed
the
refactor-create-mounts
branch
from
1d3bf99 to
96b8f9c
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors Daemon.create to prepare for integration with NRI (Node Resource Interface) plugins. The main goal is to set up the container's configuration before processing mount options and volumes, enabling NRI plugin calls that may modify container configuration including mount points.
Key changes:
- Removed the
setHostConfigmethod which was misleadingly named and performed operations beyond just setting host config - Restructured the container creation flow to set
HostConfigduring container construction rather than after - Separated OS-specific settings from volume creation into distinct functions
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| daemon/create.go | Refactored container creation flow: removed setHostConfig call, moved registerLinks and registerMountPoints to separate steps, and split OS-specific operations |
| daemon/container.go | Removed setHostConfig method; simplified setSecurityOptions to use container's existing HostConfig; set HostConfig during container construction in newContainer |
| daemon/volumes.go | Updated registerMountPoints to accept container directly and read HostConfig from it, removing separate hostConfig parameter; renamed container to ctr for consistency |
| daemon/create_unix.go | Split createContainerOSSpecificSettings into two functions: one for OS settings and a new createContainerVolumesOS for volume creation; moved mount/unmount operations to volume creation |
| daemon/create_windows.go | Split createContainerOSSpecificSettings similarly to Unix version, separating volume creation into createContainerVolumesOS |
| daemon/daemon_unix.go | Updated registerLinks to accept only container parameter and read HostConfig from it |
| daemon/daemon_windows.go | Updated registerLinks signature to match Unix version |
| daemon/daemon.go | Updated restore call to registerLinks to match new signature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vvoland
approved these changes
Nov 17, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
- What I did
For NRI, the daemon will need to call a plugin with a container's config, and the plugin may modify the container - including its mount points.
So, set up the container's config before processing mount options and finding volumes to make it possible to call NRI first.
Plus some refactoring ...
Daemon.createcalledsetHostConfigto assignopts.params.HostConfigdirectly toctr.HostConfig, after the container was constructed.opts.params.HostConfigandctr.HostConfigwere different, but they weren't.setHostConfigdid stuff that wasn't setting host config.- How I did it
See individual commits.
- How to verify it
- Human readable description for the release notes