diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index e0d154aa0..55b84187e 100644 --- a/api/v1beta1/types.go +++ b/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 + // of 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 + // with the name -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"` } diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 78c0f8571..39706c43b 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -687,6 +687,11 @@ func (in *LoadBalancer) DeepCopyInto(out *LoadBalancer) { *out = new(string) **out = **in } + if in.CreateBootstrapInstanceGroup != nil { + in, out := &in.CreateBootstrapInstanceGroup, &out.CreateBootstrapInstanceGroup + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancer. @@ -836,6 +841,13 @@ func (in *Network) DeepCopyInto(out *Network) { *out = new(string) **out = **in } + if in.APIInternalBootstrapInstanceGroup != nil { + in, out := &in.APIInternalBootstrapInstanceGroup, &out.APIInternalBootstrapInstanceGroup + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Network. diff --git a/cloud/interfaces.go b/cloud/interfaces.go index e2652a3c0..fe39dc5e6 100644 --- a/cloud/interfaces.go +++ b/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) diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index 695160bd1..dfc0963b9 100644 --- a/cloud/scope/machine.go +++ b/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) diff --git a/cloud/services/compute/instances/reconcile.go b/cloud/services/compute/instances/reconcile.go index 76277d072..d5bac4f22 100644 --- a/cloud/services/compute/instances/reconcile.go +++ b/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{ diff --git a/cloud/services/compute/instances/service.go b/cloud/services/compute/instances/service.go index a5a415d52..96759c4c0 100644 --- a/cloud/services/compute/instances/service.go +++ b/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. diff --git a/cloud/services/compute/loadbalancers/reconcile.go b/cloud/services/compute/loadbalancers/reconcile.go index 86280e7a9..f169aad84 100644 --- a/cloud/services/compute/loadbalancers/reconcile.go +++ b/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,14 @@ func (s *Service) Reconcile(ctx context.Context) error { } lbSpec := s.scope.LoadBalancer() + 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 +175,15 @@ func (s *Service) deleteInternalLoadBalancer(ctx context.Context, name string) e } s.scope.Network().APIInternalHealthCheck = nil + lbSpec := s.scope.LoadBalancer() + 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 +316,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 +816,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 +830,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) diff --git a/cloud/services/compute/loadbalancers/reconcile_test.go b/cloud/services/compute/loadbalancers/reconcile_test.go index 3572ae1ea..be48e613d 100644 --- a/cloud/services/compute/loadbalancers/reconcile_test.go +++ b/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) + } + }) + } +} diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclusters.yaml index 2b955cceb..7ea434ef5 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclusters.yaml +++ b/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 -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 diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yaml index f0e64db25..4f97a7184 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yaml +++ b/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 -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 diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml index 9dcdde341..dcc6b3a43 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml @@ -117,6 +117,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 -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 @@ -372,6 +383,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