Skip to content

Commit

Permalink
Merge pull request kubernetes#107296 from cezarygerard/smart-ig-ilb
Browse files Browse the repository at this point in the history
GCE L4 load balancer: enable migration of Instance Group management out of K/K.
  • Loading branch information
k8s-ci-robot authored Feb 9, 2022
2 parents 97e20b4 + 3f5a875 commit 0d46aee
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 9 deletions.
4 changes: 4 additions & 0 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_alpha.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ const (
// AlphaFeatureILBSubsets allows InternalLoadBalancer services to include a subset
// of cluster nodes as backends instead of all nodes.
AlphaFeatureILBSubsets = "ILBSubsets"

// AlphaFeatureSkipIGsManagement enabled L4 Regional Backend Services and
// disables instance group management in service controller
AlphaFeatureSkipIGsManagement = "SkipIGsManagement"
)

// AlphaFeatureGate contains a mapping of alpha features to whether they are enabled
Expand Down
10 changes: 10 additions & 0 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_instancegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ func (g *Cloud) DeleteInstanceGroup(name string, zone string) error {
return mc.Observe(g.c.InstanceGroups().Delete(ctx, meta.ZonalKey(name, zone)))
}

// FilterInstanceGroupsByName lists all InstanceGroups in the project and
// zone that match the name regexp.
func (g *Cloud) FilterInstanceGroupsByNamePrefix(namePrefix, zone string) ([]*compute.InstanceGroup, error) {
ctx, cancel := cloud.ContextWithCallTimeout()
defer cancel()
mc := newInstanceGroupMetricContext("filter", zone)
v, err := g.c.InstanceGroups().List(ctx, zone, filter.Regexp("name", namePrefix+".*"))
return v, mc.Observe(err)
}

// ListInstanceGroups lists all InstanceGroups in the project and
// zone.
func (g *Cloud) ListInstanceGroups(zone string) ([]*compute.InstanceGroup, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -625,11 +625,21 @@ func (g *Cloud) ensureInternalInstanceGroups(name string, nodes []*v1.Node) ([]s
klog.V(2).Infof("ensureInternalInstanceGroups(%v): %d nodes over %d zones in region %v", name, len(nodes), len(zonedNodes), g.region)
var igLinks []string
for zone, nodes := range zonedNodes {
igLink, err := g.ensureInternalInstanceGroup(name, zone, nodes)
if err != nil {
return []string{}, err
if g.AlphaFeatureGate.Enabled(AlphaFeatureSkipIGsManagement) {
igs, err := g.FilterInstanceGroupsByNamePrefix(name, zone)
if err != nil {
return nil, err
}
for _, ig := range igs {
igLinks = append(igLinks, ig.SelfLink)
}
} else {
igLink, err := g.ensureInternalInstanceGroup(name, zone, nodes)
if err != nil {
return nil, err
}
igLinks = append(igLinks, igLink)
}
igLinks = append(igLinks, igLink)
}

return igLinks, nil
Expand All @@ -642,10 +652,13 @@ func (g *Cloud) ensureInternalInstanceGroupsDeleted(name string) error {
return err
}

klog.V(2).Infof("ensureInternalInstanceGroupsDeleted(%v): attempting delete instance group in all %d zones", name, len(zones))
for _, z := range zones {
if err := g.DeleteInstanceGroup(name, z.Name); err != nil && !isNotFoundOrInUse(err) {
return err
// Skip Instance Group deletion if IG management was moved out of k/k code
if !g.AlphaFeatureGate.Enabled(AlphaFeatureSkipIGsManagement) {
klog.V(2).Infof("ensureInternalInstanceGroupsDeleted(%v): attempting delete instance group in all %d zones", name, len(zones))
for _, z := range zones {
if err := g.DeleteInstanceGroup(name, z.Name); err != nil && !isNotFoundOrInUse(err) {
return err
}
}
}
return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"context"
"fmt"
"reflect"
"sort"
"strings"
"testing"

Expand Down Expand Up @@ -165,12 +166,39 @@ func TestEnsureInternalInstanceGroupsLimit(t *testing.T) {
igName := makeInstanceGroupName(vals.ClusterID)
_, err = gce.ensureInternalInstanceGroups(igName, nodes)
require.NoError(t, err)

instances, err := gce.ListInstancesInInstanceGroup(igName, vals.ZoneName, allInstances)
require.NoError(t, err)
assert.Equal(t, maxInstancesPerInstanceGroup, len(instances))
}

func TestEnsureMultipleInstanceGroups(t *testing.T) {
t.Parallel()

vals := DefaultTestClusterValues()
gce, err := fakeGCECloud(vals)
require.NoError(t, err)
gce.AlphaFeatureGate = NewAlphaFeatureGate([]string{AlphaFeatureSkipIGsManagement})

nodes, err := createAndInsertNodes(gce, []string{"n1"}, vals.ZoneName)
require.NoError(t, err)

baseName := makeInstanceGroupName(vals.ClusterID)
clusterIGs := []string{baseName, baseName + "-1", baseName + "-2", baseName + "-3"}
for _, igName := range append(clusterIGs, "zz-another-ig", "k8s-ig--cluster2-id") {
ig := &compute.InstanceGroup{Name: igName}
err := gce.CreateInstanceGroup(ig, vals.ZoneName)
require.NoError(t, err)
}

igsFromCloud, err := gce.ensureInternalInstanceGroups(baseName, nodes)
require.NoError(t, err)
assert.Len(t, igsFromCloud, len(clusterIGs), "Incorrect number of Instance Groups")
sort.Strings(igsFromCloud)
for i, igName := range clusterIGs {
assert.True(t, strings.HasSuffix(igsFromCloud[i], igName))
}
}

func TestEnsureInternalLoadBalancer(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -525,6 +553,29 @@ func TestEnsureInternalLoadBalancerDeleted(t *testing.T) {
assertInternalLbResourcesDeleted(t, gce, svc, vals, true)
}

func TestSkipInstanceGroupDeletion(t *testing.T) {
t.Parallel()

vals := DefaultTestClusterValues()
gce, err := fakeGCECloud(vals)
require.NoError(t, err)

svc := fakeLoadbalancerService(string(LBTypeInternal))
svc, err = gce.client.CoreV1().Services(svc.Namespace).Create(context.TODO(), svc, metav1.CreateOptions{})
require.NoError(t, err)
_, err = createInternalLoadBalancer(gce, svc, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName)
assert.NoError(t, err)

gce.AlphaFeatureGate = NewAlphaFeatureGate([]string{AlphaFeatureSkipIGsManagement})
err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, svc)
assert.NoError(t, err)

igName := makeInstanceGroupName(vals.ClusterID)
ig, err := gce.GetInstanceGroup(igName, vals.ZoneName)
assert.NoError(t, err)
assert.NotNil(t, ig, "Instance group should not be deleted when flag 'NetLB_RBS' is present")
}

func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 0d46aee

Please sign in to comment.