build: device entitlement support#50386
Conversation
e59010e to
9741d95
Compare
tonistiigi
left a comment
There was a problem hiding this comment.
This entitlement is enabled by default to match current behavior in moby when requesting devices.
Wdym by "current behavior" here?
Slightly curious as well; here's on my current Docker Desktop with v28.3.x Does this config relate to the |
| type BuilderEntitlements struct { | ||
| NetworkHost *bool `json:"network-host,omitempty"` | ||
| SecurityInsecure *bool `json:"security-insecure,omitempty"` | ||
| Device *bool `json:"device,omitempty"` | ||
| } |
There was a problem hiding this comment.
This probably needs changes in the daemon docs, although it looks like we currently don't document these at all? https://github.com/docker/cli/blob/7668b683d2bdd470b6fc95082f4a1f9f3f5c756a/docs/reference/dockerd.md#on-linux
currently only shows this in the example (no docs);
"builder": {
"gc": {
"enabled": true,
"defaultKeepStorage": "10GB",
"policy": [
{ "keepStorage": "10GB", "filter": ["unused-for=2200h"] },
{ "keepStorage": "50GB", "filter": ["unused-for=3300h"] },
{ "keepStorage": "100GB", "all": true }
]
}
},| if conf.Entitlements.Device == nil || *conf.Entitlements.Device { | ||
| ents = append(ents, string(entitlements.EntitlementDevice)) | ||
| } |
There was a problem hiding this comment.
Does BuildKit have some list of its defaults? Slightly starting to wonder if we need to change the BuilderEntitlements to be a map[string]bool (or something) that allows it to stay in sync with whatever new options are added in BuildKit
That may also allow us to validate options as part of dockerd --validate (i.e., check the list of keys against known entitlements in BuildKit)
There was a problem hiding this comment.
Does BuildKit have some list of its defaults?
They are all disabled by default in buildkit
Slightly starting to wonder if we need to change the
BuilderEntitlementsto be amap[string]bool(or something) that allows it to stay in sync with whatever new options are added in BuildKit
Yes I think we could but needs to keep current defaults for moby.
There was a problem hiding this comment.
Yeah, considering we could set some defaults here (which is where we set most defaults);
Lines 311 to 345 in d8089c8
We may need to look closely if we want to import BuildKit packages there, or define local consts for the defaults (but possibly it's fine to import buildkit code).
There was a problem hiding this comment.
Possibly we can inject the defaults as well (I need to refresh my memory on the exact flow we have for these).
I meant running a container in moby allows access to specified cdi devices without any specific entitlement atm so I think it should be the same for build. |
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
9741d95 to
d95dceb
Compare
follow-up:
Adds
deviceentitlement support in Docker daemon configuration. This entitlement is enabled by default to match current behavior in moby when requesting devices.- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)