diff --git a/pkg/handlers/generic/mutation/containerdrestart/inject.go b/pkg/handlers/generic/mutation/containerdrestart/inject.go new file mode 100644 index 000000000..92edab3de --- /dev/null +++ b/pkg/handlers/generic/mutation/containerdrestart/inject.go @@ -0,0 +1,86 @@ +// Copyright 2023 D2iQ, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +package containerdrestart + +import ( + "context" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" + ctrl "sigs.k8s.io/controller-runtime" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches" + "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches/selectors" +) + +type containerdRestartPatchHandler struct{} + +func NewPatch() *containerdRestartPatchHandler { + return &containerdRestartPatchHandler{} +} + +func (h *containerdRestartPatchHandler) Mutate( + ctx context.Context, + obj *unstructured.Unstructured, + vars map[string]apiextensionsv1.JSON, + holderRef runtimehooksv1.HolderReference, + clusterKey ctrlclient.ObjectKey, +) error { + log := ctrl.LoggerFrom(ctx).WithValues( + "holderRef", holderRef, + ) + + file, command := generateContainerdRestartScript() + + if err := patches.MutateIfApplicable( + obj, vars, &holderRef, selectors.ControlPlane(), log, + func(obj *controlplanev1.KubeadmControlPlaneTemplate) error { + log.WithValues( + "patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(), + "patchedObjectName", ctrlclient.ObjectKeyFromObject(obj), + ).Info("adding containerd restart script to control plane kubeadm config spec") + obj.Spec.Template.Spec.KubeadmConfigSpec.Files = append( + obj.Spec.Template.Spec.KubeadmConfigSpec.Files, + file, + ) + + log.WithValues( + "patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(), + "patchedObjectName", ctrlclient.ObjectKeyFromObject(obj), + ).Info("adding containerd restart command to control plane kubeadm config spec") + obj.Spec.Template.Spec.KubeadmConfigSpec.PreKubeadmCommands = append( + obj.Spec.Template.Spec.KubeadmConfigSpec.PreKubeadmCommands, + command, + ) + + return nil + }); err != nil { + return err + } + + if err := patches.MutateIfApplicable( + obj, vars, &holderRef, selectors.WorkersKubeadmConfigTemplateSelector(), log, + func(obj *bootstrapv1.KubeadmConfigTemplate) error { + log.WithValues( + "patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(), + "patchedObjectName", ctrlclient.ObjectKeyFromObject(obj), + ).Info("adding containerd restart script to worker node kubeadm config template") + obj.Spec.Template.Spec.Files = append(obj.Spec.Template.Spec.Files, file) + + log.WithValues( + "patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(), + "patchedObjectName", ctrlclient.ObjectKeyFromObject(obj), + ).Info("adding containerd restart command to worker node kubeadm config template") + obj.Spec.Template.Spec.PreKubeadmCommands = append(obj.Spec.Template.Spec.PreKubeadmCommands, command) + + return nil + }); err != nil { + return err + } + + return nil +} diff --git a/pkg/handlers/generic/mutation/containerdrestart/inject_test.go b/pkg/handlers/generic/mutation/containerdrestart/inject_test.go new file mode 100644 index 000000000..c654edc1d --- /dev/null +++ b/pkg/handlers/generic/mutation/containerdrestart/inject_test.go @@ -0,0 +1,97 @@ +// Copyright 2023 D2iQ, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package containerdrestart + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" + + "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation" + "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest" + "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest/request" +) + +func TestContainerdRestartPatch(t *testing.T) { + gomega.RegisterFailHandler(Fail) + RunSpecs(t, "Containerd restart mutator suite") +} + +var _ = Describe("Generate Containerd restart patches", func() { + // only add aws region patch + patchGenerator := func() mutation.GeneratePatches { + return mutation.NewMetaGeneratePatchesHandler("", NewPatch()).(mutation.GeneratePatches) + } + + testDefs := []capitest.PatchTestDef{ + { + Name: "restart script and command added to control plane kubeadm config spec", + RequestItem: request.NewKubeadmControlPlaneTemplateRequestItem(""), + ExpectedPatchMatchers: []capitest.JSONPatchMatcher{ + { + Operation: "add", + Path: "/spec/template/spec/kubeadmConfigSpec/files", + ValueMatcher: gomega.ContainElements( + gomega.HaveKeyWithValue( + "path", ContainerdRestartScriptOnRemote, + ), + ), + }, + { + Operation: "add", + Path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands", + ValueMatcher: gomega.ContainElements( + ContainerdRestartScriptOnRemoteCommand, + ), + }, + }, + }, + { + Name: "restart script and command added to worker node kubeadm config template", + Vars: []runtimehooksv1.Variable{ + capitest.VariableWithValue( + "builtin", + map[string]any{ + "machineDeployment": map[string]any{ + "class": "*", + }, + }, + ), + }, + RequestItem: request.NewKubeadmConfigTemplateRequestItem(""), + ExpectedPatchMatchers: []capitest.JSONPatchMatcher{ + { + Operation: "add", + Path: "/spec/template/spec/files", + ValueMatcher: gomega.ContainElements( + gomega.HaveKeyWithValue( + "path", ContainerdRestartScriptOnRemote, + ), + ), + }, + { + Operation: "add", + Path: "/spec/template/spec/preKubeadmCommands", + ValueMatcher: gomega.ContainElements( + ContainerdRestartScriptOnRemoteCommand, + ), + }, + }, + }, + } + + // create test node for each case + for testIdx := range testDefs { + tt := testDefs[testIdx] + It(tt.Name, func() { + capitest.AssertGeneratePatches( + GinkgoT(), + patchGenerator, + &tt, + ) + }) + } +}) diff --git a/pkg/handlers/generic/mutation/containerdrestart/restart.go b/pkg/handlers/generic/mutation/containerdrestart/restart.go new file mode 100644 index 000000000..8826cceed --- /dev/null +++ b/pkg/handlers/generic/mutation/containerdrestart/restart.go @@ -0,0 +1,27 @@ +// Copyright 2023 D2iQ, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +package containerdrestart + +import ( + _ "embed" + + bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" +) + +const ( + ContainerdRestartScriptOnRemote = "/etc/containerd/restart.sh" + ContainerdRestartScriptOnRemoteCommand = "/bin/bash " + ContainerdRestartScriptOnRemote +) + +//go:embed templates/containerd-restart.sh +var containerdRestartScript []byte + +//nolint:gocritic // no need for named return values +func generateContainerdRestartScript() (bootstrapv1.File, string) { + return bootstrapv1.File{ + Path: ContainerdRestartScriptOnRemote, + Content: string(containerdRestartScript), + Permissions: "0700", + }, + ContainerdRestartScriptOnRemoteCommand +} diff --git a/pkg/handlers/generic/mutation/mirrors/templates/containerd-restart.sh b/pkg/handlers/generic/mutation/containerdrestart/templates/containerd-restart.sh similarity index 100% rename from pkg/handlers/generic/mutation/mirrors/templates/containerd-restart.sh rename to pkg/handlers/generic/mutation/containerdrestart/templates/containerd-restart.sh diff --git a/pkg/handlers/generic/mutation/handlers.go b/pkg/handlers/generic/mutation/handlers.go index bd71c8caa..9ec605971 100644 --- a/pkg/handlers/generic/mutation/handlers.go +++ b/pkg/handlers/generic/mutation/handlers.go @@ -9,6 +9,7 @@ import ( "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation" "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/aws/mutation/cni/calico" "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/auditpolicy" + "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/containerdrestart" "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/etcd" "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/extraapiservercertsans" "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 { mirrors.NewPatch(mgr.GetClient()), calico.NewPatch(), users.NewPatch(), + + // Some patches may have changed containerd configuration. + // We must restart containerd for the configuration to take effect. + // Therefore, we must apply this patch last. + // + // Containerd restart and readiness altogether could take ~5s. + // We want to keep patch independent of each other and not share any state. + // Therefore, We must always apply this patch regardless any other patch modified containerd configuration. + containerdrestart.NewPatch(), } } diff --git a/pkg/handlers/generic/mutation/mirrors/inject.go b/pkg/handlers/generic/mutation/mirrors/inject.go index a1e5e8e3b..42ad077d0 100644 --- a/pkg/handlers/generic/mutation/mirrors/inject.go +++ b/pkg/handlers/generic/mutation/mirrors/inject.go @@ -191,10 +191,6 @@ func generateFilesAndCommands( } files = append(files, applyPatchesFile...) commands = append(commands, applyPatchesCommand) - // generate Containerd restart script and command - restartFile, restartCommand := generateContainerdRestartScript() - files = append(files, restartFile...) - commands = append(commands, restartCommand) return files, commands, err } diff --git a/pkg/handlers/generic/mutation/mirrors/inject_test.go b/pkg/handlers/generic/mutation/mirrors/inject_test.go index 3e7c9106d..24ca2ae17 100644 --- a/pkg/handlers/generic/mutation/mirrors/inject_test.go +++ b/pkg/handlers/generic/mutation/mirrors/inject_test.go @@ -75,9 +75,6 @@ var _ = Describe("Generate Global mirror patches", func() { gomega.HaveKeyWithValue( "path", "/etc/containerd/apply-patches.sh", ), - gomega.HaveKeyWithValue( - "path", "/etc/containerd/restart.sh", - ), ), }, { @@ -85,7 +82,6 @@ var _ = Describe("Generate Global mirror patches", func() { Path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands", ValueMatcher: gomega.ContainElements( "/bin/bash /etc/containerd/apply-patches.sh", - "/bin/bash /etc/containerd/restart.sh", ), }, }, @@ -124,9 +120,6 @@ var _ = Describe("Generate Global mirror patches", func() { gomega.HaveKeyWithValue( "path", "/etc/containerd/apply-patches.sh", ), - gomega.HaveKeyWithValue( - "path", "/etc/containerd/restart.sh", - ), ), }, { @@ -134,7 +127,6 @@ var _ = Describe("Generate Global mirror patches", func() { Path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands", ValueMatcher: gomega.ContainElements( "/bin/bash /etc/containerd/apply-patches.sh", - "/bin/bash /etc/containerd/restart.sh", ), }, }, @@ -173,9 +165,6 @@ var _ = Describe("Generate Global mirror patches", func() { gomega.HaveKeyWithValue( "path", "/etc/containerd/apply-patches.sh", ), - gomega.HaveKeyWithValue( - "path", "/etc/containerd/restart.sh", - ), ), }, { @@ -183,7 +172,6 @@ var _ = Describe("Generate Global mirror patches", func() { Path: "/spec/template/spec/preKubeadmCommands", ValueMatcher: gomega.ContainElements( "/bin/bash /etc/containerd/apply-patches.sh", - "/bin/bash /etc/containerd/restart.sh", ), }, }, @@ -230,9 +218,6 @@ var _ = Describe("Generate Global mirror patches", func() { gomega.HaveKeyWithValue( "path", "/etc/containerd/apply-patches.sh", ), - gomega.HaveKeyWithValue( - "path", "/etc/containerd/restart.sh", - ), ), }, { @@ -240,7 +225,6 @@ var _ = Describe("Generate Global mirror patches", func() { Path: "/spec/template/spec/preKubeadmCommands", ValueMatcher: gomega.ContainElements( "/bin/bash /etc/containerd/apply-patches.sh", - "/bin/bash /etc/containerd/restart.sh", ), }, }, diff --git a/pkg/handlers/generic/mutation/mirrors/mirror.go b/pkg/handlers/generic/mutation/mirrors/mirror.go index ff14c2b62..8ab281f9c 100644 --- a/pkg/handlers/generic/mutation/mirrors/mirror.go +++ b/pkg/handlers/generic/mutation/mirrors/mirror.go @@ -28,9 +28,6 @@ const ( containerdPatchesDirOnRemote = "/etc/containerd/cre.d" containerdApplyPatchesScriptOnRemote = "/etc/containerd/apply-patches.sh" containerdApplyPatchesScriptOnRemoteCommand = "/bin/bash " + containerdApplyPatchesScriptOnRemote - - containerdRestartScriptOnRemote = "/etc/containerd/restart.sh" - containerdRestartScriptOnRemoteCommand = "/bin/bash " + containerdRestartScriptOnRemote ) var ( @@ -50,9 +47,6 @@ var ( //go:embed templates/containerd-apply-patches.sh.gotmpl containerdApplyConfigPatchesScript []byte - - //go:embed templates/containerd-restart.sh - containerdRestartScript []byte ) type mirrorConfig struct { @@ -226,14 +220,3 @@ func generateContainerdApplyPatchesScript() ([]cabpkv1.File, string, error) { }, }, containerdApplyPatchesScriptOnRemoteCommand, nil } - -//nolint:gocritic // no need for named return values -func generateContainerdRestartScript() ([]cabpkv1.File, string) { - return []cabpkv1.File{ - { - Path: containerdRestartScriptOnRemote, - Content: string(containerdRestartScript), - Permissions: "0700", - }, - }, containerdRestartScriptOnRemoteCommand -}