Skip to content

Commit 65dbf35

Browse files
jimmidysonjimmidyson
committed
fix: Always apply containerd patches
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 4cb10d8 commit 65dbf35

File tree

11 files changed

+142
-167
lines changed

11 files changed

+142
-167
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/containerdrestart/inject_test.go renamed to ‎pkg/handlers/generic/mutation/containerdapplypatchesandrestart/inject_test.go

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
// Copyright 2023 Nutanix. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
package containerdrestart
4+
package containerdapplypatchesandrestart
55

66
import (
77
"testing"
88

99
. "github.com/onsi/ginkgo/v2"
1010
"github.com/onsi/gomega"
11+
"github.com/stretchr/testify/assert"
12+
bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
1113
runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
1214

1315
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation"
@@ -16,12 +18,12 @@ import (
1618
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/test/helpers"
1719
)
1820

19-
func TestContainerdRestartPatch(t *testing.T) {
21+
func TestContainerdApplyPatchesAndRestartPatch(t *testing.T) {
2022
gomega.RegisterFailHandler(Fail)
21-
RunSpecs(t, "Containerd restart mutator suite")
23+
RunSpecs(t, "Containerd apply patches and restart mutator suite")
2224
}
2325

24-
var _ = Describe("Generate Containerd restart patches", func() {
26+
var _ = Describe("Generate Containerd apply patches and restart patches", func() {
2527
// only add aws region patch
2628
patchGenerator := func() mutation.GeneratePatches {
2729
return mutation.NewMetaGeneratePatchesHandler("", helpers.TestEnv.Client, NewPatch()).(mutation.GeneratePatches)
@@ -35,7 +37,10 @@ var _ = Describe("Generate Containerd restart patches", func() {
3537
{
3638
Operation: "add",
3739
Path: "/spec/template/spec/kubeadmConfigSpec/files",
38-
ValueMatcher: gomega.ContainElements(
40+
ValueMatcher: gomega.HaveExactElements(
41+
gomega.HaveKeyWithValue(
42+
"path", containerdApplyPatchesScriptOnRemote,
43+
),
3944
gomega.HaveKeyWithValue(
4045
"path", ContainerdRestartScriptOnRemote,
4146
),
@@ -44,7 +49,8 @@ var _ = Describe("Generate Containerd restart patches", func() {
4449
{
4550
Operation: "add",
4651
Path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands",
47-
ValueMatcher: gomega.ContainElements(
52+
ValueMatcher: gomega.HaveExactElements(
53+
containerdApplyPatchesScriptOnRemoteCommand,
4854
ContainerdRestartScriptOnRemoteCommand,
4955
),
5056
},
@@ -67,7 +73,10 @@ var _ = Describe("Generate Containerd restart patches", func() {
6773
{
6874
Operation: "add",
6975
Path: "/spec/template/spec/files",
70-
ValueMatcher: gomega.ContainElements(
76+
ValueMatcher: gomega.HaveExactElements(
77+
gomega.HaveKeyWithValue(
78+
"path", containerdApplyPatchesScriptOnRemote,
79+
),
7180
gomega.HaveKeyWithValue(
7281
"path", ContainerdRestartScriptOnRemote,
7382
),
@@ -76,7 +85,8 @@ var _ = Describe("Generate Containerd restart patches", func() {
7685
{
7786
Operation: "add",
7887
Path: "/spec/template/spec/preKubeadmCommands",
79-
ValueMatcher: gomega.ContainElements(
88+
ValueMatcher: gomega.HaveExactElements(
89+
containerdApplyPatchesScriptOnRemoteCommand,
8090
ContainerdRestartScriptOnRemoteCommand,
8191
),
8292
},
@@ -96,3 +106,39 @@ var _ = Describe("Generate Containerd restart patches", func() {
96106
})
97107
}
98108
})
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+
declare -r TOML_MERGE_IMAGE="ghcr.io/mesosphere/toml-merge:v0.2.0"
123+
124+
if ! ctr --namespace k8s.io images check "name==${TOML_MERGE_IMAGE}" | grep "${TOML_MERGE_IMAGE}" >/dev/null; then
125+
ctr --namespace k8s.io images pull "${TOML_MERGE_IMAGE}"
126+
fi
127+
128+
cleanup() {
129+
ctr images unmount "${tmp_ctr_mount_dir}" || true
130+
}
131+
132+
trap 'cleanup' EXIT
133+
134+
readonly tmp_ctr_mount_dir="$(mktemp -d)"
135+
136+
ctr --namespace k8s.io images mount "${TOML_MERGE_IMAGE}" "${tmp_ctr_mount_dir}"
137+
"${tmp_ctr_mount_dir}/usr/local/bin/toml-merge" -i --patch-file "/etc/containerd/cre.d/*.toml" /etc/containerd/config.toml
138+
`,
139+
}
140+
wantCmd := "/bin/bash /etc/containerd/apply-patches.sh"
141+
file, cmd, _ := generateContainerdApplyPatchesScript()
142+
assert.Equal(t, wantFile, file)
143+
assert.Equal(t, wantCmd, cmd)
144+
}

‎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/mirrors/templates/containerd-apply-patches.sh.gotmpl renamed to ‎pkg/handlers/generic/mutation/containerdapplypatchesandrestart/templates/containerd-apply-patches.sh.gotmpl

File renamed without changes.

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

File renamed without changes.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import (
99
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation"
1010
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/aws/mutation/cni/calico"
1111
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/auditpolicy"
12+
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/containerdapplypatchesandrestart"
1213
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/containerdmetrics"
13-
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/containerdrestart"
1414
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/etcd"
1515
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/extraapiservercertsans"
1616
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/httpproxy"
@@ -41,6 +41,6 @@ func MetaMutators(mgr manager.Manager) []mutation.MetaMutator {
4141
// Containerd restart and readiness altogether could take ~5s.
4242
// We want to keep patch independent of each other and not share any state.
4343
// Therefore, We must always apply this patch regardless any other patch modified containerd configuration.
44-
containerdrestart.NewPatch(),
44+
containerdapplypatchesandrestart.NewPatch(),
4545
}
4646
}

‎pkg/handlers/generic/mutation/mirrors/inject.go

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func (h *globalMirrorPatchHandler) Mutate(
101101
if err != nil {
102102
return err
103103
}
104-
files, commands, generateErr := generateFilesAndCommands(
104+
files, generateErr := generateFilesAndCommands(
105105
mirrorConfig,
106106
globalMirror)
107107
if generateErr != nil {
@@ -117,15 +117,6 @@ func (h *globalMirrorPatchHandler) Mutate(
117117
files...,
118118
)
119119

120-
log.WithValues(
121-
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
122-
"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
123-
).Info("adding PreKubeadmCommands to control plane kubeadm config spec")
124-
obj.Spec.Template.Spec.KubeadmConfigSpec.PreKubeadmCommands = append(
125-
obj.Spec.Template.Spec.KubeadmConfigSpec.PreKubeadmCommands,
126-
commands...,
127-
)
128-
129120
return nil
130121
}); err != nil {
131122
return err
@@ -143,7 +134,7 @@ func (h *globalMirrorPatchHandler) Mutate(
143134
if err != nil {
144135
return err
145136
}
146-
files, commands, generateErr := generateFilesAndCommands(
137+
files, generateErr := generateFilesAndCommands(
147138
mirrorConfig,
148139
globalMirror)
149140
if generateErr != nil {
@@ -156,12 +147,6 @@ func (h *globalMirrorPatchHandler) Mutate(
156147
).Info("adding global registry mirror files to worker node kubeadm config template")
157148
obj.Spec.Template.Spec.Files = append(obj.Spec.Template.Spec.Files, files...)
158149

159-
log.WithValues(
160-
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
161-
"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
162-
).Info("adding PreKubeadmCommands to worker node kubeadm config template")
163-
obj.Spec.Template.Spec.PreKubeadmCommands = append(obj.Spec.Template.Spec.PreKubeadmCommands, commands...)
164-
165150
return nil
166151
}); err != nil {
167152
return err
@@ -173,26 +158,18 @@ func (h *globalMirrorPatchHandler) Mutate(
173158
func generateFilesAndCommands(
174159
mirrorConfig *mirrorConfig,
175160
globalMirror v1alpha1.GlobalImageRegistryMirror,
176-
) ([]bootstrapv1.File, []string, error) {
161+
) ([]bootstrapv1.File, error) {
177162
// generate default registry mirror file
178163
files, err := generateGlobalRegistryMirrorFile(mirrorConfig)
179164
if err != nil {
180-
return nil, nil, err
165+
return nil, err
181166
}
182167
// generate CA certificate file for registry mirror
183168
mirrorCAFile := generateMirrorCACertFile(mirrorConfig, globalMirror)
184169
files = append(files, mirrorCAFile...)
185170
// generate Containerd registry config drop-in file
186171
registryConfigDropIn := generateContainerdRegistryConfigDropInFile()
187172
files = append(files, registryConfigDropIn...)
188-
// generate Containerd apply patches script and command
189-
var commands []string
190-
applyPatchesFile, applyPatchesCommand, err := generateContainerdApplyPatchesScript()
191-
if err != nil {
192-
return nil, nil, err
193-
}
194-
files = append(files, applyPatchesFile...)
195-
commands = append(commands, applyPatchesCommand)
196173

197-
return files, commands, err
174+
return files, err
198175
}

0 commit comments

Comments
 (0)