feat: add erofs snapshotter configuration for new kata-preview runtime class - #9085
feat: add erofs snapshotter configuration for new kata-preview runtime class#9085Camelron wants to merge 2 commits into
Conversation
Kata's VM templating feature requires a blockfile (erofs) snapshotter. Configure the kata runtime block as such in /etc/containerd/config.toml Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>
Windows Unit Test Results 3 files 11 suites 47s ⏱️ Results for commit 222decb. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
🟡 Not ready to approve
The containerd v1 templates can render invalid TOML due to duplicate snapshotter keys under certain flag combinations, and the new runtime class/config is not mirrored in the aks-node-controller (scriptless) containerd templates.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR updates the AgentBaker-generated containerd config.toml templates to introduce a new kata-preview runtime class intended to use the erofs snapshotter, enabling Kata VM templating-related features while keeping the existing kata runtime on overlayfs.
Changes:
- Adds
io.containerd.snapshotter.v1.erofs(and related differ/diff-service config) gated onIsKatain all containerd config templates. - Introduces a new
kata-previewruntime stanza configured to usesnapshotter = "erofs"with a new Kata ConfigPath. - Explicitly sets
snapshotter = "overlayfs"for the existingkataruntime (and in the v1 CRI containerd block whenIsKata).
File summaries
| File | Description |
|---|---|
| pkg/agent/baker.go | Extends containerd config templates to add erofs snapshotter config and a new kata-preview runtime class. |
Review details
Comments suppressed due to low confidence (2)
pkg/agent/baker.go:1848
- In the containerd v1 template this can emit two
snapshotter = ...keys in the same TOML table whenIsKataandIsArtifactStreamingEnabledare both true (overlayfs + overlaybd). Duplicate keys are invalid TOML and can cause containerd to fail to load the config. Consider making these branches mutually exclusive so only one snapshotter is written.
This issue also appears on line 2112 of the same file.
snapshotter = "overlayfs"
{{- end}}
{{- if IsArtifactStreamingEnabled }}
snapshotter = "overlaybd"
disable_snapshot_annotations = false
pkg/agent/baker.go:2116
- Same issue as the non-NoGPU template: this block can render duplicate
snapshotterkeys in the[plugins."io.containerd.grpc.v1.cri".containerd]table whenIsKataandIsArtifactStreamingEnabledare both true, producing invalid TOML.
disable_snapshot_annotations = false
snapshotter = "overlayfs"
{{- end}}
{{- if IsArtifactStreamingEnabled }}
snapshotter = "overlaybd"
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.kata-preview] | ||
| runtime_type = "io.containerd.kata.v2" | ||
| privileged_without_host_devices = true | ||
| snapshotter = "erofs" | ||
| [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.kata-preview.options] |
There was a problem hiding this comment.
I'm pushing a second commit containing the same template changes mirrored to these files. @cameronmeissner @Bickor can someone double check here and explain the difference?
There was a problem hiding this comment.
I'll let @cameronmeissner explain the difference here, I don't know the answer.
There was a problem hiding this comment.
In the past I've made changes only to baker.go (ex: https://github.com/Azure/AgentBaker/pull/6816/changes) I think with that we should be good, but I'll let Cameron chime in in case it has changed now and we need changes to both.
Mirror the same configs in baker.go to aks-node-controller templates. Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>
There was a problem hiding this comment.
🟡 Not ready to approve
The v1 containerd templates can emit duplicate TOML keys (snapshotter/disable_snapshot_annotations) when both Kata and artifact streaming are enabled, which can make the generated config invalid.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (2)
aks-node-controller/parser/templates/containerd.toml.gtpl:80
- In this template the
kataruntime block is missingprivileged_without_host_devices = true(present in the AgentBaker-generated templates and in the v2 templates). This can lead to different Kata behavior depending on provisioning mode; add it here for consistency.
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.kata]
runtime_type = "io.containerd.kata.v2"
snapshotter = "overlayfs"
aks-node-controller/parser/templates/containerd_no_GPU.toml.gtpl:64
- In this template the
kataruntime block is missingprivileged_without_host_devices = true(present in the AgentBaker-generated templates and in the v2 templates). This can lead to different Kata behavior depending on provisioning mode; add it here for consistency.
[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.kata]
runtime_type = "io.containerd.kata.v2"
snapshotter = "overlayfs"
- Files reviewed: 5/5 changed files
- Comments generated: 4
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Bickor
left a comment
There was a problem hiding this comment.
@cameronmeissner could you point us to the pipeline in aks-rp we need to run the agentbaker branch on to validate these changes still work?
@Camelron fyi validate with the pipeline before merging please, the pipeline runs the k8s conformance tests with kata.
| Root = "" | ||
| CriuPath = "" | ||
| SystemdCgroup = false | ||
| [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.kata-preview] |
There was a problem hiding this comment.
Is this the final name of the runtimeclass? CC @allyford
Maybe let's leave a TODO to define the exact name when we're ready to onboard the rc.
| [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.kata-preview] | ||
| runtime_type = "io.containerd.kata.v2" | ||
| privileged_without_host_devices = true | ||
| snapshotter = "erofs" | ||
| [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.kata-preview.options] |
There was a problem hiding this comment.
In the past I've made changes only to baker.go (ex: https://github.com/Azure/AgentBaker/pull/6816/changes) I think with that we should be good, but I'll let Cameron chime in in case it has changed now and we need changes to both.
Kata's VM templating feature requires a blockfile snapshotter (erofs). Configure the kata runtime block as such in /etc/containerd/config.toml
Erofs snapshotter will be used for VM templating, arbitrary snapshot/restore API, and kata-cc will, in a future change, switch over to using it over tardev snapshotter.
There is no risk of regression or backwards-compatibility issues in AKS kata; the erofs snapshotter configuration is gated behind a new runtime class, kata-preview.
What this PR does / why we need it:
Which issue(s) this PR fixes:
No existing issue; this is being done to enable a new incoming kata feature.