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.

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

Fix Device.InterfaceSet to not return an error. #34

Merged
merged 1 commit into from
Apr 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (s *controllerSuite) TestCreateDevice(c *gc.C) {
MACAddresses: []string{"a-mac-address"},
})
c.Assert(err, jc.ErrorIsNil)
c.Assert(device.SystemID(), gc.Equals, "4y3ha8")
c.Assert(device.SystemID(), gc.Equals, "4y3haf")
}

func (s *controllerSuite) TestCreateDeviceMissingAddress(c *gc.C) {
Expand Down
55 changes: 34 additions & 21 deletions device.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ type device struct {
hostname string
fqdn string

ipAddresses []string
zone *zone
parent string
owner string

ipAddresses []string
interfaceSet []*interface_
zone *zone
}

// SystemID implements Device.
Expand All @@ -41,6 +45,16 @@ func (d *device) FQDN() string {
return d.fqdn
}

// Parent implements Device.
func (d *device) Parent() string {
return d.parent
}

// Owner implements Device.
func (d *device) Owner() string {
return d.owner
}

// IPAddresses implements Device.
func (d *device) IPAddresses() []string {
return d.ipAddresses
Expand All @@ -52,24 +66,13 @@ func (d *device) Zone() Zone {
}

// InterfaceSet implements Device.
func (d *device) InterfaceSet() ([]Interface, error) {
// NOTE: the get and extra parse will not be necessary
// when r4900 of maas has been packaged and released.
source, err := d.controller.get(d.interfacesURI())
if err != nil {
return nil, NewUnexpectedError(err)
}
ifaces, err := readInterfaces(d.controller.apiVersion, source)
if err != nil {
return nil, errors.Trace(err)
}

result := make([]Interface, len(ifaces))
for i, v := range ifaces {
func (d *device) InterfaceSet() []Interface {
result := make([]Interface, len(d.interfaceSet))
for i, v := range d.interfaceSet {
v.controller = d.controller
result[i] = v
}
return result, nil
return result
}

// CreateInterfaceArgs is an argument struct for passing parameters to
Expand Down Expand Up @@ -240,9 +243,12 @@ func device_2_0(source map[string]interface{}) (*device, error) {
"system_id": schema.String(),
"hostname": schema.String(),
"fqdn": schema.String(),
"parent": schema.String(),
"owner": schema.String(),

"ip_addresses": schema.List(schema.String()),
"zone": schema.StringMap(schema.Any()),
"ip_addresses": schema.List(schema.String()),
"interface_set": schema.List(schema.StringMap(schema.Any())),
"zone": schema.StringMap(schema.Any()),
}
checker := schema.FieldMap(fields, nil) // no defaults
coerced, err := checker.Coerce(source, nil)
Expand All @@ -253,6 +259,10 @@ func device_2_0(source map[string]interface{}) (*device, error) {
// From here we know that the map returned from the schema coercion
// contains fields of the right type.

interfaceSet, err := readInterfaceList(valid["interface_set"].([]interface{}), interface_2_0)
if err != nil {
return nil, errors.Trace(err)
Copy link

Choose a reason for hiding this comment

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

wrap with a gomaasapi error type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

readInterfaceList already returns gomaasapi error types.

}
zone, err := zone_2_0(valid["zone"].(map[string]interface{}))
if err != nil {
return nil, errors.Trace(err)
Copy link

Choose a reason for hiding this comment

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

same again

Expand All @@ -264,9 +274,12 @@ func device_2_0(source map[string]interface{}) (*device, error) {
systemID: valid["system_id"].(string),
hostname: valid["hostname"].(string),
fqdn: valid["fqdn"].(string),
parent: valid["parent"].(string),
owner: valid["owner"].(string),

ipAddresses: convertToStringSlice(valid["ip_addresses"]),
zone: zone,
ipAddresses: convertToStringSlice(valid["ip_addresses"]),
interfaceSet: interfaceSet,
zone: zone,
}
return result, nil
}
135 changes: 95 additions & 40 deletions device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ func (*deviceSuite) TestReadDevices(c *gc.C) {
c.Assert(devices, gc.HasLen, 1)

device := devices[0]
c.Assert(device.SystemID(), gc.Equals, "4y3ha8")
c.Assert(device.Hostname(), gc.Equals, "furnacelike-brittney")
c.Assert(device.FQDN(), gc.Equals, "furnacelike-brittney.maas")
c.Assert(device.IPAddresses(), jc.DeepEquals, []string{"192.168.100.11"})
c.Check(device.SystemID(), gc.Equals, "4y3haf")
c.Check(device.Hostname(), gc.Equals, "furnacelike-brittney")
c.Check(device.FQDN(), gc.Equals, "furnacelike-brittney.maas")
c.Check(device.IPAddresses(), jc.DeepEquals, []string{"192.168.100.11"})
zone := device.Zone()
c.Assert(zone, gc.NotNil)
c.Assert(zone.Name(), gc.Equals, "default")
c.Check(zone, gc.NotNil)
c.Check(zone.Name(), gc.Equals, "default")
}

func (*deviceSuite) TestLowVersion(c *gc.C) {
Expand All @@ -54,10 +54,8 @@ func (*deviceSuite) TestHighVersion(c *gc.C) {
func (s *deviceSuite) TestInterfaceSet(c *gc.C) {
server, device := s.getServerAndDevice(c)
server.AddGetResponse(device.interfacesURI(), http.StatusOK, interfacesResponse)

ifaces, err := device.InterfaceSet()
c.Assert(err, jc.ErrorIsNil)
c.Assert(ifaces, gc.HasLen, 1)
ifaces := device.InterfaceSet()
c.Assert(ifaces, gc.HasLen, 2)
}

type fakeVLAN struct {
Expand Down Expand Up @@ -211,36 +209,93 @@ func (s *deviceSuite) TestDeleteUnknown(c *gc.C) {

const (
deviceResponse = `
{
"domain": {
"name": "maas",
"ttl": null,
"resource_record_count": 0,
"resource_uri": "/MAAS/api/2.0/domains/0/",
"id": 0,
"authoritative": true
},
"tag_names": [],
"hostname": "furnacelike-brittney",
"zone": {
"name": "default",
"description": "",
"resource_uri": "/MAAS/api/2.0/zones/default/"
{
"zone": {
"description": "",
"resource_uri": "/MAAS/api/2.0/zones/default/",
"name": "default"
},
"domain": {
"resource_record_count": 0,
"resource_uri": "/MAAS/api/2.0/domains/0/",
"authoritative": true,
"name": "maas",
"ttl": null,
"id": 0
},
"node_type_name": "Device",
"address_ttl": null,
"hostname": "furnacelike-brittney",
"node_type": 1,
"resource_uri": "/MAAS/api/2.0/devices/4y3haf/",
"ip_addresses": ["192.168.100.11"],
"owner": "thumper",
"tag_names": [],
"fqdn": "furnacelike-brittney.maas",
"system_id": "4y3haf",
"parent": "4y3ha3",
"interface_set": [
{
"resource_uri": "/MAAS/api/2.0/nodes/4y3haf/interfaces/48/",
"type": "physical",
"mac_address": "78:f0:f1:16:a7:46",
"params": "",
"discovered": null,
"effective_mtu": 1500,
"id": 48,
"children": [],
"links": [],
"name": "eth0",
"vlan": {
"secondary_rack": null,
"dhcp_on": true,
"fabric": "fabric-0",
"mtu": 1500,
"primary_rack": "4y3h7n",
"resource_uri": "/MAAS/api/2.0/vlans/1/",
"external_dhcp": null,
"name": "untagged",
"id": 1,
"vid": 0
},
"tags": [],
"parents": [],
"enabled": true
},
"parent": null,
"system_id": "4y3ha8",
"node_type": 1,
"ip_addresses": ["192.168.100.11"],
"resource_uri": "/MAAS/api/2.0/devices/4y3ha8/",
"owner": "thumper",
"fqdn": "furnacelike-brittney.maas",
"node_type_name": "Device",
"macaddress_set": [
{
"mac_address": "b8:6a:6d:58:b3:7d"
}
]
}
`
{
"resource_uri": "/MAAS/api/2.0/nodes/4y3haf/interfaces/49/",
"type": "physical",
"mac_address": "15:34:d3:2d:f7:a7",
"params": {},
"discovered": null,
"effective_mtu": 1500,
"id": 49,
"children": [],
"links": [
{
"mode": "link_up",
"id": 101
}
],
"name": "eth1",
"vlan": {
"secondary_rack": null,
"dhcp_on": true,
"fabric": "fabric-0",
"mtu": 1500,
"primary_rack": "4y3h7n",
"resource_uri": "/MAAS/api/2.0/vlans/1/",
"external_dhcp": null,
"name": "untagged",
"id": 1,
"vid": 0
},
"tags": [],
"parents": [],
"enabled": true
}
]
}
`
devicesResponse = "[" + deviceResponse + "]"
)
14 changes: 8 additions & 6 deletions interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,15 @@ type Device interface {
IPAddresses() []string
Zone() Zone

// Parent, Owner, MAC Addresses if needed
// Parent returns the SystemID of the Parent. Most often this will be a
// Machine.
Parent() string

// InterfaceSet returns all the interfaces for the Device. This is an
// interim call until r4900 is packaged, when we will be able to remove the
// error response as the device JSON will include the interface set which
// will be able to be parsed at object creation time.
InterfaceSet() ([]Interface, error)
// Owner is the username of the user that created the device.
Owner() string

// InterfaceSet returns all the interfaces for the Device.
InterfaceSet() []Interface
Copy link

Choose a reason for hiding this comment

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

Why InterfaceSet instead of Interfaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it was called "interface_set" in the JSON response. That is the only reason I chose that name.


// CreateInterface will create a physical interface for this machine.
CreateInterface(CreateInterfaceArgs) (Interface, error)
Expand Down