-
Notifications
You must be signed in to change notification settings - Fork 260
Add option in CNS to pre-provision hns network #323
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
Conversation
Add a commandline option in CNS to pre-provision hns network. The commandline option take the type of the network that needs pre-provisioning. This allows orchestrators to start CNS with this option so that the VM network blip / disconnect is avoided when calling cni add the very first time.
Codecov Report
@@ Coverage Diff @@
## master #323 +/- ##
=======================================
Coverage 45.16% 45.16%
=======================================
Files 25 25
Lines 3538 3538
=======================================
Hits 1598 1598
Misses 1685 1685
Partials 255 255Continue to review full report at Codecov.
|
jaer-tsun
left a comment
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.
/lgtm
jaer-tsun
left a comment
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.
some comments, we can discuss when you're in office
| switch r.Method { | ||
| case "POST": | ||
| service.lock.Lock() | ||
| networkInfo, ok := service.state.Networks[req.NetworkName] |
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 will encapsulate this in a function, so that people don't forget to acquire lock.
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.
Keeping it as is. Don't understand the comment. I don't think moving the networkName validity check warrants a new func if that's what you are asking for.
Addressed review comments in the subsequent updates
jaer-tsun
left a comment
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.
/lgtm, we will merge some of the functions when you move the code around later on.
Add a commandline option in CNS to pre-provision hns network.
The commandline option takes the type of the network that needs
pre-provisioning. This allows orchestrators to start CNS with the
option so that the VM network blip / disconnect is avoided when
calling cni add the very first time.