Merge lp://staging/~dooferlad/gomaasapi/subnets into lp://staging/gomaasapi

Proposed by James Tunnicliffe
Status: Merged
Approved by: James Tunnicliffe
Approved revision: 79
Merged at revision: 64
Proposed branch: lp://staging/~dooferlad/gomaasapi/subnets
Merge into: lp://staging/gomaasapi
Diff against target: 1390 lines (+1091/-40)
8 files modified
jsonobject.go (+10/-1)
maasobject.go (+2/-2)
testservice.go (+92/-27)
testservice_spaces.go (+81/-0)
testservice_subnets.go (+393/-0)
testservice_test.go (+361/-10)
testservice_utils.go (+119/-0)
testservice_vlan.go (+33/-0)
To merge this branch: bzr merge lp://staging/~dooferlad/gomaasapi/subnets
Reviewer Review Type Date Requested Status
Michael Foord (community) Approve
James Tunnicliffe (community) Needs Resubmitting
Dimiter Naydenov Pending
Review via email: mp+278342@code.staging.launchpad.net

This proposal supersedes a proposal from 2015-11-19.

Description of the change

Add subnets support.

To post a comment you must log in.
Revision history for this message
Dimiter Naydenov (dimitern) wrote : Posted in a previous version of this proposal

Most of it looks good, apart from a few concerns around marshalling/unmarshalling for numbers and reducing some duplication.

review: Needs Fixing
Revision history for this message
James Tunnicliffe (dooferlad) wrote : Posted in a previous version of this proposal

Don't worry about the JSON stuff - it works just fine if you use structs because go can convert according to the type you give it. The de-dup stuff seems worth it though.

Revision history for this message
James Tunnicliffe (dooferlad) : Posted in a previous version of this proposal
Revision history for this message
James Tunnicliffe (dooferlad) : Posted in a previous version of this proposal
72. By James Tunnicliffe

Pretty JSON is easier to read.
Fixed api/1.0/subnets/1/?op=reserved_ip_ranges when there is 1 reserved address.

73. By James Tunnicliffe

Fixed incorrect JSON name for space

74. By James Tunnicliffe

New addresses default to having a purpose of "assigned-ip"
Added AddFixedAddressRange call.
De-duplicated some code.

75. By James Tunnicliffe

Purpose back to being an array.
Fixed possible out of range error.

76. By James Tunnicliffe

AddFixedAddressRange moved to the server rather than directly acting on a subnet.

77. By James Tunnicliffe

suite.server.SetNodeNetworkLink now takes a node.SystemID rather than a node.

78. By James Tunnicliffe

Fixed subnets/1/?op=reserved_ip_ranges when no IP addresses had been explicitly assigned.

Revision history for this message
James Tunnicliffe (dooferlad) wrote :

Now working well enough for voidspace to use. Please take another look.

review: Needs Resubmitting
Revision history for this message
Michael Foord (mfoord) wrote :

The json encoding serialises uninitialised slices as null, which is incompatible with MAAS and will break clients that expect an empty array. Other than that looks good to me.

review: Needs Fixing
79. By James Tunnicliffe

Ensure that all arrays in a posted subnet are non-nil.
Fix typo.
Simplify net.IP --> uint64 logic.

Revision history for this message
Michael Foord (mfoord) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: