Skip to content

Commit

Permalink
feat: remove paged options for List and fix unit test define.
Browse files Browse the repository at this point in the history 
Signed-off-by: Asklv <[email protected]>
  • Loading branch information
@IRONICBo
IRONICBo committed Jul 24, 2024
1 parent 3e38af0 commit 735eba1
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 115 deletions.
53 changes: 34 additions & 19 deletions pkg/apiserver/registry/stats/nodelatencystats/rest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package nodelatencystats

import (
"context"
"strconv"

"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
Expand Down Expand Up @@ -58,6 +57,7 @@ func (r *REST) Destroy() {
}

func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
// Update will add the object if the key does not exist.
summary := obj.(*statsv1alpha1.NodeLatencyStats)
if err := r.indexer.Update(summary); err != nil {
return nil, errors.NewInternalError(err)
Expand Down Expand Up @@ -85,19 +85,8 @@ func (r *REST) NewList() runtime.Object {
func (r *REST) List(ctx context.Context, options *internalversion.ListOptions) (runtime.Object, error) {
objs := r.indexer.List()

if options.Continue != "" {
start, err := strconv.Atoi(options.Continue)
if err != nil {
return nil, errors.NewBadRequest("invalid continue token")
}
if start < 0 || start >= len(objs) {
return r.NewList(), nil
}
objs = objs[start:]
}
if options.Limit > 0 && int64(len(objs)) > options.Limit {
objs = objs[:options.Limit]
}
// Due to the unordered nature of map iteration and the complexity of controlling 'continue',
// we will ignore paging here and plan to implement it in the future.

entries := make([]statsv1alpha1.NodeLatencyStats, 0, len(objs))
for _, obj := range objs {
Expand All @@ -110,8 +99,10 @@ func (r *REST) List(ctx context.Context, options *internalversion.ListOptions) (
func (r *REST) ConvertToTable(ctx context.Context, obj runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) {
table := &metav1.Table{
ColumnDefinitions: []metav1.TableColumnDefinition{
{Name: "SourceNodeName", Type: "string", Format: "name", Description: "Source node name."},
{Name: "PeerNodeLatencyStats", Type: "array", Format: "string", Description: "Current Node to each peers latency."},
{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: "integer", Format: "int64", Description: "Name of the peer with the highest latency and the latency value(ns)."},
{Name: "Avg Latency", Type: "integer", Format: "int64", Description: "Average latency value across all peers(ns)."},
},
}
if m, err := meta.ListAccessor(obj); err == nil {
Expand All @@ -127,7 +118,32 @@ func (r *REST) ConvertToTable(ctx context.Context, obj runtime.Object, tableOpti
var err error
table.Rows, err = metatable.MetaToTableRow(obj, func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error) {
summary := obj.(*statsv1alpha1.NodeLatencyStats)
return []interface{}{name, summary.PeerNodeLatencyStats}, nil

// Calculate the max and average latency values.
peerNodeLatencyEntriesCount := len(summary.PeerNodeLatencyStats)
var targetIPLatencyCount int64
var maxLatency int64
var avgLatency int64

for i := range summary.PeerNodeLatencyStats {
targetIPLatency := summary.PeerNodeLatencyStats[i]

for j := range targetIPLatency.TargetIPLatencyStats {
targetIPLatencyCount++
currentLatency := targetIPLatency.TargetIPLatencyStats[j].LastMeasuredRTTNanoseconds
if currentLatency > maxLatency {
maxLatency = currentLatency
}

if targetIPLatencyCount == 1 {
avgLatency = currentLatency
} else {
avgLatency += (currentLatency - avgLatency) / targetIPLatencyCount
}
}
}

return []interface{}{name, peerNodeLatencyEntriesCount, maxLatency, avgLatency}, nil
})
return table, err
}
Expand All @@ -142,8 +158,7 @@ func (r *REST) Delete(ctx context.Context, name string, deleteValidation rest.Va
return nil, false, errors.NewNotFound(statsv1alpha1.Resource("nodelatencystats"), name)
}

err = r.indexer.Delete(obj)
if err != nil {
if err = r.indexer.Delete(obj); err != nil {
return nil, false, errors.NewInternalError(err)
}

Expand Down
156 changes: 60 additions & 96 deletions pkg/apiserver/registry/stats/nodelatencystats/rest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"testing"

"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"
Expand All @@ -34,8 +36,6 @@ func TestREST(t *testing.T) {
}

func TestRESTCreate(t *testing.T) {
// Define the test case
name := "create summary"
summary := &statsv1alpha1.NodeLatencyStats{
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
Expand All @@ -44,19 +44,13 @@ func TestRESTCreate(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
}
expectedErr := false

// Execute the test case
t.Run(name, func(t *testing.T) {
r := NewREST()
obj, err := r.Create(context.TODO(), summary, nil, nil)
if expectedErr {
assert.NotNil(t, err)
} else {
assert.Nil(t, err)
assert.Equal(t, expectedObj, obj)
}
})

r := NewREST()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
obj, err := r.Create(ctx, summary, nil, nil)
assert.NoError(t, err)
assert.Equal(t, expectedObj, obj)
}

func TestRESTGet(t *testing.T) {
Expand All @@ -66,6 +60,7 @@ func TestRESTGet(t *testing.T) {
nodeName string
expectedObj runtime.Object
expectedErr bool
err error
}{
{
name: "get summary",
Expand All @@ -79,6 +74,7 @@ func TestRESTGet(t *testing.T) {
PeerNodeLatencyStats: nil,
},
expectedErr: false,
err: nil,
},
{
name: "get summary not found",
Expand All @@ -89,18 +85,21 @@ func TestRESTGet(t *testing.T) {
nodeName: "node2",
expectedObj: nil,
expectedErr: true,
err: errors.NewNotFound(statsv1alpha1.Resource("nodelatencystats"), "node2"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := NewREST()
_, err := r.Create(context.TODO(), tt.summary, nil, nil)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
_, err := r.Create(ctx, tt.summary, nil, nil)
assert.Nil(t, err)
obj, err := r.Get(context.TODO(), tt.nodeName, nil)
obj, err := r.Get(ctx, tt.nodeName, nil)
if tt.expectedErr {
assert.NotNil(t, err)
assert.Equal(t, tt.err.(*errors.StatusError).ErrStatus.Code, err.(*errors.StatusError).ErrStatus.Code)
} else {
assert.Nil(t, err)
require.NoError(t, err)
assert.Equal(t, tt.expectedObj, obj)
}
})
Expand All @@ -114,6 +113,7 @@ func TestRESTDelete(t *testing.T) {
nodeName string
expectedObj runtime.Object
expectedErr bool
err error
}{
{
name: "delete summary",
Expand All @@ -127,6 +127,7 @@ func TestRESTDelete(t *testing.T) {
PeerNodeLatencyStats: nil,
},
expectedErr: false,
err: nil,
},
{
name: "delete summary not found",
Expand All @@ -137,18 +138,22 @@ func TestRESTDelete(t *testing.T) {
nodeName: "node2",
expectedObj: nil,
expectedErr: true,
err: errors.NewNotFound(statsv1alpha1.Resource("nodelatencystats"), "node2"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := NewREST()
_, err := r.Create(context.TODO(), tt.summary, nil, nil)
assert.Nil(t, err)
obj, deleted, err := r.Delete(context.TODO(), tt.nodeName, nil, nil)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

_, err := r.Create(ctx, tt.summary, nil, nil)
assert.NoError(t, err)
obj, deleted, err := r.Delete(ctx, tt.nodeName, nil, nil)
if tt.expectedErr {
assert.NotNil(t, err)
assert.Equal(t, tt.err.(*errors.StatusError).ErrStatus.Code, err.(*errors.StatusError).ErrStatus.Code)
} else {
assert.Nil(t, err)
require.NoError(t, err)
assert.True(t, deleted)
assert.Equal(t, tt.expectedObj, obj)
}
Expand All @@ -157,72 +162,35 @@ func TestRESTDelete(t *testing.T) {
}

func TestRESTList(t *testing.T) {
tests := []struct {
name string
summary *statsv1alpha1.NodeLatencyStats
options *internalversion.ListOptions
expectedObj runtime.Object
expectedErr bool
}{
{
name: "list summary",
summary: &statsv1alpha1.NodeLatencyStats{
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
},
options: &internalversion.ListOptions{
Limit: 10,
Continue: "",
},
expectedObj: &statsv1alpha1.NodeLatencyStatsList{
Items: []statsv1alpha1.NodeLatencyStats{
{
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
},
},
},
expectedErr: false,
},
{
name: "list summary",
summary: &statsv1alpha1.NodeLatencyStats{
summary := &statsv1alpha1.NodeLatencyStats{
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
}
options := &internalversion.ListOptions{
Limit: 10,
Continue: "",
}
expectedObj := &statsv1alpha1.NodeLatencyStatsList{
Items: []statsv1alpha1.NodeLatencyStats{
{
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
},
options: &internalversion.ListOptions{
Limit: 0,
Continue: "",
},
expectedObj: &statsv1alpha1.NodeLatencyStatsList{
Items: []statsv1alpha1.NodeLatencyStats{
{
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
},
},
},
expectedErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := NewREST()
_, err := r.Create(context.TODO(), tt.summary, nil, nil)
assert.Nil(t, err)
obj, err := r.List(context.TODO(), tt.options)
if tt.expectedErr {
assert.NotNil(t, err)
} else {
assert.Nil(t, err)
assert.Equal(t, tt.expectedObj, obj)
}
})
}

r := NewREST()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

_, err := r.Create(ctx, summary, nil, nil)
assert.Nil(t, err)
objs, err := r.List(ctx, options)
assert.NoError(t, err)
assert.Equal(t, expectedObj, objs)
}

func TestRESTConvertToTable(t *testing.T) {
name := "convert to table"
summary := &statsv1alpha1.NodeLatencyStats{
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
Expand All @@ -231,18 +199,14 @@ func TestRESTConvertToTable(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
}
expectedErr := false

t.Run(name, func(t *testing.T) {
r := NewREST()
_, err := r.Create(context.TODO(), summary, nil, nil)
assert.Nil(t, err)
obj, err := r.ConvertToTable(context.TODO(), summary, nil)
if expectedErr {
assert.NotNil(t, err)
} else {
assert.Nil(t, err)
assert.Equal(t, expectedObj, obj.Rows[0].Object.Object)
}
})

r := NewREST()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

_, err := r.Create(ctx, summary, nil, nil)
assert.Nil(t, err)
obj, err := r.ConvertToTable(ctx, summary, nil)
assert.NoError(t, err)
assert.Equal(t, expectedObj, obj.Rows[0].Object.Object)
}

0 comments on commit 735eba1

Please sign in to comment.