Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions cloud/scope/managedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
12 changes: 7 additions & 5 deletions cloud/services/container/clusters/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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: |-
Expand All @@ -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
Expand Down
16 changes: 16 additions & 0 deletions exp/api/v1beta1/gcpmanagedcontrolplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
19 changes: 17 additions & 2 deletions exp/api/v1beta1/gcpmanagedcontrolplane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
44 changes: 35 additions & 9 deletions exp/api/v1beta1/gcpmanagedcontrolplane_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be this updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update this in a separate PR, if that's okay.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

EnableAutopilot: true,
},
expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_autopilot", ControlPlaneVersion: &vV1_27_1, EnableAutopilot: true},
expectSpec: GCPManagedControlPlaneSpec{ClusterName: "cluster1_autopilot", Version: &vV1_27_1, EnableAutopilot: true},
},
}

Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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())
}
})
}
}
Expand Down
10 changes: 10 additions & 0 deletions exp/api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ metadata:
spec:
project: "${GCP_PROJECT}"
location: "${GCP_REGION}"
version: "${KUBERNETES_VERSION}"
releaseChannel: "regular"
---
apiVersion: cluster.x-k8s.io/v1beta1
Expand Down