Skip to content
This repository was archived by the owner on Apr 11, 2024. It is now read-only.

Commit 898e55c

Browse files
dlipovetskysupershal
authored and
jimmidyson
committed
feat: Make containerd restart its own patch (#18)
* feat: Make containerd restart its own patch * fix: unit tests for kubeadmconfigtemplate with containerdrestart patch * fix: add comment for always add containerd patch * test: move unit test to their own package --------- Co-authored-by: Shalin Patel <[email protected]>
1 parent c6cdfba commit 898e55c

File tree

8 files changed

+220
-37
lines changed

8 files changed

+220
-37
lines changed

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

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright 2023 D2iQ, Inc. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
package containerdrestart
4+
5+
import (
6+
"context"
7+
8+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
9+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
10+
bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
11+
controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
12+
runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
13+
ctrl "sigs.k8s.io/controller-runtime"
14+
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
15+
16+
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches"
17+
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches/selectors"
18+
)
19+
20+
type containerdRestartPatchHandler struct{}
21+
22+
func NewPatch() *containerdRestartPatchHandler {
23+
return &containerdRestartPatchHandler{}
24+
}
25+
26+
func (h *containerdRestartPatchHandler) Mutate(
27+
ctx context.Context,
28+
obj *unstructured.Unstructured,
29+
vars map[string]apiextensionsv1.JSON,
30+
holderRef runtimehooksv1.HolderReference,
31+
clusterKey ctrlclient.ObjectKey,
32+
) error {
33+
log := ctrl.LoggerFrom(ctx).WithValues(
34+
"holderRef", holderRef,
35+
)
36+
37+
file, command := generateContainerdRestartScript()
38+
39+
if err := patches.MutateIfApplicable(
40+
obj, vars, &holderRef, selectors.ControlPlane(), log,
41+
func(obj *controlplanev1.KubeadmControlPlaneTemplate) error {
42+
log.WithValues(
43+
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
44+
"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
45+
).Info("adding containerd restart script to control plane kubeadm config spec")
46+
obj.Spec.Template.Spec.KubeadmConfigSpec.Files = append(
47+
obj.Spec.Template.Spec.KubeadmConfigSpec.Files,
48+
file,
49+
)
50+
51+
log.WithValues(
52+
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
53+
"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
54+
).Info("adding containerd restart command to control plane kubeadm config spec")
55+
obj.Spec.Template.Spec.KubeadmConfigSpec.PreKubeadmCommands = append(
56+
obj.Spec.Template.Spec.KubeadmConfigSpec.PreKubeadmCommands,
57+
command,
58+
)
59+
60+
return nil
61+
}); err != nil {
62+
return err
63+
}
64+
65+
if err := patches.MutateIfApplicable(
66+
obj, vars, &holderRef, selectors.WorkersKubeadmConfigTemplateSelector(), log,
67+
func(obj *bootstrapv1.KubeadmConfigTemplate) error {
68+
log.WithValues(
69+
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
70+
"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
71+
).Info("adding containerd restart script to worker node kubeadm config template")
72+
obj.Spec.Template.Spec.Files = append(obj.Spec.Template.Spec.Files, file)
73+
74+
log.WithValues(
75+
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
76+
"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
77+
).Info("adding containerd restart command to worker node kubeadm config template")
78+
obj.Spec.Template.Spec.PreKubeadmCommands = append(obj.Spec.Template.Spec.PreKubeadmCommands, command)
79+
80+
return nil
81+
}); err != nil {
82+
return err
83+
}
84+
85+
return nil
86+
}

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

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Copyright 2023 D2iQ, Inc. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package containerdrestart
5+
6+
import (
7+
"testing"
8+
9+
. "github.com/onsi/ginkgo/v2"
10+
"github.com/onsi/gomega"
11+
runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
12+
13+
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation"
14+
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest"
15+
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest/request"
16+
)
17+
18+
func TestContainerdRestartPatch(t *testing.T) {
19+
gomega.RegisterFailHandler(Fail)
20+
RunSpecs(t, "Containerd restart mutator suite")
21+
}
22+
23+
var _ = Describe("Generate Containerd restart patches", func() {
24+
// only add aws region patch
25+
patchGenerator := func() mutation.GeneratePatches {
26+
return mutation.NewMetaGeneratePatchesHandler("", NewPatch()).(mutation.GeneratePatches)
27+
}
28+
29+
testDefs := []capitest.PatchTestDef{
30+
{
31+
Name: "restart script and command added to control plane kubeadm config spec",
32+
RequestItem: request.NewKubeadmControlPlaneTemplateRequestItem(""),
33+
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{
34+
{
35+
Operation: "add",
36+
Path: "/spec/template/spec/kubeadmConfigSpec/files",
37+
ValueMatcher: gomega.ContainElements(
38+
gomega.HaveKeyWithValue(
39+
"path", ContainerdRestartScriptOnRemote,
40+
),
41+
),
42+
},
43+
{
44+
Operation: "add",
45+
Path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands",
46+
ValueMatcher: gomega.ContainElements(
47+
ContainerdRestartScriptOnRemoteCommand,
48+
),
49+
},
50+
},
51+
},
52+
{
53+
Name: "restart script and command added to worker node kubeadm config template",
54+
Vars: []runtimehooksv1.Variable{
55+
capitest.VariableWithValue(
56+
"builtin",
57+
map[string]any{
58+
"machineDeployment": map[string]any{
59+
"class": "*",
60+
},
61+
},
62+
),
63+
},
64+
RequestItem: request.NewKubeadmConfigTemplateRequestItem(""),
65+
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{
66+
{
67+
Operation: "add",
68+
Path: "/spec/template/spec/files",
69+
ValueMatcher: gomega.ContainElements(
70+
gomega.HaveKeyWithValue(
71+
"path", ContainerdRestartScriptOnRemote,
72+
),
73+
),
74+
},
75+
{
76+
Operation: "add",
77+
Path: "/spec/template/spec/preKubeadmCommands",
78+
ValueMatcher: gomega.ContainElements(
79+
ContainerdRestartScriptOnRemoteCommand,
80+
),
81+
},
82+
},
83+
},
84+
}
85+
86+
// create test node for each case
87+
for testIdx := range testDefs {
88+
tt := testDefs[testIdx]
89+
It(tt.Name, func() {
90+
capitest.AssertGeneratePatches(
91+
GinkgoT(),
92+
patchGenerator,
93+
&tt,
94+
)
95+
})
96+
}
97+
})

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright 2023 D2iQ, Inc. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
package containerdrestart
4+
5+
import (
6+
_ "embed"
7+
8+
bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
9+
)
10+
11+
const (
12+
ContainerdRestartScriptOnRemote = "/etc/containerd/restart.sh"
13+
ContainerdRestartScriptOnRemoteCommand = "/bin/bash " + ContainerdRestartScriptOnRemote
14+
)
15+
16+
//go:embed templates/containerd-restart.sh
17+
var containerdRestartScript []byte
18+
19+
//nolint:gocritic // no need for named return values
20+
func generateContainerdRestartScript() (bootstrapv1.File, string) {
21+
return bootstrapv1.File{
22+
Path: ContainerdRestartScriptOnRemote,
23+
Content: string(containerdRestartScript),
24+
Permissions: "0700",
25+
},
26+
ContainerdRestartScriptOnRemoteCommand
27+
}

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

File renamed without changes.

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation"
1010
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/aws/mutation/cni/calico"
1111
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/auditpolicy"
12+
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/containerdrestart"
1213
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/etcd"
1314
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/extraapiservercertsans"
1415
"github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/httpproxy"
@@ -30,5 +31,14 @@ func MetaMutators(mgr manager.Manager) []mutation.MetaMutator {
3031
mirrors.NewPatch(mgr.GetClient()),
3132
calico.NewPatch(),
3233
users.NewPatch(),
34+
35+
// Some patches may have changed containerd configuration.
36+
// We must restart containerd for the configuration to take effect.
37+
// Therefore, we must apply this patch last.
38+
//
39+
// Containerd restart and readiness altogether could take ~5s.
40+
// We want to keep patch independent of each other and not share any state.
41+
// Therefore, We must always apply this patch regardless any other patch modified containerd configuration.
42+
containerdrestart.NewPatch(),
3343
}
3444
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,6 @@ func generateFilesAndCommands(
191191
}
192192
files = append(files, applyPatchesFile...)
193193
commands = append(commands, applyPatchesCommand)
194-
// generate Containerd restart script and command
195-
restartFile, restartCommand := generateContainerdRestartScript()
196-
files = append(files, restartFile...)
197-
commands = append(commands, restartCommand)
198194

199195
return files, commands, err
200196
}

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

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,13 @@ var _ = Describe("Generate Global mirror patches", func() {
7575
gomega.HaveKeyWithValue(
7676
"path", "/etc/containerd/apply-patches.sh",
7777
),
78-
gomega.HaveKeyWithValue(
79-
"path", "/etc/containerd/restart.sh",
80-
),
8178
),
8279
},
8380
{
8481
Operation: "add",
8582
Path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands",
8683
ValueMatcher: gomega.ContainElements(
8784
"/bin/bash /etc/containerd/apply-patches.sh",
88-
"/bin/bash /etc/containerd/restart.sh",
8985
),
9086
},
9187
},
@@ -124,17 +120,13 @@ var _ = Describe("Generate Global mirror patches", func() {
124120
gomega.HaveKeyWithValue(
125121
"path", "/etc/containerd/apply-patches.sh",
126122
),
127-
gomega.HaveKeyWithValue(
128-
"path", "/etc/containerd/restart.sh",
129-
),
130123
),
131124
},
132125
{
133126
Operation: "add",
134127
Path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands",
135128
ValueMatcher: gomega.ContainElements(
136129
"/bin/bash /etc/containerd/apply-patches.sh",
137-
"/bin/bash /etc/containerd/restart.sh",
138130
),
139131
},
140132
},
@@ -173,17 +165,13 @@ var _ = Describe("Generate Global mirror patches", func() {
173165
gomega.HaveKeyWithValue(
174166
"path", "/etc/containerd/apply-patches.sh",
175167
),
176-
gomega.HaveKeyWithValue(
177-
"path", "/etc/containerd/restart.sh",
178-
),
179168
),
180169
},
181170
{
182171
Operation: "add",
183172
Path: "/spec/template/spec/preKubeadmCommands",
184173
ValueMatcher: gomega.ContainElements(
185174
"/bin/bash /etc/containerd/apply-patches.sh",
186-
"/bin/bash /etc/containerd/restart.sh",
187175
),
188176
},
189177
},
@@ -230,17 +218,13 @@ var _ = Describe("Generate Global mirror patches", func() {
230218
gomega.HaveKeyWithValue(
231219
"path", "/etc/containerd/apply-patches.sh",
232220
),
233-
gomega.HaveKeyWithValue(
234-
"path", "/etc/containerd/restart.sh",
235-
),
236221
),
237222
},
238223
{
239224
Operation: "add",
240225
Path: "/spec/template/spec/preKubeadmCommands",
241226
ValueMatcher: gomega.ContainElements(
242227
"/bin/bash /etc/containerd/apply-patches.sh",
243-
"/bin/bash /etc/containerd/restart.sh",
244228
),
245229
},
246230
},

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@ const (
2828
containerdPatchesDirOnRemote = "/etc/containerd/cre.d"
2929
containerdApplyPatchesScriptOnRemote = "/etc/containerd/apply-patches.sh"
3030
containerdApplyPatchesScriptOnRemoteCommand = "/bin/bash " + containerdApplyPatchesScriptOnRemote
31-
32-
containerdRestartScriptOnRemote = "/etc/containerd/restart.sh"
33-
containerdRestartScriptOnRemoteCommand = "/bin/bash " + containerdRestartScriptOnRemote
3431
)
3532

3633
var (
@@ -50,9 +47,6 @@ var (
5047

5148
//go:embed templates/containerd-apply-patches.sh.gotmpl
5249
containerdApplyConfigPatchesScript []byte
53-
54-
//go:embed templates/containerd-restart.sh
55-
containerdRestartScript []byte
5650
)
5751

5852
type mirrorConfig struct {
@@ -226,14 +220,3 @@ func generateContainerdApplyPatchesScript() ([]cabpkv1.File, string, error) {
226220
},
227221
}, containerdApplyPatchesScriptOnRemoteCommand, nil
228222
}
229-
230-
//nolint:gocritic // no need for named return values
231-
func generateContainerdRestartScript() ([]cabpkv1.File, string) {
232-
return []cabpkv1.File{
233-
{
234-
Path: containerdRestartScriptOnRemote,
235-
Content: string(containerdRestartScript),
236-
Permissions: "0700",
237-
},
238-
}, containerdRestartScriptOnRemoteCommand
239-
}

0 commit comments

Comments
 (0)