From df417aa9b8813d0e5e1208e9039f140edeff39db Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Wed, 1 May 2024 14:45:45 +0530 Subject: [PATCH 1/2] kubectl/describe/svc: refactor TestDescribeService Signed-off-by: Daman Arora --- .../kubectl/pkg/describe/describe_test.go | 177 ++++++++++-------- 1 file changed, 101 insertions(+), 76 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/describe/describe_test.go b/staging/src/k8s.io/kubectl/pkg/describe/describe_test.go index 61d39379358..e55f39182b0 100644 --- a/staging/src/k8s.io/kubectl/pkg/describe/describe_test.go +++ b/staging/src/k8s.io/kubectl/pkg/describe/describe_test.go @@ -25,6 +25,9 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/lithammer/dedent" + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" @@ -652,9 +655,9 @@ func getResourceList(cpu, memory string) corev1.ResourceList { func TestDescribeService(t *testing.T) { singleStack := corev1.IPFamilyPolicySingleStack testCases := []struct { - name string - service *corev1.Service - expect []string + name string + service *corev1.Service + expected string }{ { name: "test1", @@ -676,24 +679,31 @@ func TestDescribeService(t *testing.T) { ClusterIP: "1.2.3.4", IPFamilies: []corev1.IPFamily{corev1.IPv4Protocol}, LoadBalancerIP: "5.6.7.8", - SessionAffinity: "None", - ExternalTrafficPolicy: "Local", + SessionAffinity: corev1.ServiceAffinityNone, + ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal, HealthCheckNodePort: 32222, }, }, - expect: []string{ - "Name", "bar", - "Namespace", "foo", - "Selector", "blah=heh", - "Type", "LoadBalancer", - "IP", "1.2.3.4", - "Port", "port-tcp", "8080/TCP", - "TargetPort", "9527/TCP", - "NodePort", "port-tcp", "31111/TCP", - "Session Affinity", "None", - "External Traffic Policy", "Local", - "HealthCheck NodePort", "32222", - }, + expected: dedent.Dedent(` + Name: bar + Namespace: foo + Labels: + Annotations: + Selector: blah=heh + Type: LoadBalancer + IP Families: IPv4 + IP: 1.2.3.4 + IPs: + IP: 5.6.7.8 + Port: port-tcp 8080/TCP + TargetPort: 9527/TCP + NodePort: port-tcp 31111/TCP + Endpoints: + Session Affinity: None + External Traffic Policy: Local + HealthCheck NodePort: 32222 + Events: + `)[1:], }, { name: "test2", @@ -715,24 +725,31 @@ func TestDescribeService(t *testing.T) { ClusterIP: "1.2.3.4", IPFamilies: []corev1.IPFamily{corev1.IPv4Protocol}, LoadBalancerIP: "5.6.7.8", - SessionAffinity: "None", - ExternalTrafficPolicy: "Local", + SessionAffinity: corev1.ServiceAffinityNone, + ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal, HealthCheckNodePort: 32222, }, }, - expect: []string{ - "Name", "bar", - "Namespace", "foo", - "Selector", "blah=heh", - "Type", "LoadBalancer", - "IP", "1.2.3.4", - "Port", "port-tcp", "8080/TCP", - "TargetPort", "targetPort/TCP", - "NodePort", "port-tcp", "31111/TCP", - "Session Affinity", "None", - "External Traffic Policy", "Local", - "HealthCheck NodePort", "32222", - }, + expected: dedent.Dedent(` + Name: bar + Namespace: foo + Labels: + Annotations: + Selector: blah=heh + Type: LoadBalancer + IP Families: IPv4 + IP: 1.2.3.4 + IPs: + IP: 5.6.7.8 + Port: port-tcp 8080/TCP + TargetPort: targetPort/TCP + NodePort: port-tcp 31111/TCP + Endpoints: + Session Affinity: None + External Traffic Policy: Local + HealthCheck NodePort: 32222 + Events: + `)[1:], }, { name: "test-ServiceIPFamily", @@ -754,25 +771,31 @@ func TestDescribeService(t *testing.T) { ClusterIP: "1.2.3.4", IPFamilies: []corev1.IPFamily{corev1.IPv4Protocol}, LoadBalancerIP: "5.6.7.8", - SessionAffinity: "None", - ExternalTrafficPolicy: "Local", + SessionAffinity: corev1.ServiceAffinityNone, + ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal, HealthCheckNodePort: 32222, }, }, - expect: []string{ - "Name", "bar", - "Namespace", "foo", - "Selector", "blah=heh", - "Type", "LoadBalancer", - "IP", "1.2.3.4", - "IP Families", "IPv4", - "Port", "port-tcp", "8080/TCP", - "TargetPort", "targetPort/TCP", - "NodePort", "port-tcp", "31111/TCP", - "Session Affinity", "None", - "External Traffic Policy", "Local", - "HealthCheck NodePort", "32222", - }, + expected: dedent.Dedent(` + Name: bar + Namespace: foo + Labels: + Annotations: + Selector: blah=heh + Type: LoadBalancer + IP Families: IPv4 + IP: 1.2.3.4 + IPs: + IP: 5.6.7.8 + Port: port-tcp 8080/TCP + TargetPort: targetPort/TCP + NodePort: port-tcp 31111/TCP + Endpoints: + Session Affinity: None + External Traffic Policy: Local + HealthCheck NodePort: 32222 + Events: + `)[1:], }, { name: "test-ServiceIPFamilyPolicy+ClusterIPs", @@ -796,43 +819,45 @@ func TestDescribeService(t *testing.T) { IPFamilyPolicy: &singleStack, ClusterIPs: []string{"1.2.3.4"}, LoadBalancerIP: "5.6.7.8", - SessionAffinity: "None", - ExternalTrafficPolicy: "Local", + SessionAffinity: corev1.ServiceAffinityNone, + ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal, HealthCheckNodePort: 32222, }, }, - expect: []string{ - "Name", "bar", - "Namespace", "foo", - "Selector", "blah=heh", - "Type", "LoadBalancer", - "IP", "1.2.3.4", - "IP Families", "IPv4", - "IP Family Policy", "SingleStack", - "IPs", "1.2.3.4", - "Port", "port-tcp", "8080/TCP", - "TargetPort", "targetPort/TCP", - "NodePort", "port-tcp", "31111/TCP", - "Session Affinity", "None", - "External Traffic Policy", "Local", - "HealthCheck NodePort", "32222", - }, + expected: dedent.Dedent(` + Name: bar + Namespace: foo + Labels: + Annotations: + Selector: blah=heh + Type: LoadBalancer + IP Family Policy: SingleStack + IP Families: IPv4 + IP: 1.2.3.4 + IPs: 1.2.3.4 + IP: 5.6.7.8 + Port: port-tcp 8080/TCP + TargetPort: targetPort/TCP + NodePort: port-tcp 31111/TCP + Endpoints: + Session Affinity: None + External Traffic Policy: Local + HealthCheck NodePort: 32222 + Events: + `)[1:], }, } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - fake := fake.NewSimpleClientset(testCase.service) - c := &describeClient{T: t, Namespace: "foo", Interface: fake} + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fakeClient := fake.NewSimpleClientset(tc.service) + c := &describeClient{T: t, Namespace: "foo", Interface: fakeClient} d := ServiceDescriber{c} out, err := d.Describe("foo", "bar", DescriberSettings{ShowEvents: true}) if err != nil { t.Errorf("unexpected error: %v", err) } - for _, expected := range testCase.expect { - if !strings.Contains(out, expected) { - t.Errorf("expected to find %q in output: %q", expected, out) - } - } + + assert.Equal(t, tc.expected, out) }) } } From 3236891023c991f1b66a49b8c9f6331ed6b4a690 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Wed, 1 May 2024 15:09:02 +0530 Subject: [PATCH 2/2] kubectl/describe: use endpointslices for describing endpoints Signed-off-by: Daman Arora --- .../k8s.io/kubectl/pkg/describe/describe.go | 54 +++++---- .../kubectl/pkg/describe/describe_test.go | 112 +++++++++++++++--- 2 files changed, 124 insertions(+), 42 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/describe/describe.go b/staging/src/k8s.io/kubectl/pkg/describe/describe.go index 18fdd31be6f..b9aa38990b5 100644 --- a/staging/src/k8s.io/kubectl/pkg/describe/describe.go +++ b/staging/src/k8s.io/kubectl/pkg/describe/describe.go @@ -2603,7 +2603,9 @@ func (i *IngressDescriber) Describe(namespace, name string, describerSettings De } func (i *IngressDescriber) describeBackendV1beta1(ns string, backend *networkingv1beta1.IngressBackend) string { - endpoints, err := i.client.CoreV1().Endpoints(ns).Get(context.TODO(), backend.ServiceName, metav1.GetOptions{}) + endpointSliceList, err := i.client.DiscoveryV1().EndpointSlices(ns).List(context.TODO(), metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", discoveryv1.LabelServiceName, backend.ServiceName), + }) if err != nil { return fmt.Sprintf("", err) } @@ -2625,20 +2627,22 @@ func (i *IngressDescriber) describeBackendV1beta1(ns string, backend *networking } } } - return formatEndpoints(endpoints, sets.NewString(spName)) + return formatEndpointSlices(endpointSliceList.Items, sets.New(spName)) } func (i *IngressDescriber) describeBackendV1(ns string, backend *networkingv1.IngressBackend) string { if backend.Service != nil { sb := serviceBackendStringer(backend.Service) - endpoints, err := i.client.CoreV1().Endpoints(ns).Get(context.TODO(), backend.Service.Name, metav1.GetOptions{}) + endpointSliceList, err := i.client.DiscoveryV1().EndpointSlices(ns).List(context.TODO(), metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", discoveryv1.LabelServiceName, backend.Service.Name), + }) if err != nil { return fmt.Sprintf("%v ()", sb, err) } service, err := i.client.CoreV1().Services(ns).Get(context.TODO(), backend.Service.Name, metav1.GetOptions{}) if err != nil { - return fmt.Sprintf("%v()", sb, err) + return fmt.Sprintf("%v ()", sb, err) } spName := "" for i := range service.Spec.Ports { @@ -2649,7 +2653,7 @@ func (i *IngressDescriber) describeBackendV1(ns string, backend *networkingv1.In spName = sp.Name } } - ep := formatEndpoints(endpoints, sets.NewString(spName)) + ep := formatEndpointSlices(endpointSliceList.Items, sets.New(spName)) return fmt.Sprintf("%s (%s)", sb, ep) } if backend.Resource != nil { @@ -2961,12 +2965,14 @@ func (d *ServiceDescriber) Describe(namespace, name string, describerSettings De return "", err } - endpoints, _ := d.CoreV1().Endpoints(namespace).Get(context.TODO(), name, metav1.GetOptions{}) + endpointSliceList, _ := d.DiscoveryV1().EndpointSlices(namespace).List(context.TODO(), metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", discoveryv1.LabelServiceName, name), + }) var events *corev1.EventList if describerSettings.ShowEvents { events, _ = searchEvents(d.CoreV1(), service, describerSettings.ChunkSize) } - return describeService(service, endpoints, events) + return describeService(service, endpointSliceList.Items, events) } func buildIngressString(ingress []corev1.LoadBalancerIngress) string { @@ -2985,10 +2991,7 @@ func buildIngressString(ingress []corev1.LoadBalancerIngress) string { return buffer.String() } -func describeService(service *corev1.Service, endpoints *corev1.Endpoints, events *corev1.EventList) (string, error) { - if endpoints == nil { - endpoints = &corev1.Endpoints{} - } +func describeService(service *corev1.Service, endpointSlices []discoveryv1.EndpointSlice, events *corev1.EventList) (string, error) { return tabbedString(func(out io.Writer) error { w := NewPrefixWriter(out) w.Write(LEVEL_0, "Name:\t%s\n", service.Name) @@ -3049,7 +3052,7 @@ func describeService(service *corev1.Service, endpoints *corev1.Endpoints, event if sp.NodePort != 0 { w.Write(LEVEL_0, "NodePort:\t%s\t%d/%s\n", name, sp.NodePort, sp.Protocol) } - w.Write(LEVEL_0, "Endpoints:\t%s\n", formatEndpoints(endpoints, sets.NewString(sp.Name))) + w.Write(LEVEL_0, "Endpoints:\t%s\n", formatEndpointSlices(endpointSlices, sets.New(sp.Name))) } w.Write(LEVEL_0, "Session Affinity:\t%s\n", service.Spec.SessionAffinity) if service.Spec.ExternalTrafficPolicy != "" { @@ -5419,39 +5422,38 @@ func translateTimestampSince(timestamp metav1.Time) string { } // Pass ports=nil for all ports. -func formatEndpoints(endpoints *corev1.Endpoints, ports sets.String) string { - if len(endpoints.Subsets) == 0 { +func formatEndpointSlices(endpointSlices []discoveryv1.EndpointSlice, ports sets.Set[string]) string { + if len(endpointSlices) == 0 { return "" } - list := []string{} + var list []string max := 3 more := false count := 0 - for i := range endpoints.Subsets { - ss := &endpoints.Subsets[i] - if len(ss.Ports) == 0 { + for i := range endpointSlices { + if len(endpointSlices[i].Ports) == 0 { // It's possible to have headless services with no ports. - for i := range ss.Addresses { + for j := range endpointSlices[i].Endpoints { if len(list) == max { more = true } if !more { - list = append(list, ss.Addresses[i].IP) + list = append(list, endpointSlices[i].Endpoints[j].Addresses[0]) } count++ } } else { // "Normal" services with ports defined. - for i := range ss.Ports { - port := &ss.Ports[i] - if ports == nil || ports.Has(port.Name) { - for i := range ss.Addresses { + for j := range endpointSlices[i].Ports { + port := endpointSlices[i].Ports[j] + if ports == nil || ports.Has(*port.Name) { + for k := range endpointSlices[i].Endpoints { if len(list) == max { more = true } - addr := &ss.Addresses[i] + addr := endpointSlices[i].Endpoints[k].Addresses[0] if !more { - hostPort := net.JoinHostPort(addr.IP, strconv.Itoa(int(port.Port))) + hostPort := net.JoinHostPort(addr, strconv.Itoa(int(*port.Port))) list = append(list, hostPort) } count++ diff --git a/staging/src/k8s.io/kubectl/pkg/describe/describe_test.go b/staging/src/k8s.io/kubectl/pkg/describe/describe_test.go index e55f39182b0..b00718d8d98 100644 --- a/staging/src/k8s.io/kubectl/pkg/describe/describe_test.go +++ b/staging/src/k8s.io/kubectl/pkg/describe/describe_test.go @@ -655,9 +655,10 @@ func getResourceList(cpu, memory string) corev1.ResourceList { func TestDescribeService(t *testing.T) { singleStack := corev1.IPFamilyPolicySingleStack testCases := []struct { - name string - service *corev1.Service - expected string + name string + service *corev1.Service + endpointSlices []*discoveryv1.EndpointSlice + expected string }{ { name: "test1", @@ -684,6 +685,25 @@ func TestDescribeService(t *testing.T) { HealthCheckNodePort: 32222, }, }, + endpointSlices: []*discoveryv1.EndpointSlice{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar-abcde", + Namespace: "foo", + Labels: map[string]string{ + "kubernetes.io/service-name": "bar", + }, + }, + Endpoints: []discoveryv1.Endpoint{ + {Addresses: []string{"10.244.0.1"}}, + {Addresses: []string{"10.244.0.2"}}, + {Addresses: []string{"10.244.0.3"}}, + }, + Ports: []discoveryv1.EndpointPort{{ + Name: ptr.To("port-tcp"), + Port: ptr.To[int32](9527), + Protocol: ptr.To(corev1.ProtocolTCP), + }}, + }}, expected: dedent.Dedent(` Name: bar Namespace: foo @@ -698,7 +718,7 @@ func TestDescribeService(t *testing.T) { Port: port-tcp 8080/TCP TargetPort: 9527/TCP NodePort: port-tcp 31111/TCP - Endpoints: + Endpoints: 10.244.0.1:9527,10.244.0.2:9527,10.244.0.3:9527 Session Affinity: None External Traffic Policy: Local HealthCheck NodePort: 32222 @@ -730,6 +750,45 @@ func TestDescribeService(t *testing.T) { HealthCheckNodePort: 32222, }, }, + endpointSlices: []*discoveryv1.EndpointSlice{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "bar-12345", + Namespace: "foo", + Labels: map[string]string{ + "kubernetes.io/service-name": "bar", + }, + }, + Endpoints: []discoveryv1.Endpoint{ + {Addresses: []string{"10.244.0.1"}}, + {Addresses: []string{"10.244.0.2"}}, + }, + Ports: []discoveryv1.EndpointPort{{ + Name: ptr.To("port-tcp"), + Port: ptr.To[int32](9527), + Protocol: ptr.To(corev1.ProtocolUDP), + }}, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "bar-54321", + Namespace: "foo", + Labels: map[string]string{ + "kubernetes.io/service-name": "bar", + }, + }, + Endpoints: []discoveryv1.Endpoint{ + {Addresses: []string{"10.244.0.3"}}, + {Addresses: []string{"10.244.0.4"}}, + {Addresses: []string{"10.244.0.5"}}, + }, + Ports: []discoveryv1.EndpointPort{{ + Name: ptr.To("port-tcp"), + Port: ptr.To[int32](9527), + Protocol: ptr.To(corev1.ProtocolUDP), + }}, + }, + }, expected: dedent.Dedent(` Name: bar Namespace: foo @@ -744,7 +803,7 @@ func TestDescribeService(t *testing.T) { Port: port-tcp 8080/TCP TargetPort: targetPort/TCP NodePort: port-tcp 31111/TCP - Endpoints: + Endpoints: 10.244.0.1:9527,10.244.0.2:9527,10.244.0.3:9527 + 2 more... Session Affinity: None External Traffic Policy: Local HealthCheck NodePort: 32222 @@ -776,6 +835,23 @@ func TestDescribeService(t *testing.T) { HealthCheckNodePort: 32222, }, }, + endpointSlices: []*discoveryv1.EndpointSlice{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar-123ab", + Namespace: "foo", + Labels: map[string]string{ + "kubernetes.io/service-name": "bar", + }, + }, + Endpoints: []discoveryv1.Endpoint{ + {Addresses: []string{"10.244.0.1"}}, + }, + Ports: []discoveryv1.EndpointPort{{ + Name: ptr.To("port-tcp"), + Port: ptr.To[int32](9527), + Protocol: ptr.To(corev1.ProtocolTCP), + }}, + }}, expected: dedent.Dedent(` Name: bar Namespace: foo @@ -790,7 +866,7 @@ func TestDescribeService(t *testing.T) { Port: port-tcp 8080/TCP TargetPort: targetPort/TCP NodePort: port-tcp 31111/TCP - Endpoints: + Endpoints: 10.244.0.1:9527 Session Affinity: None External Traffic Policy: Local HealthCheck NodePort: 32222 @@ -849,7 +925,11 @@ func TestDescribeService(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - fakeClient := fake.NewSimpleClientset(tc.service) + objects := []runtime.Object{tc.service} + for i := range tc.endpointSlices { + objects = append(objects, tc.endpointSlices[i]) + } + fakeClient := fake.NewSimpleClientset(objects...) c := &describeClient{T: t, Namespace: "foo", Interface: fakeClient} d := ServiceDescriber{c} out, err := d.Describe("foo", "bar", DescriberSettings{ShowEvents: true}) @@ -3087,7 +3167,7 @@ Rules: Host Path Backends ---- ---- -------- foo.bar.com - /foo default-backend:80 () + /foo default-backend:80 () Annotations: Events: ` + "\n", }, @@ -3103,7 +3183,7 @@ Rules: Host Path Backends ---- ---- -------- foo.bar.com - /foo default-backend:80 () + /foo default-backend:80 () Annotations: Events: ` + "\n", }, @@ -3216,12 +3296,12 @@ Labels: Namespace: foo Address: Ingress Class: test -Default backend: default-backend:80 () +Default backend: default-backend:80 () Rules: Host Path Backends ---- ---- -------- foo.bar.com - /foo default-backend:80 () + /foo default-backend:80 () Annotations: Events: ` + "\n", }, @@ -3301,7 +3381,7 @@ Rules: Host Path Backends ---- ---- -------- foo.bar.com - /foo default-backend:80 () + /foo default-backend:80 () Annotations: Events: ` + "\n", }, @@ -3321,11 +3401,11 @@ Labels: Namespace: foo Address: Ingress Class: test -Default backend: default-backend:80 () +Default backend: default-backend:80 () Rules: Host Path Backends ---- ---- -------- - * * default-backend:80 () + * * default-backend:80 () Annotations: Events: `, @@ -3369,11 +3449,11 @@ Labels: Namespace: foo Address: Ingress Class: -Default backend: default-backend:80 () +Default backend: default-backend:80 () Rules: Host Path Backends ---- ---- -------- - * * default-backend:80 () + * * default-backend:80 () Annotations: Events: `,