Skip to content

Commit 2deeee9

Browse files
jimmidysonjimmidyson
authored
fix: Always apply containerd patches (#644)
This commit ensures that applying containerd patches always happens. Previously, this only happened if mirror configuration was provided, which meant that patches such as enabling containerd metrics were never applied to containerd config. By bundling the apply patches and restart containerd script in the same handler, we ensure they both always get run.
1 parent 22bf205 commit 2deeee9

File tree

13 files changed

+303
-278
lines changed

13 files changed

+303
-278
lines changed

‎pkg/handlers/generic/mutation/containerdrestart/restart.go renamed to ‎pkg/handlers/generic/mutation/containerdapplypatchesandrestart/apply_patches.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Copyright 2023 Nutanix. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
3-
package containerdrestart
3+
package containerdapplypatchesandrestart
44

55
import (
66
_ "embed"

‎pkg/handlers/generic/mutation/containerdrestart/inject.go renamed to ‎pkg/handlers/generic/mutation/containerdapplypatchesandrestart/inject.go

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
// Copyright 2023 Nutanix. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
3-
package containerdrestart
3+
package containerdapplypatchesandrestart
44

55
import (
66
"context"
7+
_ "embed"
78

89
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
910
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -18,13 +19,13 @@ import (
1819
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches/selectors"
1920
)
2021

21-
type containerdRestartPatchHandler struct{}
22+
type containerdApplyPatchesAndRestartPatchHandler struct{}
2223

23-
func NewPatch() *containerdRestartPatchHandler {
24-
return &containerdRestartPatchHandler{}
24+
func NewPatch() *containerdApplyPatchesAndRestartPatchHandler {
25+
return &containerdApplyPatchesAndRestartPatchHandler{}
2526
}
2627

27-
func (h *containerdRestartPatchHandler) Mutate(
28+
func (h *containerdApplyPatchesAndRestartPatchHandler) Mutate(
2829
ctx context.Context,
2930
obj *unstructured.Unstructured,
3031
vars map[string]apiextensionsv1.JSON,
@@ -36,27 +37,34 @@ func (h *containerdRestartPatchHandler) Mutate(
3637
"holderRef", holderRef,
3738
)
3839

39-
file, command := generateContainerdRestartScript()
40+
restartFile, restartCommand := generateContainerdRestartScript()
41+
42+
applyPatchesFile, applyPatchesCommand, err := generateContainerdApplyPatchesScript()
43+
if err != nil {
44+
return err
45+
}
4046

4147
if err := patches.MutateIfApplicable(
4248
obj, vars, &holderRef, selectors.ControlPlane(), log,
4349
func(obj *controlplanev1.KubeadmControlPlaneTemplate) error {
4450
log.WithValues(
4551
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
4652
"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
47-
).Info("adding containerd restart script to control plane kubeadm config spec")
53+
).Info("adding containerd apply patches and restart scripts to control plane kubeadm config spec")
4854
obj.Spec.Template.Spec.KubeadmConfigSpec.Files = append(
4955
obj.Spec.Template.Spec.KubeadmConfigSpec.Files,
50-
file,
56+
applyPatchesFile,
57+
restartFile,
5158
)
5259

5360
log.WithValues(
5461
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
5562
"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
56-
).Info("adding containerd restart command to control plane kubeadm config spec")
63+
).Info("adding containerd apply patches and restart commands to control plane kubeadm config spec")
5764
obj.Spec.Template.Spec.KubeadmConfigSpec.PreKubeadmCommands = append(
5865
obj.Spec.Template.Spec.KubeadmConfigSpec.PreKubeadmCommands,
59-
command,
66+
applyPatchesCommand,
67+
restartCommand,
6068
)
6169

6270
return nil
@@ -70,14 +78,22 @@ func (h *containerdRestartPatchHandler) Mutate(
7078
log.WithValues(
7179
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
7280
"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
73-
).Info("adding containerd restart script to worker node kubeadm config template")
74-
obj.Spec.Template.Spec.Files = append(obj.Spec.Template.Spec.Files, file)
81+
).Info("adding containerd apply patches and restart scripts to worker node kubeadm config template")
82+
obj.Spec.Template.Spec.Files = append(
83+
obj.Spec.Template.Spec.Files,
84+
applyPatchesFile,
85+
restartFile,
86+
)
7587

7688
log.WithValues(
7789
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
7890
"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
79-
).Info("adding containerd restart command to worker node kubeadm config template")
80-
obj.Spec.Template.Spec.PreKubeadmCommands = append(obj.Spec.Template.Spec.PreKubeadmCommands, command)
91+
).Info("adding containerd apply patches and restart commands to worker node kubeadm config template")
92+
obj.Spec.Template.Spec.PreKubeadmCommands = append(
93+
obj.Spec.Template.Spec.PreKubeadmCommands,
94+
applyPatchesCommand,
95+
restartCommand,
96+
)
8197

8298
return nil
8399
}); err != nil {

‎pkg/handlers/generic/mutation/containerdapplypatchesandrestart/inject_test.go

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
// Copyright 2023 Nutanix. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package containerdapplypatchesandrestart
5+
6+
import (
7+
"testing"
8+
9+
. "github.com/onsi/ginkgo/v2"
10+
"github.com/onsi/gomega"
11+
"github.com/stretchr/testify/assert"
12+
bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
13+
runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
14+
15+
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation"
16+
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest"
17+
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest/request"
18+
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/test/helpers"
19+
)
20+
21+
func TestContainerdApplyPatchesAndRestartPatch(t *testing.T) {
22+
gomega.RegisterFailHandler(Fail)
23+
RunSpecs(t, "Containerd apply patches and restart mutator suite")
24+
}
25+
26+
var _ = Describe("Generate Containerd apply patches and restart patches", func() {
27+
// only add aws region patch
28+
patchGenerator := func() mutation.GeneratePatches {
29+
return mutation.NewMetaGeneratePatchesHandler("", helpers.TestEnv.Client, NewPatch()).(mutation.GeneratePatches)
30+
}
31+
32+
testDefs := []capitest.PatchTestDef{
33+
{
34+
Name: "restart script and command added to control plane kubeadm config spec",
35+
RequestItem: request.NewKubeadmControlPlaneTemplateRequestItem(""),
36+
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{
37+
{
38+
Operation: "add",
39+
Path: "/spec/template/spec/kubeadmConfigSpec/files",
40+
ValueMatcher: gomega.HaveExactElements(
41+
gomega.HaveKeyWithValue(
42+
"path", containerdApplyPatchesScriptOnRemote,
43+
),
44+
gomega.HaveKeyWithValue(
45+
"path", ContainerdRestartScriptOnRemote,
46+
),
47+
),
48+
},
49+
{
50+
Operation: "add",
51+
Path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands",
52+
ValueMatcher: gomega.HaveExactElements(
53+
containerdApplyPatchesScriptOnRemoteCommand,
54+
ContainerdRestartScriptOnRemoteCommand,
55+
),
56+
},
57+
},
58+
},
59+
{
60+
Name: "restart script and command added to worker node kubeadm config template",
61+
Vars: []runtimehooksv1.Variable{
62+
capitest.VariableWithValue(
63+
"builtin",
64+
map[string]any{
65+
"machineDeployment": map[string]any{
66+
"class": "*",
67+
},
68+
},
69+
),
70+
},
71+
RequestItem: request.NewKubeadmConfigTemplateRequestItem(""),
72+
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{
73+
{
74+
Operation: "add",
75+
Path: "/spec/template/spec/files",
76+
ValueMatcher: gomega.HaveExactElements(
77+
gomega.HaveKeyWithValue(
78+
"path", containerdApplyPatchesScriptOnRemote,
79+
),
80+
gomega.HaveKeyWithValue(
81+
"path", ContainerdRestartScriptOnRemote,
82+
),
83+
),
84+
},
85+
{
86+
Operation: "add",
87+
Path: "/spec/template/spec/preKubeadmCommands",
88+
ValueMatcher: gomega.HaveExactElements(
89+
containerdApplyPatchesScriptOnRemoteCommand,
90+
ContainerdRestartScriptOnRemoteCommand,
91+
),
92+
},
93+
},
94+
},
95+
}
96+
97+
// create test node for each case
98+
for testIdx := range testDefs {
99+
tt := testDefs[testIdx]
100+
It(tt.Name, func() {
101+
capitest.AssertGeneratePatches(
102+
GinkgoT(),
103+
patchGenerator,
104+
&tt,
105+
)
106+
})
107+
}
108+
})
109+
110+
func Test_generateContainerdApplyPatchesScript(t *testing.T) {
111+
wantFile := bootstrapv1.File{
112+
Path: "/etc/containerd/apply-patches.sh",
113+
Owner: "",
114+
Permissions: "0700",
115+
Encoding: "",
116+
Append: false,
117+
//nolint:lll // just a long string
118+
Content: `#!/bin/bash
119+
set -euo pipefail
120+
IFS=$'\n\t'
121+
122+
# This script is used to merge the TOML files in the patch directory into the containerd configuration file.
123+
124+
# Check if there are any TOML files in the patch directory, exiting if none are found.
125+
# Use a for loop that will only run a maximum of once to check if there are any files in the patch directory because
126+
# using -e does not work with globs.
127+
# See https://github.com/koalaman/shellcheck/wiki/SC2144 for an explanation of the following loop.
128+
patches_exist=false
129+
for file in "/etc/containerd/cre.d"/*.toml; do
130+
if [ -e "${file}" ]; then
131+
patches_exist=true
132+
fi
133+
# Always break after the first iteration.
134+
break
135+
done
136+
137+
if [ "${patches_exist}" = false ]; then
138+
echo "No TOML files found in the patch directory: /etc/containerd/cre.d - nothing to do"
139+
exit 0
140+
fi
141+
142+
# Use go template variable to avoid hard-coding the toml-merge image name in this script.
143+
declare -r TOML_MERGE_IMAGE="ghcr.io/mesosphere/toml-merge:v0.2.0"
144+
145+
# Check if the toml-merge image is already present in ctr images list, if not pull it.
146+
if ! ctr --namespace k8s.io images check "name==${TOML_MERGE_IMAGE}" | grep "${TOML_MERGE_IMAGE}" >/dev/null; then
147+
ctr --namespace k8s.io images pull "${TOML_MERGE_IMAGE}"
148+
fi
149+
150+
# Cleanup the temporary directory on exit.
151+
cleanup() {
152+
ctr images unmount "${tmp_ctr_mount_dir}" || true
153+
}
154+
trap 'cleanup' EXIT
155+
156+
# Create a temporary directory to mount the toml-merge image filesystem.
157+
readonly tmp_ctr_mount_dir="$(mktemp -d)"
158+
159+
# Mount the toml-merge image filesystem and run the toml-merge binary to merge the TOML files.
160+
ctr --namespace k8s.io images mount "${TOML_MERGE_IMAGE}" "${tmp_ctr_mount_dir}"
161+
"${tmp_ctr_mount_dir}/usr/local/bin/toml-merge" -i --patch-file "/etc/containerd/cre.d/*.toml" /etc/containerd/config.toml
162+
`,
163+
}
164+
wantCmd := "/bin/bash /etc/containerd/apply-patches.sh"
165+
file, cmd, _ := generateContainerdApplyPatchesScript()
166+
assert.Equal(t, wantFile, file)
167+
assert.Equal(t, wantCmd, cmd)
168+
}

‎pkg/handlers/generic/mutation/containerdapplypatchesandrestart/restart.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2023 Nutanix. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
package containerdapplypatchesandrestart
4+
5+
import (
6+
"bytes"
7+
_ "embed"
8+
"fmt"
9+
"text/template"
10+
11+
bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
12+
)
13+
14+
const (
15+
tomlMergeImage = "ghcr.io/mesosphere/toml-merge:v0.2.0"
16+
containerdPatchesDirOnRemote = "/etc/containerd/cre.d"
17+
containerdApplyPatchesScriptOnRemote = "/etc/containerd/apply-patches.sh"
18+
containerdApplyPatchesScriptOnRemoteCommand = "/bin/bash " + containerdApplyPatchesScriptOnRemote
19+
)
20+
21+
//go:embed templates/containerd-apply-patches.sh.gotmpl
22+
var containerdApplyConfigPatchesScript []byte
23+
24+
func generateContainerdApplyPatchesScript() (bootstrapv1.File, string, error) {
25+
t, err := template.New("").Parse(string(containerdApplyConfigPatchesScript))
26+
if err != nil {
27+
return bootstrapv1.File{}, "", fmt.Errorf("failed to parse go template: %w", err)
28+
}
29+
30+
templateInput := struct {
31+
TOMLMergeImage string
32+
PatchDir string
33+
}{
34+
TOMLMergeImage: tomlMergeImage,
35+
PatchDir: containerdPatchesDirOnRemote,
36+
}
37+
38+
var b bytes.Buffer
39+
err = t.Execute(&b, templateInput)
40+
if err != nil {
41+
return bootstrapv1.File{}, "", fmt.Errorf("failed executing template: %w", err)
42+
}
43+
44+
return bootstrapv1.File{
45+
Path: containerdApplyPatchesScriptOnRemote,
46+
Content: b.String(),
47+
Permissions: "0700",
48+
}, containerdApplyPatchesScriptOnRemoteCommand, nil
49+
}

‎pkg/handlers/generic/mutation/containerdapplypatchesandrestart/templates/containerd-apply-patches.sh.gotmpl

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#!/bin/bash
2+
set -euo pipefail
3+
IFS=$'\n\t'
4+
5+
# This script is used to merge the TOML files in the patch directory into the containerd configuration file.
6+
7+
# Check if there are any TOML files in the patch directory, exiting if none are found.
8+
# Use a for loop that will only run a maximum of once to check if there are any files in the patch directory because
9+
# using -e does not work with globs.
10+
# See https://github.com/koalaman/shellcheck/wiki/SC2144 for an explanation of the following loop.
11+
patches_exist=false
12+
for file in "{{ .PatchDir }}"/*.toml; do
13+
if [ -e "${file}" ]; then
14+
patches_exist=true
15+
fi
16+
# Always break after the first iteration.
17+
break
18+
done
19+
20+
if [ "${patches_exist}" = false ]; then
21+
echo "No TOML files found in the patch directory: {{ .PatchDir }} - nothing to do"
22+
exit 0
23+
fi
24+
25+
# Use go template variable to avoid hard-coding the toml-merge image name in this script.
26+
declare -r TOML_MERGE_IMAGE="{{ .TOMLMergeImage }}"
27+
28+
# Check if the toml-merge image is already present in ctr images list, if not pull it.
29+
if ! ctr --namespace k8s.io images check "name==${TOML_MERGE_IMAGE}" | grep "${TOML_MERGE_IMAGE}" >/dev/null; then
30+
ctr --namespace k8s.io images pull "${TOML_MERGE_IMAGE}"
31+
fi
32+
33+
# Cleanup the temporary directory on exit.
34+
cleanup() {
35+
ctr images unmount "${tmp_ctr_mount_dir}" || true
36+
}
37+
trap 'cleanup' EXIT
38+
39+
# Create a temporary directory to mount the toml-merge image filesystem.
40+
readonly tmp_ctr_mount_dir="$(mktemp -d)"
41+
42+
# Mount the toml-merge image filesystem and run the toml-merge binary to merge the TOML files.
43+
ctr --namespace k8s.io images mount "${TOML_MERGE_IMAGE}" "${tmp_ctr_mount_dir}"
44+
"${tmp_ctr_mount_dir}/usr/local/bin/toml-merge" -i --patch-file "{{ .PatchDir }}/*.toml" /etc/containerd/config.toml

‎pkg/handlers/generic/mutation/containerdrestart/templates/containerd-restart.sh renamed to ‎pkg/handlers/generic/mutation/containerdapplypatchesandrestart/templates/containerd-restart.sh

File renamed without changes.

0 commit comments

Comments
 (0)