Skip to content

Commit 2f1320c

Browse files
authored
Merge pull request #1434 from salasberryfin/managed-control-plane-version-api-change
feat: align GCPManagedControlPlane with CAPI >=v1.9
2 parents 52c7c87 + af6413d commit 2f1320c

File tree

9 files changed

+121
-18
lines changed

9 files changed

+121
-18
lines changed

.golangci.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,11 @@ issues:
196196
- linters:
197197
- gocritic
198198
text: "appendAssign: append result not assigned to the same slice"
199+
# Specific exclude rules for deprecated fields that are still part of the codebase.
200+
# These should be removed as the referenced deprecated item is removed from the project.
201+
- linters:
202+
- staticcheck
203+
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."
199204

200205
run:
201206
timeout: 10m

cloud/scope/managedcontrolplane.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,3 +237,14 @@ func (s *ManagedControlPlaneScope) SetEndpoint(host string) {
237237
func (s *ManagedControlPlaneScope) IsAutopilotCluster() bool {
238238
return s.GCPManagedControlPlane.Spec.EnableAutopilot
239239
}
240+
241+
// GetControlPlaneVersion returns the control plane version from the specification.
242+
func (s *ManagedControlPlaneScope) GetControlPlaneVersion() *string {
243+
if s.GCPManagedControlPlane.Spec.Version != nil {
244+
return s.GCPManagedControlPlane.Spec.Version
245+
}
246+
if s.GCPManagedControlPlane.Spec.ControlPlaneVersion != nil {
247+
return s.GCPManagedControlPlane.Spec.ControlPlaneVersion
248+
}
249+
return nil
250+
}

cloud/services/container/clusters/reconcile.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,9 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {
9696
}
9797

9898
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
100102

101103
switch cluster.GetStatus() {
102104
case containerpb.Cluster_PROVISIONING:
@@ -271,8 +273,8 @@ func (s *Service) createCluster(ctx context.Context, log *logr.Logger) error {
271273
},
272274
},
273275
}
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)
276278
}
277279
if s.scope.GCPManagedControlPlane.Spec.ClusterNetwork != nil {
278280
cn := s.scope.GCPManagedControlPlane.Spec.ClusterNetwork
@@ -434,8 +436,8 @@ func (s *Service) checkDiffAndPrepareUpdate(existingCluster *containerpb.Cluster
434436
}
435437
}
436438
// 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)
439441
existingClusterMasterVersion := convertToSdkMasterVersion(existingCluster.GetCurrentMasterVersion())
440442
if desiredMasterVersion != existingClusterMasterVersion {
441443
needUpdate = true

config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ spec:
3131
jsonPath: .status.currentVersion
3232
name: CurrentVersion
3333
type: string
34+
- description: The Kubernetes version of the GKE control plane
35+
jsonPath: .status.version
36+
name: Version
37+
type: string
3438
- description: API Endpoint
3539
jsonPath: .spec.endpoint
3640
name: Endpoint
@@ -133,6 +137,8 @@ spec:
133137
ControlPlaneVersion represents the control plane version of the GKE cluster.
134138
If not specified, the default version currently supported by GKE will be
135139
used.
140+
141+
Deprecated: This field will soon be removed and you are expected to use Version instead.
136142
type: string
137143
description:
138144
description: Description describe the cluster.
@@ -217,6 +223,12 @@ spec:
217223
- regular
218224
- stable
219225
type: string
226+
version:
227+
description: |-
228+
Version represents the control plane version of the GKE cluster.
229+
If not specified, the default version currently supported by GKE will be
230+
used.
231+
type: string
220232
required:
221233
- location
222234
- project
@@ -272,8 +284,10 @@ spec:
272284
type: object
273285
type: array
274286
currentVersion:
275-
description: CurrentVersion shows the current version of the GKE control
276-
plane.
287+
description: |-
288+
CurrentVersion shows the current version of the GKE control plane.
289+
290+
Deprecated: This field will soon be removed and you are expected to use Version instead.
277291
type: string
278292
initialized:
279293
description: |-
@@ -286,6 +300,9 @@ spec:
286300
Ready denotes that the GCPManagedControlPlane API Server is ready to
287301
receive requests.
288302
type: boolean
303+
version:
304+
description: Version represents the version of the GKE control plane.
305+
type: string
289306
required:
290307
- ready
291308
type: object

exp/api/v1beta1/gcpmanagedcontrolplane_types.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,16 @@ type GCPManagedControlPlaneSpec struct {
146146
// ControlPlaneVersion represents the control plane version of the GKE cluster.
147147
// If not specified, the default version currently supported by GKE will be
148148
// used.
149+
//
150+
// Deprecated: This field will soon be removed and you are expected to use Version instead.
151+
//
149152
// +optional
150153
ControlPlaneVersion *string `json:"controlPlaneVersion,omitempty"`
154+
// Version represents the control plane version of the GKE cluster.
155+
// If not specified, the default version currently supported by GKE will be
156+
// used.
157+
// +optional
158+
Version *string `json:"version,omitempty"`
151159
// Endpoint represents the endpoint used to communicate with the control plane.
152160
// +optional
153161
Endpoint clusterv1.APIEndpoint `json:"endpoint"`
@@ -183,8 +191,15 @@ type GCPManagedControlPlaneStatus struct {
183191
Conditions clusterv1.Conditions `json:"conditions,omitempty"`
184192

185193
// CurrentVersion shows the current version of the GKE control plane.
194+
//
195+
// Deprecated: This field will soon be removed and you are expected to use Version instead.
196+
//
186197
// +optional
187198
CurrentVersion string `json:"currentVersion,omitempty"`
199+
200+
// Version represents the version of the GKE control plane.
201+
// +optional
202+
Version *string `json:"version,omitempty"`
188203
}
189204

190205
// +kubebuilder:object:root=true
@@ -194,6 +209,7 @@ type GCPManagedControlPlaneStatus struct {
194209
// +kubebuilder:printcolumn:name="Cluster",type="string",JSONPath=".metadata.labels.cluster\\.x-k8s\\.io/cluster-name",description="Cluster to which this GCPManagedControlPlane belongs"
195210
// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.ready",description="Control plane is ready"
196211
// +kubebuilder:printcolumn:name="CurrentVersion",type="string",JSONPath=".status.currentVersion",description="The current Kubernetes version"
212+
// +kubebuilder:printcolumn:name="Version",type="string",JSONPath=".status.version",description="The Kubernetes version of the GKE control plane"
197213
// +kubebuilder:printcolumn:name="Endpoint",type="string",JSONPath=".spec.endpoint",description="API Endpoint",priority=1
198214

199215
// GCPManagedControlPlane is the Schema for the gcpmanagedcontrolplanes API.

exp/api/v1beta1/gcpmanagedcontrolplane_webhook.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ var _ webhook.Validator = &GCPManagedControlPlane{}
7676
func (r *GCPManagedControlPlane) ValidateCreate() (admission.Warnings, error) {
7777
gcpmanagedcontrolplanelog.Info("validate create", "name", r.Name)
7878
var allErrs field.ErrorList
79+
var allWarns admission.Warnings
7980

8081
if len(r.Spec.ClusterName) > maxClusterNameLength {
8182
allErrs = append(allErrs,
@@ -98,11 +99,20 @@ func (r *GCPManagedControlPlane) ValidateCreate() (admission.Warnings, error) {
9899
r.Spec.LoggingService, "can't be set when autopilot is enabled"))
99100
}
100101

102+
if r.Spec.ControlPlaneVersion != nil {
103+
if r.Spec.Version != nil {
104+
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "ControlPlaneVersion"),
105+
r.Spec.LoggingService, "spec.ControlPlaneVersion and spec.Version cannot be set at the same time: please use spec.Version"))
106+
} else {
107+
allWarns = append(allWarns, "spec.ControlPlaneVersion is deprecated and will soon be removed: please use spec.Version")
108+
}
109+
}
110+
101111
if len(allErrs) == 0 {
102-
return nil, nil
112+
return allWarns, nil
103113
}
104114

105-
return nil, apierrors.NewInvalid(GroupVersion.WithKind("GCPManagedControlPlane").GroupKind(), r.Name, allErrs)
115+
return allWarns, apierrors.NewInvalid(GroupVersion.WithKind("GCPManagedControlPlane").GroupKind(), r.Name, allErrs)
106116
}
107117

