Skip to content

add limited support for version aware containerd config patching #4042

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add limited support for version aware containerd config patching #4042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/apis/config/v1alpha4/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,14 @@ type Cluster struct {
// ContainerdConfigPatches are applied to every node's containerd config
// in the order listed.
// These should be toml stringsto be applied as merge patches
// If the version field in these patches doesn't match containerd config, it will not be a applied
// This way you can write configurations that work for both by supplying two patches
ContainerdConfigPatches []string `yaml:"containerdConfigPatches,omitempty" json:"containerdConfigPatches,omitempty"`

// ContainerdConfigPatchesJSON6902 are applied to every node's containerd config
// in the order listed.
// These should be YAML or JSON formatting RFC 6902 JSON patches
// NOTE: These are not currently version-aware.
ContainerdConfigPatchesJSON6902 []string `yaml:"containerdConfigPatchesJSON6902,omitempty" json:"containerdConfigPatchesJSON6902,omitempty"`
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/build/nodeimage/containerd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const containerdConfigPatchSystemdCgroupFalse = `
`

func configureContainerdSystemdCgroupFalse(containerCmdr exec.Cmder, config string) error {
patched, err := patch.TOML(config, []string{containerdConfigPatchSystemdCgroupFalse}, []string{})
patched, err := patch.ContainerdTOML(config, []string{containerdConfigPatchSystemdCgroupFalse}, []string{})
if err != nil {
return errors.Wrap(err, "failed to configure containerd SystemdCgroup=false")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cluster/internal/create/actions/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (a *Action) Execute(ctx *actions.ActionContext) error {
if err := node.Command("cat", containerdConfigPath).SetStdout(&buff).Run(); err != nil {
return errors.Wrap(err, "failed to read containerd config from node")
}
patched, err := patch.TOML(buff.String(), ctx.Config.ContainerdConfigPatches, ctx.Config.ContainerdConfigPatchesJSON6902)
patched, err := patch.ContainerdTOML(buff.String(), ctx.Config.ContainerdConfigPatches, ctx.Config.ContainerdConfigPatchesJSON6902)
if err != nil {
return errors.Wrap(err, "failed to patch containerd config")
}
Expand Down
32 changes: 29 additions & 3 deletions pkg/internal/patch/toml.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,33 @@ import (
"sigs.k8s.io/kind/pkg/errors"
)

// TOML patches toPatch with the patches (should be TOML merge patches) and patches6902 (should be JSON 6902 patches)
func TOML(toPatch string, patches []string, patches6902 []string) (string, error) {
// ContainerdTOML patches toPatch with the patches (should be TOML merge patches) and patches6902 (should be JSON 6902 patches)
func ContainerdTOML(toPatch string, patches []string, patches6902 []string) (string, error) {
// convert to JSON for patching
j, err := tomlToJSON([]byte(toPatch))
if err != nil {
return "", err
}
version, err := containerdConfigVersion(toPatch)
Copy link
Member Author

@BenTheElder BenTheElder Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is an assumption that we always get a valid version from the base configuration here, we could error if this is not non-zero here, or below we could always match if it is zero similar to how we ignore match for patches with version = 0

Copy link
Member Author

@BenTheElder BenTheElder Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think failing to read this one should be an error, it should only happen if we changed our base config and didn't update this properly (because version parsing changed in containerd config?) or because someone has forked kind with a custom containerd base config that doesn't set version, which I don't really care to guard against.

Copy link
Member Author

@BenTheElder BenTheElder Nov 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to require detecting the version in the config to be patched

if err != nil {
return "", errors.WithStack(err)
}
if version == 0 {
return "", errors.New("failed to detect containerd config version")
}
// apply merge patches
for _, patch := range patches {
pj, err := tomlToJSON([]byte(patch))
if err != nil {
return "", err
return "", errors.WithStack(err)
}
patchVersion, err := containerdConfigVersion(patch)
if err != nil {
return "", errors.WithStack(err)
}
// skip if patch sets version and version does not match
if patchVersion != 0 && patchVersion != version {
continue
}
patched, err := jsonpatch.MergePatch(j, pj)
if err != nil {
Expand All @@ -64,6 +79,17 @@ func TOML(toPatch string, patches []string, patches6902 []string) (string, error
return jsonToTOMLString(j)
}

func containerdConfigVersion(configTOML string) (int, error) {
type version struct {
Version int `toml:"version,omitempty"`
}
v := version{}
if err := toml.Unmarshal([]byte(configTOML), &v); err != nil {
Copy link
Contributor

@aojea aojea Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version field in the config file specifies the config’s version. If no version number is specified inside the config file then it is assumed to be a version 1 config and parsed as such. Please use version = 2 to enable version 2 config as version 1 has been deprecated.

should we default to version 1 instead of error?

Copy link
Member Author

@BenTheElder BenTheElder Nov 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

version 1 isn't the default in containerd as far as I know, but the default would depend on containerd version and that seems fragile.

This is the file to be patched, we are always specifying the version ourselves, so if it doesn't have the version or we can't parse it, something is wrong.

since this is in an internal package already, I didn't think it was worth designing for other use cases (hence the rename to Containerd and the assumption about the version key)

Copy link
Member Author

@BenTheElder BenTheElder Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah also, maybe this wasn't obvious: We won't error if there is no version, error would mean it fails to parse as toml or it has a version key with a different data type or something like that.

(You can also confirm from the existing test cases without version specified continuing to work, in additon to the updated test cases)

We have version field as omitempty, so it defaults to 0 if not specified

return 0, errors.WithStack(err)
}
return v.Version, nil
}

// tomlToJSON converts arbitrary TOML to JSON
func tomlToJSON(t []byte) ([]byte, error) {
// we use github.com.pelletier/go-toml here to unmarshal arbitrary TOML to JSON
Expand Down
64 changes: 54 additions & 10 deletions pkg/internal/patch/toml_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"sigs.k8s.io/kind/pkg/internal/assert"
)

func TestTOML(t *testing.T) {
func TestContainerdTOML(t *testing.T) {
t.Parallel()
type testCase struct {
Name string
Expand All @@ -39,15 +39,48 @@ func TestTOML(t *testing.T) {
ExpectError: true,
ExpectOutput: "",
},
{
Name: "invalid containerd versioning",
ToPatch: `version = "five"`,
ExpectError: true,
ExpectOutput: "",
},
{
Name: "no patches",
ToPatch: `disabled_plugins = ["restart"]
ToPatch: `version = 2
disabled_plugins = ["restart"]
[plugins.linux]
shim_debug = true
[plugins.cri.containerd.runtimes.runsc]
runtime_type = "io.containerd.runsc.v1"`,
ExpectError: false,
ExpectOutput: `disabled_plugins = ["restart"]
version = 2

[plugins]
[plugins.cri]
[plugins.cri.containerd]
[plugins.cri.containerd.runtimes]
[plugins.cri.containerd.runtimes.runsc]
runtime_type = "io.containerd.runsc.v1"
[plugins.linux]
shim_debug = true
`,
},
{
Name: "Only matching patches",
ToPatch: `version = 2

disabled_plugins = ["restart"]

[plugins.linux]
shim_debug = true
[plugins.cri.containerd.runtimes.runsc]
runtime_type = "io.containerd.runsc.v1"`,
Patches: []string{"version = 3\ndisabled_plugins=[\"bar\"]", "version = 2\n disabled_plugins=[\"baz\"]"},
ExpectError: false,
ExpectOutput: `disabled_plugins = ["baz"]
version = 2

[plugins]
[plugins.cri]
Expand All @@ -61,7 +94,8 @@ func TestTOML(t *testing.T) {
},
{
Name: "invalid patch TOML",
ToPatch: `disabled_plugins = ["restart"]
ToPatch: `version = 2
disabled_plugins = ["restart"]
[plugins.linux]
shim_debug = true
[plugins.cri.containerd.runtimes.runsc]
Expand All @@ -81,14 +115,16 @@ func TestTOML(t *testing.T) {
},
{
Name: "trivial patch",
ToPatch: `disabled_plugins = ["restart"]
ToPatch: `version = 2
disabled_plugins = ["restart"]
[plugins.linux]
shim_debug = true
[plugins.cri.containerd.runtimes.runsc]
runtime_type = "io.containerd.runsc.v1"`,
Patches: []string{`disabled_plugins=[]`},
ExpectError: false,
ExpectOutput: `disabled_plugins = []
version = 2

[plugins]
[plugins.cri]
Expand All @@ -102,14 +138,17 @@ func TestTOML(t *testing.T) {
},
{
Name: "trivial 6902 patch",
ToPatch: `disabled_plugins = ["restart"]
ToPatch: `version = 2
disabled_plugins = ["restart"]
[plugins.linux]
shim_debug = true
[plugins.cri.containerd.runtimes.runsc]
runtime_type = "io.containerd.runsc.v1"`,
PatchesJSON6902: []string{`[{"op": "remove", "path": "/disabled_plugins"}]`},
ExpectError: false,
ExpectOutput: `[plugins]
ExpectOutput: `version = 2

[plugins]
[plugins.cri]
[plugins.cri.containerd]
[plugins.cri.containerd.runtimes]
Expand All @@ -121,15 +160,18 @@ func TestTOML(t *testing.T) {
},
{
Name: "trivial patch and trivial 6902 patch",
ToPatch: `disabled_plugins = ["restart"]
ToPatch: `version = 2
disabled_plugins = ["restart"]
[plugins.linux]
shim_debug = true
[plugins.cri.containerd.runtimes.runsc]
runtime_type = "io.containerd.runsc.v1"`,
Patches: []string{`disabled_plugins=["foo"]`},
PatchesJSON6902: []string{`[{"op": "remove", "path": "/disabled_plugins"}]`},
ExpectError: false,
ExpectOutput: `[plugins]
ExpectOutput: `version = 2

[plugins]
[plugins.cri]
[plugins.cri.containerd]
[plugins.cri.containerd.runtimes]
Expand Down Expand Up @@ -160,7 +202,8 @@ func TestTOML(t *testing.T) {
},
{
Name: "patch registry",
ToPatch: `disabled_plugins = ["restart"]
ToPatch: `version = 2
disabled_plugins = ["restart"]
[plugins.linux]
shim_debug = true
[plugins.cri.containerd.runtimes.runsc]
Expand All @@ -170,6 +213,7 @@ func TestTOML(t *testing.T) {
endpoint = ["http://registry:5000"]`},
ExpectError: false,
ExpectOutput: `disabled_plugins = ["restart"]
version = 2

[plugins]
[plugins.cri]
Expand All @@ -190,7 +234,7 @@ func TestTOML(t *testing.T) {
tc := tc // capture test case
t.Run(tc.Name, func(t *testing.T) {
t.Parallel()
out, err := TOML(tc.ToPatch, tc.Patches, tc.PatchesJSON6902)
out, err := ContainerdTOML(tc.ToPatch, tc.Patches, tc.PatchesJSON6902)
assert.ExpectError(t, tc.ExpectError, err)
if err == nil {
assert.StringEqual(t, tc.ExpectOutput, out)
Expand Down
Loading