Add option to create an InstanceGroup to be used for bootstrap instances

Add a new configuration field to specify that an InstanceGroup should
be created for a bootstrap instance. When configured, the InstanceGroup
will be created in the first zone and if a bootstrap Instance is created
it will be added to this InstanceGroup.

api/v1beta1/types.go

@@ -105,6 +105,11 @@ type Network struct {
 	// created for the internal Load Balancer.
 	// +optional
 	APIInternalForwardingRule *string `json:"apiInternalForwardingRule,omitempty"`
+
+	// APIInternalBootstrapInstanceGroup is a map from zone to the full reference
+	// to the bootstrap instance group created in the same zone.
+	// +optional
+	APIInternalBootstrapInstanceGroup map[string]string `json:"apiInternalBootstrapInstanceGroup,omitempty"`
 }
 
 // NetworkSpec encapsulates all things related to a GCP network.
@@ -344,4 +349,14 @@ type LoadBalancer struct {
 	// required for the Load Balancer, if not defined the first configured subnet will be
 	// used.
 	Subnet *string `json:"subnet,omitempty"`
+
+	// CreateBootstrapInstanceGroup: When set to true, an InstanceGroup is created
+	// will be created named <cluster-id>-boostrap. This InstanceGroup will be used
+	// by the bootstrap instance.
+	// When set to false, no bootstrap InstanceGroup will be created. If a bootstrap
+	// instance is created, it will be added to an existing InstanceGroup.
+	//
+	// Defaults to false.
+	// +optional
+	CreateBootstrapInstanceGroup *bool `json:"createBootstrapInstanceGroup,omitempty"`
 }

cloud/interfaces.go

