diff --git a/.golangci.yml b/.golangci.yml index 4bb218a39..faa3cd58c 100644 --- 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 index a9ac54217..1587a6aef 100644 --- 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 index 64ceec390..e3e729c32 100644 --- 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 index c1de7a59d..2eb9c294d 100644 --- 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 index d4c65b4c5..53ecffcdd 100644 --- 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 index 7835b4143..d9a1ff326 100644 --- 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 index 1d5ea1208..7a89871ce 100644 --- 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,11 +120,13 @@ 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), }, @@ -132,6 +134,7 @@ func TestGCPManagedControlPlaneValidatingWebhookCreate(t *testing.T) { { 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 index f14563ca4..73a813a13 100644 --- a/exp/api/v1beta1/zz_generated.deepcopy.go +++ b/exp/api/v1beta1/zz_generated.deepcopy.go @@ -307,6 +307,11 @@ func (in *GCPManagedControlPlaneSpec) DeepCopyInto(out *GCPManagedControlPlaneSp *out = new(string) **out = **in } + if in.Version != nil { + in, out := &in.Version, &out.Version + *out = new(string) + **out = **in + } out.Endpoint = in.Endpoint if in.MasterAuthorizedNetworksConfig != nil { in, out := &in.MasterAuthorizedNetworksConfig, &out.MasterAuthorizedNetworksConfig @@ -345,6 +350,11 @@ func (in *GCPManagedControlPlaneStatus) DeepCopyInto(out *GCPManagedControlPlane (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Version != nil { + in, out := &in.Version, &out.Version + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GCPManagedControlPlaneStatus. diff --git a/test/e2e/data/infrastructure-gcp/cluster-template-ci-gke.yaml b/test/e2e/data/infrastructure-gcp/cluster-template-ci-gke.yaml index 228acdb52..72c9d4108 100644 --- 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