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

[sdk#740] Rework sandbox #811

Merged
merged 4 commits into from
Jun 23, 2021
Merged

[sdk#740] Rework sandbox #811

merged 4 commits into from
Jun 23, 2021

Conversation

Bolodya1997
Copy link

Sandbox rework for #740. Corresponding heal tests rework.

Changes:

  1. adds Node.NewNSMgr
  2. all sandbox methods now handle errors inside with require.NoError
  3. removes Builder.SetCustomConfig - replaces it with extended Builder.SetNodeSetup
  4. removes Domain.resources - every resource now has a goroutine waiting on the corresponding context
  5. adds .Name to NSMgrEntry, EndpointEntry to enable emulation of NSMgr/Forwarder/Endpoint restart with same name + URL
  6. removes Node.Forwarder - forwarder can be accessed after custom creation in Builder.SetNodeSetup

@Bolodya1997 Bolodya1997 marked this pull request as draft April 8, 2021 09:34
@Bolodya1997 Bolodya1997 force-pushed the rework-sandbox branch 3 times, most recently from ad0a051 to 458da5f Compare April 9, 2021 04:38
@Bolodya1997 Bolodya1997 marked this pull request as ready for review April 9, 2021 04:41
denis-tingaikin
denis-tingaikin approved these changes Apr 9, 2021
View reviewed changes
Copy link
Member

@denis-tingaikin denis-tingaikin left a comment

Choose a reason for hiding this comment

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

This looks awesome!
We can merge the PR when fixes for #819 will be landed.

pkg/tools/sandbox/utils.go Outdated Show resolved Hide resolved
@Bolodya1997 Bolodya1997 mentioned this pull request Apr 13, 2021
Closed
@denis-tingaikin
Copy link
Member

@Bolodya1997 Could you please rebase this?

@Bolodya1997 Bolodya1997 force-pushed the rework-sandbox branch 3 times, most recently from 7ff4dd0 to 0a5edb3 Compare April 16, 2021 05:43
@Bolodya1997 Bolodya1997 force-pushed the rework-sandbox branch 6 times, most recently from 55e14e6 to 793f046 Compare April 23, 2021 04:12
@Bolodya1997 Bolodya1997 force-pushed the rework-sandbox branch 5 times, most recently from 97ac43f to eb94474 Compare April 30, 2021 05:05
@Bolodya1997 Bolodya1997 marked this pull request as draft April 30, 2021 05:05
@Bolodya1997
Copy link
Author

Blocker for #823.

@Bolodya1997 Bolodya1997 mentioned this pull request Apr 30, 2021
Closed
@Bolodya1997 Bolodya1997 force-pushed the rework-sandbox branch 3 times, most recently from d7af9e2 to f38fd7c Compare April 30, 2021 09:36
@Bolodya1997 Bolodya1997 force-pushed the rework-sandbox branch 2 times, most recently from ed8822b to 44edf86 Compare June 1, 2021 12:03
@Bolodya1997 Bolodya1997 removed the ASAP The issue that blocking SOW items or core use-cases label Jun 7, 2021
@Bolodya1997 Bolodya1997 marked this pull request as draft June 22, 2021 14:30
@Bolodya1997 Bolodya1997 marked this pull request as ready for review June 22, 2021 14:41
Vladimir Popov added 2 commits June 22, 2021 22:03
Rework sandbox
5486e3c 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Remove default NS, NSE registry clients for the node
9ca4e05 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Add node.URL()
64d71ff 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Fix sandbox new* methods
a36485a 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@Bolodya1997 Bolodya1997 marked this pull request as draft June 23, 2021 06:53
@Bolodya1997 Bolodya1997 marked this pull request as ready for review June 23, 2021 06:58
denis-tingaikin
denis-tingaikin approved these changes Jun 23, 2021
View reviewed changes
Copy link
Member

@denis-tingaikin denis-tingaikin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think we also need to consider semantic simplification of sandbox testing tools to make them more clear for folks but it can be considered in separate PRs.
So merging this.

@denis-tingaikin denis-tingaikin merged commit 6e69831 into networkservicemesh:main Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glazychev-art glazychev-art glazychev-art approved these changes

@Mixaster995 Mixaster995 Mixaster995 approved these changes

@d-uzlov d-uzlov d-uzlov approved these changes

@denis-tingaikin denis-tingaikin denis-tingaikin approved these changes

@edwarnicke edwarnicke Awaiting requested review from edwarnicke

Assignees

@Bolodya1997 Bolodya1997

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Bolodya1997 @denis-tingaikin @edwarnicke @glazychev-art @Mixaster995 @d-uzlov