From 123a5aa079ad09a43ff581dd9264313c3b6c51a1 Mon Sep 17 00:00:00 2001 From: willie-yao Date: Fri, 11 Aug 2023 00:12:50 +0000 Subject: [PATCH] Resolve nits --- internal/controllers/topology/cluster/blueprint.go | 12 ++++++------ .../controllers/topology/cluster/current_state.go | 8 ++++---- .../controllers/topology/cluster/desired_state.go | 4 ++-- .../controllers/topology/cluster/patches/engine.go | 4 ++-- .../controllers/topology/cluster/reconcile_state.go | 6 ++---- .../controllers/topology/cluster/scope/blueprint.go | 4 ++-- internal/webhooks/clusterclass.go | 2 +- test/extension/handlers/topologymutation/handler.go | 2 +- .../api/v1beta1/dockermachinepooltemplate_types.go | 2 +- 9 files changed, 21 insertions(+), 23 deletions(-) diff --git a/internal/controllers/topology/cluster/blueprint.go b/internal/controllers/topology/cluster/blueprint.go index e853d418aaae..7e74a9bdfe53 100644 --- a/internal/controllers/topology/cluster/blueprint.go +++ b/internal/controllers/topology/cluster/blueprint.go @@ -83,7 +83,7 @@ func (r *Reconciler) getBlueprint(ctx context.Context, cluster *clusterv1.Cluste // Get the bootstrap machine template. machineDeploymentBlueprint.BootstrapTemplate, err = r.getReference(ctx, machineDeploymentClass.Template.Bootstrap.Ref) if err != nil { - return nil, errors.Wrapf(err, "failed to get bootstrap machine template for %s, MachineDeployment class %q", tlog.KObj{Obj: blueprint.ClusterClass}, machineDeploymentClass.Class) + return nil, errors.Wrapf(err, "failed to get bootstrap config template for %s, MachineDeployment class %q", tlog.KObj{Obj: blueprint.ClusterClass}, machineDeploymentClass.Class) } // If the machineDeploymentClass defines a MachineHealthCheck add it to the blueprint. @@ -103,16 +103,16 @@ func (r *Reconciler) getBlueprint(ctx context.Context, cluster *clusterv1.Cluste // for the MachinePool that is created or updated. machinePoolClass.Template.Metadata.DeepCopyInto(&machinePoolBlueprint.Metadata) - // Get the infrastructure machine template. + // Get the InfrastructureMachinePoolTemplate. machinePoolBlueprint.InfrastructureMachinePoolTemplate, err = r.getReference(ctx, machinePoolClass.Template.Infrastructure.Ref) if err != nil { - return nil, errors.Wrapf(err, "failed to get infrastructure machine template for %s, MachinePool class %q", tlog.KObj{Obj: blueprint.ClusterClass}, machinePoolClass.Class) + return nil, errors.Wrapf(err, "failed to get InfrastructureMachinePoolTemplate for %s, MachinePool class %q", tlog.KObj{Obj: blueprint.ClusterClass}, machinePoolClass.Class) } - // Get the bootstrap machine template. - machinePoolBlueprint.BootstrapTemplate, err = r.getReference(ctx, machinePoolClass.Template.Bootstrap.Ref) + // Get the bootstrap config. + machinePoolBlueprint.BootstrapConfig, err = r.getReference(ctx, machinePoolClass.Template.Bootstrap.Ref) if err != nil { - return nil, errors.Wrapf(err, "failed to get bootstrap machine template for %s, MachinePool class %q", tlog.KObj{Obj: blueprint.ClusterClass}, machinePoolClass.Class) + return nil, errors.Wrapf(err, "failed to get bootstrap config for %s, MachinePool class %q", tlog.KObj{Obj: blueprint.ClusterClass}, machinePoolClass.Class) } blueprint.MachinePools[machinePoolClass.Class] = machinePoolBlueprint diff --git a/internal/controllers/topology/cluster/current_state.go b/internal/controllers/topology/cluster/current_state.go index 8a35a5c6f4ad..e52ea3d8355b 100644 --- a/internal/controllers/topology/cluster/current_state.go +++ b/internal/controllers/topology/cluster/current_state.go @@ -340,7 +340,7 @@ func (r *Reconciler) getCurrentMachinePoolState(ctx context.Context, blueprintMa if !ok { return nil, fmt.Errorf("failed to find MachinePool class %s in ClusterClass", mpClassName) } - bootstrapRef, err = alignRefAPIVersion(mpBluePrint.BootstrapTemplate, bootstrapRef) + bootstrapRef, err = alignRefAPIVersion(mpBluePrint.BootstrapConfig, bootstrapRef) if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("%s Bootstrap reference could not be retrieved", tlog.KObj{Obj: m})) } @@ -430,9 +430,9 @@ func getMPClassName(cluster *clusterv1.Cluster, mpTopologyName string) (bool, st return false, "" } - for _, mdTopology := range cluster.Spec.Topology.Workers.MachineDeployments { - if mdTopology.Name == mpTopologyName { - return true, mdTopology.Class + for _, mpTopology := range cluster.Spec.Topology.Workers.MachinePools { + if mpTopology.Name == mpTopologyName { + return true, mpTopology.Class } } return false, "" diff --git a/internal/controllers/topology/cluster/desired_state.go b/internal/controllers/topology/cluster/desired_state.go index 84100f2fa79a..13865e89d1e8 100644 --- a/internal/controllers/topology/cluster/desired_state.go +++ b/internal/controllers/topology/cluster/desired_state.go @@ -926,8 +926,8 @@ func computeMachinePool(_ context.Context, s *scope.Scope, machinePoolTopology c } var err error desiredMachinePool.BootstrapObject, err = templateToObject(templateToInput{ - template: machinePoolBlueprint.BootstrapTemplate, - templateClonedFromRef: contract.ObjToRef(machinePoolBlueprint.BootstrapTemplate), + template: machinePoolBlueprint.BootstrapConfig, + templateClonedFromRef: contract.ObjToRef(machinePoolBlueprint.BootstrapConfig), cluster: s.Current.Cluster, namePrefix: bootstrapTemplateNamePrefix(s.Current.Cluster.Name, machinePoolTopology.Name), currentObjectRef: currentBootstrapTemplateRef, diff --git a/internal/controllers/topology/cluster/patches/engine.go b/internal/controllers/topology/cluster/patches/engine.go index 54bfe9cb0b55..a15649a13b96 100644 --- a/internal/controllers/topology/cluster/patches/engine.go +++ b/internal/controllers/topology/cluster/patches/engine.go @@ -354,12 +354,12 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat } // Add the BootstrapTemplate. - t, err := newRequestItemBuilder(mpClass.BootstrapTemplate). + t, err := newRequestItemBuilder(mpClass.BootstrapConfig). WithHolder(mp.Object, "spec.template.spec.bootstrap.configRef"). Build() if err != nil { return nil, errors.Wrapf(err, "failed to prepare BootstrapConfig template %s for MachinePool topology %s for patching", - tlog.KObj{Obj: mpClass.BootstrapTemplate}, mpTopologyName) + tlog.KObj{Obj: mpClass.BootstrapConfig}, mpTopologyName) } req.Items = append(req.Items, *t) diff --git a/internal/controllers/topology/cluster/reconcile_state.go b/internal/controllers/topology/cluster/reconcile_state.go index 9cfedb3ebd78..596ae6a8c023 100644 --- a/internal/controllers/topology/cluster/reconcile_state.go +++ b/internal/controllers/topology/cluster/reconcile_state.go @@ -94,10 +94,8 @@ func (r *Reconciler) reconcileState(ctx context.Context, s *scope.Scope) error { return err } - // Reconcile desired state of the MachinePool objects. - err := r.reconcileMachinePools(ctx, s) - - return err + // Reconcile desired state of the MachinePool object and return. + return r.reconcileMachinePools(ctx, s) } // Reconcile the Cluster shim, a temporary object used a mean to collect objects/templates diff --git a/internal/controllers/topology/cluster/scope/blueprint.go b/internal/controllers/topology/cluster/scope/blueprint.go index 1f93ba84258e..14d206828134 100644 --- a/internal/controllers/topology/cluster/scope/blueprint.go +++ b/internal/controllers/topology/cluster/scope/blueprint.go @@ -84,8 +84,8 @@ type MachinePoolBlueprint struct { // NOTE: This is a convenience copy of the metadata field from Cluster.Spec.Topology.Workers.MachinePools[x]. Metadata clusterv1.ObjectMeta - // BootstrapTemplate holds the bootstrap template for a MachinePool referenced from ClusterClass. - BootstrapTemplate *unstructured.Unstructured + // BootstrapConfig holds the bootstrap template for a MachinePool referenced from ClusterClass. + BootstrapConfig *unstructured.Unstructured // InfrastructureMachinePoolTemplate holds the infrastructure machine template for a MachinePool referenced from ClusterClass. InfrastructureMachinePoolTemplate *unstructured.Unstructured diff --git a/internal/webhooks/clusterclass.go b/internal/webhooks/clusterclass.go index b989af85489d..4466fda390ae 100644 --- a/internal/webhooks/clusterclass.go +++ b/internal/webhooks/clusterclass.go @@ -302,7 +302,7 @@ func (webhook *ClusterClass) validateRemovedMachinePoolClassesAreNotUsed(cluster for _, c := range clusters { for _, machinePoolTopology := range c.Spec.Topology.Workers.MachinePools { if removedClasses.Has(machinePoolTopology.Class) { - // TODO: Same as above for MachineDeployments + // TODO(killianmuldoon): Same as above for MachineDeployments allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "workers", "machinePools"), fmt.Sprintf("MachinePoolClass %q cannot be deleted because it is used by Cluster %q", machinePoolTopology.Class, c.Name), diff --git a/test/extension/handlers/topologymutation/handler.go b/test/extension/handlers/topologymutation/handler.go index 60fc0c105c24..52aebead0bf4 100644 --- a/test/extension/handlers/topologymutation/handler.go +++ b/test/extension/handlers/topologymutation/handler.go @@ -216,7 +216,7 @@ func patchKubeadmControlPlaneTemplate(ctx context.Context, kcpTemplate *controlp func patchKubeadmConfigTemplate(ctx context.Context, k *bootstrapv1.KubeadmConfigTemplate, templateVariables map[string]apiextensionsv1.JSON) error { log := ctrl.LoggerFrom(ctx) - // Only patch the customImage if this DockerMachineTemplate belongs to a MachineDeployment with class "default-class" + // 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 liked to MachineDeployments. mdClass, mdFound, err := topologymutation.GetStringVariable(templateVariables, "builtin.machineDeployment.class") if err != nil { diff --git a/test/infrastructure/docker/exp/api/v1beta1/dockermachinepooltemplate_types.go b/test/infrastructure/docker/exp/api/v1beta1/dockermachinepooltemplate_types.go index 70f82071beb7..a502767443fb 100644 --- a/test/infrastructure/docker/exp/api/v1beta1/dockermachinepooltemplate_types.go +++ b/test/infrastructure/docker/exp/api/v1beta1/dockermachinepooltemplate_types.go @@ -59,6 +59,6 @@ type DockerMachinePoolTemplateResource struct { // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata // +optional ObjectMeta clusterv1.ObjectMeta `json:"metadata,omitempty"` - // Spec is the specification of the desired behavior of the machine. + Spec DockerMachinePoolSpec `json:"spec"` }