|
|
|
Added network inclusion/exclusion to acquireNode
I added Networks structure as a parameter of acquireNode
in maas provider. Its two members IncludedNetworks and
ExcludedNetworks get converted, in a similar fashion than
constraints, into url encoded parameters, according to
~maas-maintainers/maas/trunk/view/head:/docs/networks.rst
they become, respectively "networks" and "not_networks"
For this I create an analog to convertConstraints which
I called convertNetworks, mostly to keep the two
conceptually separated.
I added testing for the four variants I could think of
the structure values that where representative.
I also made a small cosmetic change to the converConstraints
test since it was very hard to read and I needed to based my
own test on it
https://code.launchpad.net/~hduran-8/juju-core/add_networks_to_maas_acquireNode/+merge/211748
Requires: https://code.launchpad.net/~gz/juju-core/add_startinstance_params/+merge/211588
(do not edit description out of merge proposal)
Patch Set 1 #
Total comments: 5
Patch Set 2 : Added network inclusion/exclusion to acquireNode #Patch Set 3 : Added network inclusion/exclusion to acquireNode #
Total messages: 9
|
hduran
Please take a look.
|
11 years, 9 months ago (2014年03月19日 15:09:13 UTC) #1 | ||||||||||||||||||||||||||||||||
Please take a look.
On 2014年03月19日 15:09:13, hduran wrote: > Please take a look. LGTM with one slight change.
Doh, forgot to do the publish and mail thing. Here's the change I mentioned before. https://codereview.appspot.com/77850043/diff/1/provider/maas/environ.go File provider/maas/environ.go (right): https://codereview.appspot.com/77850043/diff/1/provider/maas/environ.go#newco... provider/maas/environ.go:172: func convertNetworks(params *url.Values, nets environs.Networks) { I'd rename this to be addNetworks, since it's adding on to the url.Values that you pass in. Also, don't actually need to make it a pointer, since url.Values is actually just a map, and internally, maps are pointers to data structures, so you can pass them like they're already a pointer.
Please take a look. https://codereview.appspot.com/77850043/diff/1/provider/maas/environ.go File provider/maas/environ.go (right): https://codereview.appspot.com/77850043/diff/1/provider/maas/environ.go#newco... provider/maas/environ.go:172: func convertNetworks(params *url.Values, nets environs.Networks) { On 2014年03月19日 15:33:52, nate.finch wrote: > I'd rename this to be addNetworks, since it's adding on to the url.Values that > you pass in. > > Also, don't actually need to make it a pointer, since url.Values is actually > just a map, and internally, maps are pointers to data structures, so you can > pass them like they're already a pointer. Done.
I did change the name, it makes more sense now. Also I changed params to be just passed as value. Thank you.
https://codereview.appspot.com/77850043/diff/1/provider/maas/environ.go File provider/maas/environ.go (right): https://codereview.appspot.com/77850043/diff/1/provider/maas/environ.go#newco... provider/maas/environ.go:175: //XXX Is this suposed to be a comma separated list? No, MAAs doesn't expect a comma-separated list, it wants a multi-valued URL parameter. As in http://server/?networks=net1&networks=net2 i.e. v := url.Values{} v.Add("networks", "<network1>") v.Add("networks", "<network2>")
Please take a look. https://codereview.appspot.com/77850043/diff/1/provider/maas/environ.go File provider/maas/environ.go (right): https://codereview.appspot.com/77850043/diff/1/provider/maas/environ.go#newco... provider/maas/environ.go:175: //XXX Is this suposed to be a comma separated list? On 2014年03月19日 15:51:33, rvb wrote: > No, MAAs doesn't expect a comma-separated list, it wants a multi-valued URL > parameter. > > As in http://server/?networks=net1&networks=net2 > i.e. > v := url.Values{} > v.Add("networks", "<network1>") > v.Add("networks", "<network2>") Done. https://codereview.appspot.com/77850043/diff/1/provider/maas/environ.go#newco... provider/maas/environ.go:175: //XXX Is this suposed to be a comma separated list? On 2014年03月19日 15:51:33, rvb wrote: > No, MAAs doesn't expect a comma-separated list, it wants a multi-valued URL > parameter. > > As in http://server/?networks=net1&networks=net2 > i.e. > v := url.Values{} > v.Add("networks", "<network1>") > v.Add("networks", "<network2>") Done.