From 2d5445153fc0d9c4df325f17e7a5563cb3e54b68 Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Thu, 25 Jul 2024 13:58:41 -0700 Subject: [PATCH] Address latest round of review comments Signed-off-by: Antonin Bas --- .../registry/stats/nodelatencystats/rest.go | 4 +-- .../stats/nodelatencystats/rest_test.go | 27 ++++++------------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/pkg/apiserver/registry/stats/nodelatencystats/rest.go b/pkg/apiserver/registry/stats/nodelatencystats/rest.go index 373f06527ad..b7f05e545bc 100644 --- a/pkg/apiserver/registry/stats/nodelatencystats/rest.go +++ b/pkg/apiserver/registry/stats/nodelatencystats/rest.go @@ -102,8 +102,8 @@ func (r *REST) ConvertToTable(ctx context.Context, obj runtime.Object, tableOpti ColumnDefinitions: []metav1.TableColumnDefinition{ {Name: "Node Name", Type: "string", Format: "name", Description: "Name of Node from which latency was measured."}, {Name: "Num Latency Entries", Type: "integer", Format: "int64", Description: "Number of peers for which latency measurements are available."}, - {Name: "Max Latency", Type: "string", Format: "", Description: "Name of the peer with the highest latency and the latency value."}, {Name: "Avg Latency", Type: "string", Format: "", Description: "Average latency value across all peers."}, + {Name: "Max Latency", Type: "string", Format: "", Description: "Largest latency value across all peers."}, }, } if m, err := meta.ListAccessor(obj); err == nil { @@ -146,7 +146,7 @@ func (r *REST) ConvertToTable(ctx context.Context, obj runtime.Object, tableOpti avgLatency = avgLatency / targetIPLatencyCount } - return []interface{}{name, peerNodeLatencyEntriesCount, time.Duration(maxLatency).String(), time.Duration(avgLatency).String()}, nil + return []interface{}{name, peerNodeLatencyEntriesCount, time.Duration(avgLatency).String(), time.Duration(maxLatency).String()}, nil }) return table, err } diff --git a/pkg/apiserver/registry/stats/nodelatencystats/rest_test.go b/pkg/apiserver/registry/stats/nodelatencystats/rest_test.go index 3f427844567..8a5d1071e90 100644 --- a/pkg/apiserver/registry/stats/nodelatencystats/rest_test.go +++ b/pkg/apiserver/registry/stats/nodelatencystats/rest_test.go @@ -22,7 +22,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -50,7 +49,7 @@ func TestRESTCreate(t *testing.T) { ctx := context.Background() obj, err := r.Create(ctx, summary, nil, nil) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, expectedObj, obj) } @@ -161,10 +160,6 @@ func TestRESTList(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "node1"}, PeerNodeLatencyStats: nil, } - options := &internalversion.ListOptions{ - Limit: 10, - Continue: "", - } expectedObj := &statsv1alpha1.NodeLatencyStatsList{ Items: []statsv1alpha1.NodeLatencyStats{ { @@ -179,7 +174,7 @@ func TestRESTList(t *testing.T) { _, err := r.Create(ctx, summary, nil, nil) require.NoError(t, err) - objs, err := r.List(ctx, options) + objs, err := r.List(ctx, nil) require.NoError(t, err) assert.Equal(t, expectedObj, objs) } @@ -193,32 +188,27 @@ func TestRESTConvertToTable(t *testing.T) { NodeName: "node2", TargetIPLatencyStats: []statsv1alpha1.TargetIPLatencyStats{ { - TargetIP: "192.168.0.1", + TargetIP: "192.168.0.2", LastSendTime: metav1.Time{Time: mockTime}, LastRecvTime: metav1.Time{Time: mockTime}, - LastMeasuredRTTNanoseconds: 0, + LastMeasuredRTTNanoseconds: 1000000, }, }, }, - }, - } - expectedObj := &statsv1alpha1.NodeLatencyStats{ - ObjectMeta: metav1.ObjectMeta{Name: "node1"}, - PeerNodeLatencyStats: []statsv1alpha1.PeerNodeLatencyStats{ { - NodeName: "node2", + NodeName: "node3", TargetIPLatencyStats: []statsv1alpha1.TargetIPLatencyStats{ { - TargetIP: "192.168.0.1", + TargetIP: "192.168.0.3", LastSendTime: metav1.Time{Time: mockTime}, LastRecvTime: metav1.Time{Time: mockTime}, - LastMeasuredRTTNanoseconds: 0, + LastMeasuredRTTNanoseconds: 2000000, }, }, }, }, } - expectedCells := []interface{}{"node1", 1, "0s", "0s"} + expectedCells := []interface{}{"node1", 2, "1.5ms", "2ms"} r := NewREST() ctx := context.Background() @@ -227,6 +217,5 @@ func TestRESTConvertToTable(t *testing.T) { require.NoError(t, err) obj, err := r.ConvertToTable(ctx, summary, nil) require.NoError(t, err) - assert.Equal(t, expectedObj, obj.Rows[0].Object.Object) assert.Equal(t, expectedCells, obj.Rows[0].Cells) }