From 0726effb2c3ddeb9c18afcedc5e3c3ea437a283b Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Tue, 2 Jun 2020 13:01:03 +0100 Subject: [PATCH 01/17] Emit event to orphaned vsrs --- internal/k8s/controller.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 194b325802..b731e0b5cd 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -879,6 +879,10 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { if err != nil { glog.Errorf("Error when deleting configuration for %v: %v", key, err) } + vsr := lbc.getVirtualServerRoutes() + for _, r := range vsr { + lbc.recorder.Eventf(r, api_v1.EventTypeWarning, "NoVirtualServerFound", "No VirtualServer references VirtualServerRoute %v/%v", r.Namespace, r.Name) + } return } @@ -905,6 +909,10 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } // TO-DO: emit events for referenced VirtualServerRoutes + vsr := lbc.getVirtualServerRoutes() + for _, r := range vsr { + lbc.recorder.Eventf(r, api_v1.EventTypeWarning, "NoVirtualServerFound", "No VirtualServer references VirtualServerRoute %v/%v", r.Namespace, r.Name) + } return } From ff2676f936c4b786f48a847ce92d3c0fe8001ae3 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Tue, 2 Jun 2020 14:35:01 +0100 Subject: [PATCH 02/17] Removed comment * add flow control logic --- internal/k8s/controller.go | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index b731e0b5cd..cf16f3894c 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -875,14 +875,15 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { glog.V(2).Infof("Deleting VirtualServer: %v\n", key) err := lbc.configurator.DeleteVirtualServer(key) - // TO-DO: emit events for referenced VirtualServerRoutes if err != nil { glog.Errorf("Error when deleting configuration for %v: %v", key, err) + } else { + vsr := lbc.getVirtualServerRoutes() + for _, r := range vsr { + lbc.recorder.Eventf(r, api_v1.EventTypeWarning, "NoVirtualServerFound", "No VirtualServer references VirtualServerRoute %v/%v", r.Namespace, r.Name) + } } - vsr := lbc.getVirtualServerRoutes() - for _, r := range vsr { - lbc.recorder.Eventf(r, api_v1.EventTypeWarning, "NoVirtualServerFound", "No VirtualServer references VirtualServerRoute %v/%v", r.Namespace, r.Name) - } + return } @@ -895,24 +896,13 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { err := lbc.configurator.DeleteVirtualServer(key) if err != nil { glog.Errorf("Error when deleting configuration for %v: %v", key, err) - } - reason := "Rejected" - msg := fmt.Sprintf("VirtualServer %v is invalid and was rejected: %v", key, validationErr) - - lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, reason, msg) - if lbc.reportVsVsrStatusEnabled() { - err = lbc.statusUpdater.UpdateVirtualServerStatus(vs, conf_v1.StateInvalid, reason, msg) - - if err != nil { - glog.Errorf("Error when updating the status for VirtualServer %v/%v: %v", vs.Namespace, vs.Name, err) + } else { + vsr := lbc.getVirtualServerRoutes() + for _, r := range vsr { + lbc.recorder.Eventf(r, api_v1.EventTypeWarning, "NoVirtualServerFound", "No VirtualServer references VirtualServerRoute %v/%v", r.Namespace, r.Name) } } - - // TO-DO: emit events for referenced VirtualServerRoutes - vsr := lbc.getVirtualServerRoutes() - for _, r := range vsr { - lbc.recorder.Eventf(r, api_v1.EventTypeWarning, "NoVirtualServerFound", "No VirtualServer references VirtualServerRoute %v/%v", r.Namespace, r.Name) - } + lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "Rejected", "VirtualServer %v is invalid and was rejected: %v", key, validationErr) return } From 132d9ecf2106355676ea191433866475dcef6cfe Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Tue, 16 Jun 2020 00:20:04 +0100 Subject: [PATCH 03/17] Update vsr when not referenced --- internal/k8s/controller.go | 108 +++++++++++++++++++++++++++++++++---- 1 file changed, 97 insertions(+), 11 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index cf16f3894c..5438a01e75 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -864,6 +864,7 @@ func (lbc *LoadBalancerController) syncGlobalConfiguration(task task) { } func (lbc *LoadBalancerController) syncVirtualServer(task task) { + glog.Errorf("Problem %v", 1) key := task.Key obj, vsExists, err := lbc.virtualServerLister.GetByKey(key) if err != nil { @@ -871,6 +872,23 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { return } + glog.Errorf("Problem %v", 2) + var vs *conf_v1.VirtualServer + var vsrlist []*conf_v1.VirtualServerRoute + var vsrNotRef []*conf_v1.VirtualServerRoute + vslist := lbc.getVirtualServers() + + for _, v := range vslist { + vskey := fmt.Sprintf("%s/%s", v.Namespace, v.Name) + if vskey == key { + vs = v + break + } + } + glog.Errorf("Problem %v", 3) + vsrlist = findVirtualServerRoutesForVirtualServer(vs, lbc.getVirtualServerRoutes()) + glog.Errorf("num of vsr %d", lbc.getVirtualServerRoutes()) + if !vsExists { glog.V(2).Infof("Deleting VirtualServer: %v\n", key) @@ -878,9 +896,11 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { if err != nil { glog.Errorf("Error when deleting configuration for %v: %v", key, err) } else { - vsr := lbc.getVirtualServerRoutes() - for _, r := range vsr { - lbc.recorder.Eventf(r, api_v1.EventTypeWarning, "NoVirtualServerFound", "No VirtualServer references VirtualServerRoute %v/%v", r.Namespace, r.Name) + reason := "NoVirtualServerFound" + for _, vsr := range vsrlist { + msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) + lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) + lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) } } @@ -888,19 +908,20 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } glog.V(2).Infof("Adding or Updating VirtualServer: %v\n", key) - - vs := obj.(*conf_v1.VirtualServer) + glog.Errorf("Problem %v", 4) + vs = obj.(*conf_v1.VirtualServer) validationErr := validation.ValidateVirtualServer(vs, lbc.isNginxPlus) if validationErr != nil { err := lbc.configurator.DeleteVirtualServer(key) if err != nil { glog.Errorf("Error when deleting configuration for %v: %v", key, err) - } else { - vsr := lbc.getVirtualServerRoutes() - for _, r := range vsr { - lbc.recorder.Eventf(r, api_v1.EventTypeWarning, "NoVirtualServerFound", "No VirtualServer references VirtualServerRoute %v/%v", r.Namespace, r.Name) - } + } + reason := "NoVirtualServerFound" + for _, vsr := range vsrlist { + msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) + lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) + lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) } lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "Rejected", "VirtualServer %v is invalid and was rejected: %v", key, validationErr) return @@ -940,7 +961,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { state = conf_v1.StateWarning } - msg := fmt.Sprintf("Configuration for %v was added or updated %s", key, vsEventWarningMessage) + msg := fmt.Sprintf("Configuration for %v was adddddded or updated %s", key, vsEventWarningMessage) lbc.recorder.Eventf(vs, vsEventType, vsEventTitle, msg) if lbc.reportVsVsrStatusEnabled() { @@ -951,12 +972,20 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } } + vsrRemoved := 0 + vsrlist = findAllVirtualServerRoutesForVirtualServer(vs, lbc.getVirtualServerRoutes()) + vsrNotRef = vsrlist + //lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "num vsr", "%d", len(vsrlist)) + glog.Errorf("later num of vsr %d", vsrlist) + for _, vsr := range vsEx.VirtualServerRoutes { vsrEventType := eventType vsrEventTitle := eventTitle vsrEventWarningMessage := eventWarningMessage state := conf_v1.StateValid + glog.Errorf("meta owenerreference %v", vsr.Name) + if messages, ok := warnings[vsr]; ok && addErr == nil { vsrEventType = api_v1.EventTypeWarning vsrEventTitle = "AddedOrUpdatedWithWarning" @@ -974,8 +1003,65 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) } } + + vsrkey := fmt.Sprintf("%s/%s", vsr.Namespace, vsr.Name) + glog.Errorf("outter loop vsr %v", vsrkey) + //if len(vsrlist) > 1 { + for i, r := range vsrlist { + key := fmt.Sprintf("%s/%s", r.Namespace, r.Name) + glog.Errorf("Inner loop vsr %v", key) + if key == vsrkey { + if len(vsrlist) > 1 { + vsrNotRef = append(vsrNotRef[:i-vsrRemoved], vsrNotRef[(i-vsrRemoved)+1:]...) + vsrRemoved++ + glog.Errorf("Removing %v", key) + } else { + glog.Errorf("making list empty %v", r) + vsrNotRef = []*conf_v1.VirtualServerRoute{} + } + } + } + /*} else { + glog.Errorf("less than or equal to %s", 1) + if len(vsrlist) == 1 { + key = fmt.Sprintf("%s/%s", vsrlist[0].Namespace, vsrlist[0].Name) + glog.Errorf("%s", key) + } + if key == vsrkey { + vsrNotRef = []*conf_v1.VirtualServerRoute{} + } + }*/ } + glog.Errorf("Number of not reference : %d", len(vsrNotRef)) + reason := "Ignored" + for _, vsr := range vsrNotRef { + msg := fmt.Sprintf("Ignored by VirtualServer %v/%v", vs.Namespace, vs.Name) + lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "Ignored", msg) + lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) + } +} +func findAllVirtualServerRoutesForVirtualServer(vs *conf_v1.VirtualServer, vsrs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { + var result []*conf_v1.VirtualServerRoute + key := fmt.Sprintf("%s/%s", vs.Namespace, vs.Name) + for _, r := range vsrs { + if r.Status.ReferencedBy == key { + result = append(result, r) + } + } + return result +} + +func findVirtualServerRoutesForVirtualServer(virtualserver *conf_v1.VirtualServer, virtualServerRoutes []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { + var result []*conf_v1.VirtualServerRoute + vslist := []*conf_v1.VirtualServer{virtualserver} + for _, vsr := range virtualServerRoutes { + key := fmt.Sprintf("%s/%s", vsr.Namespace, vsr.Name) + if len(findVirtualServersForVirtualServerRouteKey(vslist, key)) != 0 { + result = append(result, vsr) + } + } + return result } func (lbc *LoadBalancerController) syncVirtualServerRoute(task task) { From be8aa9f29e4629adbc421b05040a7e68ba6a06c4 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Tue, 16 Jun 2020 10:23:21 +0100 Subject: [PATCH 04/17] Removed comments and unneeeded code --- internal/k8s/controller.go | 35 ++++------------------------------- 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 5438a01e75..8ee02d81ec 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -864,7 +864,6 @@ func (lbc *LoadBalancerController) syncGlobalConfiguration(task task) { } func (lbc *LoadBalancerController) syncVirtualServer(task task) { - glog.Errorf("Problem %v", 1) key := task.Key obj, vsExists, err := lbc.virtualServerLister.GetByKey(key) if err != nil { @@ -872,12 +871,9 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { return } - glog.Errorf("Problem %v", 2) var vs *conf_v1.VirtualServer - var vsrlist []*conf_v1.VirtualServerRoute - var vsrNotRef []*conf_v1.VirtualServerRoute - vslist := lbc.getVirtualServers() + vslist := lbc.getVirtualServers() for _, v := range vslist { vskey := fmt.Sprintf("%s/%s", v.Namespace, v.Name) if vskey == key { @@ -885,9 +881,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { break } } - glog.Errorf("Problem %v", 3) - vsrlist = findVirtualServerRoutesForVirtualServer(vs, lbc.getVirtualServerRoutes()) - glog.Errorf("num of vsr %d", lbc.getVirtualServerRoutes()) + vsrlist := findVirtualServerRoutesForVirtualServer(vs, lbc.getVirtualServerRoutes()) if !vsExists { glog.V(2).Infof("Deleting VirtualServer: %v\n", key) @@ -908,7 +902,6 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } glog.V(2).Infof("Adding or Updating VirtualServer: %v\n", key) - glog.Errorf("Problem %v", 4) vs = obj.(*conf_v1.VirtualServer) validationErr := validation.ValidateVirtualServer(vs, lbc.isNginxPlus) @@ -961,7 +954,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { state = conf_v1.StateWarning } - msg := fmt.Sprintf("Configuration for %v was adddddded or updated %s", key, vsEventWarningMessage) + msg := fmt.Sprintf("Configuration for %v was added or updated %s", key, vsEventWarningMessage) lbc.recorder.Eventf(vs, vsEventType, vsEventTitle, msg) if lbc.reportVsVsrStatusEnabled() { @@ -974,9 +967,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { vsrRemoved := 0 vsrlist = findAllVirtualServerRoutesForVirtualServer(vs, lbc.getVirtualServerRoutes()) - vsrNotRef = vsrlist - //lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "num vsr", "%d", len(vsrlist)) - glog.Errorf("later num of vsr %d", vsrlist) + vsrNotRef := vsrlist for _, vsr := range vsEx.VirtualServerRoutes { vsrEventType := eventType @@ -984,8 +975,6 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { vsrEventWarningMessage := eventWarningMessage state := conf_v1.StateValid - glog.Errorf("meta owenerreference %v", vsr.Name) - if messages, ok := warnings[vsr]; ok && addErr == nil { vsrEventType = api_v1.EventTypeWarning vsrEventTitle = "AddedOrUpdatedWithWarning" @@ -1005,34 +994,18 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } vsrkey := fmt.Sprintf("%s/%s", vsr.Namespace, vsr.Name) - glog.Errorf("outter loop vsr %v", vsrkey) - //if len(vsrlist) > 1 { for i, r := range vsrlist { key := fmt.Sprintf("%s/%s", r.Namespace, r.Name) - glog.Errorf("Inner loop vsr %v", key) if key == vsrkey { if len(vsrlist) > 1 { vsrNotRef = append(vsrNotRef[:i-vsrRemoved], vsrNotRef[(i-vsrRemoved)+1:]...) vsrRemoved++ - glog.Errorf("Removing %v", key) } else { - glog.Errorf("making list empty %v", r) vsrNotRef = []*conf_v1.VirtualServerRoute{} } } } - /*} else { - glog.Errorf("less than or equal to %s", 1) - if len(vsrlist) == 1 { - key = fmt.Sprintf("%s/%s", vsrlist[0].Namespace, vsrlist[0].Name) - glog.Errorf("%s", key) - } - if key == vsrkey { - vsrNotRef = []*conf_v1.VirtualServerRoute{} - } - }*/ } - glog.Errorf("Number of not reference : %d", len(vsrNotRef)) reason := "Ignored" for _, vsr := range vsrNotRef { msg := fmt.Sprintf("Ignored by VirtualServer %v/%v", vs.Namespace, vs.Name) From c08208a362bd50da04c9ecdc2b278aa3386491b2 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Wed, 17 Jun 2020 09:49:19 +0100 Subject: [PATCH 05/17] Add tests --- internal/k8s/controller_test.go | 120 ++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index feda7a6181..1d2e15a2d3 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -1382,6 +1382,126 @@ func TestFindVirtualServerRoutesForService(t *testing.T) { } } +func TestFindVirtualServerRoutesForVirtualServer(t *testing.T) { + vsr1 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-1", + Namespace: "ns-1", + }, + } + + vsr2 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-2", + Namespace: "ns-1", + }, + } + + vsr3 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-3", + Namespace: "ns-2", + }, + } + + vsr4 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-4", + Namespace: "ns-1", + }, + } + + virtualserverroutes := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr3, &vsr4} + + vs1 := conf_v1.VirtualServer{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vs-1", + Namespace: "ns-1", + }, + Spec: conf_v1.VirtualServerSpec{ + Routes: []conf_v1.Route{ + { + Route: "ns-1/vsr-1", + }, + { + Route: "ns-1/vsr-2", + }, + { + Route: "ns-1/vsr-4", + }, + }, + }, + } + + expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr4} + + result := findVirtualServerRoutesForVirtualServer(&vs1, virtualserverroutes) + + if !reflect.DeepEqual(result, expected) { + t.Errorf("findVirtualServerRoutesForVirtualServer returned %v but expected %v", result, expected) + } + +} + +func TestFindAllVirtualServerRoutesForVirtualServer(t *testing.T) { + + vsr1 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-1", + Namespace: "ns-1", + }, + Status: conf_v1.VirtualServerRouteStatus{ + State: "Valid", + Reason: "AddedOrUpdated", + Message: fmt.Sprint("Configuration for ns-1/vs-1 was added or updated"), + ReferencedBy: "ns-1/vs-1", + }, + } + + vsr2 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-2", + Namespace: "ns-1", + }, + Status: conf_v1.VirtualServerRouteStatus{ + State: "InValid", + Reason: "Ignored", + Message: fmt.Sprint("Ignored by VirtualServer ns-1/vs-1"), + ReferencedBy: "ns-1/vs-1", + }, + } + + vsr3 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-3", + Namespace: "ns-1", + }, + Status: conf_v1.VirtualServerRouteStatus{ + State: "InValid", + Reason: "NoVirtualServerFound", + Message: fmt.Sprint("No VirtualServer references VirtualServerRoute vsr-3/ns-1"), + ReferencedBy: "", + }, + } + + virtualserverroutes := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr3} + + expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2} + + vs1 := conf_v1.VirtualServer{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vs-1", + Namespace: "ns-1", + }, + } + + result := findAllVirtualServerRoutesForVirtualServer(&vs1, virtualserverroutes) + + if !reflect.DeepEqual(result, expected) { + t.Errorf("findAllVirtualServerRoutes return %v but expected %v", result, expected) + } +} + func TestFindTransportServersForService(t *testing.T) { ts1 := conf_v1alpha1.TransportServer{ ObjectMeta: meta_v1.ObjectMeta{ From 2cff0cb4b8872fdd0877ff430e0e5d1821333525 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Wed, 17 Jun 2020 16:10:04 +0100 Subject: [PATCH 06/17] Feedback --- internal/k8s/controller.go | 48 +++++++++++++++---------- internal/k8s/controller_test.go | 62 ++++++++++++++++++++++++++++----- 2 files changed, 83 insertions(+), 27 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 8ee02d81ec..80d5be6fe1 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -914,7 +914,10 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { for _, vsr := range vsrlist { msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) - lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) + err := lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) + if err != nil { + glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + } } lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "Rejected", "VirtualServer %v is invalid and was rejected: %v", key, validationErr) return @@ -965,9 +968,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } } - vsrRemoved := 0 vsrlist = findAllVirtualServerRoutesForVirtualServer(vs, lbc.getVirtualServerRoutes()) - vsrNotRef := vsrlist for _, vsr := range vsEx.VirtualServerRoutes { vsrEventType := eventType @@ -994,26 +995,35 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } vsrkey := fmt.Sprintf("%s/%s", vsr.Namespace, vsr.Name) - for i, r := range vsrlist { - key := fmt.Sprintf("%s/%s", r.Namespace, r.Name) - if key == vsrkey { - if len(vsrlist) > 1 { - vsrNotRef = append(vsrNotRef[:i-vsrRemoved], vsrNotRef[(i-vsrRemoved)+1:]...) - vsrRemoved++ - } else { - vsrNotRef = []*conf_v1.VirtualServerRoute{} - } - } - } + vsrlist = checkForVirtualServerRoute(vsrkey, vsrlist) } reason := "Ignored" - for _, vsr := range vsrNotRef { + for _, vsr := range vsrlist { msg := fmt.Sprintf("Ignored by VirtualServer %v/%v", vs.Namespace, vs.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "Ignored", msg) - lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) + err := lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) + if err != nil { + glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + } } } +func checkForVirtualServerRoute(vsrkey string, vsrlist []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { + var vsrNotRef []*conf_v1.VirtualServerRoute + for i, r := range vsrlist { + key := fmt.Sprintf("%s/%s", r.Namespace, r.Name) + if key == vsrkey { + if len(vsrlist) > 1 { + vsrNotRef = append(vsrlist[:i], vsrlist[i+1:]...) + } else { + vsrNotRef = []*conf_v1.VirtualServerRoute{} + } + } + } + + return vsrNotRef +} + func findAllVirtualServerRoutesForVirtualServer(vs *conf_v1.VirtualServer, vsrs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { var result []*conf_v1.VirtualServerRoute key := fmt.Sprintf("%s/%s", vs.Namespace, vs.Name) @@ -1025,10 +1035,10 @@ func findAllVirtualServerRoutesForVirtualServer(vs *conf_v1.VirtualServer, vsrs return result } -func findVirtualServerRoutesForVirtualServer(virtualserver *conf_v1.VirtualServer, virtualServerRoutes []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { +func findVirtualServerRoutesForVirtualServer(vs *conf_v1.VirtualServer, vsrs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { var result []*conf_v1.VirtualServerRoute - vslist := []*conf_v1.VirtualServer{virtualserver} - for _, vsr := range virtualServerRoutes { + vslist := []*conf_v1.VirtualServer{vs} + for _, vsr := range vsrs { key := fmt.Sprintf("%s/%s", vsr.Namespace, vsr.Name) if len(findVirtualServersForVirtualServerRouteKey(vslist, key)) != 0 { result = append(result, vsr) diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 1d2e15a2d3..d041356e66 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -1382,6 +1382,54 @@ func TestFindVirtualServerRoutesForService(t *testing.T) { } } +func TestCheckForVirtualServerRoute(t *testing.T) { + + vsr1 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-1", + Namespace: "ns-1", + }, + } + + vsr2 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-2", + Namespace: "ns-1", + }, + } + + vsr3 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-3", + Namespace: "ns-2", + }, + } + + vsr4 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-4", + Namespace: "ns-1", + }, + } + + vsr5 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-5", + Namespace: "ns-3", + }, + } + + vsrlist := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr3, &vsr4, &vsr5} + + expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr4, &vsr5} + + result := checkForVirtualServerRoute("ns-2/vsr-3", vsrlist) + + if !reflect.DeepEqual(result, expected) { + t.Errorf("checkForVirtualServerRoute return %v but expected %v", result, expected) + } + +} func TestFindVirtualServerRoutesForVirtualServer(t *testing.T) { vsr1 := conf_v1.VirtualServerRoute{ ObjectMeta: meta_v1.ObjectMeta{ @@ -1413,7 +1461,7 @@ func TestFindVirtualServerRoutesForVirtualServer(t *testing.T) { virtualserverroutes := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr3, &vsr4} - vs1 := conf_v1.VirtualServer{ + vs := conf_v1.VirtualServer{ ObjectMeta: meta_v1.ObjectMeta{ Name: "vs-1", Namespace: "ns-1", @@ -1435,16 +1483,14 @@ func TestFindVirtualServerRoutesForVirtualServer(t *testing.T) { expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr4} - result := findVirtualServerRoutesForVirtualServer(&vs1, virtualserverroutes) + result := findVirtualServerRoutesForVirtualServer(&vs, virtualserverroutes) if !reflect.DeepEqual(result, expected) { t.Errorf("findVirtualServerRoutesForVirtualServer returned %v but expected %v", result, expected) } - } func TestFindAllVirtualServerRoutesForVirtualServer(t *testing.T) { - vsr1 := conf_v1.VirtualServerRoute{ ObjectMeta: meta_v1.ObjectMeta{ Name: "vsr-1", @@ -1464,7 +1510,7 @@ func TestFindAllVirtualServerRoutesForVirtualServer(t *testing.T) { Namespace: "ns-1", }, Status: conf_v1.VirtualServerRouteStatus{ - State: "InValid", + State: "Invalid", Reason: "Ignored", Message: fmt.Sprint("Ignored by VirtualServer ns-1/vs-1"), ReferencedBy: "ns-1/vs-1", @@ -1477,7 +1523,7 @@ func TestFindAllVirtualServerRoutesForVirtualServer(t *testing.T) { Namespace: "ns-1", }, Status: conf_v1.VirtualServerRouteStatus{ - State: "InValid", + State: "Invalid", Reason: "NoVirtualServerFound", Message: fmt.Sprint("No VirtualServer references VirtualServerRoute vsr-3/ns-1"), ReferencedBy: "", @@ -1488,14 +1534,14 @@ func TestFindAllVirtualServerRoutesForVirtualServer(t *testing.T) { expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2} - vs1 := conf_v1.VirtualServer{ + vs := conf_v1.VirtualServer{ ObjectMeta: meta_v1.ObjectMeta{ Name: "vs-1", Namespace: "ns-1", }, } - result := findAllVirtualServerRoutesForVirtualServer(&vs1, virtualserverroutes) + result := findAllVirtualServerRoutesForVirtualServer(&vs, virtualserverroutes) if !reflect.DeepEqual(result, expected) { t.Errorf("findAllVirtualServerRoutes return %v but expected %v", result, expected) From ffeba6b3aee6cb9be1fb8e44c0dde062fe098f8e Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Thu, 18 Jun 2020 18:37:59 +0100 Subject: [PATCH 07/17] Refactor code --- internal/k8s/controller.go | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 80d5be6fe1..d6ec2f0227 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -870,18 +870,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { lbc.syncQueue.Requeue(task, err) return } - - var vs *conf_v1.VirtualServer - - vslist := lbc.getVirtualServers() - for _, v := range vslist { - vskey := fmt.Sprintf("%s/%s", v.Namespace, v.Name) - if vskey == key { - vs = v - break - } - } - vsrlist := findVirtualServerRoutesForVirtualServer(vs, lbc.getVirtualServerRoutes()) + vsrlist := findAllVirtualServerRoutesForVirtualServer(key, lbc.getVirtualServerRoutes()) if !vsExists { glog.V(2).Infof("Deleting VirtualServer: %v\n", key) @@ -902,7 +891,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } glog.V(2).Infof("Adding or Updating VirtualServer: %v\n", key) - vs = obj.(*conf_v1.VirtualServer) + vs := obj.(*conf_v1.VirtualServer) validationErr := validation.ValidateVirtualServer(vs, lbc.isNginxPlus) if validationErr != nil { @@ -968,7 +957,8 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } } - vsrlist = findAllVirtualServerRoutesForVirtualServer(vs, lbc.getVirtualServerRoutes()) + key = fmt.Sprintf("%v/%v", vs.Namespace, vs.Name) + vsrlist = findAllVirtualServerRoutesForVirtualServer(key, lbc.getVirtualServerRoutes()) for _, vsr := range vsEx.VirtualServerRoutes { vsrEventType := eventType @@ -1024,9 +1014,8 @@ func checkForVirtualServerRoute(vsrkey string, vsrlist []*conf_v1.VirtualServerR return vsrNotRef } -func findAllVirtualServerRoutesForVirtualServer(vs *conf_v1.VirtualServer, vsrs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { +func findAllVirtualServerRoutesForVirtualServer(key string, vsrs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { var result []*conf_v1.VirtualServerRoute - key := fmt.Sprintf("%s/%s", vs.Namespace, vs.Name) for _, r := range vsrs { if r.Status.ReferencedBy == key { result = append(result, r) From 6df9a090eeb0743a4f08f33efb27d49384c81f8e Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Thu, 18 Jun 2020 18:50:25 +0100 Subject: [PATCH 08/17] Feedback --- internal/k8s/controller.go | 25 ++++++++++++------------- internal/k8s/controller_test.go | 1 - 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index d6ec2f0227..abe2dc0175 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -870,7 +870,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { lbc.syncQueue.Requeue(task, err) return } - vsrlist := findAllVirtualServerRoutesForVirtualServer(key, lbc.getVirtualServerRoutes()) + vsrList := findAllVirtualServerRoutesForVirtualServer(key, lbc.getVirtualServerRoutes()) if !vsExists { glog.V(2).Infof("Deleting VirtualServer: %v\n", key) @@ -880,7 +880,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { glog.Errorf("Error when deleting configuration for %v: %v", key, err) } else { reason := "NoVirtualServerFound" - for _, vsr := range vsrlist { + for _, vsr := range vsrList { msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) @@ -900,7 +900,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { glog.Errorf("Error when deleting configuration for %v: %v", key, err) } reason := "NoVirtualServerFound" - for _, vsr := range vsrlist { + for _, vsr := range vsrList { msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) err := lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) @@ -958,7 +958,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } key = fmt.Sprintf("%v/%v", vs.Namespace, vs.Name) - vsrlist = findAllVirtualServerRoutesForVirtualServer(key, lbc.getVirtualServerRoutes()) + vsrList = findAllVirtualServerRoutesForVirtualServer(key, lbc.getVirtualServerRoutes()) for _, vsr := range vsEx.VirtualServerRoutes { vsrEventType := eventType @@ -985,10 +985,10 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } vsrkey := fmt.Sprintf("%s/%s", vsr.Namespace, vsr.Name) - vsrlist = checkForVirtualServerRoute(vsrkey, vsrlist) + vsrList = removeVirtualServerRouteByKey(vsrkey, vsrList) } reason := "Ignored" - for _, vsr := range vsrlist { + for _, vsr := range vsrList { msg := fmt.Sprintf("Ignored by VirtualServer %v/%v", vs.Namespace, vs.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "Ignored", msg) err := lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) @@ -998,20 +998,19 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } } -func checkForVirtualServerRoute(vsrkey string, vsrlist []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { - var vsrNotRef []*conf_v1.VirtualServerRoute - for i, r := range vsrlist { +func removeVirtualServerRouteByKey(vsrkey string, vsrList []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { + for i, r := range vsrList { key := fmt.Sprintf("%s/%s", r.Namespace, r.Name) if key == vsrkey { - if len(vsrlist) > 1 { - vsrNotRef = append(vsrlist[:i], vsrlist[i+1:]...) + if len(vsrList) > 1 { + vsrList = append(vsrList[:i], vsrList[i+1:]...) } else { - vsrNotRef = []*conf_v1.VirtualServerRoute{} + vsrList = []*conf_v1.VirtualServerRoute{} } } } - return vsrNotRef + return vsrList } func findAllVirtualServerRoutesForVirtualServer(key string, vsrs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index d041356e66..2a5297c301 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -1383,7 +1383,6 @@ func TestFindVirtualServerRoutesForService(t *testing.T) { } func TestCheckForVirtualServerRoute(t *testing.T) { - vsr1 := conf_v1.VirtualServerRoute{ ObjectMeta: meta_v1.ObjectMeta{ Name: "vsr-1", From fd2b898e0399e3c5396f8c7f1dd7634a52d0914d Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Thu, 18 Jun 2020 18:59:10 +0100 Subject: [PATCH 09/17] Fix tests --- internal/k8s/controller_test.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 2a5297c301..38bc58d482 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -1422,7 +1422,7 @@ func TestCheckForVirtualServerRoute(t *testing.T) { expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr4, &vsr5} - result := checkForVirtualServerRoute("ns-2/vsr-3", vsrlist) + result := removeVirtualServerRouteByKey("ns-2/vsr-3", vsrlist) if !reflect.DeepEqual(result, expected) { t.Errorf("checkForVirtualServerRoute return %v but expected %v", result, expected) @@ -1533,14 +1533,9 @@ func TestFindAllVirtualServerRoutesForVirtualServer(t *testing.T) { expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2} - vs := conf_v1.VirtualServer{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "vs-1", - Namespace: "ns-1", - }, - } + vsKey := "ns-1/vs-1" - result := findAllVirtualServerRoutesForVirtualServer(&vs, virtualserverroutes) + result := findAllVirtualServerRoutesForVirtualServer(vsKey, virtualserverroutes) if !reflect.DeepEqual(result, expected) { t.Errorf("findAllVirtualServerRoutes return %v but expected %v", result, expected) From 872ec422a9864d2ea4858fa3e88edf346fa0edbb Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Mon, 22 Jun 2020 10:27:48 +0100 Subject: [PATCH 10/17] Feedback 2 --- internal/configs/configurator.go | 10 +++ internal/k8s/controller.go | 90 +++++++++++-------------- internal/k8s/controller_test.go | 112 ------------------------------- 3 files changed, 49 insertions(+), 163 deletions(-) diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 2a157d34cb..fd6d985480 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -247,6 +247,16 @@ func (cnf *Configurator) addOrUpdateTransportServer(transportServerEx *Transport return nil } +// GetVirtualServerRoutesForVirtualServer returns the virtualServerRoutes that a virtualServer +// references, if that virtualServer exists +func (cnf *Configurator) GetVirtualServerRoutesForVirtualServer(key string) []*conf_v1.VirtualServerRoute { + vsFileName := getFileNameForVirtualServerFromKey(key) + if cnf.virtualServers[vsFileName] != nil { + return cnf.virtualServers[vsFileName].VirtualServerRoutes + } + return nil +} + func (cnf *Configurator) updateTLSPassthroughHostsConfig() error { cfg, duplicatedHosts := generateTLSPassthroughHostsConfig(cnf.tlsPassthroughPairs) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index abe2dc0175..0012edfd46 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -870,23 +870,28 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { lbc.syncQueue.Requeue(task, err) return } - vsrList := findAllVirtualServerRoutesForVirtualServer(key, lbc.getVirtualServerRoutes()) - + vsrs := lbc.configurator.GetVirtualServerRoutesForVirtualServer(key) if !vsExists { glog.V(2).Infof("Deleting VirtualServer: %v\n", key) err := lbc.configurator.DeleteVirtualServer(key) if err != nil { glog.Errorf("Error when deleting configuration for %v: %v", key, err) - } else { - reason := "NoVirtualServerFound" - for _, vsr := range vsrList { - msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) - lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) - lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) - } } + reason := "NoVirtualServerFound" + for _, vsr := range vsrs { + msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) + lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) + + if lbc.reportVsVsrStatusEnabled() { + virtualServersForVSR := []*conf_v1.VirtualServer{} + err = lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, conf_v1.StateInvalid, reason, msg, virtualServersForVSR) + if err != nil { + glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + } + } + } return } @@ -899,16 +904,22 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { if err != nil { glog.Errorf("Error when deleting configuration for %v: %v", key, err) } + + lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "Rejected", "VirtualServer %v is invalid and was rejected: %v", key, validationErr) + reason := "NoVirtualServerFound" - for _, vsr := range vsrList { + for _, vsr := range vsrs { msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) - err := lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) - if err != nil { - glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + + if lbc.reportVsVsrStatusEnabled() { + virtualServersForVSR := []*conf_v1.VirtualServer{} + err = lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, conf_v1.StateInvalid, reason, msg, virtualServersForVSR) + if err != nil { + glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + } } } - lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "Rejected", "VirtualServer %v is invalid and was rejected: %v", key, validationErr) return } @@ -918,6 +929,8 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "IgnoredVirtualServerRoute", "Ignored VirtualServerRoute %v: %v", vsrError.VirtualServerRouteNsName, vsrError.Error) if vsrError.VirtualServerRoute != nil { lbc.recorder.Eventf(vsrError.VirtualServerRoute, api_v1.EventTypeWarning, "Ignored", "Ignored by VirtualServer %v/%v: %v", vs.Namespace, vs.Name, vsrError.Error) + vsrKey := fmt.Sprintf("%v/%v", vsrError.VirtualServerRoute.Namespace, vsrError.VirtualServerRoute.Name) + vsrs = removeVirtualServerRouteByKey(vsrKey, vsrs) } } @@ -957,9 +970,6 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } } - key = fmt.Sprintf("%v/%v", vs.Namespace, vs.Name) - vsrList = findAllVirtualServerRoutesForVirtualServer(key, lbc.getVirtualServerRoutes()) - for _, vsr := range vsEx.VirtualServerRoutes { vsrEventType := eventType vsrEventTitle := eventTitle @@ -976,8 +986,8 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { lbc.recorder.Eventf(vsr, vsrEventType, vsrEventTitle, msg) if lbc.reportVsVsrStatusEnabled() { - virtualServersForVSR := findVirtualServersForVirtualServerRoute(lbc.getVirtualServers(), vsr) - err = lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, state, vsrEventTitle, msg, virtualServersForVSR) + vss := []*conf_v1.VirtualServer{vs} + err = lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, state, vsrEventTitle, msg, vss) if err != nil { glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) @@ -985,15 +995,19 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } vsrkey := fmt.Sprintf("%s/%s", vsr.Namespace, vsr.Name) - vsrList = removeVirtualServerRouteByKey(vsrkey, vsrList) + vsrs = removeVirtualServerRouteByKey(vsrkey, vsrs) } + reason := "Ignored" - for _, vsr := range vsrList { + for _, vsr := range vsrs { msg := fmt.Sprintf("Ignored by VirtualServer %v/%v", vs.Namespace, vs.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "Ignored", msg) - err := lbc.statusUpdater.UpdateVirtualServerRouteStatus(vsr, conf_v1.StateInvalid, reason, msg) - if err != nil { - glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + if lbc.reportVsVsrStatusEnabled() { + var emptyVSS []*conf_v1.VirtualServer + err := lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, conf_v1.StateInvalid, reason, msg, emptyVSS) + if err != nil { + glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) + } } } } @@ -1002,39 +1016,13 @@ func removeVirtualServerRouteByKey(vsrkey string, vsrList []*conf_v1.VirtualServ for i, r := range vsrList { key := fmt.Sprintf("%s/%s", r.Namespace, r.Name) if key == vsrkey { - if len(vsrList) > 1 { - vsrList = append(vsrList[:i], vsrList[i+1:]...) - } else { - vsrList = []*conf_v1.VirtualServerRoute{} - } + vsrList = append(vsrList[:i], vsrList[i+1:]...) } } return vsrList } -func findAllVirtualServerRoutesForVirtualServer(key string, vsrs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { - var result []*conf_v1.VirtualServerRoute - for _, r := range vsrs { - if r.Status.ReferencedBy == key { - result = append(result, r) - } - } - return result -} - -func findVirtualServerRoutesForVirtualServer(vs *conf_v1.VirtualServer, vsrs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { - var result []*conf_v1.VirtualServerRoute - vslist := []*conf_v1.VirtualServer{vs} - for _, vsr := range vsrs { - key := fmt.Sprintf("%s/%s", vsr.Namespace, vsr.Name) - if len(findVirtualServersForVirtualServerRouteKey(vslist, key)) != 0 { - result = append(result, vsr) - } - } - return result -} - func (lbc *LoadBalancerController) syncVirtualServerRoute(task task) { key := task.Key diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 38bc58d482..87949b3941 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -1429,118 +1429,6 @@ func TestCheckForVirtualServerRoute(t *testing.T) { } } -func TestFindVirtualServerRoutesForVirtualServer(t *testing.T) { - vsr1 := conf_v1.VirtualServerRoute{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "vsr-1", - Namespace: "ns-1", - }, - } - - vsr2 := conf_v1.VirtualServerRoute{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "vsr-2", - Namespace: "ns-1", - }, - } - - vsr3 := conf_v1.VirtualServerRoute{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "vsr-3", - Namespace: "ns-2", - }, - } - - vsr4 := conf_v1.VirtualServerRoute{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "vsr-4", - Namespace: "ns-1", - }, - } - - virtualserverroutes := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr3, &vsr4} - - vs := conf_v1.VirtualServer{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "vs-1", - Namespace: "ns-1", - }, - Spec: conf_v1.VirtualServerSpec{ - Routes: []conf_v1.Route{ - { - Route: "ns-1/vsr-1", - }, - { - Route: "ns-1/vsr-2", - }, - { - Route: "ns-1/vsr-4", - }, - }, - }, - } - - expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr4} - - result := findVirtualServerRoutesForVirtualServer(&vs, virtualserverroutes) - - if !reflect.DeepEqual(result, expected) { - t.Errorf("findVirtualServerRoutesForVirtualServer returned %v but expected %v", result, expected) - } -} - -func TestFindAllVirtualServerRoutesForVirtualServer(t *testing.T) { - vsr1 := conf_v1.VirtualServerRoute{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "vsr-1", - Namespace: "ns-1", - }, - Status: conf_v1.VirtualServerRouteStatus{ - State: "Valid", - Reason: "AddedOrUpdated", - Message: fmt.Sprint("Configuration for ns-1/vs-1 was added or updated"), - ReferencedBy: "ns-1/vs-1", - }, - } - - vsr2 := conf_v1.VirtualServerRoute{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "vsr-2", - Namespace: "ns-1", - }, - Status: conf_v1.VirtualServerRouteStatus{ - State: "Invalid", - Reason: "Ignored", - Message: fmt.Sprint("Ignored by VirtualServer ns-1/vs-1"), - ReferencedBy: "ns-1/vs-1", - }, - } - - vsr3 := conf_v1.VirtualServerRoute{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "vsr-3", - Namespace: "ns-1", - }, - Status: conf_v1.VirtualServerRouteStatus{ - State: "Invalid", - Reason: "NoVirtualServerFound", - Message: fmt.Sprint("No VirtualServer references VirtualServerRoute vsr-3/ns-1"), - ReferencedBy: "", - }, - } - - virtualserverroutes := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr3} - - expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2} - - vsKey := "ns-1/vs-1" - - result := findAllVirtualServerRoutesForVirtualServer(vsKey, virtualserverroutes) - - if !reflect.DeepEqual(result, expected) { - t.Errorf("findAllVirtualServerRoutes return %v but expected %v", result, expected) - } -} func TestFindTransportServersForService(t *testing.T) { ts1 := conf_v1alpha1.TransportServer{ From bc09a6a475fad70cee6cff199c82f0978c7912b1 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Mon, 22 Jun 2020 11:48:42 +0100 Subject: [PATCH 11/17] Refactor test --- internal/k8s/controller_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 87949b3941..e9cce9bc5b 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -1418,11 +1418,11 @@ func TestCheckForVirtualServerRoute(t *testing.T) { }, } - vsrlist := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr3, &vsr4, &vsr5} + vsrs := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr3, &vsr4, &vsr5} expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr4, &vsr5} - result := removeVirtualServerRouteByKey("ns-2/vsr-3", vsrlist) + result := removeVirtualServerRouteByKey("ns-2/vsr-3", vsrs) if !reflect.DeepEqual(result, expected) { t.Errorf("checkForVirtualServerRoute return %v but expected %v", result, expected) From c74f48c92a2dbbc5e53db214e4505d91b021f5a8 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Tue, 23 Jun 2020 10:00:05 +0100 Subject: [PATCH 12/17] WIP --- internal/k8s/controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 0012edfd46..f345a657a0 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -998,9 +998,9 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { vsrs = removeVirtualServerRouteByKey(vsrkey, vsrs) } - reason := "Ignored" + reason := "NoVirtualServerFound" for _, vsr := range vsrs { - msg := fmt.Sprintf("Ignored by VirtualServer %v/%v", vs.Namespace, vs.Name) + msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "Ignored", msg) if lbc.reportVsVsrStatusEnabled() { var emptyVSS []*conf_v1.VirtualServer From eb6da4727a567d97fd2d2b08979639ee9b2bf92a Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Tue, 23 Jun 2020 16:45:22 +0100 Subject: [PATCH 13/17] Address feedback --- internal/k8s/controller.go | 37 ++++++++++++++++++++------------- internal/k8s/controller_test.go | 4 +++- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index f345a657a0..acb0112db5 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -870,7 +870,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { lbc.syncQueue.Requeue(task, err) return } - vsrs := lbc.configurator.GetVirtualServerRoutesForVirtualServer(key) + previousVSRs := lbc.configurator.GetVirtualServerRoutesForVirtualServer(key) if !vsExists { glog.V(2).Infof("Deleting VirtualServer: %v\n", key) @@ -879,7 +879,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { glog.Errorf("Error when deleting configuration for %v: %v", key, err) } reason := "NoVirtualServerFound" - for _, vsr := range vsrs { + for _, vsr := range previousVSRs { msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) @@ -908,7 +908,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "Rejected", "VirtualServer %v is invalid and was rejected: %v", key, validationErr) reason := "NoVirtualServerFound" - for _, vsr := range vsrs { + for _, vsr := range previousVSRs { msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) @@ -923,14 +923,15 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { return } + var handledVSRs []*conf_v1.VirtualServerRoute + vsEx, vsrErrors := lbc.createVirtualServer(vs) for _, vsrError := range vsrErrors { lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "IgnoredVirtualServerRoute", "Ignored VirtualServerRoute %v: %v", vsrError.VirtualServerRouteNsName, vsrError.Error) if vsrError.VirtualServerRoute != nil { + handledVSRs = append(handledVSRs, vsrError.VirtualServerRoute) lbc.recorder.Eventf(vsrError.VirtualServerRoute, api_v1.EventTypeWarning, "Ignored", "Ignored by VirtualServer %v/%v: %v", vs.Namespace, vs.Name, vsrError.Error) - vsrKey := fmt.Sprintf("%v/%v", vsrError.VirtualServerRoute.Namespace, vsrError.VirtualServerRoute.Name) - vsrs = removeVirtualServerRouteByKey(vsrKey, vsrs) } } @@ -994,12 +995,12 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } } - vsrkey := fmt.Sprintf("%s/%s", vsr.Namespace, vsr.Name) - vsrs = removeVirtualServerRouteByKey(vsrkey, vsrs) + handledVSRs = append(handledVSRs, vsr) } + orphanedVSRs := filterOutVirtualServerRoutes(previousVSRs, handledVSRs) reason := "NoVirtualServerFound" - for _, vsr := range vsrs { + for _, vsr := range orphanedVSRs { msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "Ignored", msg) if lbc.reportVsVsrStatusEnabled() { @@ -1012,15 +1013,23 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } } -func removeVirtualServerRouteByKey(vsrkey string, vsrList []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { - for i, r := range vsrList { +func filterOutVirtualServerRoutes(previousVSRs []*conf_v1.VirtualServerRoute, handledVSRs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { + var orphanedVSRs []*conf_v1.VirtualServerRoute + for _, r := range previousVSRs { + isIn := false key := fmt.Sprintf("%s/%s", r.Namespace, r.Name) - if key == vsrkey { - vsrList = append(vsrList[:i], vsrList[i+1:]...) + for _, r1 := range handledVSRs { + vsrKey := fmt.Sprintf("%s/%s", r1.Namespace, r1.Name) + if key == vsrKey { + isIn = true + break + } + } + if !isIn { + orphanedVSRs = append(orphanedVSRs, r) } } - - return vsrList + return orphanedVSRs } func (lbc *LoadBalancerController) syncVirtualServerRoute(task task) { diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index e9cce9bc5b..83ec0bbecb 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -1420,9 +1420,11 @@ func TestCheckForVirtualServerRoute(t *testing.T) { vsrs := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr3, &vsr4, &vsr5} + handledVSRs := []*conf_v1.VirtualServerRoute{&vsr3} + expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr4, &vsr5} - result := removeVirtualServerRouteByKey("ns-2/vsr-3", vsrs) + result := filterOutVirtualServerRoutes(vsrs, handledVSRs) if !reflect.DeepEqual(result, expected) { t.Errorf("checkForVirtualServerRoute return %v but expected %v", result, expected) From 66a07e387cda63c2a2b6ecf25731a5c8917c1095 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Wed, 24 Jun 2020 10:06:13 +0100 Subject: [PATCH 14/17] feedback 3 --- internal/k8s/controller.go | 19 ++++++++++++------- internal/k8s/controller_test.go | 7 +++---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index acb0112db5..29bfc50663 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -906,6 +906,11 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "Rejected", "VirtualServer %v is invalid and was rejected: %v", key, validationErr) + if lbc.reportVsVsrStatusEnabled() { + reason = "AddedOrUpdatedWithError" + msg := fmt.Sprintf("Configuration for %v was added or updated %s", key, validationErr) + err = lbc.statusUpdater.UpdateVirtualServerStatus(vs, conf_v1.StateInvalid, reason, msg) + } reason := "NoVirtualServerFound" for _, vsr := range previousVSRs { @@ -998,7 +1003,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { handledVSRs = append(handledVSRs, vsr) } - orphanedVSRs := filterOutVirtualServerRoutes(previousVSRs, handledVSRs) + orphanedVSRs := findOrphanedVirtualServerRoutes(previousVSRs, handledVSRs) reason := "NoVirtualServerFound" for _, vsr := range orphanedVSRs { msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) @@ -1013,14 +1018,14 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { } } -func filterOutVirtualServerRoutes(previousVSRs []*conf_v1.VirtualServerRoute, handledVSRs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { +func findOrphanedVirtualServerRoutes(previousVSRs []*conf_v1.VirtualServerRoute, handledVSRs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute { var orphanedVSRs []*conf_v1.VirtualServerRoute - for _, r := range previousVSRs { + for _, prev := range previousVSRs { isIn := false - key := fmt.Sprintf("%s/%s", r.Namespace, r.Name) - for _, r1 := range handledVSRs { - vsrKey := fmt.Sprintf("%s/%s", r1.Namespace, r1.Name) - if key == vsrKey { + prevKey := fmt.Sprintf("%s/%s", r.Namespace, r.Name) + for _, handled := range handledVSRs { + handledKey := fmt.Sprintf("%s/%s", r1.Namespace, r1.Name) + if prevKey == handledKey { isIn = true break } diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 83ec0bbecb..6cb2db8c10 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -1382,7 +1382,7 @@ func TestFindVirtualServerRoutesForService(t *testing.T) { } } -func TestCheckForVirtualServerRoute(t *testing.T) { +func TestFindOrphanedVirtualServerRoute(t *testing.T) { vsr1 := conf_v1.VirtualServerRoute{ ObjectMeta: meta_v1.ObjectMeta{ Name: "vsr-1", @@ -1424,12 +1424,11 @@ func TestCheckForVirtualServerRoute(t *testing.T) { expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr4, &vsr5} - result := filterOutVirtualServerRoutes(vsrs, handledVSRs) + result := findOrphanedVirtualServerRoutes(vsrs, handledVSRs) if !reflect.DeepEqual(result, expected) { - t.Errorf("checkForVirtualServerRoute return %v but expected %v", result, expected) + t.Errorf("findOrphanedVirtualServerRoutes return %v but expected %v", result, expected) } - } func TestFindTransportServersForService(t *testing.T) { From f5fa9cc0992238a4e2293fb170a0986b5a5ac3d4 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Wed, 24 Jun 2020 10:10:29 +0100 Subject: [PATCH 15/17] feedback 4 --- internal/k8s/controller.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 29bfc50663..57903efabb 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -905,10 +905,11 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { glog.Errorf("Error when deleting configuration for %v: %v", key, err) } - lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "Rejected", "VirtualServer %v is invalid and was rejected: %v", key, validationErr) + reason = "Rejected" + msg := fmt.Sprintf("VirtualServer %v is invalid and was rejected: %v", key, validationErr) + + lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, reason, msg) if lbc.reportVsVsrStatusEnabled() { - reason = "AddedOrUpdatedWithError" - msg := fmt.Sprintf("Configuration for %v was added or updated %s", key, validationErr) err = lbc.statusUpdater.UpdateVirtualServerStatus(vs, conf_v1.StateInvalid, reason, msg) } From 20de3dd66181196097797eed60910c126afa56c1 Mon Sep 17 00:00:00 2001 From: Lorcan McVeigh Date: Wed, 24 Jun 2020 10:12:50 +0100 Subject: [PATCH 16/17] apply feedback Co-authored-by: Michael Pleshakov --- internal/k8s/controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 57903efabb..00b9ff8463 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -1008,9 +1008,9 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { reason := "NoVirtualServerFound" for _, vsr := range orphanedVSRs { msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) - lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "Ignored", msg) + lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) if lbc.reportVsVsrStatusEnabled() { - var emptyVSS []*conf_v1.VirtualServer + var emptyVSes []*conf_v1.VirtualServer err := lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, conf_v1.StateInvalid, reason, msg, emptyVSS) if err != nil { glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) From ba7abbef0e69188c2e5cd7f38710774aa26d9d60 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Wed, 24 Jun 2020 15:37:27 +0100 Subject: [PATCH 17/17] feedback 5 --- internal/k8s/controller.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 00b9ff8463..15eb07d983 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -905,7 +905,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { glog.Errorf("Error when deleting configuration for %v: %v", key, err) } - reason = "Rejected" + reason := "Rejected" msg := fmt.Sprintf("VirtualServer %v is invalid and was rejected: %v", key, validationErr) lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, reason, msg) @@ -913,7 +913,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { err = lbc.statusUpdater.UpdateVirtualServerStatus(vs, conf_v1.StateInvalid, reason, msg) } - reason := "NoVirtualServerFound" + reason = "NoVirtualServerFound" for _, vsr := range previousVSRs { msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name) lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) @@ -1011,7 +1011,7 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) { lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg) if lbc.reportVsVsrStatusEnabled() { var emptyVSes []*conf_v1.VirtualServer - err := lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, conf_v1.StateInvalid, reason, msg, emptyVSS) + err := lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, conf_v1.StateInvalid, reason, msg, emptyVSes) if err != nil { glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err) } @@ -1023,16 +1023,16 @@ func findOrphanedVirtualServerRoutes(previousVSRs []*conf_v1.VirtualServerRoute, var orphanedVSRs []*conf_v1.VirtualServerRoute for _, prev := range previousVSRs { isIn := false - prevKey := fmt.Sprintf("%s/%s", r.Namespace, r.Name) + prevKey := fmt.Sprintf("%s/%s", prev.Namespace, prev.Name) for _, handled := range handledVSRs { - handledKey := fmt.Sprintf("%s/%s", r1.Namespace, r1.Name) + handledKey := fmt.Sprintf("%s/%s", handled.Namespace, handled.Name) if prevKey == handledKey { isIn = true break } } if !isIn { - orphanedVSRs = append(orphanedVSRs, r) + orphanedVSRs = append(orphanedVSRs, prev) } } return orphanedVSRs