Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Make storage detachable/attachable #7186
Conversation
wallyworld
approved these changes
Mar 31, 2017
There's a lot of txn asserts being used, but it seems there's not a lot of concurrent access test coverage? Maybe add a test or two with txn hooks to test the correct behaviour?
| - storage, err := common.UnitStorage(api.backend, unit.UnitTag()) | ||
| + storage, err := storagecommon.UnitStorage(api.backend, unit.UnitTag()) | ||
| + if err != nil { | ||
| + return nil, err |
| @@ -57,6 +58,10 @@ type StorageInterface interface { | ||
| // BlockDevices returns information about block devices published | ||
| // for the specified machine. | ||
| BlockDevices(names.MachineTag) ([]state.BlockDeviceInfo, error) | ||
| + | ||
| + // UnitStorageAttachments returns the storage attachments for the | ||
| + // identified unit. |
wallyworld
Mar 31, 2017
Owner
specified unit
(to be consistent with the comment above and general usage elsewhere)
axw
Mar 31, 2017
Member
sure, though several of the methods above use identified as well :) I've changed them all
| + } | ||
| + for tag, volumeAttachment := range args.volumeAttachments { | ||
| + volumeAttachments = append(volumeAttachments, volumeAttachmentTemplate{ | ||
| + tag, volumeAttachment, true, |
wallyworld
Mar 31, 2017
Owner
would be nice to have a const ExistingStorage=true and NewStorage=false or something and use the consts instead of true/false
|
Also, is it worth adding a feature test or two? |
axw
added some commits
Mar 30, 2017
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
axw commentedMar 31, 2017
Description of change
When detaching storage from a unit, we now
leave the storage in the model, and just
clear the storage instance's ownership. The
storage can then be attached to another unit
using the new State.AttachStorage method.
Hook up the attach-storage backend so it calls
State.AttachStorage.
The existing model storage cleanups have been
replaced with one that destroys storage
instances, rather than volumes and filesystems.
The machine storage will be destroyed as a
consequence.
QA steps
(wait)
(wait - should fail because storagetest/0 and storagetest/1 are in different AZs)
(wait - should attach cleanly)
Documentation changes
When this branch lands into develop, after 2.2 is released, we need to document detach-storage and attach-storage in the devel docs.
Bug reference
https://bugs.launchpad.net/bugs/1592887