Merge pull request #1207 from bfournie/api-server-name-override

Allow configuration of API server instance group name

Author: k8s-ci-robot

api/v1beta1/gcpcluster_types.go

@@ -64,6 +64,10 @@ type GCPClusterSpec struct {
 	// supplied then the credentials of the controller will be used.
 	// +optional
 	CredentialsRef *ObjectReference `json:"credentialsRef,omitempty"`
+
+	// LoadBalancer contains configuration for one or more LoadBalancers.
+	// +optional
+	LoadBalancer LoadBalancerSpec `json:"loadBalancer,omitempty"`
 }
 
 // GCPClusterStatus defines the observed state of GCPCluster.

api/v1beta1/types.go

@@ -114,6 +114,18 @@ type NetworkSpec struct {
 	LoadBalancerBackendPort *int32 `json:"loadBalancerBackendPort,omitempty"`
 }
 
+// LoadBalancerSpec contains configuration for one or more LoadBalancers.
+type LoadBalancerSpec struct {
+
+	// APIServerInstanceGroupTagOverride overrides the default setting for the
+	// tag used when creating the API Server Instance Group.
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:validation:MaxLength=16
+	// +kubebuilder:validation:Pattern=`(^[1-9][0-9]{0,31}$)|(^[a-z][a-z0-9-]{4,28}[a-z0-9]$)`
+	// +optional
+	APIServerInstanceGroupTagOverride *string `json:"apiServerInstanceGroupTagOverride,omitempty"`
+}
+
 // SubnetSpec configures an GCP Subnet.
 type SubnetSpec struct {
 	// Name defines a unique identifier to reference this resource.

api/v1beta1/zz_generated.deepcopy.go

@@ -61,6 +61,7 @@ type ClusterGetter interface {
 	FailureDomains() clusterv1.FailureDomains
 	ControlPlaneEndpoint() clusterv1.APIEndpoint
 	ResourceManagerTags() infrav1.ResourceManagerTags
+	LoadBalancer() infrav1.LoadBalancerSpec
 }
 
 // ClusterSetter is an interface which can set cluster information.

cloud/interfaces.go

@@ -130,6 +130,11 @@ func (s *ClusterScope) AdditionalLabels() infrav1.Labels {
 	return s.GCPCluster.Spec.AdditionalLabels
 }
 
+// LoadBalancer returns the LoadBalancer configuration.
+func (s *ClusterScope) LoadBalancer() infrav1.LoadBalancerSpec {
+	return s.GCPCluster.Spec.LoadBalancer
+}
+
 // ResourceManagerTags returns ResourceManagerTags from the scope's GCPCluster. The returned value will never be nil.
 func (s *ClusterScope) ResourceManagerTags() infrav1.ResourceManagerTags {
 	if len(s.GCPCluster.Spec.ResourceManagerTags) == 0 {
@@ -339,8 +344,9 @@ func (s *ClusterScope) HealthCheckSpec() *compute.HealthCheck {
 // InstanceGroupSpec returns google compute instance-group spec.
 func (s *ClusterScope) InstanceGroupSpec(zone string) *compute.InstanceGroup {
 	port := ptr.Deref(s.GCPCluster.Spec.Network.LoadBalancerBackendPort, 6443)
+	tag := ptr.Deref(s.GCPCluster.Spec.LoadBalancer.APIServerInstanceGroupTagOverride, infrav1.APIServerRoleTagValue)
 	return &compute.InstanceGroup{
-		Name: fmt.Sprintf("%s-%s-%s", s.Name(), infrav1.APIServerRoleTagValue, zone),
+		Name: fmt.Sprintf("%s-%s-%s", s.Name(), tag, zone),
 		NamedPorts: []*compute.NamedPort{
 			{
 				Name: "apiserver",

cloud/scope/cluster.go

@@ -128,7 +128,8 @@ func (m *MachineScope) Namespace() string {
 
 // ControlPlaneGroupName returns the control-plane instance group name.
 func (m *MachineScope) ControlPlaneGroupName() string {
-	return fmt.Sprintf("%s-%s-%s", m.ClusterGetter.Name(), infrav1.APIServerRoleTagValue, m.Zone())
+	tag := ptr.Deref(m.ClusterGetter.LoadBalancer().APIServerInstanceGroupTagOverride, infrav1.APIServerRoleTagValue)
+	return fmt.Sprintf("%s-%s-%s", m.ClusterGetter.Name(), tag, m.Zone())
 }
 
 // IsControlPlane returns true if the machine is a control plane.

cloud/scope/machine.go

@@ -133,6 +133,11 @@ func (s *ManagedClusterScope) AdditionalLabels() infrav1.Labels {
 	return s.GCPManagedCluster.Spec.AdditionalLabels
 }
 
+// LoadBalancer returns the LoadBalancer configuration.
+func (s *ManagedClusterScope) LoadBalancer() infrav1.LoadBalancerSpec {
+	return s.GCPManagedCluster.Spec.LoadBalancer
+}
+
 // ResourceManagerTags returns ResourceManagerTags from cluster. The returned value will never be nil.
 func (s *ManagedClusterScope) ResourceManagerTags() infrav1.ResourceManagerTags {
 	if len(s.GCPManagedCluster.Spec.ResourceManagerTags) == 0 {

cloud/scope/managedcluster.go

@@ -0,0 +1,164 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package loadbalancers
+
+import (
+	"context"
+	"net/http"
+	"testing"
+
+	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
+	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
+	"github.com/google/go-cmp/cmp"
+	"google.golang.org/api/compute/v1"
+	"google.golang.org/api/googleapi"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/scheme"
+	infrav1 "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-gcp/cloud/scope"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+func init() {
+	_ = clusterv1.AddToScheme(scheme.Scheme)
+	_ = infrav1.AddToScheme(scheme.Scheme)
+}
+
+func getBaseClusterScope() (*scope.ClusterScope, error) {
+	fakec := fake.NewClientBuilder().
+		WithScheme(scheme.Scheme).
+		Build()
+
+	fakeCluster := &clusterv1.Cluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "my-cluster",
+			Namespace: "default",
+		},
+		Spec: clusterv1.ClusterSpec{},
+	}
+
+	fakeGCPCluster := &infrav1.GCPCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "my-cluster",
+			Namespace: "default",
+		},
+		Spec: infrav1.GCPClusterSpec{
+			Project: "my-proj",
+			Region:  "us-central1",
+		},
+		Status: infrav1.GCPClusterStatus{
+			FailureDomains: clusterv1.FailureDomains{
+				"us-central1-a": clusterv1.FailureDomainSpec{ControlPlane: true},
+			},
+		},
+	}
+	clusterScope, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{
+		Client:     fakec,
+		Cluster:    fakeCluster,
+		GCPCluster: fakeGCPCluster,
+		GCPServices: scope.GCPServices{
+			Compute: &compute.Service{},
+		},
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return clusterScope, nil
+}
+
+func TestService_createOrGetInstanceGroup(t *testing.T) {
+	tests := []struct {
+		name              string
+		scope             func(s *scope.ClusterScope) Scope
+		mockInstanceGroup *cloud.MockInstanceGroups
+		want              []*compute.InstanceGroup
+		wantErr           bool
+	}{
+		{
+			name:  "error getting instanceGroup with non 404 error code (should return an error)",
+			scope: func(s *scope.ClusterScope) Scope { return s },
+			mockInstanceGroup: &cloud.MockInstanceGroups{
+				ProjectRouter: &cloud.SingleProjectRouter{ID: "proj-id"},
+				Objects:       map[meta.Key]*cloud.MockInstanceGroupsObj{},
+				GetHook: func(_ context.Context, _ *meta.Key, _ *cloud.MockInstanceGroups) (bool, *compute.InstanceGroup, error) {
+					return true, &compute.InstanceGroup{}, &googleapi.Error{Code: http.StatusBadRequest}
+				},
+			},
+			want:    []*compute.InstanceGroup{},
+			wantErr: true,
+		},
+		{
+			name: "instanceGroup name is overridden (should create instanceGroup)",
+			scope: func(s *scope.ClusterScope) Scope {
+				var tagOverride = "master"
+				s.GCPCluster.Spec.LoadBalancer = infrav1.LoadBalancerSpec{
+					APIServerInstanceGroupTagOverride: &tagOverride,
+				}
+				return s
+			},
+			mockInstanceGroup: &cloud.MockInstanceGroups{
+				ProjectRouter: &cloud.SingleProjectRouter{ID: "proj-id"},
+				Objects:       map[meta.Key]*cloud.MockInstanceGroupsObj{},
+			},
+			want: []*compute.InstanceGroup{
+				{
+					Name:       "my-cluster-master-us-central1-a",
+					NamedPorts: []*compute.NamedPort{{Name: "apiserver", Port: 6443}},
+					SelfLink:   "https://www.googleapis.com/compute/v1/projects/proj-id/zones/us-central1-a/instanceGroups/my-cluster-master-us-central1-a",
+				},
+			},
+		},
+		{
+			name:  "instanceGroup does not exist (should create instanceGroup)",
+			scope: func(s *scope.ClusterScope) Scope { return s },
+			mockInstanceGroup: &cloud.MockInstanceGroups{
+				ProjectRouter: &cloud.SingleProjectRouter{ID: "proj-id"},
+				Objects:       map[meta.Key]*cloud.MockInstanceGroupsObj{},
+			},
+			want: []*compute.InstanceGroup{
+				{
+					Name:       "my-cluster-apiserver-us-central1-a",
+					NamedPorts: []*compute.NamedPort{{Name: "apiserver", Port: 6443}},
+					SelfLink:   "https://www.googleapis.com/compute/v1/projects/proj-id/zones/us-central1-a/instanceGroups/my-cluster-apiserver-us-central1-a",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.TODO()
+
+			clusterScope, err := getBaseClusterScope()
+			if err != nil {
+				t.Fatal(err)
+			}
+			s := New(tt.scope(clusterScope))
+			s.instancegroups = tt.mockInstanceGroup
+			got, err := s.createOrGetInstanceGroups(ctx)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Service s.createOrGetInstanceGroups() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if d := cmp.Diff(tt.want, got); d != "" {
+				t.Errorf("Service s.createOrGetInstanceGroups() mismatch (-want +got):\n%s", d)
+			}
+		})
+	}
+}

cloud/services/compute/loadbalancers/reconcile_test.go

@@ -108,6 +108,17 @@ spec:
                 items:
                   type: string
                 type: array
+              loadBalancer:
+                description: LoadBalancer contains configuration for one or more LoadBalancers.
+                properties:
+                  apiServerInstanceGroupTagOverride:
+                    description: |-
+                      APIServerInstanceGroupTagOverride overrides the default setting for the
+                      tag used when creating the API Server Instance Group.
+                    maxLength: 16
+                    pattern: (^[1-9][0-9]{0,31}$)|(^[a-z][a-z0-9-]{4,28}[a-z0-9]$)
+                    type: string
+                type: object
               network:
                 description: NetworkSpec encapsulates all things related to GCP network.
                 properties:

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

@@ -123,6 +123,18 @@ spec:
                         items:
                           type: string
                         type: array
+                      loadBalancer:
+                        description: LoadBalancer contains configuration for one or
+                          more LoadBalancers.
+                        properties:
+                          apiServerInstanceGroupTagOverride:
+                            description: |-
+                              APIServerInstanceGroupTagOverride overrides the default setting for the
+                              tag used when creating the API Server Instance Group.
+                            maxLength: 16
+                            pattern: (^[1-9][0-9]{0,31}$)|(^[a-z][a-z0-9-]{4,28}[a-z0-9]$)
+                            type: string
+                        type: object
                       network:
                         description: NetworkSpec encapsulates all things related to
                           GCP network.