Merge pull request #92 from Charliekenney23/feat/hostname-only-ingress-ann

add loadbalancer-hostname-only-ingress annotation

Author: 0xch4z

README.md

@@ -55,6 +55,7 @@ Annotation (Suffix) | Values | Default | Description
 `check-passive` | [bool](#annotation-bool-values) | `false` | When `true`, `5xx` status codes will cause the health check to fail
 `preserve` | [bool](#annotation-bool-values) | `false` | When `true`, deleting a `LoadBalancer` service does not delete the underlying NodeBalancer. This will also prevent deletion of the former LoadBalancer when another one is specified with the `nodebalancer-id` annotation.
 `nodebalancer-id` | string | | The ID of the NodeBalancer to front the service. When not specified, a new NodeBalancer will be created. This can be configured on service creation or patching
+`hostname-only-ingress` | [bool](#annotation-bool-values) | `false` | When `true`, the LoadBalancerStatus for the service will only contain the Hostname. This is useful for bypassing kube-proxy's rerouting of in-cluster requests originally intended for the external LoadBalancer to the service's constituent pod IPs.
 
 #### Deprecated Annotations
 

cloud/linode/loadbalancers.go

@@ -44,6 +44,8 @@ const (
 
 	annLinodeLoadBalancerPreserve = "service.beta.kubernetes.io/linode-loadbalancer-preserve"
 	annLinodeNodeBalancerID       = "service.beta.kubernetes.io/linode-loadbalancer-nodebalancer-id"
+
+	annLinodeHostnameOnlyIngress = "service.beta.kubernetes.io/linode-loadbalancer-hostname-only-ingress"
 )
 
 type lbNotFoundError struct {
@@ -122,12 +124,12 @@ func (l *loadbalancers) getLatestServiceLoadBalancerStatus(ctx context.Context,
 // most recent LoadBalancer status.
 func (l *loadbalancers) getNodeBalancerByStatus(ctx context.Context, service *v1.Service) (nb *linodego.NodeBalancer, err error) {
 	for _, ingress := range service.Status.LoadBalancer.Ingress {
-		if ingress.Hostname != "" {
-			return l.getNodeBalancerByHostname(ctx, service, ingress.Hostname)
-		}
 		if ingress.IP != "" {
 			return l.getNodeBalancerByIPv4(ctx, service, ingress.IP)
 		}
+		if ingress.Hostname != "" {
+			return l.getNodeBalancerByHostname(ctx, service, ingress.Hostname)
+		}
 	}
 	return nil, lbNotFoundError{serviceNn: getServiceNn(service)}
 }
@@ -195,7 +197,7 @@ func (l *loadbalancers) GetLoadBalancer(ctx context.Context, clusterName string,
 		return nil, false, err
 	}
 
-	return makeLoadBalancerStatus(nb), true, nil
+	return makeLoadBalancerStatus(service, nb), true, nil
 }
 
 // EnsureLoadBalancer ensures that the cluster is running a load balancer for
@@ -231,7 +233,7 @@ func (l *loadbalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri
 	}
 
 	klog.Infof("NodeBalancer (%d) has been ensured for service (%s)", nb.ID, serviceNn)
-	lbStatus = makeLoadBalancerStatus(nb)
+	lbStatus = makeLoadBalancerStatus(service, nb)
 
 	if !l.shouldPreserveNodeBalancer(service) {
 		if err := l.cleanupOldNodeBalancer(ctx, service); err != nil {
@@ -383,12 +385,7 @@ func (l *loadbalancers) deleteUnusedConfigs(ctx context.Context, nbConfigs []lin
 // shouldPreserveNodeBalancer determines whether a NodeBalancer should be deleted based on the
 // service's preserve annotation.
 func (l *loadbalancers) shouldPreserveNodeBalancer(service *v1.Service) bool {
-	preserveRaw, ok := getServiceAnnotation(service, annLinodeLoadBalancerPreserve)
-	if !ok {
-		return false
-	}
-	preserve, err := strconv.ParseBool(preserveRaw)
-	return err == nil && preserve
+	return getServiceBoolAnnotation(service, annLinodeLoadBalancerPreserve)
 }
 
 // EnsureLoadBalancerDeleted deletes the specified loadbalancer if it exists.
@@ -769,11 +766,16 @@ func getConnectionThrottle(service *v1.Service) int {
 	return connThrottle
 }
 
-func makeLoadBalancerStatus(nb *linodego.NodeBalancer) *v1.LoadBalancerStatus {
+func makeLoadBalancerStatus(service *v1.Service, nb *linodego.NodeBalancer) *v1.LoadBalancerStatus {
+	ingress := v1.LoadBalancerIngress{
+		Hostname: *nb.Hostname,
+	}
+	if !getServiceBoolAnnotation(service, annLinodeHostnameOnlyIngress) {
+		ingress.IP = *nb.IPv4
+	}
+
 	return &v1.LoadBalancerStatus{
-		Ingress: []v1.LoadBalancerIngress{{
-			Hostname: *nb.Hostname,
-		}},
+		Ingress: []v1.LoadBalancerIngress{ingress},
 	}
 }
 
@@ -789,3 +791,12 @@ func getServiceAnnotation(service *v1.Service, name string) (string, bool) {
 	val, ok := service.Annotations[name]
 	return val, ok
 }
+
+func getServiceBoolAnnotation(service *v1.Service, name string) bool {
+	value, ok := getServiceAnnotation(service, name)
+	if !ok {
+		return false
+	}
+	boolValue, err := strconv.ParseBool(value)
+	return err == nil && boolValue
+}

cloud/linode/loadbalancers_test.go

@@ -162,6 +162,10 @@ func TestCCMLoadBalancers(t *testing.T) {
 			name: "getNodeBalancerForService - NodeBalancerID does not exist",
 			f:    testGetNodeBalancerForServiceIDDoesNotExist,
 		},
+		{
+			name: "makeLoadBalancerStatus",
+			f:    testMakeLoadBalancerStatus,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -571,7 +575,7 @@ func testUpdateLoadBalancerAddProxyProtocol(t *testing.T, client *linodego.Clien
 				t.Fatalf("failed to create NodeBalancer: %s", err)
 			}
 
-			svc.Status.LoadBalancer = *makeLoadBalancerStatus(nodeBalancer)
+			svc.Status.LoadBalancer = *makeLoadBalancerStatus(svc, nodeBalancer)
 			svc.ObjectMeta.SetAnnotations(map[string]string{
 				annLinodeDefaultProxyProtocol: string(tc.proxyProtocolConfig),
 			})
@@ -649,7 +653,7 @@ func testUpdateLoadBalancerAddNodeBalancerID(t *testing.T, client *linodego.Clie
 		t.Fatalf("failed to create NodeBalancer: %s", err)
 	}
 
-	svc.Status.LoadBalancer = *makeLoadBalancerStatus(nodeBalancer)
+	svc.Status.LoadBalancer = *makeLoadBalancerStatus(svc, nodeBalancer)
 
 	newNodeBalancer, err := client.CreateNodeBalancer(context.TODO(), linodego.NodeBalancerCreateOptions{
 		Region: lb.zone,
@@ -672,7 +676,7 @@ func testUpdateLoadBalancerAddNodeBalancerID(t *testing.T, client *linodego.Clie
 		t.Errorf("GetLoadBalancer returned an error: %s", err)
 	}
 
-	expectedLBStatus := makeLoadBalancerStatus(newNodeBalancer)
+	expectedLBStatus := makeLoadBalancerStatus(svc, newNodeBalancer)
 	if !reflect.DeepEqual(expectedLBStatus, lbStatus) {
 		t.Errorf("LoadBalancer status mismatch: expected %v, got %v", expectedLBStatus, lbStatus)
 	}
@@ -1185,7 +1189,7 @@ func testEnsureLoadBalancerPreserveAnnotation(t *testing.T, client *linodego.Cli
 				t.Fatal(err)
 			}
 
-			svc.Status.LoadBalancer = *makeLoadBalancerStatus(nb)
+			svc.Status.LoadBalancer = *makeLoadBalancerStatus(svc, nb)
 			err = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc)
 
 			didDelete := fake.didRequestOccur(http.MethodDelete, fmt.Sprintf("/nodebalancers/%d", nb.ID), "")
@@ -1318,7 +1322,7 @@ func testEnsureExistingLoadBalancer(t *testing.T, client *linodego.Client, _ *fa
 		t.Fatal(err)
 	}
 
-	svc.Status.LoadBalancer = *makeLoadBalancerStatus(nb)
+	svc.Status.LoadBalancer = *makeLoadBalancerStatus(svc, nb)
 	defer func() { _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) }()
 	getLBStatus, exists, err := lb.GetLoadBalancer(context.TODO(), "linodelb", svc)
 	if err != nil {
@@ -1406,6 +1410,40 @@ func testEnsureExistingLoadBalancer(t *testing.T, client *linodego.Client, _ *fa
 	}
 }
 
+func testMakeLoadBalancerStatus(t *testing.T, client *linodego.Client, _ *fakeAPI) {
+	ipv4 := "192.168.0.1"
+	hostname := "nb-192-168-0-1.newark.nodebalancer.linode.com"
+	nb := &linodego.NodeBalancer{
+		IPv4:     &ipv4,
+		Hostname: &hostname,
+	}
+
+	svc := &v1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "test",
+			Annotations: make(map[string]string, 1),
+		},
+	}
+
+	expectedStatus := &v1.LoadBalancerStatus{
+		Ingress: []v1.LoadBalancerIngress{{
+			Hostname: hostname,
+			IP:       ipv4,
+		}},
+	}
+	status := makeLoadBalancerStatus(svc, nb)
+	if !reflect.DeepEqual(status, expectedStatus) {
+		t.Errorf("expected status for basic service to be %#v; got %#v", expectedStatus, status)
+	}
+
+	svc.Annotations[annLinodeHostnameOnlyIngress] = "true"
+	expectedStatus.Ingress[0] = v1.LoadBalancerIngress{Hostname: hostname}
+	status = makeLoadBalancerStatus(svc, nb)
+	if !reflect.DeepEqual(status, expectedStatus) {
+		t.Errorf("expected status for %q annotated service to be %#v; got %#v", annLinodeHostnameOnlyIngress, expectedStatus, status)
+	}
+}
+
 func testGetNodeBalancerForServiceIDDoesNotExist(t *testing.T, client *linodego.Client, _ *fakeAPI) {
 	lb := &loadbalancers{client, "us-west", nil}
 	bogusNodeBalancerID := "123456"
@@ -1579,7 +1617,7 @@ func testGetLoadBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) {
 	}
 	defer func() { _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) }()
 
-	lbStatus := makeLoadBalancerStatus(nb)
+	lbStatus := makeLoadBalancerStatus(svc, nb)
 	svc.Status.LoadBalancer = *lbStatus
 
 	testcases := []struct {

e2e/test/ccm_e2e_test.go

@@ -40,6 +40,7 @@ var _ = Describe("e2e tests", func() {
 		annLinodeHealthCheckAttempts     = "service.beta.kubernetes.io/linode-loadbalancer-check-attempts"
 		annLinodeHealthCheckPassive      = "service.beta.kubernetes.io/linode-loadbalancer-check-passive"
 		annLinodeNodeBalancerID          = "service.beta.kubernetes.io/linode-loadbalancer-nodebalancer-id"
+		annLinodeHostnameOnlyIngress     = "service.beta.kubernetes.io/linode-loadbalancer-hostname-only-ingress"
 	)
 
 	BeforeEach(func() {
@@ -174,6 +175,26 @@ var _ = Describe("e2e tests", func() {
 		Expect(err).NotTo(HaveOccurred())
 	}
 
+	var checkLBStatus = func(service string, hasIP bool) {
+		Eventually(func() bool {
+			nb, err := f.LoadBalancer.GetNodeBalancer(service)
+			Expect(err).NotTo(HaveOccurred())
+
+			svc, err := f.LoadBalancer.GetServiceWithLoadBalancerStatus(service, f.LoadBalancer.Namespace())
+			Expect(err).NotTo(HaveOccurred())
+
+			ingress := svc.Status.LoadBalancer.Ingress[0]
+			Expect(nb.Hostname).ToNot(BeNil())
+			Expect(ingress.Hostname).Should(Equal(*nb.Hostname))
+
+			if hasIP {
+				return nb.IPv4 != nil && ingress.IP == *nb.IPv4
+			} else {
+				return ingress.IP == ""
+			}
+		}).Should(BeTrue())
+	}
+
 	var checkNodeBalancerConfigForPort = func(port int, args checkArgs) {
 		By("Getting NodeBalancer Configuration for port " + strconv.Itoa(port))
 		nbConfig, err := f.LoadBalancer.GetNodeBalancerConfigForPort(framework.TestServerResourceName, port)
@@ -406,6 +427,72 @@ var _ = Describe("e2e tests", func() {
 				})
 			})
 
+			Context("With Hostname only ingress", func() {
+				var (
+					pods         []string
+					labels       map[string]string
+					servicePorts []core.ServicePort
+
+					annotations = map[string]string{}
+				)
+
+				BeforeEach(func() {
+					pods = []string{"test-pod-1"}
+					ports := []core.ContainerPort{
+						{
+							Name:          "http-1",
+							ContainerPort: 80,
+						},
+					}
+					servicePorts = []core.ServicePort{
+						{
+							Name:       "http-1",
+							Port:       80,
+							TargetPort: intstr.FromInt(80),
+							Protocol:   "TCP",
+						},
+					}
+
+					labels = map[string]string{
+						"app": "test-loadbalancer-with-hostname-only-ingress",
+					}
+
+					By("Creating Pod")
+					createPodWithLabel(pods, ports, framework.TestServerImage, labels, false)
+
+					By("Creating Service")
+					createServiceWithAnnotations(labels, map[string]string{}, servicePorts, false)
+				})
+
+				AfterEach(func() {
+					By("Deleting the Pods")
+					deletePods(pods)
+
+					By("Deleting the Service")
+					deleteService()
+				})
+
+				It("can update service to only use Hostname in ingress", func() {
+					By("Checking LB Status has IP")
+					checkLBStatus(framework.TestServerResourceName, true)
+
+					By("Annotating service with " + annLinodeHostnameOnlyIngress)
+					updateServiceWithAnnotations(labels, map[string]string{
+						annLinodeHostnameOnlyIngress: "true",
+					}, servicePorts, false)
+
+					By("Checking LB Status does not have IP")
+					checkLBStatus(framework.TestServerResourceName, false)
+				})
+
+				annotations[annLinodeHostnameOnlyIngress] = "true"
+
+				It("can create a service that only uses Hostname in ingress", func() {
+					By("Creating a service annotated with " + annLinodeHostnameOnlyIngress)
+					checkLBStatus(framework.TestServerResourceName, true)
+				})
+			})
+
 			Context("With ProxyProtocol", func() {
 				var (
 					pods         []string

e2e/test/framework/loadbalancer_suite.go

@@ -14,24 +14,32 @@ func (i *lbInvocation) GetHTTPEndpoints() ([]string, error) {
 	return i.getLoadBalancerURLs()
 }
 
-func (i *lbInvocation) GetNodeBalancerID(svcName string) (int, error) {
+func (i *lbInvocation) GetNodeBalancer(svcName string) (*linodego.NodeBalancer, error) {
 	hostname, err := i.waitForLoadBalancerHostname(svcName)
 	if err != nil {
-		return -1, err
+		return nil, err
 	}
 
 	nbList, errListNodeBalancers := i.linodeClient.ListNodeBalancers(context.Background(), nil)
 
 	if errListNodeBalancers != nil {
-		return -1, errListNodeBalancers
+		return nil, errListNodeBalancers
 	}
 
 	for _, nb := range nbList {
 		if *nb.Hostname == hostname {
-			return nb.ID, nil
+			return &nb, nil
 		}
 	}
-	return -1, fmt.Errorf("no NodeBalancer Found for service %v", svcName)
+	return nil, fmt.Errorf("no NodeBalancer Found for service %v", svcName)
+}
+
+func (i *lbInvocation) GetNodeBalancerID(svcName string) (int, error) {
+	nb, err := i.GetNodeBalancer(svcName)
+	if err != nil {
+		return -1, err
+	}
+	return nb.ID, nil
 }
 
 func (i *lbInvocation) WaitForNodeBalancerReady(svcName string, expectedID int) error {