From eaca5834fa074a846d7feba9570da802235930ec Mon Sep 17 00:00:00 2001 From: Richard Chen Date: Thu, 26 Jan 2023 09:48:27 -0800 Subject: [PATCH] Remove NodeVersion field --- cloud/scope/managedcontrolplane.go | 24 +++++++++++++------ cloud/scope/managedmachinepool.go | 23 +++++++++++++----- .../services/container/clusters/reconcile.go | 6 ++--- .../services/container/nodepools/reconcile.go | 6 ++--- ...uster.x-k8s.io_gcpmanagedmachinepools.yaml | 5 ---- .../v1beta1/gcpmanagedmachinepool_types.go | 4 ---- exp/api/v1beta1/zz_generated.deepcopy.go | 5 ---- .../gcpmanagedmachinepool_controller.go | 1 + 8 files changed, 41 insertions(+), 33 deletions(-) diff --git a/cloud/scope/managedcontrolplane.go b/cloud/scope/managedcontrolplane.go index 0f6ba7d769..028c9b31dd 100644 --- a/cloud/scope/managedcontrolplane.go +++ b/cloud/scope/managedcontrolplane.go @@ -30,6 +30,7 @@ import ( "github.com/pkg/errors" infrav1exp "sigs.k8s.io/cluster-api-provider-gcp/exp/api/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + clusterv1exp "sigs.k8s.io/cluster-api/exp/api/v1beta1" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -125,7 +126,8 @@ type ManagedControlPlaneScope struct { credentialsClient *credentials.IamCredentialsClient credential *Credential - AllNodePools []infrav1exp.GCPManagedMachinePool + AllMachinePools []clusterv1exp.MachinePool + AllManagedMachinePools []infrav1exp.GCPManagedMachinePool } // PatchObject persists the managed control plane configuration and status. @@ -174,21 +176,29 @@ func (s *ManagedControlPlaneScope) GetCredential() *Credential { } // GetAllNodePools gets all node pools for the control plane. -func (s *ManagedControlPlaneScope) GetAllNodePools(ctx context.Context) ([]infrav1exp.GCPManagedMachinePool, error) { - if s.AllNodePools == nil { +func (s *ManagedControlPlaneScope) GetAllNodePools(ctx context.Context) ([]infrav1exp.GCPManagedMachinePool, []clusterv1exp.MachinePool, error) { + if s.AllManagedMachinePools == nil || len(s.AllManagedMachinePools) == 0 { opt1 := client.InNamespace(s.GCPManagedControlPlane.Namespace) opt2 := client.MatchingLabels(map[string]string{ clusterv1.ClusterLabelName: s.Cluster.Name, }) - machinePoolList := &infrav1exp.GCPManagedMachinePoolList{} + machinePoolList := &clusterv1exp.MachinePoolList{} if err := s.client.List(ctx, machinePoolList, opt1, opt2); err != nil { - return nil, err + return nil, nil, err } - s.AllNodePools = machinePoolList.Items + managedMachinePoolList := &infrav1exp.GCPManagedMachinePoolList{} + if err := s.client.List(ctx, managedMachinePoolList, opt1, opt2); err != nil { + return nil, nil, err + } + if len(machinePoolList.Items) != len(managedMachinePoolList.Items) { + return nil, nil, fmt.Errorf("machinePoolList length (%d) != managedMachinePoolList length (%d)", len(machinePoolList.Items), len(managedMachinePoolList.Items)) + } + s.AllMachinePools = machinePoolList.Items + s.AllManagedMachinePools = managedMachinePoolList.Items } - return s.AllNodePools, nil + return s.AllManagedMachinePools, s.AllMachinePools, nil } func parseLocation(location string) string { diff --git a/cloud/scope/managedmachinepool.go b/cloud/scope/managedmachinepool.go index e4221c29dd..3d64427b13 100644 --- a/cloud/scope/managedmachinepool.go +++ b/cloud/scope/managedmachinepool.go @@ -30,6 +30,7 @@ import ( "github.com/pkg/errors" infrav1exp "sigs.k8s.io/cluster-api-provider-gcp/exp/api/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + clusterv1exp "sigs.k8s.io/cluster-api/exp/api/v1beta1" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -40,6 +41,7 @@ type ManagedMachinePoolScopeParams struct { InstanceGroupManagersClient *compute.InstanceGroupManagersClient Client client.Client Cluster *clusterv1.Cluster + MachinePool *clusterv1exp.MachinePool GCPManagedCluster *infrav1exp.GCPManagedCluster GCPManagedControlPlane *infrav1exp.GCPManagedControlPlane GCPManagedMachinePool *infrav1exp.GCPManagedMachinePool @@ -51,6 +53,9 @@ func NewManagedMachinePoolScope(ctx context.Context, params ManagedMachinePoolSc if params.Cluster == nil { return nil, errors.New("failed to generate new scope from nil Cluster") } + if params.MachinePool == nil { + return nil, errors.New("failed to generate new scope from nil MachinePool") + } if params.GCPManagedCluster == nil { return nil, errors.New("failed to generate new scope from nil GCPManagedCluster") } @@ -111,6 +116,7 @@ type ManagedMachinePoolScope struct { patchHelper *patch.Helper Cluster *clusterv1.Cluster + MachinePool *clusterv1exp.MachinePool GCPManagedCluster *infrav1exp.GCPManagedCluster GCPManagedControlPlane *infrav1exp.GCPManagedControlPlane GCPManagedMachinePool *infrav1exp.GCPManagedMachinePool @@ -153,6 +159,11 @@ func (s *ManagedMachinePoolScope) InstanceGroupManagersClient() *compute.Instanc return s.migClient } +// NodePoolVersion returns the k8s version of the node pool. +func (s *ManagedMachinePoolScope) NodePoolVersion() *string { + return s.MachinePool.Spec.Template.Spec.Version +} + // ParseInstanceGroupURL parses an instance group URL. func ParseInstanceGroupURL(url string) (project string, zone string, mig string) { parts := strings.Split(url, "/") @@ -163,7 +174,7 @@ func ParseInstanceGroupURL(url string) (project string, zone string, mig string) } // ConvertToSdkNodePool convertsSetReplicas a node pool to format that is used by GCP SDK. -func ConvertToSdkNodePool(nodePool infrav1exp.GCPManagedMachinePool) *containerpb.NodePool { +func ConvertToSdkNodePool(nodePool infrav1exp.GCPManagedMachinePool, machinePool clusterv1exp.MachinePool) *containerpb.NodePool { nodePoolName := nodePool.Spec.NodePoolName if len(nodePoolName) == 0 { nodePoolName = nodePool.Name @@ -177,17 +188,17 @@ func ConvertToSdkNodePool(nodePool infrav1exp.GCPManagedMachinePool) *containerp Metadata: nodePool.Spec.AdditionalLabels, }, } - if nodePool.Spec.NodeVersion != nil { - sdkNodePool.Version = *nodePool.Spec.NodeVersion + if machinePool.Spec.Template.Spec.Version != nil { + sdkNodePool.Version = *machinePool.Spec.Template.Spec.Version } return &sdkNodePool } // ConvertToSdkNodePools converts node pools to format that is used by GCP SDK. -func ConvertToSdkNodePools(nodePools []infrav1exp.GCPManagedMachinePool) []*containerpb.NodePool { +func ConvertToSdkNodePools(nodePools []infrav1exp.GCPManagedMachinePool, machinePools []clusterv1exp.MachinePool) []*containerpb.NodePool { res := []*containerpb.NodePool{} - for _, nodePool := range nodePools { - res = append(res, ConvertToSdkNodePool(nodePool)) + for i := range nodePools { + res = append(res, ConvertToSdkNodePool(nodePools[i], machinePools[i])) } return res } diff --git a/cloud/services/container/clusters/reconcile.go b/cloud/services/container/clusters/reconcile.go index aaa42bc623..65d98158fb 100644 --- a/cloud/services/container/clusters/reconcile.go +++ b/cloud/services/container/clusters/reconcile.go @@ -49,7 +49,7 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) { log.Info("Cluster not found, creating") s.scope.GCPManagedControlPlane.Status.Ready = false - nodePools, err := s.scope.GetAllNodePools(ctx) + nodePools, _, err := s.scope.GetAllNodePools(ctx) if err != nil { conditions.MarkFalse(s.scope.ConditionSetter(), clusterv1.ReadyCondition, infrav1exp.GKEControlPlaneReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) conditions.MarkFalse(s.scope.ConditionSetter(), infrav1exp.GKEControlPlaneReadyCondition, infrav1exp.GKEControlPlaneReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) @@ -215,14 +215,14 @@ func (s *Service) describeCluster(ctx context.Context) (*containerpb.Cluster, er func (s *Service) createCluster(ctx context.Context) error { log := log.FromContext(ctx) - nodePools, _ := s.scope.GetAllNodePools(ctx) + nodePools, machinePools, _ := s.scope.GetAllNodePools(ctx) cluster := &containerpb.Cluster{ Name: s.scope.GCPManagedControlPlane.Spec.ClusterName, Network: *s.scope.GCPManagedCluster.Spec.Network.Name, Autopilot: &containerpb.Autopilot{ Enabled: false, }, - NodePools: scope.ConvertToSdkNodePools(nodePools), + NodePools: scope.ConvertToSdkNodePools(nodePools, machinePools), ReleaseChannel: &containerpb.ReleaseChannel{ Channel: convertToSdkReleaseChannel(s.scope.GCPManagedControlPlane.Spec.ReleaseChannel), }, diff --git a/cloud/services/container/nodepools/reconcile.go b/cloud/services/container/nodepools/reconcile.go index a22fe3a206..e174b7b0d0 100644 --- a/cloud/services/container/nodepools/reconcile.go +++ b/cloud/services/container/nodepools/reconcile.go @@ -246,7 +246,7 @@ func (s *Service) getInstances(ctx context.Context, nodePool *containerpb.NodePo func (s *Service) createNodePool(ctx context.Context) error { createNodePoolRequest := &containerpb.CreateNodePoolRequest{ - NodePool: scope.ConvertToSdkNodePool(*s.scope.GCPManagedMachinePool), + NodePool: scope.ConvertToSdkNodePool(*s.scope.GCPManagedMachinePool, *s.scope.MachinePool), Parent: s.scope.NodePoolLocation(), } _, err := s.scope.ManagedMachinePoolClient().CreateNodePool(ctx, createNodePoolRequest) @@ -293,9 +293,9 @@ func (s *Service) checkDiffAndPrepareUpdateVersionOrImage(existingNodePool *cont Name: s.scope.NodePoolFullName(), } // Node version - if s.scope.GCPManagedMachinePool.Spec.NodeVersion != nil && *s.scope.GCPManagedMachinePool.Spec.NodeVersion != existingNodePool.Version { + if s.scope.NodePoolVersion() != nil && *s.scope.NodePoolVersion() != existingNodePool.Version { needUpdate = true - updateNodePoolRequest.NodeVersion = *s.scope.GCPManagedMachinePool.Spec.NodeVersion + updateNodePoolRequest.NodeVersion = *s.scope.NodePoolVersion() } // Kubernetes labels if !reflect.DeepEqual(map[string]string(s.scope.GCPManagedMachinePool.Spec.KubernetesLabels), existingNodePool.Config.Labels) { diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedmachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedmachinepools.yaml index feec695ba5..a3a4747031 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedmachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedmachinepools.yaml @@ -94,11 +94,6 @@ spec: a default name will be created based on the namespace and name of the managed machine pool. type: string - nodeVersion: - description: NodeVersion represents the node version of the node pool. - If not specified, the GKE cluster control plane version will be - used. - type: string providerIDList: description: ProviderIDList are the provider IDs of instances in the managed instance group corresponding to the nodegroup represented diff --git a/exp/api/v1beta1/gcpmanagedmachinepool_types.go b/exp/api/v1beta1/gcpmanagedmachinepool_types.go index ada9ee06cf..ce7b5c2a4f 100644 --- a/exp/api/v1beta1/gcpmanagedmachinepool_types.go +++ b/exp/api/v1beta1/gcpmanagedmachinepool_types.go @@ -34,10 +34,6 @@ type GCPManagedMachinePoolSpec struct { // then a default name will be created based on the namespace and name of the managed machine pool. // +optional NodePoolName string `json:"nodePoolName,omitempty"` - // NodeVersion represents the node version of the node pool. - // If not specified, the GKE cluster control plane version will be used. - // +optional - NodeVersion *string `json:"nodeVersion,omitempty"` // NodeCount represents the initial number of nodes for the pool. // In regional or multi-zonal clusters, this is the number of nodes per zone. NodeCount int32 `json:"nodeCount"` diff --git a/exp/api/v1beta1/zz_generated.deepcopy.go b/exp/api/v1beta1/zz_generated.deepcopy.go index cef156b106..502f060b3f 100644 --- a/exp/api/v1beta1/zz_generated.deepcopy.go +++ b/exp/api/v1beta1/zz_generated.deepcopy.go @@ -314,11 +314,6 @@ func (in *GCPManagedMachinePoolList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GCPManagedMachinePoolSpec) DeepCopyInto(out *GCPManagedMachinePoolSpec) { *out = *in - if in.NodeVersion != nil { - in, out := &in.NodeVersion, &out.NodeVersion - *out = new(string) - **out = **in - } if in.KubernetesLabels != nil { in, out := &in.KubernetesLabels, &out.KubernetesLabels *out = make(apiv1beta1.Labels, len(*in)) diff --git a/exp/controllers/gcpmanagedmachinepool_controller.go b/exp/controllers/gcpmanagedmachinepool_controller.go index 704fc0383e..262ce5d96d 100644 --- a/exp/controllers/gcpmanagedmachinepool_controller.go +++ b/exp/controllers/gcpmanagedmachinepool_controller.go @@ -289,6 +289,7 @@ func (r *GCPManagedMachinePoolReconciler) Reconcile(ctx context.Context, req ctr managedMachinePoolScope, err := scope.NewManagedMachinePoolScope(ctx, scope.ManagedMachinePoolScopeParams{ Client: r.Client, Cluster: cluster, + MachinePool: machinePool, GCPManagedCluster: gcpManagedCluster, GCPManagedControlPlane: gcpManagedControlPlane, GCPManagedMachinePool: gcpManagedMachinePool,