From 1d0f700be98289cdf754a9026d0ffb5c6cafe237 Mon Sep 17 00:00:00 2001 From: Dimitri Koshkin Date: Fri, 2 May 2025 15:44:07 -0700 Subject: [PATCH 1/3] feat: use registryMirror addon as Containerd mirror --- .../generic/mutation/mirrors/inject.go | 59 +++++++++++- .../generic/mutation/mirrors/inject_test.go | 92 ++++++++++++++++++- 2 files changed, 144 insertions(+), 7 deletions(-) diff --git a/pkg/handlers/generic/mutation/mirrors/inject.go b/pkg/handlers/generic/mutation/mirrors/inject.go index 03f6eec7d..2a0819c1e 100644 --- a/pkg/handlers/generic/mutation/mirrors/inject.go +++ b/pkg/handlers/generic/mutation/mirrors/inject.go @@ -11,6 +11,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" 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" @@ -22,6 +23,7 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches/selectors" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/variables" + registrymirrorutils "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/lifecycle/registrymirror/utils" handlersutils "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/utils" ) @@ -62,8 +64,8 @@ func (h *globalMirrorPatchHandler) Mutate( obj *unstructured.Unstructured, vars map[string]apiextensionsv1.JSON, holderRef runtimehooksv1.HolderReference, - clusterKey ctrlclient.ObjectKey, - _ mutation.ClusterGetter, + _ ctrlclient.ObjectKey, + clusterGetter mutation.ClusterGetter, ) error { log := ctrl.LoggerFrom(ctx).WithValues( "holderRef", holderRef, @@ -82,14 +84,24 @@ func (h *globalMirrorPatchHandler) Mutate( v1alpha1.ImageRegistriesVariableName, ) + _, registryMirrorErr := variables.Get[v1alpha1.RegistryMirror]( + vars, + v1alpha1.ClusterConfigVariableName, + []string{"addons", v1alpha1.RegistryMirrorVariableName}...) + switch { - case variables.IsNotFoundError(imageRegistriesErr) && variables.IsNotFoundError(globalMirrorErr): - log.V(5).Info("Image Registry Credentials and Global Registry Mirror variable not defined") + case variables.IsNotFoundError(imageRegistriesErr) && + variables.IsNotFoundError(globalMirrorErr) && + variables.IsNotFoundError(registryMirrorErr): + log.V(5). + Info("Image Registry Credentials and Global Registry Mirror and Registry Mirror Addon variable not defined") return nil case imageRegistriesErr != nil && !variables.IsNotFoundError(imageRegistriesErr): return imageRegistriesErr case globalMirrorErr != nil && !variables.IsNotFoundError(globalMirrorErr): return globalMirrorErr + case registryMirrorErr != nil && !variables.IsNotFoundError(registryMirrorErr): + return registryMirrorErr } var registriesWithOptionalCA []containerdConfig //nolint:prealloc // We don't know the size of the slice yet. @@ -121,6 +133,26 @@ func (h *globalMirrorPatchHandler) Mutate( registryWithOptionalCredentials, ) } + if registryMirrorErr == nil { + cluster, err := clusterGetter(ctx) + if err != nil { + log.Error( + err, + "failed to get cluster from Global Mirror mutation handler", + ) + return err + } + + registryConfig, err := containerdConfigFromRegistryMirrorAddon( + ctx, + h.client, + cluster, + ) + if err != nil { + return err + } + registriesWithOptionalCA = append(registriesWithOptionalCA, registryConfig) + } needConfiguration := needContainerdConfiguration( registriesWithOptionalCA, @@ -234,6 +266,25 @@ func containerdConfigFromImageRegistry( return configWithOptionalCACert, nil } +func containerdConfigFromRegistryMirrorAddon( + _ context.Context, + _ ctrlclient.Client, + cluster *clusterv1.Cluster, +) (containerdConfig, error) { + serviceIP, err := registrymirrorutils.ServiceIPForCluster(cluster) + if err != nil { + return containerdConfig{}, fmt.Errorf("error getting service IP for the registry mirror addon: %w", err) + } + + config := containerdConfig{ + // FIXME: Generate a self-signed CA. + URL: fmt.Sprintf("http://%s", serviceIP), + Mirror: true, + } + + return config, nil +} + func generateFiles( registriesWithOptionalCA []containerdConfig, ) ([]bootstrapv1.File, error) { diff --git a/pkg/handlers/generic/mutation/mirrors/inject_test.go b/pkg/handlers/generic/mutation/mirrors/inject_test.go index f0bee8a64..de66188b1 100644 --- a/pkg/handlers/generic/mutation/mirrors/inject_test.go +++ b/pkg/handlers/generic/mutation/mirrors/inject_test.go @@ -11,7 +11,11 @@ import ( "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apiserver/pkg/storage/names" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" @@ -32,6 +36,10 @@ func TestMirrorsPatch(t *testing.T) { } var _ = Describe("Generate Global mirror patches", func() { + clientScheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(clientScheme)) + utilruntime.Must(clusterv1.AddToScheme(clientScheme)) + patchGenerator := func() mutation.GeneratePatches { // Always initialize the testEnv variable in the closure. // This will allow ginkgo to initialize testEnv variable during test execution time. @@ -40,7 +48,7 @@ var _ = Describe("Generate Global mirror patches", func() { // that are written by the tests. // Test cases writes credentials secret that the mutator handler reads. // Using direct client will enable reading it immediately. - client, err := testEnv.GetK8sClient() + client, err := testEnv.GetK8sClientWithScheme(clientScheme) gomega.Expect(err).To(gomega.BeNil()) return mutation.NewMetaGeneratePatchesHandler("", client, NewPatch(client)).(mutation.GeneratePatches) } @@ -330,11 +338,69 @@ var _ = Describe("Generate Global mirror patches", func() { }, }, }, + { + Name: "files added in KubeadmControlPlaneTemplate for registry mirror addon", + Vars: []runtimehooksv1.Variable{ + capitest.VariableWithValue( + v1alpha1.ClusterConfigVariableName, + v1alpha1.RegistryMirror{}, + []string{"addons", v1alpha1.RegistryMirrorVariableName}..., + ), + }, + RequestItem: request.NewKubeadmControlPlaneTemplateRequestItem(""), + ExpectedPatchMatchers: []capitest.JSONPatchMatcher{ + { + Operation: "add", + Path: "/spec/template/spec/kubeadmConfigSpec/files", + ValueMatcher: gomega.HaveExactElements( + gomega.HaveKeyWithValue( + "path", "/etc/containerd/certs.d/_default/hosts.toml", + ), + gomega.HaveKeyWithValue( + "path", "/etc/caren/containerd/patches/registry-config.toml", + ), + ), + }, + }, + }, + { + Name: "files added in KubeadmConfigTemplate for registry mirror addon", + Vars: []runtimehooksv1.Variable{ + capitest.VariableWithValue( + v1alpha1.ClusterConfigVariableName, + v1alpha1.RegistryMirror{}, + []string{"addons", v1alpha1.RegistryMirrorVariableName}..., + ), + capitest.VariableWithValue( + "builtin", + map[string]any{ + "machineDeployment": map[string]any{ + "class": names.SimpleNameGenerator.GenerateName("worker-"), + }, + }, + ), + }, + RequestItem: request.NewKubeadmConfigTemplateRequestItem(""), + ExpectedPatchMatchers: []capitest.JSONPatchMatcher{ + { + Operation: "add", + Path: "/spec/template/spec/files", + ValueMatcher: gomega.HaveExactElements( + gomega.HaveKeyWithValue( + "path", "/etc/containerd/certs.d/_default/hosts.toml", + ), + gomega.HaveKeyWithValue( + "path", "/etc/caren/containerd/patches/registry-config.toml", + ), + ), + }, + }, + }, } // Create credentials secret before each test BeforeEach(func(ctx SpecContext) { - client, err := helpers.TestEnv.GetK8sClient() + client, err := helpers.TestEnv.GetK8sClientWithScheme(clientScheme) gomega.Expect(err).To(gomega.BeNil()) gomega.Expect(client.Create( ctx, @@ -344,11 +410,21 @@ var _ = Describe("Generate Global mirror patches", func() { ctx, newMirrorSecretWithoutCA(validMirrorNoCASecretName, request.Namespace), )).To(gomega.BeNil()) + + gomega.Expect(client.Create( + ctx, + &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: request.ClusterName, + Namespace: request.Namespace, + }, + }, + )).To(gomega.BeNil()) }) // Delete credentials secret after each test AfterEach(func(ctx SpecContext) { - client, err := helpers.TestEnv.GetK8sClient() + client, err := helpers.TestEnv.GetK8sClientWithScheme(clientScheme) gomega.Expect(err).To(gomega.BeNil()) gomega.Expect(client.Delete( ctx, @@ -358,6 +434,16 @@ var _ = Describe("Generate Global mirror patches", func() { ctx, newMirrorSecretWithoutCA(validMirrorNoCASecretName, request.Namespace), )).To(gomega.BeNil()) + + gomega.Expect(client.Delete( + ctx, + &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: request.ClusterName, + Namespace: request.Namespace, + }, + }, + )).To(gomega.BeNil()) }) // create test node for each case From 1054c68fe82c07d727fc08c02ee0750da6156f30 Mon Sep 17 00:00:00 2001 From: Dimitri Koshkin Date: Wed, 7 May 2025 11:34:00 -0700 Subject: [PATCH 2/3] fixup! fix: remove unused function args --- pkg/handlers/generic/mutation/mirrors/inject.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/pkg/handlers/generic/mutation/mirrors/inject.go b/pkg/handlers/generic/mutation/mirrors/inject.go index 2a0819c1e..0aad77e69 100644 --- a/pkg/handlers/generic/mutation/mirrors/inject.go +++ b/pkg/handlers/generic/mutation/mirrors/inject.go @@ -143,11 +143,7 @@ func (h *globalMirrorPatchHandler) Mutate( return err } - registryConfig, err := containerdConfigFromRegistryMirrorAddon( - ctx, - h.client, - cluster, - ) + registryConfig, err := containerdConfigFromRegistryMirrorAddon(cluster) if err != nil { return err } @@ -266,11 +262,7 @@ func containerdConfigFromImageRegistry( return configWithOptionalCACert, nil } -func containerdConfigFromRegistryMirrorAddon( - _ context.Context, - _ ctrlclient.Client, - cluster *clusterv1.Cluster, -) (containerdConfig, error) { +func containerdConfigFromRegistryMirrorAddon(cluster *clusterv1.Cluster) (containerdConfig, error) { serviceIP, err := registrymirrorutils.ServiceIPForCluster(cluster) if err != nil { return containerdConfig{}, fmt.Errorf("error getting service IP for the registry mirror addon: %w", err) From 71bc9ca501a7af4e352dfa5c4c8b9875d80d39eb Mon Sep 17 00:00:00 2001 From: Dimitri Koshkin Date: Thu, 8 May 2025 15:57:03 -0700 Subject: [PATCH 3/3] fixup! after rebase --- pkg/handlers/generic/mutation/mirrors/inject.go | 12 ++++++------ pkg/handlers/generic/mutation/mirrors/inject_test.go | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/handlers/generic/mutation/mirrors/inject.go b/pkg/handlers/generic/mutation/mirrors/inject.go index 0aad77e69..7ff2dcee4 100644 --- a/pkg/handlers/generic/mutation/mirrors/inject.go +++ b/pkg/handlers/generic/mutation/mirrors/inject.go @@ -23,7 +23,7 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches/selectors" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/variables" - registrymirrorutils "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/lifecycle/registrymirror/utils" + registryutils "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/lifecycle/registry/utils" handlersutils "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/utils" ) @@ -84,10 +84,10 @@ func (h *globalMirrorPatchHandler) Mutate( v1alpha1.ImageRegistriesVariableName, ) - _, registryMirrorErr := variables.Get[v1alpha1.RegistryMirror]( + _, registryMirrorErr := variables.Get[v1alpha1.RegistryAddon]( vars, v1alpha1.ClusterConfigVariableName, - []string{"addons", v1alpha1.RegistryMirrorVariableName}...) + []string{"addons", v1alpha1.RegistryAddonVariableName}...) switch { case variables.IsNotFoundError(imageRegistriesErr) && @@ -143,7 +143,7 @@ func (h *globalMirrorPatchHandler) Mutate( return err } - registryConfig, err := containerdConfigFromRegistryMirrorAddon(cluster) + registryConfig, err := containerdConfigFromRegistryAddon(cluster) if err != nil { return err } @@ -262,8 +262,8 @@ func containerdConfigFromImageRegistry( return configWithOptionalCACert, nil } -func containerdConfigFromRegistryMirrorAddon(cluster *clusterv1.Cluster) (containerdConfig, error) { - serviceIP, err := registrymirrorutils.ServiceIPForCluster(cluster) +func containerdConfigFromRegistryAddon(cluster *clusterv1.Cluster) (containerdConfig, error) { + serviceIP, err := registryutils.ServiceIPForCluster(cluster) if err != nil { return containerdConfig{}, fmt.Errorf("error getting service IP for the registry mirror addon: %w", err) } diff --git a/pkg/handlers/generic/mutation/mirrors/inject_test.go b/pkg/handlers/generic/mutation/mirrors/inject_test.go index de66188b1..bc2b60d36 100644 --- a/pkg/handlers/generic/mutation/mirrors/inject_test.go +++ b/pkg/handlers/generic/mutation/mirrors/inject_test.go @@ -343,8 +343,8 @@ var _ = Describe("Generate Global mirror patches", func() { Vars: []runtimehooksv1.Variable{ capitest.VariableWithValue( v1alpha1.ClusterConfigVariableName, - v1alpha1.RegistryMirror{}, - []string{"addons", v1alpha1.RegistryMirrorVariableName}..., + v1alpha1.RegistryAddon{}, + []string{"addons", v1alpha1.RegistryAddonVariableName}..., ), }, RequestItem: request.NewKubeadmControlPlaneTemplateRequestItem(""), @@ -368,8 +368,8 @@ var _ = Describe("Generate Global mirror patches", func() { Vars: []runtimehooksv1.Variable{ capitest.VariableWithValue( v1alpha1.ClusterConfigVariableName, - v1alpha1.RegistryMirror{}, - []string{"addons", v1alpha1.RegistryMirrorVariableName}..., + v1alpha1.RegistryAddon{}, + []string{"addons", v1alpha1.RegistryAddonVariableName}..., ), capitest.VariableWithValue( "builtin",