108118
// 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
149159
r.Spec.LoggingService, "can't be set when autopilot is enabled"))
150160
}
151161

162+
if old.Spec.Version != nil && r.Spec.ControlPlaneVersion != nil {
163+
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "ControlPlaneVersion"),
164+
r.Spec.LoggingService, "spec.ControlPlaneVersion and spec.Version cannot be set at the same time: please use spec.Version"))
165+
}
166+
152167
if r.Spec.LoggingService != nil {
153168
err := r.Spec.LoggingService.Validate()
154169
if err != nil {

exp/api/v1beta1/gcpmanagedcontrolplane_webhook_test.go

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,21 @@ func TestGCPManagedControlPlaneDefaultingWebhook(t *testing.T) {
7272
resourceName: "cluster1",
7373
resourceNS: "default",
7474
spec: GCPManagedControlPlaneSpec{
75-
ClusterName: "cluster1_27_1",
76-
ControlPlaneVersion: &vV1_27_1,
75+
ClusterName: "cluster1_27_1",
76+
Version: &vV1_27_1,
7777
},
78-
expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_27_1", ControlPlaneVersion: &vV1_27_1},
78+
expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_27_1", Version: &vV1_27_1},
7979
},
8080
{
8181
name: "with autopilot enabled",
8282
resourceName: "cluster1",
8383
resourceNS: "default",
8484
spec: GCPManagedControlPlaneSpec{
85-
ClusterName: "cluster1_autopilot",
86-
ControlPlaneVersion: &vV1_27_1,
87-
EnableAutopilot: true,
85+
ClusterName: "cluster1_autopilot",
86+
Version: &vV1_27_1,
87+
EnableAutopilot: true,
8888
},
89-
expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_autopilot", ControlPlaneVersion: &vV1_27_1, EnableAutopilot: true},
89+
expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_autopilot", Version: &vV1_27_1, EnableAutopilot: true},
9090
},
9191
}
9292

@@ -120,18 +120,21 @@ func TestGCPManagedControlPlaneValidatingWebhookCreate(t *testing.T) {
120120
tests := []struct {
121121
name string
122122
expectError bool
123+
expectWarn bool
123124
spec GCPManagedControlPlaneSpec
124125
}{
125126
{
126127
name: "cluster name too long should cause an error",
127128
expectError: true,
129+
expectWarn: false,
128130
spec: GCPManagedControlPlaneSpec{
129131
ClusterName: strings.Repeat("A", maxClusterNameLength+1),
130132
},
131133
},
132134
{
133135
name: "autopilot enabled without release channel should cause an error",
134136
expectError: true,
137+
expectWarn: false,
135138
spec: GCPManagedControlPlaneSpec{
136139
ClusterName: "",
137140
EnableAutopilot: true,
@@ -141,12 +144,32 @@ func TestGCPManagedControlPlaneValidatingWebhookCreate(t *testing.T) {
141144
{
142145
name: "autopilot enabled with release channel",
143146
expectError: false,
147+
expectWarn: false,
144148
spec: GCPManagedControlPlaneSpec{
145149
ClusterName: "",
146150
EnableAutopilot: true,
147151
ReleaseChannel: &releaseChannel,
148152
},
149153
},
154+
{
155+
name: "using deprecated ControlPlaneVersion should cause a warning",
156+
expectError: false,
157+
expectWarn: true,
158+
spec: GCPManagedControlPlaneSpec{
159+
ClusterName: "",
160+
ControlPlaneVersion: &vV1_27_1,
161+
},
162+
},
163+
{
164+
name: "using both ControlPlaneVersion and Version should cause a warning and an error",
165+
expectError: true,
166+
expectWarn: false,
167+
spec: GCPManagedControlPlaneSpec{
168+
ClusterName: "",
169+
ControlPlaneVersion: &vV1_27_1,
170+
Version: &vV1_27_1,
171+
},
172+
},
150173
}
151174

152175
for _, tc := range tests {
@@ -163,8 +186,11 @@ func TestGCPManagedControlPlaneValidatingWebhookCreate(t *testing.T) {
163186
} else {
164187
g.Expect(err).ToNot(HaveOccurred())
165188
}
166-
// Nothing emits warnings yet
167-
g.Expect(warn).To(BeEmpty())
189+
if tc.expectWarn {
190+
g.Expect(warn).ToNot(BeEmpty())
191+
} else {
192+
g.Expect(warn).To(BeEmpty())
193+
}
168194
})
169195
}
170196
}

exp/api/v1beta1/zz_generated.deepcopy.go

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/e2e/data/infrastructure-gcp/cluster-template-ci-gke.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ metadata:
3333
spec:
3434
project: "${GCP_PROJECT}"
3535
location: "${GCP_REGION}"
36+
version: "${KUBERNETES_VERSION}"
3637
releaseChannel: "regular"
3738
---
3839
apiVersion: cluster.x-k8s.io/v1beta1

0 commit comments

Comments
 (0)