From aedaf8d7c8f9a42fae0697855d4882756b409de0 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Fri, 27 Jan 2023 18:15:59 +0100 Subject: [PATCH] update --- exp/runtime/topologymutation/walker.go | 10 ++++------ exp/runtime/topologymutation/walkler_test.go | 4 ++-- test/extension/handlers/topologymutation/handler.go | 12 ++++++------ 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/exp/runtime/topologymutation/walker.go b/exp/runtime/topologymutation/walker.go index 893ae36b6399..181d480ea384 100644 --- a/exp/runtime/topologymutation/walker.go +++ b/exp/runtime/topologymutation/walker.go @@ -31,7 +31,6 @@ import ( runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" patchvariables "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/variables" - "sigs.k8s.io/cluster-api/test/extension/api" ) // WalkTemplatesOption is some configuration that modifies WalkTemplates behavior. @@ -81,7 +80,7 @@ func (d PatchFormat) ApplyToWalkTemplates(in *WalkTemplatesOptions) { // Also, by using this func it is possible to ignore most of the details of the GeneratePatchesRequest // and GeneratePatchesResponse messages format and focus on writing patches/modifying the templates. func WalkTemplates(ctx context.Context, decoder runtime.Decoder, req *runtimehooksv1.GeneratePatchesRequest, - resp *runtimehooksv1.GeneratePatchesResponse, mutateFunc func(ctx context.Context, obj runtime.Object, + resp *runtimehooksv1.GeneratePatchesResponse, variablesType interface{}, mutateFunc func(ctx context.Context, obj runtime.Object, builtinVariable patchvariables.Builtins, variables interface{}, holderRef runtimehooksv1.HolderReference) error, opts ...WalkTemplatesOption) { log := ctrl.LoggerFrom(ctx) globalVariables := patchvariables.ToMap(req.Variables) @@ -126,9 +125,8 @@ func WalkTemplates(ctx context.Context, decoder runtime.Decoder, req *runtimehoo fmt.Println(err) } - // FIXME: must be generic. Try generics? - var variablesValue api.Variables - if err := json.Unmarshal(variableValuesJSONAll, &variablesValue); err != nil { + // FIXME: just using variablesType directly is probably not clean enough + if err := json.Unmarshal(variableValuesJSONAll, &variablesType); err != nil { fmt.Println(err) } @@ -170,7 +168,7 @@ func WalkTemplates(ctx context.Context, decoder runtime.Decoder, req *runtimehoo // Calls the mutateFunc. requestItemLog.V(4).Info("Generating patch for template") modified := original.DeepCopyObject() - if err := mutateFunc(requestItemCtx, modified, builtinVariableValue, variablesValue, requestItem.HolderReference); err != nil { + if err := mutateFunc(requestItemCtx, modified, builtinVariableValue, variablesType, requestItem.HolderReference); err != nil { resp.Status = runtimehooksv1.ResponseStatusFailure resp.Message = err.Error() return diff --git a/exp/runtime/topologymutation/walkler_test.go b/exp/runtime/topologymutation/walkler_test.go index 7644ef1ab4d3..1effb99c32f9 100644 --- a/exp/runtime/topologymutation/walkler_test.go +++ b/exp/runtime/topologymutation/walkler_test.go @@ -51,7 +51,7 @@ func Test_WalkTemplates(t *testing.T) { controlplanev1.GroupVersion, bootstrapv1.GroupVersion, ) - mutatingFunc := func(ctx context.Context, obj runtime.Object, variables map[string]apiextensionsv1.JSON, holderRef runtimehooksv1.HolderReference) error { + mutatingFunc := func(ctx context.Context, obj runtime.Object, builtinVariable variables.Builtins, variables interface{}, holderRef runtimehooksv1.HolderReference) error { switch obj := obj.(type) { case *controlplanev1.KubeadmControlPlaneTemplate: obj.Annotations = map[string]string{"a": "a"} @@ -222,7 +222,7 @@ func Test_WalkTemplates(t *testing.T) { response := &runtimehooksv1.GeneratePatchesResponse{} request := &runtimehooksv1.GeneratePatchesRequest{Variables: tt.globalVariables, Items: tt.requestItems} - WalkTemplates(context.Background(), decoder, request, response, mutatingFunc, tt.options...) + WalkTemplates(context.Background(), decoder, request, response, struct{}{}, mutatingFunc, tt.options...) g.Expect(response.Status).To(Equal(tt.expectedResponse.Status)) g.Expect(response.Message).To(Equal(tt.expectedResponse.Message)) diff --git a/test/extension/handlers/topologymutation/handler.go b/test/extension/handlers/topologymutation/handler.go index 889e44e4e9ef..0e8a79fcd867 100644 --- a/test/extension/handlers/topologymutation/handler.go +++ b/test/extension/handlers/topologymutation/handler.go @@ -99,10 +99,10 @@ func (h *ExtensionHandlers) GeneratePatches(ctx context.Context, req *runtimehoo // By using WalkTemplates it is possible to implement patches using typed API objects, which makes code // easier to read and less error prone than using unstructured or working with raw json/yaml. // IMPORTANT: by unit testing this func/nested func properly, it is possible to prevent unexpected rollouts when patches are modified. - topologymutation.WalkTemplates(ctx, h.decoder, req, resp, func(ctx context.Context, obj runtime.Object, builtinVariable patchvariables.Builtins, variables interface{}, holderRef runtimehooksv1.HolderReference) error { + topologymutation.WalkTemplates(ctx, h.decoder, req, resp, &api.Variables{}, func(ctx context.Context, obj runtime.Object, builtinVariable patchvariables.Builtins, variables interface{}, holderRef runtimehooksv1.HolderReference) error { log := ctrl.LoggerFrom(ctx) - vars, ok := variables.(api.Variables) + vars, ok := variables.(*api.Variables) if !ok { return errors.Errorf("wrong variable type") } @@ -145,7 +145,7 @@ func (h *ExtensionHandlers) GeneratePatches(ctx context.Context, req *runtimehoo // patchDockerClusterTemplate patches the DockerClusterTemplate. // It sets the LoadBalancer.ImageRepository if the lbImageRepository variable is provided. // NOTE: this patch is not required for any special reason, it is used for testing the patch machinery itself. -func patchDockerClusterTemplate(_ context.Context, dockerClusterTemplate *infrav1.DockerClusterTemplate, builtinVariable patchvariables.Builtins, variables api.Variables) error { +func patchDockerClusterTemplate(_ context.Context, dockerClusterTemplate *infrav1.DockerClusterTemplate, builtinVariable patchvariables.Builtins, variables *api.Variables) error { if variables.LBImageRepository != "" { dockerClusterTemplate.Spec.Template.Spec.LoadBalancer.ImageRepository = variables.LBImageRepository } @@ -157,7 +157,7 @@ func patchDockerClusterTemplate(_ context.Context, dockerClusterTemplate *infrav // to work with older kind images. // It also sets the RolloutStrategy.RollingUpdate.MaxSurge if the kubeadmControlPlaneMaxSurge is provided. // NOTE: RolloutStrategy.RollingUpdate.MaxSurge patch is not required for any special reason, it is used for testing the patch machinery itself. -func patchKubeadmControlPlaneTemplate(ctx context.Context, kcpTemplate *controlplanev1.KubeadmControlPlaneTemplate, builtinVariable patchvariables.Builtins, variables api.Variables) error { +func patchKubeadmControlPlaneTemplate(ctx context.Context, kcpTemplate *controlplanev1.KubeadmControlPlaneTemplate, builtinVariable patchvariables.Builtins, variables *api.Variables) error { log := ctrl.LoggerFrom(ctx) // 1) If the Kubernetes version from builtin.controlPlane.version is below 1.24.0 set "cgroup-driver": "cgroupfs" to @@ -216,7 +216,7 @@ func patchKubeadmControlPlaneTemplate(ctx context.Context, kcpTemplate *controlp // patchKubeadmConfigTemplate patches the ControlPlaneTemplate. // Only for the templates linked to the default-worker MachineDeployment class, It sets KubeletExtraArgs["cgroup-driver"] // to cgroupfs for Kubernetes < 1.24; this patch is required for tests to work with older kind images. -func patchKubeadmConfigTemplate(ctx context.Context, k *bootstrapv1.KubeadmConfigTemplate, builtinVariable patchvariables.Builtins, variables api.Variables) error { +func patchKubeadmConfigTemplate(ctx context.Context, k *bootstrapv1.KubeadmConfigTemplate, builtinVariable patchvariables.Builtins, variables *api.Variables) error { log := ctrl.LoggerFrom(ctx) // This is a required variable. Return an error if it's not found. @@ -259,7 +259,7 @@ func patchKubeadmConfigTemplate(ctx context.Context, k *bootstrapv1.KubeadmConfi // patchDockerMachineTemplate patches the DockerMachineTemplate. // It sets the CustomImage to an image for the version in use by the controlPlane or by the MachineDeployment // the DockerMachineTemplate belongs to. This patch is required to pick up the kind image with the required Kubernetes version. -func patchDockerMachineTemplate(ctx context.Context, dockerMachineTemplate *infrav1.DockerMachineTemplate, builtinVariable patchvariables.Builtins, variables api.Variables) error { +func patchDockerMachineTemplate(ctx context.Context, dockerMachineTemplate *infrav1.DockerMachineTemplate, builtinVariable patchvariables.Builtins, variables *api.Variables) error { log := ctrl.LoggerFrom(ctx) // If the DockerMachineTemplate belongs to the ControlPlane, set the images using the ControlPlane version.