From 921a83f8580c87dee549e65022ddf00bfb721991 Mon Sep 17 00:00:00 2001 From: Alex Masi Date: Mon, 9 Oct 2023 16:53:00 -0700 Subject: [PATCH] fix error case with intentional empty services (#439) --- topo/node/node.go | 3 +++ topo/node/node_test.go | 10 +++----- topo/topo.go | 2 +- topo/topo_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 8 deletions(-) diff --git a/topo/node/node.go b/topo/node/node.go index dc668851..ed5ad384 100644 --- a/topo/node/node.go +++ b/topo/node/node.go @@ -660,6 +660,9 @@ func (n *Impl) Pods(ctx context.Context) ([]*corev1.Pod, error) { // Service returns the service definition for the node. func (n *Impl) Services(ctx context.Context) ([]*corev1.Service, error) { + if len(n.Proto.Services) == 0 { + return nil, nil + } s, err := n.KubeClient.CoreV1().Services(n.Namespace).Get(ctx, fmt.Sprintf("service-%s", n.Name()), metav1.GetOptions{}) if err != nil { return nil, err diff --git a/topo/node/node_test.go b/topo/node/node_test.go index 213c4a27..c75bc2e3 100644 --- a/topo/node/node_test.go +++ b/topo/node/node_test.go @@ -219,10 +219,9 @@ func TestService(t *testing.T) { wantServiceErr string want []*corev1.Service }{{ - desc: "no services", - node: &topopb.Node{Name: "dev1", Vendor: topopb.Vendor(1001)}, - kClient: kfake.NewSimpleClientset(), - wantServiceErr: `"service-dev1" not found`, + desc: "no services", + node: &topopb.Node{Name: "dev1", Vendor: topopb.Vendor(1001)}, + kClient: kfake.NewSimpleClientset(), }, { desc: "services valid", node: &topopb.Node{ @@ -337,9 +336,6 @@ func TestService(t *testing.T) { if s := errdiff.Check(err, tt.wantCreateErr); s != "" { t.Fatalf("CreateService() failed: %s", s) } - if tt.wantServiceErr != "" { - return - } got, err := n.Services(context.Background()) if s := errdiff.Check(err, tt.wantServiceErr); s != "" { diff --git a/topo/topo.go b/topo/topo.go index aa79df20..bd0896a3 100644 --- a/topo/topo.go +++ b/topo/topo.go @@ -368,7 +368,7 @@ func (m *Manager) Show(ctx context.Context) (*cpb.ShowTopologyResponse, error) { } for _, n := range m.topo.Nodes { if len(n.Services) == 0 { - n.Services = map[uint32]*tpb.Service{} + continue } services, ok := r.Services[n.Name] if !ok { diff --git a/topo/topo_test.go b/topo/topo_test.go index 8c7bf7bf..be7eccef 100644 --- a/topo/topo_test.go +++ b/topo/topo_test.go @@ -955,6 +955,10 @@ func TestShow(t *testing.T) { }, }, }, + { + Name: "r3", + Vendor: tpb.Vendor(1004), + }, }, } @@ -1013,6 +1017,16 @@ func TestShow(t *testing.T) { Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, }, }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "r3", + Namespace: "test", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, + }, + }, &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "service-r1", @@ -1100,6 +1114,16 @@ func TestShow(t *testing.T) { Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, }, }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "r3", + Namespace: "test", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, + }, + }, &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "service-r1", @@ -1209,6 +1233,16 @@ func TestShow(t *testing.T) { Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, }, }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "r3", + Namespace: "test", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, + }, + }, }, wantErr: "could not get services", }, { @@ -1236,6 +1270,16 @@ func TestShow(t *testing.T) { Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, }, }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "r3", + Namespace: "test", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, + }, + }, &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "service-r1", @@ -1320,6 +1364,16 @@ func TestShow(t *testing.T) { Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, }, }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "r3", + Namespace: "test", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, + }, + }, &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "service-r1",