Add LoadBalancer to getExternalIPs (#995)

* add LoadBalancer to getExternalIPs * fix up network_routes_controller tests * update ecmp_vip tests
cloudnativelabs · Oct 2, 2020 · 8e3f36c · 8e3f36c
1 parent 92b914e
commit 8e3f36c
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 21 deletions.
diff --git a/pkg/controllers/routing/ecmp_vip.go b/pkg/controllers/routing/ecmp_vip.go
@@ -315,7 +315,7 @@ func (nrc *NetworkRoutingController) getClusterIP(svc *v1core.Service) string {
 
 func (nrc *NetworkRoutingController) getExternalIPs(svc *v1core.Service) []string {
 	externalIPList := make([]string, 0)
-	if svc.Spec.Type == "ClusterIP" || svc.Spec.Type == "NodePort" {
+	if svc.Spec.Type == "ClusterIP" || svc.Spec.Type == "NodePort" || svc.Spec.Type == "LoadBalancer" {
 
 		// skip headless services
 		if svc.Spec.ClusterIP != "None" && svc.Spec.ClusterIP != "" {

diff --git a/pkg/controllers/routing/ecmp_vip_test.go b/pkg/controllers/routing/ecmp_vip_test.go
@@ -65,10 +65,8 @@ func Test_getVIPsForService(t *testing.T) {
 				Name: "svc-loadbalancer",
 			},
 			Spec: v1core.ServiceSpec{
-				Type:      "LoadBalancer",
-				ClusterIP: "10.0.0.1",
-				// External IPs are ignored since LoadBalancer services don't
-				// advertise external IPs.
+				Type:        "LoadBalancer",
+				ClusterIP:   "10.0.0.1",
 				ExternalIPs: []string{"1.1.1.1"},
 			},
 			Status: v1core.ServiceStatus{
@@ -116,7 +114,7 @@ func Test_getVIPsForService(t *testing.T) {
 				},
 				{
 					services["loadbalancer"],
-					[]string{"10.0.0.1", "10.0.255.1", "10.0.255.2"},
+					[]string{"10.0.0.1", "1.1.1.1", "10.0.255.1", "10.0.255.2"},
 					[]string{},
 					nil,
 				},
@@ -206,7 +204,7 @@ func Test_getVIPsForService(t *testing.T) {
 				},
 				{
 					services["loadbalancer"],
-					[]string{},
+					[]string{"1.1.1.1"},
 					[]string{},
 					nil,
 				},
@@ -278,7 +276,7 @@ func Test_getVIPsForService(t *testing.T) {
 				},
 				{
 					services["loadbalancer"],
-					[]string{"10.0.0.1", "10.0.255.1", "10.0.255.2"},
+					[]string{"10.0.0.1", "1.1.1.1", "10.0.255.1", "10.0.255.2"},
 					[]string{},
 					map[string]string{
 						svcAdvertiseClusterAnnotation:      "true",
@@ -289,7 +287,7 @@ func Test_getVIPsForService(t *testing.T) {
 				{
 					// Special case to test svcAdvertiseLoadBalancerAnnotation vs legacy svcSkipLbIpsAnnotation
 					services["loadbalancer"],
-					[]string{"10.0.0.1"},
+					[]string{"10.0.0.1", "1.1.1.1"},
 					[]string{},
 					map[string]string{
 						svcAdvertiseClusterAnnotation:      "true",

diff --git a/pkg/controllers/routing/network_routes_controller_test.go b/pkg/controllers/routing/network_routes_controller_test.go
@@ -295,10 +295,8 @@ func Test_advertiseExternalIPs(t *testing.T) {
 						Name: "svc-2",
 					},
 					Spec: v1core.ServiceSpec{
-						Type:      "LoadBalancer",
-						ClusterIP: "10.0.0.2",
-						// ignored since LoadBalancer services don't
-						// advertise external IPs.
+						Type:        "LoadBalancer",
+						ClusterIP:   "10.0.0.2",
 						ExternalIPs: []string{"2.2.2.2"},
 					},
 				},
@@ -315,6 +313,7 @@ func Test_advertiseExternalIPs(t *testing.T) {
 			},
 			map[string]bool{
 				"1.1.1.1/32": true,
+				"2.2.2.2/32": true,
 				"3.3.3.3/32": true,
 				"4.4.4.4/32": true,
 			},
@@ -635,10 +634,8 @@ func Test_advertiseAnnotationOptOut(t *testing.T) {
 						Name: "svc-3",
 					},
 					Spec: v1core.ServiceSpec{
-						Type:      "LoadBalancer",
-						ClusterIP: "10.0.0.3",
-						// ignored since LoadBalancer services don't
-						// advertise external IPs.
+						Type:        "LoadBalancer",
+						ClusterIP:   "10.0.0.3",
 						ExternalIPs: []string{"4.4.4.4"},
 					},
 					Status: v1core.ServiceStatus{
@@ -662,6 +659,7 @@ func Test_advertiseAnnotationOptOut(t *testing.T) {
 				"1.1.1.1/32":    true,
 				"2.2.2.2/32":    true,
 				"3.3.3.3/32":    true,
+				"4.4.4.4/32":    true,
 				"10.0.255.1/32": true,
 				"10.0.255.2/32": true,
 			},
@@ -895,10 +893,8 @@ func Test_advertiseAnnotationOptIn(t *testing.T) {
 						},
 					},
 					Spec: v1core.ServiceSpec{
-						Type:      "LoadBalancer",
-						ClusterIP: "10.0.0.3",
-						// ignored since LoadBalancer services don't
-						// advertise external IPs.
+						Type:        "LoadBalancer",
+						ClusterIP:   "10.0.0.3",
 						ExternalIPs: []string{"4.4.4.4"},
 					},
 					Status: v1core.ServiceStatus{
@@ -922,6 +918,7 @@ func Test_advertiseAnnotationOptIn(t *testing.T) {
 				"1.1.1.1/32":    true,
 				"2.2.2.2/32":    true,
 				"3.3.3.3/32":    true,
+				"4.4.4.4/32":    true,
 				"10.0.255.1/32": true,
 				"10.0.255.2/32": true,
 			},