-
Notifications
You must be signed in to change notification settings - Fork 36
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
heal: fix dataplane healing #1362
heal: fix dataplane healing #1362
Conversation
timeout = 10 * time.Second | ||
tick = 10 * time.Millisecond | ||
requireTimeout = 2 * time.Second | ||
ctxTimeout = 5 * time.Second |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do these timers impact reconvergence time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, these were unnecessary changes, just for testing.
I just wanted to check timeouts because I changed the check from require.Eventually
to require.Never
00675a7
to
55b0d4a
Compare
Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>
55b0d4a
to
9de6f1d
Compare
@@ -278,7 +278,7 @@ func TestNSMGRHealEndpoint_DatapathHealthy_CtrlPlaneBroken(t *testing.T) { | |||
domain.Nodes[0].NewEndpoint(ctx, nseReg2, sandbox.GenerateTestToken, counter) | |||
|
|||
// Should not connect to new NSE | |||
require.Eventually(t, func() bool { return counter.UniqueRequests() == 1 }, timeout, tick) | |||
require.Never(t, func() bool { return counter.UniqueRequests() > 1 }, time.Second*2, tick) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an invalid condition in the test. It only worked on ci because of the timeout settings.
This test checks that if the dataplan is alive, we will not select another endpoint. Therefore, the number of unique requests to endpoints should never exceed 1.
In the previous implementation, the exit from Eventually
happened almost instantly, just without waiting for the second request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I very much like the channel approach for the datapath healing.
Can we add a test for this scenario to avoid regressions?
@denis-tingaikin |
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1362 Commit: 95cee7e Author: Artem Glazychev Date: 2022-09-29 17:03:43 +0700 Message: - heal: fix dataplane healing (#1362) Signed-off-by: Artem Glazychev <artem.glazychev@xored.com> Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
Signed-off-by: Artem Glazychev artem.glazychev@xored.com
Description
Issue link
#1359
How Has This Been Tested?
AddedFixed unit testTypes of changes