From 7b29f60daf838aeae951b616c6c7b03e3918f01a Mon Sep 17 00:00:00 2001 From: Lior Noy Date: Thu, 20 Jun 2024 17:15:01 +0300 Subject: [PATCH 1/2] Fix the pod name for comDetails Modifies the function for creating comDetails, so the pod name will be more accurate. We fetch the name from the OwnerReference, and modify the name if needed. Added unit tests for the function. Signed-off-by: Lior Noy --- endpointslices/endpointslices.go | 37 +++++++++++- endpointslices/endpointslices_test.go | 85 +++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 endpointslices/endpointslices_test.go diff --git a/endpointslices/endpointslices.go b/endpointslices/endpointslices.go index 274d35d6..5c412dd8 100644 --- a/endpointslices/endpointslices.go +++ b/endpointslices/endpointslices.go @@ -3,6 +3,7 @@ package endpointslices import ( "context" "fmt" + "strings" log "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" @@ -200,6 +201,37 @@ func getContainerName(portNum int, pods []corev1.Pod) (string, error) { return res, nil } +func getPodName(pod *corev1.Pod) (string, error) { + var ( + res string + found bool + ) + + if len(pod.OwnerReferences) == 0 { + res, found = strings.CutSuffix(pod.Name, fmt.Sprintf("-%s", pod.Spec.NodeName)) + if !found { + return "", fmt.Errorf("pod name %s is not ending with node name %s", pod.Name, pod.Spec.NodeName) + } + + return res, nil + } + + name := pod.OwnerReferences[0].Name + switch pod.OwnerReferences[0].Kind { + case "Node": + res, found = strings.CutSuffix(pod.Name, fmt.Sprintf("-%s", pod.Spec.NodeName)) + if !found { + return "", fmt.Errorf("pod name %s is not ending with node name %s", pod.Name, pod.Spec.NodeName) + } + case "ReplicaSet": + res = name[:strings.LastIndex(name, "-")] + default: + res = name + } + + return res, nil +} + func (epSliceinfo *EndpointSlicesInfo) toComDetails(nodes []corev1.Node) ([]types.ComDetails, error) { if len(epSliceinfo.EndpointSlice.OwnerReferences) == 0 { return nil, fmt.Errorf("empty OwnerReferences in EndpointSlice %s/%s. skipping", epSliceinfo.EndpointSlice.Namespace, epSliceinfo.EndpointSlice.Name) @@ -209,7 +241,10 @@ func (epSliceinfo *EndpointSlicesInfo) toComDetails(nodes []corev1.Node) ([]type // Get the Namespace and Pod's name from the service. namespace := epSliceinfo.Service.Namespace - name := epSliceinfo.EndpointSlice.OwnerReferences[0].Name + name, err := getPodName(&epSliceinfo.Pods[0]) + if err != nil { + return nil, fmt.Errorf("failed to get pod name for endpointslice %s: %w", epSliceinfo.EndpointSlice.Name, err) + } // Get the node roles of this endpointslice. (master or worker or both). roles := getEndpointSliceNodeRoles(epSliceinfo, nodes) diff --git a/endpointslices/endpointslices_test.go b/endpointslices/endpointslices_test.go new file mode 100644 index 00000000..fbc37d12 --- /dev/null +++ b/endpointslices/endpointslices_test.go @@ -0,0 +1,85 @@ +package endpointslices + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type TestCase struct { + desc string + podName string + nodeName string + ownerRefs []metav1.OwnerReference + expected string +} + +func TestGetPodName(t *testing.T) { + tests := []TestCase{ + { + desc: "with-no-owner-reference", + nodeName: "worker-node", + podName: "kube-rbac-proxy-worker-node", + expected: "kube-rbac-proxy", + }, + { + desc: "with-owner-reference-kind-node", + nodeName: "worker-node", + podName: "kube-rbac-proxy-worker-node", + ownerRefs: []metav1.OwnerReference{ + { + Kind: "Node", + }, + }, + expected: "kube-rbac-proxy", + }, + { + desc: "with-owner-reference-kind-ReplicaSet", + ownerRefs: []metav1.OwnerReference{ + { + Kind: "ReplicaSet", + Name: "kube-rbac-proxy-7b7df454c7", + }, + }, + expected: "kube-rbac-proxy", + }, + { + desc: "with-owner-reference-kind-DaemonSet", + ownerRefs: []metav1.OwnerReference{ + { + Kind: "DaemonSet", + Name: "kube-rbac-proxy", + }, + }, + expected: "kube-rbac-proxy", + }, + { + desc: "with-owner-reference-kind-StatefulSet", + ownerRefs: []metav1.OwnerReference{ + { + Kind: "StatefulSet", + Name: "kube-rbac-proxy", + }, + }, + expected: "kube-rbac-proxy", + }, + } + for _, test := range tests { + p := defineTestPod(&test) + res, err := getPodName(p) + if err != nil { + t.Fatalf("test %s failed. got error: %s", test.desc, err) + } + if res != test.expected { + t.Fatalf("test %s failed. expected %v got %v", test.desc, test.expected, res) + } + } +} + +func defineTestPod(t *TestCase) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: t.podName, OwnerReferences: t.ownerRefs}, + Spec: corev1.PodSpec{NodeName: t.nodeName}, + } +} From 3a65418ca61f9ea4edd650c0eda59466bc54e894 Mon Sep 17 00:00:00 2001 From: Sabina Aledort Date: Mon, 1 Jul 2024 12:20:52 +0300 Subject: [PATCH 2/2] Fix extractPodName func --- endpointslices/endpointslices.go | 31 +++++++++++---------------- endpointslices/endpointslices_test.go | 13 ++++++----- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/endpointslices/endpointslices.go b/endpointslices/endpointslices.go index 5c412dd8..82d28792 100644 --- a/endpointslices/endpointslices.go +++ b/endpointslices/endpointslices.go @@ -201,35 +201,28 @@ func getContainerName(portNum int, pods []corev1.Pod) (string, error) { return res, nil } -func getPodName(pod *corev1.Pod) (string, error) { - var ( - res string - found bool - ) - +func extractPodName(pod *corev1.Pod) (string, error) { if len(pod.OwnerReferences) == 0 { - res, found = strings.CutSuffix(pod.Name, fmt.Sprintf("-%s", pod.Spec.NodeName)) - if !found { - return "", fmt.Errorf("pod name %s is not ending with node name %s", pod.Name, pod.Spec.NodeName) - } - - return res, nil + return pod.Name, nil } - name := pod.OwnerReferences[0].Name + ownerRefName := pod.OwnerReferences[0].Name switch pod.OwnerReferences[0].Kind { case "Node": - res, found = strings.CutSuffix(pod.Name, fmt.Sprintf("-%s", pod.Spec.NodeName)) + res, found := strings.CutSuffix(pod.Name, fmt.Sprintf("-%s", pod.Spec.NodeName)) if !found { return "", fmt.Errorf("pod name %s is not ending with node name %s", pod.Name, pod.Spec.NodeName) } + return res, nil case "ReplicaSet": - res = name[:strings.LastIndex(name, "-")] - default: - res = name + return ownerRefName[:strings.LastIndex(ownerRefName, "-")], nil + case "DaemonSet": + return ownerRefName, nil + case "StatefulSet": + return ownerRefName, nil } - return res, nil + return "", fmt.Errorf("failed to extract pod name for %s", pod.Name) } func (epSliceinfo *EndpointSlicesInfo) toComDetails(nodes []corev1.Node) ([]types.ComDetails, error) { @@ -241,7 +234,7 @@ func (epSliceinfo *EndpointSlicesInfo) toComDetails(nodes []corev1.Node) ([]type // Get the Namespace and Pod's name from the service. namespace := epSliceinfo.Service.Namespace - name, err := getPodName(&epSliceinfo.Pods[0]) + name, err := extractPodName(&epSliceinfo.Pods[0]) if err != nil { return nil, fmt.Errorf("failed to get pod name for endpointslice %s: %w", epSliceinfo.EndpointSlice.Name, err) } diff --git a/endpointslices/endpointslices_test.go b/endpointslices/endpointslices_test.go index fbc37d12..f8b44774 100644 --- a/endpointslices/endpointslices_test.go +++ b/endpointslices/endpointslices_test.go @@ -15,12 +15,11 @@ type TestCase struct { expected string } -func TestGetPodName(t *testing.T) { +func TestExtractPodName(t *testing.T) { tests := []TestCase{ { desc: "with-no-owner-reference", - nodeName: "worker-node", - podName: "kube-rbac-proxy-worker-node", + podName: "kube-rbac-proxy", expected: "kube-rbac-proxy", }, { @@ -35,7 +34,7 @@ func TestGetPodName(t *testing.T) { expected: "kube-rbac-proxy", }, { - desc: "with-owner-reference-kind-ReplicaSet", + desc: "with-owner-reference-kind-replicaset", ownerRefs: []metav1.OwnerReference{ { Kind: "ReplicaSet", @@ -45,7 +44,7 @@ func TestGetPodName(t *testing.T) { expected: "kube-rbac-proxy", }, { - desc: "with-owner-reference-kind-DaemonSet", + desc: "with-owner-reference-kind-daemonset", ownerRefs: []metav1.OwnerReference{ { Kind: "DaemonSet", @@ -55,7 +54,7 @@ func TestGetPodName(t *testing.T) { expected: "kube-rbac-proxy", }, { - desc: "with-owner-reference-kind-StatefulSet", + desc: "with-owner-reference-kind-statefulset", ownerRefs: []metav1.OwnerReference{ { Kind: "StatefulSet", @@ -67,7 +66,7 @@ func TestGetPodName(t *testing.T) { } for _, test := range tests { p := defineTestPod(&test) - res, err := getPodName(p) + res, err := extractPodName(p) if err != nil { t.Fatalf("test %s failed. got error: %s", test.desc, err) }