Skip to content

Commit

Permalink
Fix file that use GetVariable function
Browse files Browse the repository at this point in the history
  • Loading branch information
@lubronzhan
lubronzhan committed Nov 7, 2023
1 parent 34970f9 commit 1520a9c
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 69 deletions.
2 changes: 1 addition & 1 deletion exp/runtime/topologymutation/variables.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
patchvariables "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/variables"
)

// TODO: Add func for validating received variables are of the expected types
// TODO: Add func for validating received variables are of the expected types.

// GetVariable get the variable value.
func GetVariable(templateVariables map[string]apiextensionsv1.JSON, variableName string) (*apiextensionsv1.JSON, error) {
Expand Down
145 changes: 77 additions & 68 deletions test/extension/handlers/topologymutation/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
"sigs.k8s.io/cluster-api/exp/runtime/topologymutation"
patchvariables "sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/patches/variables"
infrav1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/api/v1beta1"
infraexpv1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/test/infrastructure/kind"
Expand Down Expand Up @@ -129,13 +130,16 @@ func (h *ExtensionHandlers) GeneratePatches(ctx context.Context, req *runtimehoo
// It sets the LoadBalancer.ImageRepository if the imageRepository 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, templateVariables map[string]apiextensionsv1.JSON) error {
imageRepo, found, err := topologymutation.GetStringVariable(templateVariables, "imageRepository")
imageRepo, err := topologymutation.GetStringVariable(templateVariables, "imageRepository")
if err != nil {
if patchvariables.IsNotFoundError(err) {
return nil
}
return errors.Wrap(err, "could not set DockerClusterTemplate loadBalancer imageRepository")
}
if found {
dockerClusterTemplate.Spec.Template.Spec.LoadBalancer.ImageRepository = imageRepo
}

dockerClusterTemplate.Spec.Template.Spec.LoadBalancer.ImageRepository = imageRepo

return nil
}

Expand All @@ -152,15 +156,15 @@ func patchKubeadmControlPlaneTemplate(ctx context.Context, kcpTemplate *controlp
// 1) If the Kubernetes version from builtin.controlPlane.version is below 1.24.0 set "cgroup-driver": "cgroupfs" to
// - kubeadmConfigSpec.InitConfiguration.NodeRegistration.KubeletExtraArgs
// - kubeadmConfigSpec.JoinConfiguration.NodeRegistration.KubeletExtraArgs
cpVersion, found, err := topologymutation.GetStringVariable(templateVariables, "builtin.controlPlane.version")
cpVersion, err := topologymutation.GetStringVariable(templateVariables, "builtin.controlPlane.version")
if err != nil {
// This is a required variable. Return an error if it's not found.
// NOTE: this should never happen because it is enforced by the patch engine.
if patchvariables.IsNotFoundError(err) {
return errors.New("could not set cgroup-driver to control plane template kubeletExtraArgs: variable \"builtin.controlPlane.version\" not found")
}
return errors.Wrap(err, "could not set cgroup-driver to control plane template kubeletExtraArgs")
}
// This is a required variable. Return an error if it's not found.
// NOTE: this should never happen because it is enforced by the patch engine.
if !found {
return errors.New("could not set cgroup-driver to control plane template kubeletExtraArgs: variable \"builtin.controlPlane.version\" not found")
}

controlPlaneVersion, err := version.ParseMajorMinorPatchTolerant(cpVersion)
if err != nil {
Expand Down Expand Up @@ -189,22 +193,24 @@ func patchKubeadmControlPlaneTemplate(ctx context.Context, kcpTemplate *controlp

// 2) Patch RolloutStrategy RollingUpdate MaxSurge with the value from the Cluster Topology variable.
// If this is unset continue as this variable is not required.
kcpControlPlaneMaxSurge, found, err := topologymutation.GetStringVariable(templateVariables, "kubeadmControlPlaneMaxSurge")
kcpControlPlaneMaxSurge, err := topologymutation.GetStringVariable(templateVariables, "kubeadmControlPlaneMaxSurge")
if err != nil {
if patchvariables.IsNotFoundError(err) {
return nil
}
return errors.Wrap(err, "could not set KubeadmControlPlaneTemplate MaxSurge")
}
if found {
// This has to be converted to IntOrString type.
kubeadmControlPlaneMaxSurgeIntOrString := intstrutil.Parse(kcpControlPlaneMaxSurge)
log.Info(fmt.Sprintf("Setting KubeadmControlPlaneMaxSurge to %q", kubeadmControlPlaneMaxSurgeIntOrString.String()))
if kcpTemplate.Spec.Template.Spec.RolloutStrategy == nil {
kcpTemplate.Spec.Template.Spec.RolloutStrategy = &controlplanev1.RolloutStrategy{}
}
if kcpTemplate.Spec.Template.Spec.RolloutStrategy.RollingUpdate == nil {
kcpTemplate.Spec.Template.Spec.RolloutStrategy.RollingUpdate = &controlplanev1.RollingUpdate{}
}
kcpTemplate.Spec.Template.Spec.RolloutStrategy.RollingUpdate.MaxSurge = &kubeadmControlPlaneMaxSurgeIntOrString

// This has to be converted to IntOrString type.
kubeadmControlPlaneMaxSurgeIntOrString := intstrutil.Parse(kcpControlPlaneMaxSurge)
log.Info(fmt.Sprintf("Setting KubeadmControlPlaneMaxSurge to %q", kubeadmControlPlaneMaxSurgeIntOrString.String()))
if kcpTemplate.Spec.Template.Spec.RolloutStrategy == nil {
kcpTemplate.Spec.Template.Spec.RolloutStrategy = &controlplanev1.RolloutStrategy{}
}
if kcpTemplate.Spec.Template.Spec.RolloutStrategy.RollingUpdate == nil {
kcpTemplate.Spec.Template.Spec.RolloutStrategy.RollingUpdate = &controlplanev1.RollingUpdate{}
}
kcpTemplate.Spec.Template.Spec.RolloutStrategy.RollingUpdate.MaxSurge = &kubeadmControlPlaneMaxSurgeIntOrString
return nil
}

Expand All @@ -218,19 +224,19 @@ func patchKubeadmConfigTemplate(ctx context.Context, k *bootstrapv1.KubeadmConfi

// Only patch the customImage if this DockerMachineTemplate belongs to a MachineDeployment or MachinePool with class "default-class"
// NOTE: This works by checking the existence of a builtin variable that exists only for templates linked to MachineDeployments.
mdClass, mdFound, err := topologymutation.GetStringVariable(templateVariables, "builtin.machineDeployment.class")
if err != nil {
return errors.Wrap(err, "could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs")
mdClass, err1 := topologymutation.GetStringVariable(templateVariables, "builtin.machineDeployment.class")
if err1 != nil && !patchvariables.IsNotFoundError(err1) {
return errors.Wrap(err1, "could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs")
}

mpClass, mpFound, err := topologymutation.GetStringVariable(templateVariables, "builtin.machinePool.class")
if err != nil {
return errors.Wrap(err, "could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs")
mpClass, err2 := topologymutation.GetStringVariable(templateVariables, "builtin.machinePool.class")
if err2 != nil && !patchvariables.IsNotFoundError(err2) {
return errors.Wrap(err2, "could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs")
}

// This is a required variable. Return an error if it's not found.
// NOTE: this should never happen because it is enforced by the patch engine.
if !mdFound && !mpFound {
if patchvariables.IsNotFoundError(err1) && patchvariables.IsNotFoundError(err2) {
return errors.New("could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs: could find neither \"builtin.machineDeployment.class\" nor \"builtin.machinePool.class\" variable")
}

Expand All @@ -240,15 +246,15 @@ func patchKubeadmConfigTemplate(ctx context.Context, k *bootstrapv1.KubeadmConfi
// - JoinConfiguration.KubeletExtraArgs
// NOTE: MachineDeployment version might be different than Cluster.version or other MachineDeployment's versions;
// the builtin variables provides the right version to use.
mdVersion, found, err := topologymutation.GetStringVariable(templateVariables, "builtin.machineDeployment.version")
mdVersion, err := topologymutation.GetStringVariable(templateVariables, "builtin.machineDeployment.version")
if err != nil {
// This is a required variable. Return an error if it's not found.
if patchvariables.IsNotFoundError(err) {
return errors.New("could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs: variable \"builtin.machineDeployment.version\" not found")
}
return errors.Wrap(err, "could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs")
}

// This is a required variable. Return an error if it's not found.
if !found {
return errors.New("could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs: variable \"builtin.machineDeployment.version\" not found")
}
machineDeploymentVersion, err := version.ParseMajorMinorPatchTolerant(mdVersion)
if err != nil {
return errors.Wrap(err, "could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs")
Expand All @@ -274,15 +280,15 @@ func patchKubeadmConfigTemplate(ctx context.Context, k *bootstrapv1.KubeadmConfi
// - JoinConfiguration.KubeletExtraArgs
// NOTE: MachinePool version might be different than Cluster.version or other MachinePool's versions;
// the builtin variables provides the right version to use.
mpVersion, found, err := topologymutation.GetStringVariable(templateVariables, "builtin.machinePool.version")
mpVersion, err := topologymutation.GetStringVariable(templateVariables, "builtin.machinePool.version")
if err != nil {
// This is a required variable. Return an error if it's not found.
if patchvariables.IsNotFoundError(err) {
return errors.New("could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs: variable \"builtin.machinePool.version\" not found")
}
return errors.Wrap(err, "could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs")
}

// This is a required variable. Return an error if it's not found.
if !found {
return errors.New("could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs: variable \"builtin.machinePool.version\" not found")
}
machinePoolVersion, err := version.ParseMajorMinorPatchTolerant(mpVersion)
if err != nil {
return errors.Wrap(err, "could not set cgroup-driver to KubeadmConfigTemplate template kubeletExtraArgs")
Expand Down Expand Up @@ -317,11 +323,13 @@ func patchDockerMachineTemplate(ctx context.Context, dockerMachineTemplate *infr
// NOTE: ControlPlane version might be different than Cluster.version or MachineDeployment's versions;
// the builtin variables provides the right version to use.
// NOTE: This works by checking the existence of a builtin variable that exists only for templates linked to the ControlPlane.
cpVersion, found, err := topologymutation.GetStringVariable(templateVariables, "builtin.controlPlane.version")
if err != nil {
cpVersion, err := topologymutation.GetStringVariable(templateVariables, "builtin.controlPlane.version")
if err != nil && !patchvariables.IsNotFoundError(err) {
return errors.Wrap(err, "could not set customImage to control plane dockerMachineTemplate")
}
if found {

// if found
if err == nil {
semVer, err := version.ParseMajorMinorPatchTolerant(cpVersion)
if err != nil {
return errors.Wrap(err, "could not parse control plane version")
Expand All @@ -338,25 +346,26 @@ func patchDockerMachineTemplate(ctx context.Context, dockerMachineTemplate *infr
// NOTE: MachineDeployment version might be different from Cluster.version or other MachineDeployment's versions;
// the builtin variables provides the right version to use.
// NOTE: This works by checking the existence of a builtin variable that exists only for templates linked to MachineDeployments.
mdVersion, found, err := topologymutation.GetStringVariable(templateVariables, "builtin.machineDeployment.version")
mdVersion, err := topologymutation.GetStringVariable(templateVariables, "builtin.machineDeployment.version")
if err != nil {
if patchvariables.IsNotFoundError(err) {
// If the DockerMachineTemplate didn't have variables for either a control plane or a machineDeployment return an error.
// NOTE: this should never happen because it is enforced by the patch engine.
return errors.New("no version variables found for DockerMachineTemplate patch")
}
return errors.Wrap(err, "could not set customImage to MachineDeployment DockerMachineTemplate")
}
if found {
semVer, err := version.ParseMajorMinorPatchTolerant(mdVersion)
if err != nil {
return errors.Wrap(err, "could not parse MachineDeployment version")
}
kindMapping := kind.GetMapping(semVer, "")

log.Info(fmt.Sprintf("Setting MachineDeployment customImage to %q", kindMapping.Image))
dockerMachineTemplate.Spec.Template.Spec.CustomImage = kindMapping.Image
return nil
semVer, err := version.ParseMajorMinorPatchTolerant(mdVersion)
if err != nil {
return errors.Wrap(err, "could not parse MachineDeployment version")
}
kindMapping := kind.GetMapping(semVer, "")

log.Info(fmt.Sprintf("Setting MachineDeployment customImage to %q", kindMapping.Image))
dockerMachineTemplate.Spec.Template.Spec.CustomImage = kindMapping.Image
return nil

// If the DockerMachineTemplate didn't have variables for either a control plane or a machineDeployment return an error.
// NOTE: this should never happen because it is enforced by the patch engine.
return errors.New("no version variables found for DockerMachineTemplate patch")
}

// patchDockerMachinePoolTemplate patches the DockerMachinePoolTemplate.
Expand All @@ -370,25 +379,25 @@ func patchDockerMachinePoolTemplate(ctx context.Context, dockerMachinePoolTempla
// NOTE: MachinePool version might be different from Cluster.version or other MachinePool's versions;
// the builtin variables provides the right version to use.
// NOTE: This works by checking the existence of a builtin variable that exists only for templates linked to MachinePools.
mpVersion, found, err := topologymutation.GetStringVariable(templateVariables, "builtin.machinePool.version")
mpVersion, err := topologymutation.GetStringVariable(templateVariables, "builtin.machinePool.version")
if err != nil {
// If the DockerMachinePoolTemplate didn't have variables for a machinePool return an error.
// NOTE: this should never happen because it is enforced by the patch engine.
if patchvariables.IsNotFoundError(err) {
return errors.New("no version variables found for DockerMachinePoolTemplate patch")
}
return errors.Wrap(err, "could not set customImage to MachinePool DockerMachinePoolTemplate")
}
if found {
semVer, err := version.ParseMajorMinorPatchTolerant(mpVersion)
if err != nil {
return errors.Wrap(err, "could not parse MachinePool version")
}
kindMapping := kind.GetMapping(semVer, "")

log.Info(fmt.Sprintf("Setting MachinePool customImage to %q", kindMapping.Image))
dockerMachinePoolTemplate.Spec.Template.Spec.Template.CustomImage = kindMapping.Image
return nil
semVer, err := version.ParseMajorMinorPatchTolerant(mpVersion)
if err != nil {
return errors.Wrap(err, "could not parse MachinePool version")
}
kindMapping := kind.GetMapping(semVer, "")

// If the DockerMachinePoolTemplate didn't have variables for a machinePool return an error.
// NOTE: this should never happen because it is enforced by the patch engine.
return errors.New("no version variables found for DockerMachinePoolTemplate patch")
log.Info(fmt.Sprintf("Setting MachinePool customImage to %q", kindMapping.Image))
dockerMachinePoolTemplate.Spec.Template.Spec.Template.CustomImage = kindMapping.Image
return nil
}

// ValidateTopology implements the HandlerFunc for the ValidateTopology hook.
Expand Down

0 comments on commit 1520a9c

Please sign in to comment.