From c20c7351e3725b956e06a45461c0ba26f9e562e2 Mon Sep 17 00:00:00 2001 From: denis-tingaikin Date: Tue, 24 Sep 2024 16:03:41 +0300 Subject: [PATCH] apply review comments Signed-off-by: denis-tingaikin --- .golangci.yml | 4 ++++ pkg/registry/etcd/context.go | 7 ------- pkg/registry/etcd/ns_server.go | 2 +- pkg/registry/etcd/ns_server_test.go | 5 ++--- pkg/registry/etcd/nse_server.go | 7 +++---- pkg/registry/etcd/nse_server_test.go | 9 ++++++--- 6 files changed, 16 insertions(+), 18 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5816fbe..1697381 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -168,3 +168,7 @@ issues: - goheader - dupl - revive + # It's ok to have dupl code in tests files + - path: ".*.test.go" + linters: + - dupl \ No newline at end of file diff --git a/pkg/registry/etcd/context.go b/pkg/registry/etcd/context.go index 1aa2bb8..1c06dff 100644 --- a/pkg/registry/etcd/context.go +++ b/pkg/registry/etcd/context.go @@ -32,13 +32,6 @@ func nseVersionFromContext(ctx context.Context) (string, bool) { return version, ok } -func max(a, b time.Duration) time.Duration { - if a > b { - return a - } - return b -} - func min(a, b time.Duration) time.Duration { if a > b { return b diff --git a/pkg/registry/etcd/ns_server.go b/pkg/registry/etcd/ns_server.go index bbed77a..ff3838b 100644 --- a/pkg/registry/etcd/ns_server.go +++ b/pkg/registry/etcd/ns_server.go @@ -85,7 +85,7 @@ func (n *etcdNSRegistryServer) watchRemoteStorage() { sleepTime = min(sleepTime, maxSleepTime) continue } - sleepTime = max(sleepTime, minSleepTime) + sleepTime = minSleepTime isWatcherFine := true for isWatcherFine { diff --git a/pkg/registry/etcd/ns_server_test.go b/pkg/registry/etcd/ns_server_test.go index 7ca47fc..b7ea61b 100644 --- a/pkg/registry/etcd/ns_server_test.go +++ b/pkg/registry/etcd/ns_server_test.go @@ -155,7 +155,7 @@ func Test_K8sNSRegistry_FindWatch(t *testing.T) { }, time.Second, time.Millisecond*100) } -func Test_NSHightloadWatch_ShouldNotFail(t *testing.T) { +func Test_NSHighloadWatch_ShouldNotFail(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) defer cancel() @@ -194,9 +194,8 @@ func Test_NSHightloadWatch_ShouldNotFail(t *testing.T) { Name: uuid.NewString(), }, }, metav1.CreateOptions{}) - time.Sleep(time.Millisecond * 10) } }() doneWg.Wait() - require.Equal(t, updateCount, actual.Load()/clinetCount) + require.InDelta(t, updateCount, actual.Load()/clinetCount, 20) } diff --git a/pkg/registry/etcd/nse_server.go b/pkg/registry/etcd/nse_server.go index d210d10..a291aae 100644 --- a/pkg/registry/etcd/nse_server.go +++ b/pkg/registry/etcd/nse_server.go @@ -88,7 +88,7 @@ func (n *etcdNSERegistryServer) watchRemoteStorage() { sleepTime = min(sleepTime, maxSleepTime) continue } - sleepTime = max(sleepTime, minSleepTime) + sleepTime = minSleepTime isWatcherFine := true for isWatcherFine { @@ -110,15 +110,13 @@ func (n *etcdNSERegistryServer) watchRemoteStorage() { if item.Name == "" { item.Name = model.GetName() } - if v, ok := n.versions.Load(item.Name); ok && v == model.ResourceVersion { - continue - } resp := ®istry.NetworkServiceEndpointResponse{ NetworkServiceEndpoint: item, Deleted: deleted, } n.sendEvent(resp) if !deleted && item.ExpirationTime != nil && item.ExpirationTime.AsTime().Local().Before(time.Now()) { + n.versions.Delete(item.GetName()) n.deleteExecutor.AsyncExec(func() { _ = n.client.NetworkservicemeshV1().NetworkServiceEndpoints(n.ns).Delete(n.chainContext, item.GetName(), metav1.DeleteOptions{ Preconditions: &metav1.Preconditions{ @@ -252,6 +250,7 @@ func (n *etcdNSERegistryServer) Unregister(ctx context.Context, request *registr if err != nil { return nil, errors.Wrapf(err, "failed to delete a NetworkServiceEndpoints %s in a namespace %s", request.Name, n.ns) } + n.versions.Delete(request.GetName()) } return resp, nil } diff --git a/pkg/registry/etcd/nse_server_test.go b/pkg/registry/etcd/nse_server_test.go index 8399e7c..e1feacc 100644 --- a/pkg/registry/etcd/nse_server_test.go +++ b/pkg/registry/etcd/nse_server_test.go @@ -127,10 +127,13 @@ func Test_K8sNSERegistry_FindWatch(t *testing.T) { require.NoError(t, err) require.Equal(t, "nse-1", nseResp.NetworkServiceEndpoint.Name) - // NSE reregisteration. We shouldn't get any updates _, err = s.Register(ctx, nse.Clone()) require.NoError(t, err) + nseResp, err = stream.Recv() + require.NoError(t, err) + require.Equal(t, "nse-1", nseResp.NetworkServiceEndpoint.Name) + // Update NSE again - add labels updatedNSE := nse.Clone() updatedNSE.NetworkServiceLabels = map[string]*registry.NetworkServiceLabels{"label": {}} @@ -149,7 +152,7 @@ func Test_K8sNSERegistry_FindWatch(t *testing.T) { require.Equal(t, 1, len(nseResp.GetNetworkServiceEndpoint().NetworkServiceLabels)) } -func Test_NSEHightloadWatch_ShouldNotFail(t *testing.T) { +func Test_NSEHighloadWatch_ShouldNotFail(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) defer cancel() @@ -191,7 +194,7 @@ func Test_NSEHightloadWatch_ShouldNotFail(t *testing.T) { } }() doneWg.Wait() - require.Equal(t, updateCount, actual.Load()/clinetCount) + require.InDelta(t, updateCount, actual.Load()/clinetCount, 20) } func Test_K8sNSERegistry_Find_ExpiredNSE(t *testing.T) {