Merge pull request juju#75 from blakerouse/partition-storage-constraints

juju#75

MAAS 2.5 has added the ability for tags to match a partition and not just a block device when performing allocation with storage constraints.

This change allows this to on MAAS 2.5, while also not breaking versions before. This adds a new interface `StorageDevice` that can be either a partition or a block device.

blockdevice.go

@@ -13,6 +13,7 @@ type blockdevice struct {
 	resourceURI string
 
 	id      int
+	uuid    string
 	name    string
 	model   string
 	idPath  string
@@ -24,14 +25,25 @@ type blockdevice struct {
 	usedSize  uint64
 	size      uint64
 
+	filesystem *filesystem
 	partitions []*partition
 }
 
+// Type implements BlockDevice
+func (b *blockdevice) Type() string {
+	return "blockdevice"
+}
+
 // ID implements BlockDevice.
 func (b *blockdevice) ID() int {
 	return b.id
 }
 
+// UUID implements BlockDevice.
+func (b *blockdevice) UUID() string {
+	return b.uuid
+}
+
 // Name implements BlockDevice.
 func (b *blockdevice) Name() string {
 	return b.name
@@ -77,6 +89,11 @@ func (b *blockdevice) Size() uint64 {
 	return b.size
 }
 
+// FileSystem implements BlockDevice.
+func (b *blockdevice) FileSystem() FileSystem {
+	return b.filesystem
+}
+
 // Partitions implements BlockDevice.
 func (b *blockdevice) Partitions() []Partition {
 	result := make([]Partition, len(b.partitions))
@@ -135,6 +152,7 @@ func blockdevice_2_0(source map[string]interface{}) (*blockdevice, error) {
 		"resource_uri": schema.String(),
 
 		"id":       schema.ForceInt(),
+		"uuid":     schema.OneOf(schema.Nil(""), schema.String()),
 		"name":     schema.String(),
 		"model":    schema.OneOf(schema.Nil(""), schema.String()),
 		"id_path":  schema.OneOf(schema.Nil(""), schema.String()),
@@ -146,6 +164,7 @@ func blockdevice_2_0(source map[string]interface{}) (*blockdevice, error) {
 		"used_size":  schema.ForceUint(),
 		"size":       schema.ForceUint(),
 
+		"filesystem": schema.OneOf(schema.Nil(""), schema.StringMap(schema.Any())),
 		"partitions": schema.List(schema.StringMap(schema.Any())),
 	}
 	checker := schema.FieldMap(fields, nil)
@@ -157,17 +176,25 @@ func blockdevice_2_0(source map[string]interface{}) (*blockdevice, error) {
 	// From here we know that the map returned from the schema coercion
 	// contains fields of the right type.
 
+	var filesystem *filesystem
+	if fsSource, ok := valid["filesystem"].(map[string]interface{}); ok {
+		if filesystem, err = filesystem2_0(fsSource); err != nil {
+			return nil, errors.Trace(err)
+		}
+	}
 	partitions, err := readPartitionList(valid["partitions"].([]interface{}), partition_2_0)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 
+	uuid, _ := valid["uuid"].(string)
 	model, _ := valid["model"].(string)
 	idPath, _ := valid["id_path"].(string)
 	result := &blockdevice{
 		resourceURI: valid["resource_uri"].(string),
 
 		id:      valid["id"].(int),
+		uuid:    uuid,
 		name:    valid["name"].(string),
 		model:   model,
 		idPath:  idPath,
@@ -179,6 +206,7 @@ func blockdevice_2_0(source map[string]interface{}) (*blockdevice, error) {
 		usedSize:  valid["used_size"].(uint64),
 		size:      valid["size"].(uint64),
 
+		filesystem: filesystem,
 		partitions: partitions,
 	}
 	return result, nil

blockdevice_test.go

@@ -30,6 +30,7 @@ func (*blockdeviceSuite) TestReadBlockDevices(c *gc.C) {
 	c.Check(blockdevice.Model(), gc.Equals, "QEMU HARDDISK")
 	c.Check(blockdevice.Path(), gc.Equals, "/dev/disk/by-dname/sda")
 	c.Check(blockdevice.IDPath(), gc.Equals, "/dev/disk/by-id/ata-QEMU_HARDDISK_QM00001")
+	c.Check(blockdevice.UUID(), gc.Equals, "6199b7c9-b66f-40f6-a238-a938a58a0adf")
 	c.Check(blockdevice.UsedFor(), gc.Equals, "MBR partitioned with 1 partition")
 	c.Check(blockdevice.Tags(), jc.DeepEquals, []string{"rotary"})
 	c.Check(blockdevice.BlockSize(), gc.Equals, uint64(4096))
@@ -41,6 +42,11 @@ func (*blockdeviceSuite) TestReadBlockDevices(c *gc.C) {
 	partition := partitions[0]
 	c.Check(partition.ID(), gc.Equals, 1)
 	c.Check(partition.UsedFor(), gc.Equals, "ext4 formatted filesystem mounted at /")
+
+	fs := blockdevice.FileSystem()
+	c.Assert(fs, gc.NotNil)
+	c.Assert(fs.Type(), gc.Equals, "ext4")
+	c.Assert(fs.MountPoint(), gc.Equals, "/srv")
 }
 
 func (*blockdeviceSuite) TestReadBlockDevicesWithNulls(c *gc.C) {
@@ -51,6 +57,7 @@ func (*blockdeviceSuite) TestReadBlockDevicesWithNulls(c *gc.C) {
 
 	c.Check(blockdevice.Model(), gc.Equals, "")
 	c.Check(blockdevice.IDPath(), gc.Equals, "")
+	c.Check(blockdevice.FileSystem(), gc.IsNil)
 }
 
 func (*blockdeviceSuite) TestLowVersion(c *gc.C) {
@@ -89,7 +96,13 @@ var blockdevicesResponse = `
                 "size": 8581545984
             }
         ],
-        "filesystem": null,
+        "filesystem": {
+            "fstype": "ext4",
+            "mount_point": "/srv",
+            "label": "root",
+            "mount_options": null,
+            "uuid": "fcd7745e-f1b5-4f5d-9575-9b0bb796b752"
+        },
         "id_path": "/dev/disk/by-id/ata-QEMU_HARDDISK_QM00001",
         "resource_uri": "/MAAS/api/2.0/nodes/4y3ha3/blockdevices/34/",
         "id": 34,
@@ -99,7 +112,7 @@ var blockdevicesResponse = `
         "used_size": 8586788864,
         "available_size": 0,
         "partition_table_type": "MBR",
-        "uuid": null,
+        "uuid": "6199b7c9-b66f-40f6-a238-a938a58a0adf",
         "size": 8589934592,
         "model": "QEMU HARDDISK",
         "tags": [

controller.go

@@ -11,6 +11,7 @@ import (
 	"net/http"
 	"net/url"
 	"path"
+	"strconv"
 	"strings"
 	"sync/atomic"
 
@@ -530,9 +531,9 @@ type ConstraintMatches struct {
 	// that match that constraint.
 	Interfaces map[string][]Interface
 
-	// Storage is a mapping of the constraint label specified to the BlockDevices
+	// Storage is a mapping of the constraint label specified to the StorageDevice
 	// that match that constraint.
-	Storage map[string][]BlockDevice
+	Storage map[string][]StorageDevice
 }
 
 // AllocateMachine implements Controller.
@@ -912,7 +913,7 @@ func (c *controller) readAPIVersionInfo() (set.Strings, error) {
 func parseAllocateConstraintsResponse(source interface{}, machine *machine) (ConstraintMatches, error) {
 	var empty ConstraintMatches
 	matchFields := schema.Fields{
-		"storage":    schema.StringMap(schema.List(schema.ForceInt())),
+		"storage":    schema.StringMap(schema.List(schema.Any())),
 		"interfaces": schema.StringMap(schema.List(schema.ForceInt())),
 	}
 	matchDefaults := schema.Defaults{
@@ -931,11 +932,11 @@ func parseAllocateConstraintsResponse(source interface{}, machine *machine) (Con
 	constraintsMap := valid["constraints_by_type"].(map[string]interface{})
 	result := ConstraintMatches{
 		Interfaces: make(map[string][]Interface),
-		Storage:    make(map[string][]BlockDevice),
+		Storage:    make(map[string][]StorageDevice),
 	}
 
 	if interfaceMatches, found := constraintsMap["interfaces"]; found {
-		matches := convertConstraintMatches(interfaceMatches)
+		matches := convertConstraintMatchesInt(interfaceMatches)
 		for label, ids := range matches {
 			interfaces := make([]Interface, len(ids))
 			for index, id := range ids {
@@ -950,23 +951,45 @@ func parseAllocateConstraintsResponse(source interface{}, machine *machine) (Con
 	}
 
 	if storageMatches, found := constraintsMap["storage"]; found {
-		matches := convertConstraintMatches(storageMatches)
+		matches := convertConstraintMatchesAny(storageMatches)
 		for label, ids := range matches {
-			blockDevices := make([]BlockDevice, len(ids))
-			for index, id := range ids {
-				blockDevice := machine.BlockDevice(id)
-				if blockDevice == nil {
-					return empty, NewDeserializationError("constraint match storage %q: %d does not match a block device for the machine", label, id)
+			storageDevices := make([]StorageDevice, len(ids))
+			for index, storageId := range ids {
+				// The key value can be either an `int` which `json.Unmarshal` converts to a `float64` or a
+				// `string` when the key is "partition:{part_id}".
+				if id, ok := storageId.(float64); ok {
+					// Links to a block device.
+					blockDevice := machine.BlockDevice(int(id))
+					if blockDevice == nil {
+						return empty, NewDeserializationError("constraint match storage %q: %d does not match a block device for the machine", label, int(id))
+					}
+					storageDevices[index] = blockDevice
+				} else if id, ok := storageId.(string); ok {
+					// Should link to a partition.
+					const partPrefix = "partition:"
+					if !strings.HasPrefix(id, partPrefix) {
+						return empty, NewDeserializationError("constraint match storage %q: %s is not prefixed with partition", label, id)
+					}
+					partId, err := strconv.Atoi(id[len(partPrefix):])
+					if err != nil {
+						return empty, NewDeserializationError("constraint match storage %q: %s cannot convert to int.", label, id[len(partPrefix):])
+					}
+					partition := machine.Partition(partId)
+					if partition == nil {
+						return empty, NewDeserializationError("constraint match storage %q: %d does not match a partition for the machine", label, partId)
+					}
+					storageDevices[index] = partition
+				} else {
+					return empty, NewDeserializationError("constraint match storage %q: %v is not an int or string", label, storageId)
 				}
-				blockDevices[index] = blockDevice
 			}
-			result.Storage[label] = blockDevices
+			result.Storage[label] = storageDevices
 		}
 	}
 	return result, nil
 }
 
-func convertConstraintMatches(source interface{}) map[string][]int {
+func convertConstraintMatchesInt(source interface{}) map[string][]int {
 	// These casts are all safe because of the schema check.
 	result := make(map[string][]int)
 	matchMap := source.(map[string]interface{})
@@ -979,3 +1002,17 @@ func convertConstraintMatches(source interface{}) map[string][]int {
 	}
 	return result
 }
+
+func convertConstraintMatchesAny(source interface{}) map[string][]interface{} {
+	// These casts are all safe because of the schema check.
+	result := make(map[string][]interface{})
+	matchMap := source.(map[string]interface{})
+	for label, values := range matchMap {
+		items := values.([]interface{})
+		result[label] = make([]interface{}, len(items))
+		for index, value := range items {
+			result[label][index] = value
+		}
+	}
+	return result
+}

controller_test.go

@@ -644,15 +644,23 @@ 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"},
-		}},
+		Storage: []StorageSpec{
+			{
+				Tags: []string{"raid0"},
+			},
+			{
+				Tags: []string{"partition"},
+			},
+		},
 	})
 	c.Assert(err, jc.ErrorIsNil)
 	var virtualDeviceID = 23
+	var partitionID = 1
 
 	//matches storage must contain the "raid0" virtual block device
 	c.Assert(matches.Storage["0"][0], gc.Equals, machine.BlockDevice(virtualDeviceID))
+	//matches storage must contain the partition from physical block device
+	c.Assert(matches.Storage["1"][0], gc.Equals, machine.Partition(partitionID))
 }
 
 func (s *controllerSuite) TestAllocateMachineStorageMatchMissing(c *gc.C) {

domain.go

@@ -37,10 +37,10 @@ func domain_(source map[string]interface{}) (*domain, error) {
 	fields := schema.Fields{
 		"authoritative":         schema.Bool(),
 		"resource_record_count": schema.ForceInt(),
-		"ttl":          schema.OneOf(schema.Nil("null"), schema.ForceInt()),
-		"resource_uri": schema.String(),
-		"id":           schema.ForceInt(),
-		"name":         schema.String(),
+		"ttl":                   schema.OneOf(schema.Nil("null"), schema.ForceInt()),
+		"resource_uri":          schema.String(),
+		"id":                    schema.ForceInt(),
+		"name":                  schema.String(),
 	}
 	checker := schema.FieldMap(fields, nil) // no defaults
 	coerced, err := checker.Coerce(source, nil)

interfaces.go

@@ -235,6 +235,10 @@ type Machine interface {
 	// id specified. If there is no match, nil is returned.
 	BlockDevice(id int) BlockDevice
 
+	// Partition returns the partition for the machine that matches the
+	// id specified. If there is no match, nil is returned.
+	Partition(id int) Partition
+
 	Zone() Zone
 
 	// Start the machine and install the operating system specified in the args.
@@ -345,33 +349,41 @@ type FileSystem interface {
 	UUID() string
 }
 
-// Partition represents a partition of a block device. It may be mounted
-// as a filesystem.
-type Partition interface {
+// StorageDevice represents any piece of storage on a machine. Partition
+// and BlockDevice are storage devices.
+type StorageDevice interface {
+	// Type is the type of item.
+	Type() string
+
+	// ID is the unique ID of the item of that type.
 	ID() int
+
 	Path() string
-	// FileSystem may be nil if not mounted.
-	FileSystem() FileSystem
-	UUID() string
-	// UsedFor is a human readable string.
 	UsedFor() string
-	// Size is the number of bytes in the partition.
 	Size() uint64
+	UUID() string
+	Tags() []string
+
+	// FileSystem may be nil if not mounted.
+	FileSystem() FileSystem
+}
+
+// Partition represents a partition of a block device. It may be mounted
+// as a filesystem.
+type Partition interface {
+	StorageDevice
 }
 
 // BlockDevice represents an entire block device on the machine.
 type BlockDevice interface {
-	ID() int
+	StorageDevice
+
 	Name() string
 	Model() string
 	IDPath() string
-	Path() string
-	UsedFor() string
-	Tags() []string
 
 	BlockSize() uint64
 	UsedSize() uint64
-	Size() uint64
 
 	Partitions() []Partition
 

machine.go

@@ -204,6 +204,22 @@ func blockDeviceById(id int, blockDevices []BlockDevice) BlockDevice {
 	return nil
 }
 
+// Partition implements Machine.
+func (m *machine) Partition(id int) Partition {
+	return partitionById(id, m.BlockDevices())
+}
+
+func partitionById(id int, blockDevices []BlockDevice) Partition {
+	for _, blockDevice := range blockDevices {
+		for _, partition := range blockDevice.Partitions() {
+			if partition.ID() == id {
+				return partition
+			}
+		}
+	}
+	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