Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified refresh client to calculate the shortest token expiry #1190

Merged
merged 3 commits into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions pkg/networkservice/common/refresh/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,31 @@ func (t *refreshClient) Close(ctx context.Context, conn *networkservice.Connecti
func after(ctx context.Context, conn *networkservice.Connection) (time.Duration, error) {
clockTime := clock.FromContext(ctx)

expireTime, err := ptypes.Timestamp(conn.GetCurrentPathSegment().GetExpires())
if err != nil {
return 0, errors.WithStack(err)
var minTimeout *time.Duration
var expireTime time.Time
for _, segment := range conn.GetPath().GetPathSegments() {
expTime, err := ptypes.Timestamp(segment.GetExpires())
if err != nil {
return 0, errors.WithStack(err)
}

timeout := clockTime.Until(expTime)

if minTimeout == nil || timeout < *minTimeout {
if minTimeout == nil {
minTimeout = new(time.Duration)
}

*minTimeout = timeout
expireTime = expTime
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}
log.FromContext(ctx).Infof("expiration after %s at %s", timeout.String(), expireTime.UTC())
if timeout <= 0 {
return 1, nil
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


timeout := clockTime.Until(expireTime)
log.FromContext(ctx).Infof("expiration after %s at %s", timeout.String(), expireTime.UTC())
if minTimeout != nil {
log.FromContext(ctx).Infof("expiration after %s at %s", minTimeout.String(), expireTime.UTC())
}

if timeout <= 0 {
if minTimeout == nil || *minTimeout <= 0 {
return 1, nil
}

Expand All @@ -129,7 +145,7 @@ func after(ctx context.Context, conn *networkservice.Connection) (time.Duration,
if len(path.PathSegments) > 1 {
scale = 0.2 + 0.2*float64(path.Index)/float64(len(path.PathSegments))
}
duration := time.Duration(float64(timeout) * scale)
duration := time.Duration(float64(*minTimeout) * scale)

return duration, nil
}
100 changes: 97 additions & 3 deletions pkg/networkservice/common/refresh/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,25 @@ package refresh_test

import (
"context"
"fmt"
"sync/atomic"
"testing"
"time"

"github.com/networkservicemesh/api/pkg/api/networkservice"
"github.com/networkservicemesh/api/pkg/api/registry"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
"google.golang.org/grpc/credentials"

"github.com/networkservicemesh/api/pkg/api/networkservice"
"github.com/networkservicemesh/api/pkg/api/registry"

"github.com/networkservicemesh/sdk/pkg/networkservice/common/begin"
"github.com/networkservicemesh/sdk/pkg/networkservice/common/refresh"
"github.com/networkservicemesh/sdk/pkg/networkservice/common/updatepath"
"github.com/networkservicemesh/sdk/pkg/networkservice/common/updatetoken"
"github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters"
"github.com/networkservicemesh/sdk/pkg/networkservice/core/chain"
"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
countutil "github.com/networkservicemesh/sdk/pkg/networkservice/utils/count"
"github.com/networkservicemesh/sdk/pkg/networkservice/utils/inject/injectclock"
"github.com/networkservicemesh/sdk/pkg/networkservice/utils/inject/injecterror"
"github.com/networkservicemesh/sdk/pkg/networkservice/utils/metadata"
Expand All @@ -62,6 +63,28 @@ func testTokenFunc(clockTime clock.Clock) token.GeneratorFunc {
}
}

func testTokenFuncWithTimeout(clockTime clock.Clock, timeout time.Duration) token.GeneratorFunc {
return func(_ credentials.AuthInfo) (token string, expireTime time.Time, err error) {
return "", clockTime.Now().Add(timeout), err
}
}

type captureTickerDuration struct {
*clockmock.Mock

tickerDuration time.Duration
}

func (m *captureTickerDuration) Ticker(d time.Duration) clock.Ticker {
m.tickerDuration = d
return m.Mock.Ticker(d)
}

func (m *captureTickerDuration) Reset(t time.Time) {
m.tickerDuration = 0
m.Set(t)
}

func testClient(
ctx context.Context,
tokenGenerator token.GeneratorFunc,
Expand Down Expand Up @@ -302,6 +325,77 @@ func TestRefreshClient_NoRefreshOnFailure(t *testing.T) {
require.Never(t, cloneClient.validator(2), testWait, testTick)
}

func TestRefreshClient_CalculatesShortestTokenTimeout(t *testing.T) {
t.Cleanup(func() { goleak.VerifyNone(t) })

ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

testData := []struct {
Chain []time.Duration
ExpectedRefreshTimeout time.Duration
}{
{
Chain: []time.Duration{time.Hour},
ExpectedRefreshTimeout: 20 * time.Minute,
},
{
Chain: []time.Duration{time.Hour, 3 * time.Minute},
ExpectedRefreshTimeout: 54 * time.Second,
},
{
Chain: []time.Duration{time.Hour, 5 * time.Second, 3 * time.Minute},
ExpectedRefreshTimeout: 5 * time.Second / 3.,
},
{
Chain: []time.Duration{200 * time.Millisecond, 1 * time.Minute, 100 * time.Millisecond, time.Hour},
ExpectedRefreshTimeout: 100 * time.Millisecond / 3.,
},
}

timeNow := time.Date(2009, 11, 10, 23, 0, 0, 0, time.Local)

clockMock := captureTickerDuration{
Mock: clockmock.New(ctx),
}

var countClient = &countutil.Client{}

const timeoutDelta = 10 * time.Millisecond
for _, testDataElement := range testData {
clockMock.Reset(timeNow)

var pathChain []networkservice.NetworkServiceClient
var clientChain = []networkservice.NetworkServiceClient{
begin.NewClient(),
metadata.NewClient(),
injectclock.NewClient(&clockMock),
refresh.NewClient(ctx),
}

for _, expireTimeValue := range testDataElement.Chain {
pathChain = append(pathChain,
updatepath.NewClient(fmt.Sprintf("test-%v", expireTimeValue)),
adapters.NewServerToClient(updatetoken.NewServer(testTokenFuncWithTimeout(&clockMock, expireTimeValue))),
)
}

clientChain = append(pathChain, clientChain...)
clientChain = append(clientChain, countClient)
client := chain.NewNetworkServiceClient(clientChain...)

_, err := client.Request(ctx, &networkservice.NetworkServiceRequest{
Connection: new(networkservice.Connection),
})
require.NoError(t, err)

require.Less(t, clockMock.tickerDuration, testDataElement.ExpectedRefreshTimeout+timeoutDelta)
require.Greater(t, clockMock.tickerDuration, testDataElement.ExpectedRefreshTimeout-timeoutDelta)
}

require.Equal(t, countClient.Requests(), len(testData))
}

func TestRefreshClient_RefreshOnRefreshFailure(t *testing.T) {
t.Cleanup(func() { goleak.VerifyNone(t) })

Expand Down
3 changes: 3 additions & 0 deletions pkg/networkservice/common/timeout/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ func TestTimeoutServer_RefreshFailure(t *testing.T) {
injecterror.WithRequestErrorTimes(1, -1),
injecterror.WithCloseErrorTimes(),
),
updatetoken.NewServer(func(_ credentials.AuthInfo) (string, time.Time, error) {
return "test", clock.FromContext(ctx).Now().Add(tokenTimeout), nil
}),
connServer,
),
tokenTimeout,
Expand Down