state: short-circuit remove machine filesystems #8127

Merged
merged 1 commit into from Nov 24, 2017

Conversation

Projects
None yet
3 participants
Member

axw commented Nov 24, 2017

Description of change

When a machine is destroyed and its cleanup is
run, short-circuit the removal of non-detachable
filesystems as long as the machine is non-manual.
For manual machines, the machine is not going
anywhere, so the filesystem must be detached and
destroyed by the machine's storage provisioner.

Note that the cleanup will still continue to fail
while the filesystem has a related a storage instance,
meaning the unit has not run the storage-detaching
hook. This ensures that we don't pull the filesystem
out from underneath the unit during destruction.

QA steps

  1. juju bootstrap openstack
  2. juju deploy jenkins --storage jenkins=cinder
    (wait)
  3. juju ssh 0
    cd /srv/mnt/jenkins
    (just sit there to prevent the filesystem being unmounted)
  4. juju remove-application jenkins
    (the machine should be removed successfully, and the cinder volume should be removed)

Documentation changes

None.

Bug reference

Fixes the other half of https://bugs.launchpad.net/juju/+bug/1722818

state: short-circuit remove machine filesystems
When a machine is destroyed and its cleanup is
run, short-circuit the removal of non-detachable
filesystems as long as the machine is non-manual.
For manual machines, the machine is not going
anywhere, so the filesystem must be detached and
destroyed by the machine's storage provisioner.

Note that the cleanup will still continue to fail
while the filesystem has a related a storage instance,
meaning the unit has not run the storage-detaching
hook. This ensures that we don't pull the filesystem
out from underneath the unit during destruction.

Fixes the other half of https://bugs.launchpad.net/juju/+bug/1722818

Is there a test we can add to replicate the real world behaviour which triggered the original issue ie sit in a mounted dir preventing its unmounting?

Member

axw commented Nov 24, 2017

Is there a test we can add to replicate the real world behaviour which triggered the original issue ie sit in a mounted dir preventing its unmounting?

We could certainly do that as a CI test, but it doesn't make sense as a unit test in the state package.

Member

axw commented Nov 24, 2017

$$merge$$

Contributor

jujubot commented Nov 24, 2017

Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju

@jujubot jujubot merged commit f315588 into juju:2.2 Nov 24, 2017

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment