Skip to content

Commit

Permalink
update
Browse files Browse the repository at this point in the history
  • Loading branch information
sbueringer committed Jan 27, 2023
1 parent 9520e6b commit aedaf8d
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 14 deletions.
10 changes: 4 additions & 6 deletions exp/runtime/topologymutation/walker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions exp/runtime/topologymutation/walkler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down Expand Up @@ -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))
Expand Down
12 changes: 6 additions & 6 deletions test/extension/handlers/topologymutation/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit aedaf8d

Please sign in to comment.