@@ -87,7 +87,9 @@ type MachineGetter interface {
 	Project() string
 	Role() string
 	IsControlPlane() bool
+	IsBootstrap() bool
 	ControlPlaneGroupName() string
+	BootstrapGroupName() string
 	GetInstanceID() *string
 	GetProviderID() string
 	GetBootstrapData() (string, error)

cloud/scope/machine.go

@@ -131,12 +131,25 @@ func (m *MachineScope) Namespace() string {
 	return m.GCPMachine.Namespace
 }
 
+// BootstrapGroupName returns the instance group name for bootstrap machine.
+func (m *MachineScope) BootstrapGroupName() string {
+	return fmt.Sprintf("%s-%s", m.ClusterGetter.Name(), "bootstrap")
+}
+
 // ControlPlaneGroupName returns the control-plane instance group name.
 func (m *MachineScope) ControlPlaneGroupName() string {
 	tag := ptr.Deref(m.ClusterGetter.LoadBalancer().APIServerInstanceGroupTagOverride, infrav1.APIServerRoleTagValue)
 	return fmt.Sprintf("%s-%s-%s", m.ClusterGetter.Name(), tag, m.Zone())
 }
 
+// IsBootstrap returns true if the machine is a control plane machine that is used for bootstrap.
+func (m *MachineScope) IsBootstrap() bool {
+	if util.IsControlPlaneMachine(m.Machine) {
+		return strings.Contains(m.Machine.ObjectMeta.Name, "bootstrap")
+	}
+	return false
+}
+
 // IsControlPlane returns true if the machine is a control plane.
 func (m *MachineScope) IsControlPlane() bool {
 	return util.IsControlPlaneMachine(m.Machine)

cloud/services/compute/instances/reconcile.go

@@ -168,8 +168,13 @@ func (s *Service) createOrGetInstance(ctx context.Context) (*compute.Instance, e
 func (s *Service) registerControlPlaneInstance(ctx context.Context, instance *compute.Instance) error {
 	log := log.FromContext(ctx)
 	instancegroupName := s.scope.ControlPlaneGroupName()
-	log.V(2).Info("Ensuring instance already registered in the instancegroup", "name", instance.Name, "instancegroup", instancegroupName)
+	if s.scope.IsBootstrap() && s.bootstrapGroupExists(ctx) {
+		// If this is the bootstrap instance, use the bootstrap instance group if one exists
+		instancegroupName = s.scope.BootstrapGroupName()
+	}
 	instancegroupKey := meta.ZonalKey(instancegroupName, s.scope.Zone())
+
+	log.V(2).Info("Ensuring instance already registered in the instancegroup", "name", instance.Name, "instancegroup", instancegroupName)
 	instanceList, err := s.instancegroups.ListInstances(ctx, instancegroupKey, &compute.InstanceGroupsListInstancesRequest{
 		InstanceState: "RUNNING",
 	}, filter.None)
@@ -200,9 +205,20 @@ func (s *Service) registerControlPlaneInstance(ctx context.Context, instance *co
 	return nil
 }
 
+func (s *Service) bootstrapGroupExists(ctx context.Context) bool {
+	bootstrapGroupName := s.scope.BootstrapGroupName()
+	bootstrapgroupKey := meta.ZonalKey(bootstrapGroupName, s.scope.Zone())
+	_, err := s.instancegroups.Get(ctx, bootstrapgroupKey)
+	return err == nil
+}
+
 func (s *Service) deregisterControlPlaneInstance(ctx context.Context, instance *compute.Instance) error {
 	log := log.FromContext(ctx)
 	instancegroupName := s.scope.ControlPlaneGroupName()
+	if s.scope.IsBootstrap() && s.bootstrapGroupExists(ctx) {
+		// If this is the bootstrap instance, use the bootstrap instance group if one exists
+		instancegroupName = s.scope.BootstrapGroupName()
+	}
 	log.V(2).Info("Ensuring instance already registered in the instancegroup", "name", instance.Name, "instancegroup", instancegroupName)
 	instancegroupKey := meta.ZonalKey(instancegroupName, s.scope.Zone())
 	instanceList, err := s.instancegroups.ListInstances(ctx, instancegroupKey, &compute.InstanceGroupsListInstancesRequest{

cloud/services/compute/instances/service.go

@@ -39,6 +39,7 @@ type instancegroupsInterface interface {
 	AddInstances(ctx context.Context, key *meta.Key, req *compute.InstanceGroupsAddInstancesRequest, options ...k8scloud.Option) error
 	ListInstances(ctx context.Context, key *meta.Key, req *compute.InstanceGroupsListInstancesRequest, fl *filter.F, options ...k8scloud.Option) ([]*compute.InstanceWithNamedPorts, error)
 	RemoveInstances(ctx context.Context, key *meta.Key, req *compute.InstanceGroupsRemoveInstancesRequest, options ...k8scloud.Option) error
+	Get(ctx context.Context, key *meta.Key, options ...k8scloud.Option) (*compute.InstanceGroup, error)
 }
 
 // Scope is an interfaces that hold used methods.

cloud/services/compute/loadbalancers/reconcile.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
@@ -44,6 +45,8 @@ const (
 	loadBalancingModeConnection = loadBalancingMode("CONNECTION")
 
 	loadBalanceTrafficInternal = "INTERNAL"
+
+	bootstrapInstanceGroup = "bootstrap"
 )
 
 // Reconcile reconcile cluster control-plane loadbalancer components.
@@ -58,6 +61,17 @@ func (s *Service) Reconcile(ctx context.Context) error {
 	}
 
 	lbSpec := s.scope.LoadBalancer()
+	createBootstrapInstanceGroup := false
+	if lbSpec.InternalLoadBalancer != nil {
+		createBootstrapInstanceGroup = ptr.Deref(lbSpec.InternalLoadBalancer.CreateBootstrapInstanceGroup, false)
+	}
+	if createBootstrapInstanceGroup {
+		instancegroups, err = s.addBootstrapInstanceGroup(ctx, instancegroups)
+		if err != nil {
+			return err
+		}
+	}
+
 	lbType := ptr.Deref(lbSpec.LoadBalancerType, infrav1.External)
 	// Create a Global External Proxy Load Balancer by default
 	if lbType == infrav1.External || lbType == infrav1.InternalExternal {
@@ -164,6 +178,18 @@ func (s *Service) deleteInternalLoadBalancer(ctx context.Context, name string) e
 	}
 	s.scope.Network().APIInternalHealthCheck = nil
 
+	lbSpec := s.scope.LoadBalancer()
+	createBootstrapInstanceGroup := false
+	if lbSpec.InternalLoadBalancer != nil {
+		createBootstrapInstanceGroup = ptr.Deref(lbSpec.InternalLoadBalancer.CreateBootstrapInstanceGroup, false)
+	}
+	if createBootstrapInstanceGroup {
+		if err := s.deleteBootstrapInstanceGroups(ctx); err != nil {
+			return fmt.Errorf("deleting BootstrapInstanceGroup: %w", err)
+		}
+		s.scope.Network().APIInternalBootstrapInstanceGroup = nil
+	}
+
 	return nil
 }
 
@@ -296,6 +322,57 @@ func (s *Service) createOrGetInstanceGroups(ctx context.Context) ([]*compute.Ins
 	return groups, nil
 }
 
+func (s *Service) addBootstrapInstanceGroup(ctx context.Context, groups []*compute.InstanceGroup) ([]*compute.InstanceGroup, error) {
+	log := log.FromContext(ctx)
+	fd := s.scope.FailureDomains()
+	zones := make([]string, 0, len(fd))
+	for zone := range fd {
+		zones = append(zones, zone)
+	}
+	sort.Strings(zones)
+	bootstrapzone := zones[0] // use first zone
+
+	instancegroupSpec := &compute.InstanceGroup{
+		Name: fmt.Sprintf("%s-%s", s.scope.Name(), bootstrapInstanceGroup),
+		NamedPorts: []*compute.NamedPort{
+			{
+				Name: bootstrapInstanceGroup,
+				Port: 6443,
+			},
+		},
+	}
+	groupsMap := s.scope.Network().APIInternalBootstrapInstanceGroup
+	if groupsMap == nil {
+		groupsMap = make(map[string]string)
+	}
+
+	log.V(2).Info("Looking for bootstrap instancegroup in zone", "zone", bootstrapzone)
+	instancegroup, err := s.instancegroups.Get(ctx, meta.ZonalKey(instancegroupSpec.Name, bootstrapzone))
+	if err != nil {
+		if !gcperrors.IsNotFound(err) {
+			log.Error(err, "Error looking for bootstrap instancegroup in zone", "zone", bootstrapzone)
+			return groups, err
+		}
+
+		log.V(2).Info("Creating bootstrap instancegroup in zone", "zone", bootstrapzone)
+		err := s.instancegroups.Insert(ctx, meta.ZonalKey(instancegroupSpec.Name, bootstrapzone), instancegroupSpec)
+		if err != nil {
+			log.Error(err, "Error creating bootstrap instancegroup")
+			return groups, err
+		}
+
+		instancegroup, err = s.instancegroups.Get(ctx, meta.ZonalKey(instancegroupSpec.Name, bootstrapzone))
+		if err != nil {
+			return groups, err
+		}
+	}
+
+	groups = append(groups, instancegroup)
+	groupsMap[bootstrapzone] = instancegroup.SelfLink
+	s.scope.Network().APIInternalBootstrapInstanceGroup = groupsMap
+	return groups, nil
+}
+
 func (s *Service) createOrGetHealthCheck(ctx context.Context, lbname string) (*compute.HealthCheck, error) {
 	log := log.FromContext(ctx)
 	healthcheckSpec := s.scope.HealthCheckSpec(lbname)
@@ -745,10 +822,10 @@ func (s *Service) deleteInstanceGroups(ctx context.Context) error {
 	for zone := range s.scope.Network().APIServerInstanceGroups {
 		spec := s.scope.InstanceGroupSpec(zone)
 		key := meta.ZonalKey(spec.Name, zone)
-		log.V(2).Info("Deleting a instancegroup", "name", spec.Name)
+		log.V(2).Info("Deleting an instancegroup", "name", spec.Name)
 		if err := s.instancegroups.Delete(ctx, key); err != nil {
 			if !gcperrors.IsNotFound(err) {
-				log.Error(err, "Error deleting a instancegroup", "name", spec.Name)
+				log.Error(err, "Error deleting an instancegroup", "name", spec.Name)
 				return err
 			}
 
@@ -759,6 +836,22 @@ func (s *Service) deleteInstanceGroups(ctx context.Context) error {
 	return nil
 }
 
+func (s *Service) deleteBootstrapInstanceGroups(ctx context.Context) error {
+	log := log.FromContext(ctx)
+	for zone := range s.scope.Network().APIInternalBootstrapInstanceGroup {
+		key := meta.ZonalKey(bootstrapInstanceGroup, zone)
+		log.V(2).Info("Deleting bootstrap instancegroup")
+		if err := s.instancegroups.Delete(ctx, key); err != nil {
+			if !gcperrors.IsNotFound(err) {
+				log.Error(err, "Error deleting bootstrap instancegroup")
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 // getSubnet gets the subnet to use for an internal Load Balancer.
 func (s *Service) getSubnet(ctx context.Context) (*compute.Subnetwork, error) {
 	log := log.FromContext(ctx)

cloud/services/compute/loadbalancers/reconcile_test.go

@@ -713,3 +713,61 @@ func TestService_createOrGetRegionalForwardingRule(t *testing.T) {
 		})
 	}
 }
+
+func TestService_addBootstrapInstanceGroup(t *testing.T) {
+	tests := []struct {
+		name              string
+		scope             func(s *scope.ClusterScope) Scope
+		mockInstanceGroup *cloud.MockInstanceGroups
+		inputGroups       []*compute.InstanceGroup
+		want              []*compute.InstanceGroup
+		wantErr           bool
+	}{
+		{
+			name:  "bootstrap instanceGroup does not exist (should create bootstrap instanceGroup)",
+			scope: func(s *scope.ClusterScope) Scope { return s },
+			mockInstanceGroup: &cloud.MockInstanceGroups{
+				ProjectRouter: &cloud.SingleProjectRouter{ID: "proj-id"},
+				Objects:       map[meta.Key]*cloud.MockInstanceGroupsObj{},
+			},
+			inputGroups: []*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",
+				},
+			},
+			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",
+				},
+				{
+					Name:       "my-cluster-bootstrap",
+					NamedPorts: []*compute.NamedPort{{Name: "bootstrap", Port: 6443}},
+					SelfLink:   "https://www.googleapis.com/compute/v1/projects/proj-id/zones/us-central1-a/instanceGroups/my-cluster-bootstrap",
+				},
+			},
+		},
+	}
+	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.addBootstrapInstanceGroup(ctx, tt.inputGroups)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Service s.addBootstrapInstanceGroup() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if d := cmp.Diff(tt.want, got); d != "" {
+				t.Errorf("Service s.addBootstrapInstanceGroup() mismatch (-want +got):\n%s", d)
+			}
+		})
+	}
+}

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

@@ -122,6 +122,17 @@ spec:
                     description: InternalLoadBalancer is the configuration for an
                       Internal Passthrough Network Load Balancer.
                     properties:
+                      createBootstrapInstanceGroup:
+                        description: |-
+                          CreateBootstrapInstanceGroup: When set to true, an InstanceGroup is created
+                          will be created named <cluster-id>-boostrap. This InstanceGroup will be used
+                          by the bootstrap instance.
+                          When set to false, no bootstrap InstanceGroup will be created. If a bootstrap
+                          instance is created, it will be added to an existing InstanceGroup.
+
+
+                          Defaults to false.
+                        type: boolean
                       name:
                         description: |-
                           Name is the name of the Load Balancer. If not set a default name
@@ -330,6 +341,13 @@ spec:
                       APIInternalBackendService is the full reference to the backend service
                       created for the internal Load Balancer.
                     type: string
+                  apiInternalBootstrapInstanceGroup:
+                    additionalProperties:
+                      type: string
+                    description: |-
+                      APIInternalBootstrapInstanceGroup is a map from zone to the full reference
+                      to the bootstrap instance group created in the same zone.
+                    type: object
                   apiInternalForwardingRule:
                     description: |-
                       APIInternalForwardingRule is the full reference to the forwarding rule

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

@@ -138,6 +138,17 @@ spec:
                             description: InternalLoadBalancer is the configuration
                               for an Internal Passthrough Network Load Balancer.
                             properties:
+                              createBootstrapInstanceGroup:
+                                description: |-
+                                  CreateBootstrapInstanceGroup: When set to true, an InstanceGroup is created
+                                  will be created named <cluster-id>-boostrap. This InstanceGroup will be used
+                                  by the bootstrap instance.
+                                  When set to false, no bootstrap InstanceGroup will be created. If a bootstrap
+                                  instance is created, it will be added to an existing InstanceGroup.
+
+
+                                  Defaults to false.
+                                type: boolean
                               name:
                                 description: |-
                                   Name is the name of the Load Balancer. If not set a default name