From 114dbd3575680229474a7f2d88b0d93d21c6ade5 Mon Sep 17 00:00:00 2001 From: bprashanth Date: Tue, 9 Aug 2016 18:27:45 -0700 Subject: [PATCH] Don't clobber backends inserted by other controllers. --- controllers/gce/backends/backends.go | 11 ++++- controllers/gce/backends/backends_test.go | 60 +++++++++++++++++++++-- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/controllers/gce/backends/backends.go b/controllers/gce/backends/backends.go index 84b029ea9..0b15b21f0 100644 --- a/controllers/gce/backends/backends.go +++ b/controllers/gce/backends/backends.go @@ -220,12 +220,19 @@ func (b *Backends) edgeHop(be *compute.BackendService, igs []*compute.InstanceGr for _, igToBE := range igs { igLinks.Insert(igToBE.SelfLink) } - if igLinks.Equal(beIGs) { + if beIGs.IsSuperset(igLinks) { return nil } glog.Infof("Backend %v has a broken edge, expected igs %+v, current igs %+v", be.Name, igLinks.List(), beIGs.List()) - be.Backends = getBackendsForIGs(igs) + + newBackends := []*compute.Backend{} + for _, b := range getBackendsForIGs(igs) { + if !beIGs.Has(b.Group) { + newBackends = append(newBackends, b) + } + } + be.Backends = append(be.Backends, newBackends...) if err := b.cloud.UpdateBackendService(be); err != nil { return err } diff --git a/controllers/gce/backends/backends_test.go b/controllers/gce/backends/backends_test.go index 65ffbbb7e..4f4a1ece5 100644 --- a/controllers/gce/backends/backends_test.go +++ b/controllers/gce/backends/backends_test.go @@ -75,7 +75,9 @@ func TestBackendPoolAdd(t *testing.T) { // This simulates a user doing foolish things through the UI. f.calls = []int{} be, err = f.GetBackendService(beName) - be.Backends[0].Group = "test edge hop" + be.Backends = []*compute.Backend{ + {Group: "test edge hop"}, + } f.UpdateBackendService(be) pool.Add(nodePort) @@ -92,10 +94,14 @@ func TestBackendPoolAdd(t *testing.T) { if err != nil { t.Fatalf("Failed to find instance group %v", namer.IGName()) } - if gotBackend.Backends[0].Group != gotGroup.SelfLink { + backendLinks := sets.NewString() + for _, be := range gotBackend.Backends { + backendLinks.Insert(be.Group) + } + if !backendLinks.Has(gotGroup.SelfLink) { t.Fatalf( - "Broken instance group link: %v %v", - gotBackend.Backends[0].Group, + "Broken instance group link, got: %+v expected: %v", + backendLinks.List(), gotGroup.SelfLink) } } @@ -178,5 +184,49 @@ func TestBackendPoolShutdown(t *testing.T) { if _, err := f.GetBackendService(namer.BeName(80)); err == nil { t.Fatalf("%v", err) } - +} + +func TestBackendInstanceGroupClobbering(t *testing.T) { + f := NewFakeBackendServices() + fakeIGs := instances.NewFakeInstanceGroups(sets.NewString()) + pool := newBackendPool(f, fakeIGs, false) + namer := utils.Namer{} + + // This will add the instance group k8s-ig to the instance pool + pool.Add(80) + + be, err := f.GetBackendService(namer.BeName(80)) + if err != nil { + t.Fatalf("%v", err) + } + // Simulate another controller updating the same backend service with + // a different instance group + newGroups := []*compute.Backend{ + {Group: "k8s-ig-bar"}, + {Group: "k8s-ig-foo"}, + } + be.Backends = append(be.Backends, newGroups...) + if err := f.UpdateBackendService(be); err != nil { + t.Fatalf("Failed to update backend service %v", be.Name) + } + + // Make sure repeated adds don't clobber the inserted instance group + pool.Add(80) + be, err = f.GetBackendService(namer.BeName(80)) + if err != nil { + t.Fatalf("%v", err) + } + gotGroups := sets.NewString() + for _, g := range be.Backends { + gotGroups.Insert(g.Group) + } + + // seed expectedGroups with the first group native to this controller + expectedGroups := sets.NewString("k8s-ig") + for _, newGroup := range newGroups { + expectedGroups.Insert(newGroup.Group) + } + if !expectedGroups.Equal(gotGroups) { + t.Fatalf("Expected %v Got %v", expectedGroups, gotGroups) + } }