Fixed creationTimestamp field being null as per #6569

- Initialized fakeClock for each test case. - Refactored NewRest function to prevent API breakage and updated tests. - Improved test logic and fixed lint issues. - Made various improvements for better code readability and efficiency. Signed-off-by: HEMANT KUMAR <hkbiet@gmail.com>
antrea-io · Aug 8, 2024 · 11c7f08 · 11c7f08
1 parent 6c05655
commit 11c7f08
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 26 deletions.
diff --git a/pkg/apiserver/registry/stats/nodelatencystats/rest.go b/pkg/apiserver/registry/stats/nodelatencystats/rest.go
@@ -18,6 +18,8 @@ import (
 	"context"
 	"time"
 
+	"k8s.io/utils/clock"
+
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metatable "k8s.io/apimachinery/pkg/api/meta/table"
@@ -32,6 +34,7 @@ import (
 
 type REST struct {
 	indexer cache.Indexer
+	clock   clock.Clock
 }
 
 var (
@@ -45,8 +48,13 @@ var (
 
 // NewREST returns a REST object that will work against API services.
 func NewREST() *REST {
+	return newRESTWithClock(clock.RealClock{})
+}
+
+func newRESTWithClock(clock clock.Clock) *REST {
 	return &REST{
 		indexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}),
+		clock:   clock,
 	}
 }
 
@@ -60,6 +68,9 @@ 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 summary.ObjectMeta.CreationTimestamp.IsZero() {
+		summary.ObjectMeta.CreationTimestamp = metav1.Time{Time: r.clock.Now()}
+	}
 	if err := r.indexer.Update(summary); err != nil {
 		return nil, errors.NewInternalError(err)
 	}

diff --git a/pkg/apiserver/registry/stats/nodelatencystats/rest_test.go b/pkg/apiserver/registry/stats/nodelatencystats/rest_test.go
@@ -19,6 +19,8 @@ import (
 	"testing"
 	"time"
 
+	clocktesting "k8s.io/utils/clock/testing"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -29,31 +31,61 @@ import (
 )
 
 func TestREST(t *testing.T) {
-	r := NewREST()
+	fakeClock := clocktesting.NewFakeClock(time.Now())
+	r := newRESTWithClock(fakeClock)
 	assert.Equal(t, &statsv1alpha1.NodeLatencyStats{}, r.New())
 	assert.Equal(t, &statsv1alpha1.NodeLatencyStats{}, r.NewList())
 	assert.False(t, r.NamespaceScoped())
 }
 
 func TestRESTCreate(t *testing.T) {
-	summary := &statsv1alpha1.NodeLatencyStats{
-		ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
-		PeerNodeLatencyStats: nil,
-	}
-	expectedObj := &statsv1alpha1.NodeLatencyStats{
-		ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
-		PeerNodeLatencyStats: nil,
-	}
-
-	r := NewREST()
 	ctx := context.Background()
+	now := time.Now()
+	const timeStep = 1 * time.Minute
+	tests := []struct {
+		name        string
+		summary     *statsv1alpha1.NodeLatencyStats
+		expectedObj *statsv1alpha1.NodeLatencyStats
+	}{
+		{
+			name: "create with existing timestamp",
+			summary: &statsv1alpha1.NodeLatencyStats{
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now}},
+				PeerNodeLatencyStats: nil,
+			},
+			expectedObj: &statsv1alpha1.NodeLatencyStats{
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now}},
+				PeerNodeLatencyStats: nil,
+			},
+		},
+		{
+			name: "create with no existing timestamp",
+			summary: &statsv1alpha1.NodeLatencyStats{
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
+				PeerNodeLatencyStats: nil,
+			},
+			expectedObj: &statsv1alpha1.NodeLatencyStats{
+				// the test case advances the clock by timeStep
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now.Add(timeStep)}},
+				PeerNodeLatencyStats: nil,
+			},
+		},
+	}
 
-	obj, err := r.Create(ctx, summary, nil, nil)
-	require.NoError(t, err)
-	assert.Equal(t, expectedObj, obj)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fakeClock := clocktesting.NewFakeClock(now)
+			r := newRESTWithClock(fakeClock)
+			fakeClock.Step(timeStep)
+			obj, err := r.Create(ctx, tt.summary, nil, nil)
+			require.NoError(t, err)
+			assert.Equal(t, tt.expectedObj, obj)
+		})
+	}
 }
 
 func TestRESTGet(t *testing.T) {
+	now := time.Now()
 	tests := []struct {
 		name        string
 		summary     *statsv1alpha1.NodeLatencyStats
@@ -64,20 +96,20 @@ func TestRESTGet(t *testing.T) {
 		{
 			name: "get summary",
 			summary: &statsv1alpha1.NodeLatencyStats{
-				ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now}},
 				PeerNodeLatencyStats: nil,
 			},
 			nodeName: "node1",
 			expectedObj: &statsv1alpha1.NodeLatencyStats{
-				ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now}},
 				PeerNodeLatencyStats: nil,
 			},
 			err: nil,
 		},
 		{
 			name: "get summary not found",
 			summary: &statsv1alpha1.NodeLatencyStats{
-				ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now}},
 				PeerNodeLatencyStats: nil,
 			},
 			nodeName:    "node2",
@@ -87,12 +119,12 @@ func TestRESTGet(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			r := NewREST()
+			fakeClock := clocktesting.NewFakeClock(now)
+			r := newRESTWithClock(fakeClock)
 			ctx := context.Background()
 
 			_, err := r.Create(ctx, tt.summary, nil, nil)
 			require.NoError(t, err)
-
 			obj, err := r.Get(ctx, tt.nodeName, nil)
 			if tt.err != nil {
 				assert.EqualError(t, tt.err, err.Error())
@@ -105,6 +137,7 @@ func TestRESTGet(t *testing.T) {
 }
 
 func TestRESTDelete(t *testing.T) {
+	now := time.Now()
 	tests := []struct {
 		name        string
 		summary     *statsv1alpha1.NodeLatencyStats
@@ -115,20 +148,20 @@ func TestRESTDelete(t *testing.T) {
 		{
 			name: "delete summary",
 			summary: &statsv1alpha1.NodeLatencyStats{
-				ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now}},
 				PeerNodeLatencyStats: nil,
 			},
 			nodeName: "node1",
 			expectedObj: &statsv1alpha1.NodeLatencyStats{
-				ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now}},
 				PeerNodeLatencyStats: nil,
 			},
 			err: nil,
 		},
 		{
 			name: "delete summary not found",
 			summary: &statsv1alpha1.NodeLatencyStats{
-				ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now}},
 				PeerNodeLatencyStats: nil,
 			},
 			nodeName:    "node2",
@@ -138,7 +171,8 @@ func TestRESTDelete(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			r := NewREST()
+			fakeClock := clocktesting.NewFakeClock(now)
+			r := newRESTWithClock(fakeClock)
 			ctx := context.Background()
 
 			_, err := r.Create(ctx, tt.summary, nil, nil)
@@ -156,20 +190,22 @@ func TestRESTDelete(t *testing.T) {
 }
 
 func TestRESTList(t *testing.T) {
+	now := time.Now()
+	fakeClock := clocktesting.NewFakeClock(now)
 	summary := &statsv1alpha1.NodeLatencyStats{
-		ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
+		ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now}},
 		PeerNodeLatencyStats: nil,
 	}
 	expectedObj := &statsv1alpha1.NodeLatencyStatsList{
 		Items: []statsv1alpha1.NodeLatencyStats{
 			{
-				ObjectMeta:           metav1.ObjectMeta{Name: "node1"},
+				ObjectMeta:           metav1.ObjectMeta{Name: "node1", CreationTimestamp: metav1.Time{Time: now}},
 				PeerNodeLatencyStats: nil,
 			},
 		},
 	}
 
-	r := NewREST()
+	r := newRESTWithClock(fakeClock)
 	ctx := context.Background()
 
 	_, err := r.Create(ctx, summary, nil, nil)