Skip to content

Commit

Permalink
Fixes from code review.
Browse files Browse the repository at this point in the history
  • Loading branch information
Blake Rouse committed Sep 19, 2018
1 parent 9a70fcc commit f688dd4
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 17 deletions.
3 changes: 1 addition & 2 deletions blockdevice.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,7 @@ func blockdevice_2_0(source map[string]interface{}) (*blockdevice, error) {
"filesystem": schema.OneOf(schema.Nil(""), schema.StringMap(schema.Any())),
"partitions": schema.List(schema.StringMap(schema.Any())),
}
defaults := schema.Defaults{}
checker := schema.FieldMap(fields, defaults)
checker := schema.FieldMap(fields, nil)
coerced, err := checker.Coerce(source, nil)
if err != nil {
return nil, WrapWithDeserializationError(err, "blockdevice 2.0 schema check failed")
Expand Down
45 changes: 30 additions & 15 deletions controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -936,11 +936,11 @@ func parseAllocateConstraintsResponse(source interface{}, machine *machine) (Con
}

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 {
iface := machine.Interface(id.(int))
iface := machine.Interface(id)
if iface == nil {
return empty, NewDeserializationError("constraint match interface %q: %d does not match an interface for the machine", label, id)
}
Expand All @@ -951,10 +951,12 @@ func parseAllocateConstraintsResponse(source interface{}, machine *machine) (Con
}

if storageMatches, found := constraintsMap["storage"]; found {
matches := convertConstraintMatches(storageMatches)
matches := convertConstraintMatchesAny(storageMatches)
for label, ids := range matches {
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))
Expand All @@ -965,19 +967,18 @@ func parseAllocateConstraintsResponse(source interface{}, machine *machine) (Con
} else if id, ok := storageId.(string); ok {
// Should link to a partition.
const partPrefix = "partition:"
if strings.HasPrefix(id, partPrefix) {
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 {
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)
}
Expand All @@ -988,7 +989,21 @@ func parseAllocateConstraintsResponse(source interface{}, machine *machine) (Con
return result, nil
}

func convertConstraintMatches(source interface{}) map[string][]interface{} {
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{})
for label, values := range matchMap {
items := values.([]interface{})
result[label] = make([]int, len(items))
for index, value := range items {
result[label][index] = value.(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{})
Expand Down
1 change: 1 addition & 0 deletions interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ type Machine interface {
// 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

// Partition returns the partition for the machine that matches the
// id specified. If there is no match, nil is returned.
Partition(id int) Partition
Expand Down

0 comments on commit f688dd4

Please sign in to comment.