diff --git a/README.md b/README.md index 5a39ee1b..53637d92 100644 --- a/README.md +++ b/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 diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 26ec737b..14320539 100644 --- a/cloud/linode/loadbalancers.go +++ b/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 +} diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index cb62b55e..7620fe68 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/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 { diff --git a/e2e/test/ccm_e2e_test.go b/e2e/test/ccm_e2e_test.go index a5e76644..dcb6c269 100644 --- a/e2e/test/ccm_e2e_test.go +++ b/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 diff --git a/e2e/test/framework/loadbalancer_suite.go b/e2e/test/framework/loadbalancer_suite.go index e75957ab..610f8644 100644 --- a/e2e/test/framework/loadbalancer_suite.go +++ b/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 {