Merge pull request #1434 from salasberryfin/managed-control-plane-version-api-change

k8s-ci-robot · web-flow · commit 2f1320cc585e · 2025-03-05T06:25:46.000-08:00
feat: align GCPManagedControlPlane with CAPI &gt;=v1.9
diff --git a/.golangci.yml b/.golangci.yml
@@ -196,6 +196,11 @@ issues:
     - linters:
         - gocritic
       text: "appendAssign: append result not assigned to the same slice"
+    # Specific exclude rules for deprecated fields that are still part of the codebase.
+    # These should be removed as the referenced deprecated item is removed from the project.
+    - linters:
+        - staticcheck
+      text: "SA1019: (s.GCPManagedControlPlane.Status.CurrentVersion|s.scope.GCPManagedControlPlane.Status.CurrentVersion|spec.ControlPlaneVersion|s.GCPManagedControlPlane.Spec.ControlPlaneVersion|s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion) is deprecated: This field will soon be removed and you are expected to use Version instead."
 
 run:
   timeout: 10m
diff --git a/cloud/scope/managedcontrolplane.go b/cloud/scope/managedcontrolplane.go
@@ -237,3 +237,14 @@ func (s *ManagedControlPlaneScope) SetEndpoint(host string) {
 func (s *ManagedControlPlaneScope) IsAutopilotCluster() bool {
 	return s.GCPManagedControlPlane.Spec.EnableAutopilot
 }
+
+// GetControlPlaneVersion returns the control plane version from the specification.
+func (s *ManagedControlPlaneScope) GetControlPlaneVersion() *string {
+	if s.GCPManagedControlPlane.Spec.Version != nil {
+		return s.GCPManagedControlPlane.Spec.Version
+	}
+	if s.GCPManagedControlPlane.Spec.ControlPlaneVersion != nil {
+		return s.GCPManagedControlPlane.Spec.ControlPlaneVersion
+	}
+	return nil
+}
diff --git a/cloud/services/container/clusters/reconcile.go b/cloud/services/container/clusters/reconcile.go
@@ -96,7 +96,9 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {
 	}
 
 	log.V(2).Info("gke cluster found", "status", cluster.GetStatus())
-	s.scope.GCPManagedControlPlane.Status.CurrentVersion = convertToSdkMasterVersion(cluster.GetCurrentMasterVersion())
+	controlPlaneVersion := convertToSdkMasterVersion(cluster.GetCurrentMasterVersion())
+	s.scope.GCPManagedControlPlane.Status.CurrentVersion = controlPlaneVersion
+	s.scope.GCPManagedControlPlane.Status.Version = &controlPlaneVersion
 
 	switch cluster.GetStatus() {
 	case containerpb.Cluster_PROVISIONING:
@@ -271,8 +273,8 @@ func (s *Service) createCluster(ctx context.Context, log *logr.Logger) error {
 			},
 		},
 	}
-	if s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion != nil {
-		cluster.InitialClusterVersion = convertToSdkMasterVersion(*s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion)
+	if initialClusterVersionFromSpec := s.scope.GetControlPlaneVersion(); initialClusterVersionFromSpec != nil {
+		cluster.InitialClusterVersion = convertToSdkMasterVersion(*initialClusterVersionFromSpec)
 	}
 	if s.scope.GCPManagedControlPlane.Spec.ClusterNetwork != nil {
 		cn := s.scope.GCPManagedControlPlane.Spec.ClusterNetwork
@@ -434,8 +436,8 @@ func (s *Service) checkDiffAndPrepareUpdate(existingCluster *containerpb.Cluster
 		}
 	}
 	// Master version
-	if s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion != nil {
-		desiredMasterVersion := convertToSdkMasterVersion(*s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion)
+	if desiredMasterVersionFromSpec := s.scope.GetControlPlaneVersion(); desiredMasterVersionFromSpec != nil {
+		desiredMasterVersion := convertToSdkMasterVersion(*desiredMasterVersionFromSpec)
 		existingClusterMasterVersion := convertToSdkMasterVersion(existingCluster.GetCurrentMasterVersion())
 		if desiredMasterVersion != existingClusterMasterVersion {
 			needUpdate = true
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml
@@ -31,6 +31,10 @@ spec:
       jsonPath: .status.currentVersion
       name: CurrentVersion
       type: string
+    - description: The Kubernetes version of the GKE control plane
+      jsonPath: .status.version
+      name: Version
+      type: string
     - description: API Endpoint
       jsonPath: .spec.endpoint
       name: Endpoint
@@ -133,6 +137,8 @@ spec:
                   ControlPlaneVersion represents the control plane version of the GKE cluster.
                   If not specified, the default version currently supported by GKE will be
                   used.
+
+                  Deprecated: This field will soon be removed and you are expected to use Version instead.
                 type: string
               description:
                 description: Description describe the cluster.
@@ -217,6 +223,12 @@ spec:
                 - regular
                 - stable
                 type: string
+              version:
+                description: |-
+                  Version represents the control plane version of the GKE cluster.
+                  If not specified, the default version currently supported by GKE will be
+                  used.
+                type: string
             required:
             - location
             - project
@@ -272,8 +284,10 @@ spec:
                   type: object
                 type: array
               currentVersion:
-                description: CurrentVersion shows the current version of the GKE control
-                  plane.
+                description: |-
+                  CurrentVersion shows the current version of the GKE control plane.
+
+                  Deprecated: This field will soon be removed and you are expected to use Version instead.
                 type: string
               initialized:
                 description: |-
@@ -286,6 +300,9 @@ spec:
                   Ready denotes that the GCPManagedControlPlane API Server is ready to
                   receive requests.
                 type: boolean
+              version:
+                description: Version represents the version of the GKE control plane.
+                type: string
             required:
             - ready
             type: object
diff --git a/exp/api/v1beta1/gcpmanagedcontrolplane_types.go b/exp/api/v1beta1/gcpmanagedcontrolplane_types.go
@@ -146,8 +146,16 @@ type GCPManagedControlPlaneSpec struct {
 	// ControlPlaneVersion represents the control plane version of the GKE cluster.
 	// If not specified, the default version currently supported by GKE will be
 	// used.
+	//
+	// Deprecated: This field will soon be removed and you are expected to use Version instead.
+	//
 	// +optional
 	ControlPlaneVersion *string `json:"controlPlaneVersion,omitempty"`
+	// Version represents the control plane version of the GKE cluster.
+	// If not specified, the default version currently supported by GKE will be
+	// used.
+	// +optional
+	Version *string `json:"version,omitempty"`
 	// Endpoint represents the endpoint used to communicate with the control plane.
 	// +optional
 	Endpoint clusterv1.APIEndpoint `json:"endpoint"`
@@ -183,8 +191,15 @@ type GCPManagedControlPlaneStatus struct {
 	Conditions clusterv1.Conditions `json:"conditions,omitempty"`
 
 	// CurrentVersion shows the current version of the GKE control plane.
+	//
+	// Deprecated: This field will soon be removed and you are expected to use Version instead.
+	//
 	// +optional
 	CurrentVersion string `json:"currentVersion,omitempty"`
+
+	// Version represents the version of the GKE control plane.
+	// +optional
+	Version *string `json:"version,omitempty"`
 }
 
 // +kubebuilder:object:root=true
@@ -194,6 +209,7 @@ type GCPManagedControlPlaneStatus struct {
 // +kubebuilder:printcolumn:name="Cluster",type="string",JSONPath=".metadata.labels.cluster\\.x-k8s\\.io/cluster-name",description="Cluster to which this GCPManagedControlPlane belongs"
 // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.ready",description="Control plane is ready"
 // +kubebuilder:printcolumn:name="CurrentVersion",type="string",JSONPath=".status.currentVersion",description="The current Kubernetes version"
+// +kubebuilder:printcolumn:name="Version",type="string",JSONPath=".status.version",description="The Kubernetes version of the GKE control plane"
 // +kubebuilder:printcolumn:name="Endpoint",type="string",JSONPath=".spec.endpoint",description="API Endpoint",priority=1
 
 // GCPManagedControlPlane is the Schema for the gcpmanagedcontrolplanes API.
diff --git a/exp/api/v1beta1/gcpmanagedcontrolplane_webhook.go b/exp/api/v1beta1/gcpmanagedcontrolplane_webhook.go
@@ -76,6 +76,7 @@ var _ webhook.Validator = &GCPManagedControlPlane{}
 func (r *GCPManagedControlPlane) ValidateCreate() (admission.Warnings, error) {
 	gcpmanagedcontrolplanelog.Info("validate create", "name", r.Name)
 	var allErrs field.ErrorList
+	var allWarns admission.Warnings
 
 	if len(r.Spec.ClusterName) > maxClusterNameLength {
 		allErrs = append(allErrs,
@@ -98,11 +99,20 @@ func (r *GCPManagedControlPlane) ValidateCreate() (admission.Warnings, error) {
 			r.Spec.LoggingService, "can't be set when autopilot is enabled"))
 	}
 
+	if r.Spec.ControlPlaneVersion != nil {
+		if r.Spec.Version != nil {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "ControlPlaneVersion"),
+				r.Spec.LoggingService, "spec.ControlPlaneVersion and spec.Version cannot be set at the same time: please use spec.Version"))
+		} else {
+			allWarns = append(allWarns, "spec.ControlPlaneVersion is deprecated and will soon be removed: please use spec.Version")
+		}
+	}
+
 	if len(allErrs) == 0 {
-		return nil, nil
+		return allWarns, nil
 	}
 
-	return nil, apierrors.NewInvalid(GroupVersion.WithKind("GCPManagedControlPlane").GroupKind(), r.Name, allErrs)
+	return allWarns, apierrors.NewInvalid(GroupVersion.WithKind("GCPManagedControlPlane").GroupKind(), r.Name, allErrs)
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
@@ -149,6 +159,11 @@ func (r *GCPManagedControlPlane) ValidateUpdate(oldRaw runtime.Object) (admissio
 			r.Spec.LoggingService, "can't be set when autopilot is enabled"))
 	}
 
+	if old.Spec.Version != nil && r.Spec.ControlPlaneVersion != nil {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "ControlPlaneVersion"),
+			r.Spec.LoggingService, "spec.ControlPlaneVersion and spec.Version cannot be set at the same time: please use spec.Version"))
+	}
+
 	if r.Spec.LoggingService != nil {
 		err := r.Spec.LoggingService.Validate()
 		if err != nil {
diff --git a/exp/api/v1beta1/gcpmanagedcontrolplane_webhook_test.go b/exp/api/v1beta1/gcpmanagedcontrolplane_webhook_test.go
@@ -72,21 +72,21 @@ func TestGCPManagedControlPlaneDefaultingWebhook(t *testing.T) {
 			resourceName: "cluster1",
 			resourceNS:   "default",
 			spec: GCPManagedControlPlaneSpec{
-				ClusterName:         "cluster1_27_1",
-				ControlPlaneVersion: &vV1_27_1,
+				ClusterName: "cluster1_27_1",
+				Version:     &vV1_27_1,
 			},
-			expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_27_1", ControlPlaneVersion: &vV1_27_1},
+			expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_27_1", Version: &vV1_27_1},
 		},
 		{
 			name:         "with autopilot enabled",
 			resourceName: "cluster1",
 			resourceNS:   "default",
 			spec: GCPManagedControlPlaneSpec{
-				ClusterName:         "cluster1_autopilot",
-				ControlPlaneVersion: &vV1_27_1,
-				EnableAutopilot:     true,
+				ClusterName:     "cluster1_autopilot",
+				Version:         &vV1_27_1,
+				EnableAutopilot: true,
 			},
-			expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_autopilot", ControlPlaneVersion: &vV1_27_1, EnableAutopilot: true},
+			expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_autopilot", Version: &vV1_27_1, EnableAutopilot: true},
 		},
 	}
 
@@ -120,18 +120,21 @@ func TestGCPManagedControlPlaneValidatingWebhookCreate(t *testing.T) {
 	tests := []struct {
 		name        string
 		expectError bool
+		expectWarn  bool
 		spec        GCPManagedControlPlaneSpec
 	}{
 		{
 			name:        "cluster name too long should cause an error",
 			expectError: true,
+			expectWarn:  false,
 			spec: GCPManagedControlPlaneSpec{
 				ClusterName: strings.Repeat("A", maxClusterNameLength+1),
 			},
 		},
 		{
 			name:        "autopilot enabled without release channel should cause an error",
 			expectError: true,
+			expectWarn:  false,
 			spec: GCPManagedControlPlaneSpec{
 				ClusterName:     "",
 				EnableAutopilot: true,
@@ -141,12 +144,32 @@ func TestGCPManagedControlPlaneValidatingWebhookCreate(t *testing.T) {
 		{
 			name:        "autopilot enabled with release channel",
 			expectError: false,
+			expectWarn:  false,
 			spec: GCPManagedControlPlaneSpec{
 				ClusterName:     "",
 				EnableAutopilot: true,
 				ReleaseChannel:  &releaseChannel,
 			},
 		},
+		{
+			name:        "using deprecated ControlPlaneVersion should cause a warning",
+			expectError: false,
+			expectWarn:  true,
+			spec: GCPManagedControlPlaneSpec{
+				ClusterName:         "",
+				ControlPlaneVersion: &vV1_27_1,
+			},
+		},
+		{
+			name:        "using both ControlPlaneVersion and Version should cause a warning and an error",
+			expectError: true,
+			expectWarn:  false,
+			spec: GCPManagedControlPlaneSpec{
+				ClusterName:         "",
+				ControlPlaneVersion: &vV1_27_1,
+				Version:             &vV1_27_1,
+			},
+		},
 	}
 
 	for _, tc := range tests {
@@ -163,8 +186,11 @@ func TestGCPManagedControlPlaneValidatingWebhookCreate(t *testing.T) {
 			} else {
 				g.Expect(err).ToNot(HaveOccurred())
 			}
-			// Nothing emits warnings yet
-			g.Expect(warn).To(BeEmpty())
+			if tc.expectWarn {
+				g.Expect(warn).ToNot(BeEmpty())
+			} else {
+				g.Expect(warn).To(BeEmpty())
+			}
 		})
 	}
 }
diff --git a/exp/api/v1beta1/zz_generated.deepcopy.go b/exp/api/v1beta1/zz_generated.deepcopy.go
diff --git a/test/e2e/data/infrastructure-gcp/cluster-template-ci-gke.yaml b/test/e2e/data/infrastructure-gcp/cluster-template-ci-gke.yaml
@@ -33,6 +33,7 @@ metadata:
 spec:
   project: "${GCP_PROJECT}"
   location: "${GCP_REGION}"
+  version: "${KUBERNETES_VERSION}"
   releaseChannel: "regular"
 ---
 apiVersion: cluster.x-k8s.io/v1beta1

Original file line number	Diff line number	Diff line change
`@@ -96,7 +96,9 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {`
`96`	`96`	`}`
`97`	`97`
`98`	`98`	`log.V(2).Info("gke cluster found", "status", cluster.GetStatus())`
`99`		`- s.scope.GCPManagedControlPlane.Status.CurrentVersion = convertToSdkMasterVersion(cluster.GetCurrentMasterVersion())`
	`99`	`+ controlPlaneVersion := convertToSdkMasterVersion(cluster.GetCurrentMasterVersion())`
	`100`	`+ s.scope.GCPManagedControlPlane.Status.CurrentVersion = controlPlaneVersion`
	`101`	`+ s.scope.GCPManagedControlPlane.Status.Version = &controlPlaneVersion`
`100`	`102`
`101`	`103`	`switch cluster.GetStatus() {`
`102`	`104`	`case containerpb.Cluster_PROVISIONING:`
`@@ -271,8 +273,8 @@ func (s Service) createCluster(ctx context.Context, log logr.Logger) error {`
`271`	`273`	`},`
`272`	`274`	`},`
`273`	`275`	`}`
`274`		`- if s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion != nil {`
`275`		`- cluster.InitialClusterVersion = convertToSdkMasterVersion(*s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion)`
	`276`	`+ if initialClusterVersionFromSpec := s.scope.GetControlPlaneVersion(); initialClusterVersionFromSpec != nil {`
	`277`	`+ cluster.InitialClusterVersion = convertToSdkMasterVersion(*initialClusterVersionFromSpec)`
`276`	`278`	`}`
`277`	`279`	`if s.scope.GCPManagedControlPlane.Spec.ClusterNetwork != nil {`
`278`	`280`	`cn := s.scope.GCPManagedControlPlane.Spec.ClusterNetwork`
`@@ -434,8 +436,8 @@ func (s Service) checkDiffAndPrepareUpdate(existingCluster containerpb.Cluster`
`434`	`436`	`}`
`435`	`437`	`}`
`436`	`438`	`// Master version`
`437`		`- if s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion != nil {`
`438`		`- desiredMasterVersion := convertToSdkMasterVersion(*s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion)`
	`439`	`+ if desiredMasterVersionFromSpec := s.scope.GetControlPlaneVersion(); desiredMasterVersionFromSpec != nil {`
	`440`	`+ desiredMasterVersion := convertToSdkMasterVersion(*desiredMasterVersionFromSpec)`
`439`	`441`	`existingClusterMasterVersion := convertToSdkMasterVersion(existingCluster.GetCurrentMasterVersion())`
`440`	`442`	`if desiredMasterVersion != existingClusterMasterVersion {`
`441`	`443`	`needUpdate = true`