Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(318)

Issue 85060043: provider/maas: StartInstance makes []NetworkInfo (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 11 months ago by dimitern
Modified:
9 years, 11 months ago
Reviewers:
mp+214592, fwereade
Visibility:
Public.

Description

provider/maas: StartInstance makes []NetworkInfo This refactors a few bits related to networking in the MAAS provider (mainly unexporting internal methods and fixing types). It also adds a method to get all MAC addresses connected to a network. Combining that with the previous work, now StartInstance returns a populated []environs.NetworkInfo when networks are specified. Any errors from MAAS will cancel StartInstance. Last two steps to enable VLANs on MAAS will follow: * Provisioner uses []NetworkInfo from StartInstance to add the networks and interfaces in state, as needed; * Implement the changes to cloudinit scripts for MAAS so that network interfaces are brought up on boot. Few other steps will be finished after that, but with the ones above VLAN support will be complete (i.e. juju status to show networks and CLI changes to validate networks). https://code.launchpad.net/~dimitern/juju-core/391-maas-map-networks-to-macs/+merge/214592 Requires: https://code.launchpad.net/~dimitern/juju-core/390-maas-get-node-nics/+merge/214528 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+191 lines, -112 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M provider/maas/environ.go View 7 chunks +185 lines, -110 lines 0 comments Download
M provider/maas/environ_whitebox_test.go View 1 chunk +4 lines, -2 lines 2 comments Download

Messages

Total messages: 3
dimitern
Please take a look.
9 years, 11 months ago (2014-04-07 18:25:45 UTC) #1
fwereade
LGTM, but somewhat grudgingly. This probably doesn't *reduce* the average quality of maas testing but ...
9 years, 11 months ago (2014-04-07 19:05:46 UTC) #2
dimitern
9 years, 11 months ago (2014-04-08 09:02:35 UTC) #3
https://codereview.appspot.com/85060043/diff/1/provider/maas/environ_whitebox...
File provider/maas/environ_whitebox_test.go (right):

https://codereview.appspot.com/85060043/diff/1/provider/maas/environ_whitebox...
provider/maas/environ_whitebox_test.go:537: {Name: "test_network", IP:
"127.0.0.1", Mask: "255.255.255.0", VLANTag: 1, Description: ""},
On 2014/04/07 19:05:47, fwereade wrote:
> I'd really like to see a test for the exported interface method, not just for
> getInstanceNetworks.
> 
> Ideally there'd be a couple of networks here -- one VLAN, one not, ideally
> neither with an address of 127.0.0.1 -- and a separate test at a similar level
> of complexity for StartInstance itself. How complex is it to wire that up,
> though?

I'll add more tests for StartInstance when we can do it - gomaasapi's test
server needs to be extended to support the new calls. I'm adding a card for
that, as I want to finish the juju part of the work more quickly. All changes
are tested live on my maas.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b