Skip to content

Commit

Permalink
Support IG backends removal in regional_ig_linker.go.
Browse files Browse the repository at this point in the history
  • Loading branch information
@mmamczur
mmamczur committed Apr 6, 2023
1 parent 6397325 commit 558cb4d
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 26 deletions.
13 changes: 7 additions & 6 deletions pkg/backends/ig_linker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (igl *instanceGroupLinker) Link(sp utils.ServicePort, groups []GroupKey) er
return err
}

addIGs, err := getInstanceGroupsToAdd(be, igLinks)
addIGs, _, err := getInstanceGroupsToAddAndRemove(be, igLinks)
if err != nil {
return err
}
Expand Down Expand Up @@ -167,12 +167,12 @@ func backendsForIGs(igLinks []string, bm BalancingMode) []*composite.Backend {
return backends
}

func getInstanceGroupsToAdd(be *composite.BackendService, igLinks []string) ([]string, error) {
func getInstanceGroupsToAddAndRemove(be *composite.BackendService, igLinks []string) ([]string, sets.String, error) {
existingIGs := sets.String{}
for _, existingBe := range be.Backends {
path, err := utils.RelativeResourceName(existingBe.Group)
if err != nil {
return nil, fmt.Errorf("failed to parse instance group: %w", err)
return nil, nil, fmt.Errorf("failed to parse instance group: %w", err)
}
existingIGs.Insert(path)
}
Expand All @@ -181,15 +181,16 @@ func getInstanceGroupsToAdd(be *composite.BackendService, igLinks []string) ([]s
for _, igLink := range igLinks {
path, err := utils.RelativeResourceName(igLink)
if err != nil {
return nil, fmt.Errorf("failed to parse instance group: %w", err)
return nil, nil, fmt.Errorf("failed to parse instance group: %w", err)
}
wantIGs.Insert(path)
}

missingIGs := wantIGs.Difference(existingIGs)
if missingIGs.Len() > 0 {
removeIGs := existingIGs.Difference(wantIGs)
if missingIGs.Len() > 0 || removeIGs.Len() > 0 {
klog.V(2).Infof("Backend service %q has instance groups %+v, want %+v",
be.Name, existingIGs.List(), wantIGs.List())
}
return missingIGs.List(), nil
return missingIGs.List(), removeIGs, nil
}
46 changes: 29 additions & 17 deletions pkg/backends/ig_linker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,62 +183,71 @@ func TestBackendsForIG(t *testing.T) {
}
}

func TestGetIGLinksToAdd(t *testing.T) {
func TestGetIGLinksToAddAndRemove(t *testing.T) {
url := "https://googleapis.com/v1/compute/projects/my-project/global/backendServices/%s"
link := "projects/my-project/global/backendServices/%s"
for _, tc := range []struct {
name string
igLinks []string
currentIGLinks []string
want []string
wantAdd []string
wantRemove []string
}{
{
name: "empty",
igLinks: []string{},
currentIGLinks: []string{},
want: []string{},
wantAdd: []string{},
wantRemove: []string{},
},
{
name: "No IGs to add",
igLinks: []string{fmt.Sprintf(url, "same-backend")},
currentIGLinks: []string{fmt.Sprintf(url, "same-backend")},
want: []string{},
wantAdd: []string{},
wantRemove: []string{},
},
{
name: "same IGs in wrong order",
igLinks: []string{fmt.Sprintf(url, "same-backend2"), fmt.Sprintf(url, "same-backend")},
currentIGLinks: []string{fmt.Sprintf(url, "same-backend"), fmt.Sprintf(url, "same-backend2")},
want: []string{},
wantAdd: []string{},
wantRemove: []string{},
},
{
name: "one IG to add",
igLinks: []string{fmt.Sprintf(url, "same-backend"), fmt.Sprintf(url, "same-backend2"), fmt.Sprintf(url, "same-backend3")},
currentIGLinks: []string{fmt.Sprintf(url, "same-backend"), fmt.Sprintf(url, "same-backend2")},
want: []string{fmt.Sprintf(link, "same-backend3")},
wantAdd: []string{fmt.Sprintf(link, "same-backend3")},
wantRemove: []string{},
},
{
name: "IGs in wrong order",
igLinks: []string{fmt.Sprintf(url, "same-backend2"), fmt.Sprintf(url, "same-backend")},
currentIGLinks: []string{fmt.Sprintf(url, "same-backend3"), fmt.Sprintf(url, "same-backend2")},
want: []string{fmt.Sprintf(link, "same-backend")},
wantAdd: []string{fmt.Sprintf(link, "same-backend")},
wantRemove: []string{fmt.Sprintf(link, "same-backend3")},
},
{
name: "different IGs",
igLinks: []string{fmt.Sprintf(url, "same-backend"), fmt.Sprintf(url, "same-backend2")},
currentIGLinks: []string{fmt.Sprintf(url, "same-backend3"), fmt.Sprintf(url, "same-backend4")},
want: []string{fmt.Sprintf(link, "same-backend"), fmt.Sprintf(link, "same-backend2")},
wantAdd: []string{fmt.Sprintf(link, "same-backend"), fmt.Sprintf(link, "same-backend2")},
wantRemove: []string{fmt.Sprintf(link, "same-backend3"), fmt.Sprintf(link, "same-backend4")},
},
{
name: "empty current",
igLinks: []string{fmt.Sprintf(url, "same-backend"), fmt.Sprintf(url, "same-backend2")},
currentIGLinks: []string{},
want: []string{fmt.Sprintf(link, "same-backend"), fmt.Sprintf(link, "same-backend2")},
wantAdd: []string{fmt.Sprintf(link, "same-backend"), fmt.Sprintf(link, "same-backend2")},
wantRemove: []string{},
},
{
name: "empty IGs and non-empty current",
igLinks: []string{},
currentIGLinks: []string{fmt.Sprintf(url, "same-backend"), fmt.Sprintf(url, "same-backend2")},
want: []string{},
wantAdd: []string{},
wantRemove: []string{fmt.Sprintf(link, "same-backend"), fmt.Sprintf(link, "same-backend2")},
},
} {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -251,15 +260,18 @@ func TestGetIGLinksToAdd(t *testing.T) {
newBackends = append(newBackends, b)
}
be.Backends = newBackends
toAdd, err := getInstanceGroupsToAdd(&be, tc.igLinks)
toAdd, toRemove, err := getInstanceGroupsToAddAndRemove(&be, tc.igLinks)
if err != nil {
t.Fatalf("getInstanceGroupsToAdd(_,_): err:%v ", err)
t.Fatalf("getInstanceGroupsToAddAndRemove(_,_): err:%v ", err)
}
sort.Slice(toAdd, func(i, j int) bool {
return toAdd[i] <= toAdd[j]
})
if !reflect.DeepEqual(toAdd, tc.want) {
t.Fatalf("getInstanceGroupsToAdd(_,_) error. Got:%v, Want:%v", toAdd, tc.want)
sort.Strings(toAdd)
if !reflect.DeepEqual(toAdd, tc.wantAdd) {
t.Fatalf("getInstanceGroupsToAddAndRemove(_,_) toAdd error. Got:%v, Want:%v", toAdd, tc.wantAdd)
}
toRemoveList := toRemove.List()
sort.Strings(tc.wantRemove)
if !reflect.DeepEqual(toRemoveList, tc.wantRemove) {
t.Fatalf("getInstanceGroupsToAddAndRemove(_,_) toRemove error. Got:%v, Want:%v", toRemoveList, tc.wantRemove)
}
})
}
Expand Down
20 changes: 17 additions & 3 deletions pkg/backends/regional_ig_linker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,29 @@ func (linker *RegionalInstanceGroupLinker) Link(sp utils.ServicePort, projectID
if err != nil {
return err
}
addIGs, err := getInstanceGroupsToAdd(bs, igLinks)
addIGs, removeIGs, err := getInstanceGroupsToAddAndRemove(bs, igLinks)
if err != nil {
return err
}
if len(addIGs) == 0 {
klog.V(3).Infof("No backends to add for %s, skipping update.", sp.BackendName())
if len(addIGs) == 0 && len(removeIGs) == 0 {
klog.V(3).Infof("No backends to add or remove for %s, skipping update.", sp.BackendName())
return nil
}

if len(removeIGs) != 0 {
var backendsWithoutRemoved []*composite.Backend
for _, b := range bs.Backends {
path, err := utils.RelativeResourceName(b.Group)
if err != nil {
return err
}
if !removeIGs.Has(path) {
backendsWithoutRemoved = append(backendsWithoutRemoved, b)
}
}
bs.Backends = backendsWithoutRemoved
}

for _, igLink := range addIGs {
b := &composite.Backend{
Group: igLink,
Expand Down
44 changes: 44 additions & 0 deletions pkg/backends/regional_ig_linker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,50 @@ func TestRegionalUpdateLinkWithNewBackends(t *testing.T) {
}
}

func TestRegionalUpdateLinkWithRemovedBackends(t *testing.T) {
t.Parallel()
fakeGCE := gce.NewFakeGCECloud(linkerTestClusterValues())
clusterID, _ := fakeGCE.ClusterID.GetID()
l4Namer := namer.NewL4Namer("uid1", namer.NewNamer(clusterID, ""))
sp := utils.ServicePort{NodePort: 8080, BackendNamer: l4Namer}
fakeBackendPool := NewPool(fakeGCE, l4Namer)
linker := newTestRegionalIgLinker(fakeGCE, fakeBackendPool, l4Namer)
if _, err := linker.instancePool.EnsureInstanceGroupsAndPorts(l4Namer.InstanceGroup(), []int64{sp.NodePort}); err != nil {
t.Fatalf("Unexpected error when ensuring IG for ServicePort %+v: %v", sp, err)
}
createBackendService(t, sp, fakeBackendPool)

if err := linker.Link(sp, fakeGCE.ProjectID(), []string{usCentral1AZone, usCentral1CZone}); err != nil {
t.Fatalf("Unexpected error in Link(_). Error: %v", err)
}

if err := linker.Link(sp, fakeGCE.ProjectID(), []string{usCentral1AZone}); err != nil {
t.Fatalf("Unexpected error in Link(_). Error: %v", err)
}
be, err := fakeGCE.GetRegionBackendService(sp.BackendName(), fakeGCE.Region())
if err != nil {
t.Fatalf("Get Regional Backend Service failed %v", err)
}
var backendGroups []string
for _, b := range be.Backends {
backendGroups = append(backendGroups, b.Group)
}
if len(be.Backends) != 1 {
t.Errorf("Expected that Backends are updated with the added zone with len=1 but got=%+v", strings.Join(backendGroups, ", "))
}

igA, _ := linker.instancePool.Get(sp.IGName(), usCentral1AZone)
expectedLinkA, _ := utils.RelativeResourceName(igA.SelfLink)
igC, _ := linker.instancePool.Get(sp.IGName(), usCentral1CZone)
expectedLinkC, _ := utils.RelativeResourceName(igC.SelfLink)
if !slice.ContainsString(backendGroups, expectedLinkA, nil) {
t.Errorf("expected that BackendService backend contains %v, got=%v", expectedLinkA, backendGroups)
}
if slice.ContainsString(backendGroups, expectedLinkC, nil) {
t.Errorf("expected that BackendService backend does not contain %v, got=%v", expectedLinkC, backendGroups)
}
}

func createBackendService(t *testing.T, sp utils.ServicePort, backendPool *Backends) {
t.Helper()
namespacedName := types.NamespacedName{Name: "service.Name", Namespace: "service.Namespace"}
Expand Down

0 comments on commit 558cb4d

Please sign in to comment.