Skip to content

Commit

Permalink
Merge pull request #71 from ExternalReality/bug#1729127
Browse files Browse the repository at this point in the history
Fix allow storage constraints to search both physical and virtual(logi…

…cal) block devices.
  • Loading branch information
jujubot authored Dec 5, 2017
2 parents 91b8ec5 + 2199ae2 commit 663f786
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 10 deletions.
4 changes: 2 additions & 2 deletions controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -937,9 +937,9 @@ func parseAllocateConstraintsResponse(source interface{}, machine *machine) (Con
for label, ids := range matches {
blockDevices := make([]BlockDevice, len(ids))
for index, id := range ids {
blockDevice := machine.PhysicalBlockDevice(id)
blockDevice := machine.BlockDevice(id)
if blockDevice == nil {
return empty, NewDeserializationError("constraint match storage %q: %d does not match a physical block device for the machine", label, id)
return empty, NewDeserializationError("constraint match storage %q: %d does not match a block device for the machine", label, id)
}
blockDevices[index] = blockDevice
}
Expand Down
15 changes: 15 additions & 0 deletions controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,21 @@ func (s *controllerSuite) TestAllocateMachineStorageMatches(c *gc.C) {
c.Assert(storages[1].ID(), gc.Equals, 98)
}

func (s *controllerSuite) TestAllocateMachineStorageLogicalMatches(c *gc.C) {
s.server.AddPostResponse("/api/2.0/machines/?op=allocate", http.StatusOK, machineResponse)
controller := s.getController(c)
machine, matches, err := controller.AllocateMachine(AllocateMachineArgs{
Storage: []StorageSpec{{
Tags: []string{"raid0"},
}},
})
c.Assert(err, jc.ErrorIsNil)
var virtualDeviceID = 23

//matches storage must contain the "raid0" virtual block device
c.Assert(matches.Storage["0"][0], gc.Equals, machine.BlockDevice(virtualDeviceID))
}

func (s *controllerSuite) TestAllocateMachineStorageMatchMissing(c *gc.C) {
// This should never happen, but if it does it is a clear indication of a
// bug somewhere.
Expand Down
3 changes: 3 additions & 0 deletions interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,9 @@ type Machine interface {

// BlockDevices returns all the physical and virtual block devices on the machine.
BlockDevices() []BlockDevice
// BlockDevice returns the block device for the machine that matches the
// id specified. If there is no match, nil is returned.
BlockDevice(id int) BlockDevice

Zone() Zone

Expand Down
21 changes: 15 additions & 6 deletions machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,7 @@ func (m *machine) PhysicalBlockDevices() []BlockDevice {

// PhysicalBlockDevice implements Machine.
func (m *machine) PhysicalBlockDevice(id int) BlockDevice {
for _, blockDevice := range m.physicalBlockDevices {
if blockDevice.ID() == id {
return blockDevice
}
}
return nil
return blockDeviceById(id, m.PhysicalBlockDevices())
}

// BlockDevices implements Machine.
Expand All @@ -195,6 +190,20 @@ func (m *machine) BlockDevices() []BlockDevice {
return result
}

// BlockDevice implements Machine.
func (m *machine) BlockDevice(id int) BlockDevice {
return blockDeviceById(id, m.BlockDevices())
}

func blockDeviceById(id int, blockDevices []BlockDevice) BlockDevice {
for _, blockDevice := range blockDevices {
if blockDevice.ID() == id {
return blockDevice
}
}
return nil
}

// Devices implements Machine.
func (m *machine) Devices(args DevicesArgs) ([]Device, error) {
// Perhaps in the future, MAAS will give us a way to query just for the
Expand Down
60 changes: 58 additions & 2 deletions machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ func (*machineSuite) TestReadMachines(c *gc.C) {
c.Assert(machine.Interface(id+5), gc.IsNil)

blockDevices := machine.BlockDevices()
c.Assert(blockDevices, gc.HasLen, 2)
c.Assert(blockDevices, gc.HasLen, 3)
c.Assert(blockDevices[0].Name(), gc.Equals, "sda")
c.Assert(blockDevices[1].Name(), gc.Equals, "sdb")
c.Assert(blockDevices[2].Name(), gc.Equals, "md0")

blockDevices = machine.PhysicalBlockDevices()
c.Assert(blockDevices, gc.HasLen, 2)
Expand Down Expand Up @@ -420,11 +421,17 @@ const (
machineOwnerDataTemplate = `
{
"netboot": false,
"constraints_by_type": {
"storage": {
"0": [
23
]
}
},
"system_id": "4y3ha3",
"ip_addresses": [
"192.168.100.4"
],
"virtualblockdevice_set": [],
"memory": 1024,
"cpu_count": 1,
"hwe_kernel": "hwe-t",
Expand All @@ -438,6 +445,32 @@ const (
],
"special_filesystems": [],
"status": 6,
"virtualblockdevice_set": [
{
"block_size": 512,
"serial": null,
"path": "/dev/disk/by-dname/md0",
"system_id": "xc3e6q",
"available_size": 256599130112,
"size": 256599130112,
"uuid": "b76de3fd-d05f-4a3f-b515-189de53d6c03",
"tags": [
"raid0"
],
"used_size": 0,
"name": "md0",
"type": "virtual",
"filesystem": null,
"used_for": "Unused",
"partitions": [],
"id": 23,
"partition_table_type": null,
"model": null,
"id_path": null,
"resource_uri": "/MAAS/api/2.0/nodes/xc3e6q/blockdevices/23/"
}
],
"physicalblockdevice_set": [
{
"path": "/dev/disk/by-dname/sda",
Expand Down Expand Up @@ -776,6 +809,29 @@ const (
"tags": [
"rotary"
]
},
{
"tags": [
"raid0"
],
"used_size": 0,
"path": "/dev/disk/by-dname/md0",
"serial": null,
"available_size": 256599130112,
"system_id": "xc3e6q",
"uuid": "b76de3fd-d05f-4a3f-b515-189de53d6c03",
"block_size": 512,
"size": 256599130112,
"type": "virtual",
"filesystem": null,
"used_for": "Unused",
"partitions": [],
"id": 23,
"name": "md0",
"partition_table_type": null,
"model": null,
"id_path": null,
"resource_uri": "/MAAS/api/2.0/nodes/xc3e6q/blockdevices/23/"
}
],
"zone": {
Expand Down

0 comments on commit 663f786

Please sign in to comment.