Moar storage-attached #11

Merged
merged 5 commits into from Jan 28, 2015

Conversation

Projects
None yet
2 participants
Collaborator

axw commented Jan 28, 2015

  • Fixes to uniter StorageChanged operation
  • Made storage-get more like relation-get:
    • you can specify a storage ID with "-s",
      and it defaults to the hook storage instance.
    • you can specify at most one key. If none
      are specified, you get a map; if one is
      specified you just get that attribute's value.
  • When a machine is provisioned, we set the location for associated "block" type storage instances to the device paths.
  • When a disk is formatted, we mount it and then set the location on the associated storage instance.

axw added some commits Jan 27, 2015

Various uniter changes to support storage-attached
- storage-get now is more like relation-get:
    * you can specify a storage ID with "-s",
      and it defaults to the hook storage instance.
    * you can specify at most one key. If none
      are specified, you get a map; if one is
      specified you just get that attribute's value.
- uniter is hacked up a bit to work better, needs
  thorough review from fwereade to figure out the
  appropriate course.
- when we set provisioned block device info, we
  tack the location onto storage instance info.
  This is a dirty hack that will go away when we
  come to doing all this properly (model refactor).
storage/storage.go
@@ -35,4 +35,6 @@ type StorageInstance struct {
// Location is the location relevant to the datastore (block device, filesystem).
Location string `yaml:"location" json:"location"`
+
+ RequestedLocation string `yaml:"requested-location" json:"requested-location"`
@wallyworld

wallyworld Jan 28, 2015

Owner

Doc comment please

@axw

axw Jan 28, 2015

Collaborator

Done.

- if err := createFilesystem(devicePath); err != nil {
- logger.Errorf("failed to create filesystem on block device %q: %v", blockDevices[i].Name, err)
+ if blockDevices[i].FilesystemType == "" {
+ if err := createFilesystem(devicePath); err != nil {
@wallyworld

wallyworld Jan 28, 2015

Owner

I can't see how we know to create a file system as opposed to leaving it as a raw block device

@axw

axw Jan 28, 2015

Collaborator

Line 101/103:
if storageInstance.Kind != storage.StorageKindFilesystem {

+ // TODO(axw) determine what the appropriate
+ // course of action is here. For now, this
+ // works.
+ StorageId: "fake/0",
@wallyworld

wallyworld Jan 28, 2015

Owner

don't quite follow this but if it works....
do we have to have a value for StorageId?

@axw

axw Jan 28, 2015

Collaborator

Yes, see worker/uniter/modes.go. This is a hack, I'm not sure how it's meant to work. Will need to talk with William.

- case "location":
- values[key] = storageInstance.Location
- default:
- return errors.Errorf("invalid storage instance key %q", key)
@wallyworld

wallyworld Jan 28, 2015

Owner

We just want to ignore invalid keys rather than print an error? I'd prefer an error. Maybe a todo for now?

@axw

axw Jan 28, 2015

Collaborator

Look at the bottom of the function, we're not ignoring.

Owner

wallyworld commented Jan 28, 2015

LGTM with questions answered. Main question - how does diskformatter know whether to format vs just keeping a raw block device

wallyworld added a commit that referenced this pull request Jan 28, 2015

@wallyworld wallyworld merged commit 6406f42 into wallyworld:storage-feature Jan 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment