Volume backed filesystems take on scope of backing volume #7320

Merged
merged 4 commits into from May 10, 2017

Conversation

Projects
None yet
3 participants
Member

axw commented May 9, 2017

Description of change

Volume-backed filesystems now take on the scope of
the backing volume. If the volume is model-scoped,
then so is the filesystem. This enables us to detach
and reattach volume-backed filesystem storage.

The watcher implementation for machine filesystems
and filesystem attachments has been updated to
report filesystems whose backing volumes are attached
to the specified machine. The apiserver/storageprovisioner
allows access to a volume-backed filesystem if access
to the backing volume is allowed.

There's also a drive-by fix for a cmd/juju/storage bug
that appeared while testing. If storage is attached to a
unit, but not yet a machine, "juju storage" would panic.

QA steps

  1. juju bootstrap aws

  2. juju deploy postgresql --storage pgdata=ebs-ssd,10G
    The filesystem should have ID "0" (previously it would have been "0/0"). Confirm with "juju storage --filesystem".

  3. juju remove-application postgresql
    The output of "juju remove-application" should inform the user that the storage will be detached.

  4. wait for application, unit, and machine to be removed
    The storage should remain.

Documentation changes

None.

Bug reference

None.

axw added some commits May 9, 2017

state: volume-backed filesystems have volume scope
Volume-backed filesystems now take on the scope of
the backing volume. If the volume is model-scoped,
then so is the filesystem. This enables us to detach
and reattach volume-backed filesystem storage.

The watcher implementation for machine filesystems
and filesystem attachments has been updated to
report filesystems whose backing volumes are attached
to the specified machine.
cmd/juju/storage: don't panic
Don't panic if there are unit/storage attachments,
but no machine volume/filesystem attachments.
apiserver/storageprovisioner: update access rules
The storage provisioner may access volume-backed filesystems
if it may access the backing volumes.

Looks good. I would prefer to see the various watcher tests include more excluded data to ensure full coverage of the filters used in the watcher.

state/watcher.go
+ _, err = st.VolumeAttachment(m, volumeTag)
+ return err == nil
+ }
+ return newLifecycleWatcher(st, filesystemsC, members, filter, nil)
}
// WatchEnvironVolumeAttachments returns a StringsWatcher that notifies of
// changes to the lifecycles of all volume attachments related to environ-
// scoped volumes.
func (st *State) WatchEnvironVolumeAttachments() StringsWatcher {
@wallyworld

wallyworld May 10, 2017

Owner

Can we rename here and in doc comments etc environ-scoped -> model-scoped

@axw

axw May 10, 2017

Member

done

Member

axw commented May 10, 2017

I would prefer to see the various watcher tests include more excluded data to ensure full coverage of the filters used in the watcher.

All of the volume and filesystem watch methods has 100% coverage. What do you think is missing?

Member

axw commented May 10, 2017

$$merge$$

Contributor

jujubot commented May 10, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit fbe714f into juju:feature-persistent-storage May 10, 2017

1 check failed

github-check-merge-juju Built PR, ran unit tests, and tested LXD deploy. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment