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.

Sign up for GitHub

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

Jump to bottom

Cover nsmgr remote use-case via grpc unit tests #460

Merged
merged 6 commits into from
Sep 16, 2020
Merged

Cover nsmgr remote use-case via grpc unit tests #460

merged 6 commits into from
Sep 16, 2020

Conversation

denis-tingaikin
Copy link
Member

Motivation

  1. Remote nsmgr use-case is required for proxy-nsmgr testing.
  2. These tests cover this fix fix: Interpose chain element wrong compares urls #459

@denis-tingaikin
Add nsmgr local/remote grpc tests
d96000c 
Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>
@denis-tingaikin
fix linter issues
36e0fdb 
Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>
@denis-tingaikin
extract chaintest
77ea61d 
Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>
denis-tingaikin
denis-tingaikin commented Sep 15, 2020
View reviewed changes
pkg/networkservice/chains/chainstest/doc.go
// See the License for the specific language governing permissions and
// limitations under the License.

// Package chainstest provides API for testing NSM chains such as Forwarder, NSC, NSMGR, NSE.
Copy link
Member Author

@denis-tingaikin denis-tingaikin Sep 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@edwarnicke , @haiodo Here I suggesting to add chainstest pkg that has the next advantages:

  1. This pkg adds a possibility to test real nsmgr, nsc, nse, registries via grpc unit tests.
  2. This pkg simplifies the logic of creating, cleaning grpc servers/connection.

This can be super useful for testing the logic of interdomain/multi nodes chains and so on. For example for testing proxy-nsmgr we need to start these grpc servers:

1. forwarder1
2. forwarder2
3. nsmgr1
4. nsmgr2
5. nsc
6. nse
7. proxy-registry1
8. proxy-registyr2
9. proxy-nsmgr1
10. proxy-nsmgr2
11. registry
12. *floating registry

With this pkg we can hide the logic of creating these 12 servers that should simplify the testing.

Note: this pkg has a name convention with https://golang.org/pkg/net/http/httptest/.

Please share your thoughts about this approach :)

@denis-tingaikin
add possible to pass custom context
3b73562 
Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>
haiodo
haiodo approved these changes Sep 15, 2020
View reviewed changes
Copy link
Contributor

@haiodo haiodo left a comment

Choose a reason for hiding this comment

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

I'd like in general.

pkg/networkservice/chains/chainstest/builder.go Outdated Show resolved Hide resolved
pkg/networkservice/chains/chainstest/builder.go Outdated
dialContext: b.dialContext,
ctx: ctx,
}
b.resources = nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed if the builder will be instantly used for creating the second NSM domain.

pkg/networkservice/chains/chainstest/builder.go Show resolved Hide resolved
pkg/networkservice/chains/chainstest/builder.go Outdated Show resolved Hide resolved
pkg/networkservice/chains/chainstest/builder.go Show resolved Hide resolved
pkg/networkservice/chains/chainstest/builder.go Show resolved Hide resolved
pkg/networkservice/chains/chainstest/supplier.go Outdated Show resolved Hide resolved
@denis-tingaikin
apply review comments
15cf09c 
Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>
@denis-tingaikin
self code review
d2b34a2 
Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>
@haiodo haiodo merged commit 5b64906 into networkservicemesh:master Sep 16, 2020
nsmbot pushed a commit to networkservicemesh/cmd-registry-memory that referenced this pull request Sep 16, 2020
Update go.mod and go.sum to latest version from networkservicemesh/sd…
414d2d5 
…k@master networkservicemesh/sdk#460

networkservicemesh/sdk PR link: networkservicemesh/sdk#460

networkservicemesh/sdk commit message:
commit 5b64906a352b8b04a60e5eaf386a4d59a4bbe58d
Author: Denis Tingaikin <49399980+denis-tingajkin@users.noreply.github.com>
Date:   Wed Sep 16 15:33:09 2020 +0700

    Cover nsmgr remote use-case via grpc unit tests (#460)

    * Add nsmgr local/remote grpc tests

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * fix linter issues

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * extract chaintest

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * add possible to pass custom context

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * apply review comments

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * self code review

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-registry-proxy-dns that referenced this pull request Sep 16, 2020
Update go.mod and go.sum to latest version from networkservicemesh/sd…
6077386 
…k@master networkservicemesh/sdk#460

networkservicemesh/sdk PR link: networkservicemesh/sdk#460

networkservicemesh/sdk commit message:
commit 5b64906a352b8b04a60e5eaf386a4d59a4bbe58d
Author: Denis Tingaikin <49399980+denis-tingajkin@users.noreply.github.com>
Date:   Wed Sep 16 15:33:09 2020 +0700

    Cover nsmgr remote use-case via grpc unit tests (#460)

    * Add nsmgr local/remote grpc tests

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * fix linter issues

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * extract chaintest

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * add possible to pass custom context

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * apply review comments

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * self code review

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsmgr that referenced this pull request Sep 16, 2020
Update go.mod and go.sum to latest version from networkservicemesh/sd…
bb00a36 
…k@master networkservicemesh/sdk#460

networkservicemesh/sdk PR link: networkservicemesh/sdk#460

networkservicemesh/sdk commit message:
commit 5b64906a352b8b04a60e5eaf386a4d59a4bbe58d
Author: Denis Tingaikin <49399980+denis-tingajkin@users.noreply.github.com>
Date:   Wed Sep 16 15:33:09 2020 +0700

    Cover nsmgr remote use-case via grpc unit tests (#460)

    * Add nsmgr local/remote grpc tests

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * fix linter issues

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * extract chaintest

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * add possible to pass custom context

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * apply review comments

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * self code review

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-kernel that referenced this pull request Sep 16, 2020
Update go.mod and go.sum to latest version from networkservicemesh/sd…
030761c 
…k@master networkservicemesh/sdk#460

networkservicemesh/sdk PR link: networkservicemesh/sdk#460

networkservicemesh/sdk commit message:
commit 5b64906a352b8b04a60e5eaf386a4d59a4bbe58d
Author: Denis Tingaikin <49399980+denis-tingajkin@users.noreply.github.com>
Date:   Wed Sep 16 15:33:09 2020 +0700

    Cover nsmgr remote use-case via grpc unit tests (#460)

    * Add nsmgr local/remote grpc tests

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * fix linter issues

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * extract chaintest

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * add possible to pass custom context

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * apply review comments

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * self code review

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-vppagent that referenced this pull request Sep 16, 2020
Update go.mod and go.sum to latest version from networkservicemesh/sd…
3a6a344 
…k@master networkservicemesh/sdk#460

networkservicemesh/sdk PR link: networkservicemesh/sdk#460

networkservicemesh/sdk commit message:
commit 5b64906a352b8b04a60e5eaf386a4d59a4bbe58d
Author: Denis Tingaikin <49399980+denis-tingajkin@users.noreply.github.com>
Date:   Wed Sep 16 15:33:09 2020 +0700

    Cover nsmgr remote use-case via grpc unit tests (#460)

    * Add nsmgr local/remote grpc tests

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * fix linter issues

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * extract chaintest

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * add possible to pass custom context

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * apply review comments

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * self code review

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
@Bolodya1997 Bolodya1997 mentioned this pull request Feb 18, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haiodo haiodo haiodo approved these changes

@edwarnicke edwarnicke edwarnicke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@denis-tingaikin @haiodo @edwarnicke