/juju

resources: Don't run multiple cleanups for one storage path #7664

Merged
merged 2 commits into from Jul 21, 2017

Conversation

Reviewers

@axw axw

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@babbageclunk @jujubot @axw
@babbageclunk
Member

babbageclunk commented Jul 21, 2017 •
Edited 1 time

Description of change

Destroying an application with units would result in multiple cleanups being scheduled for the same storage path, because there are resource records for the application and the units. Change ResourcePersistence.NewRemoveResourcesOps to only schedule a cleanup once for a given path.

If a resource blob had already been deleted from storage when the cleanup ran, it would fail (logging an error) and leave the cleanup record around. This would prevent migrations, since the source precheck
ensures there aren't any cleanups. If the blob's already gone, the cleanup should succeed.

QA steps

  • Deploy an application that takes a resource (like cs:~cmars/mattermost).
  • Remove the application.
  • Check that there are no errors in the log about failed cleanups, and that only one "resourceBlob" cleanup is run.
  • Check that there are no cleanups remaining in the database once the system is idle.

Bug reference

Fixes https://bugs.launchpad.net/juju/+bug/1705352

babbageclunk added some commits Jul 21, 2017

@babbageclunk
state: Fix resource blob cleanup when the blob is already gone 
If a resource blob had already been deleted from storage when the
cleanup ran, it would fail (logging an error) and leave the cleanup
record around. This would prevent migrations, since the source precheck
ensures there aren't any cleanups.

If the blob's already gone, the cleanup should succeed.

Fixes https://bugs.launchpad.net/juju/+bug/1705352
9d32485
@babbageclunk
resources: Don't schedule multiple cleanups for one path 
This was the cause of the failed cleanups for resources that were
blocking migrations. Unit and applications resources were both found and
cleanups were scheduled for them even if they had the same
storage-path. This meant that the first cleanup would succeed and the
second would fail.

Change ResourcePersistence.NewRemoveResourcesOps to only emit one
cleanup for each path.
da721e5

@babbageclunk babbageclunk changed the title from state: Fix resource blob cleanup when the blob is already gone to resources: Don't run multiple cleanups for one storage path Jul 21, 2017

@axw

axw approved these changes Jul 21, 2017
View changes

@babbageclunk
Member

babbageclunk commented Jul 21, 2017

$$merge$$
@jujubot
Contributor

jujubot commented Jul 21, 2017

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

@jujubot jujubot merged commit 099ed19 into juju:2.2 Jul 21, 2017
1 check failed

1 check failed

github-check-merge-juju Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details

@babbageclunk babbageclunk deleted the babbageclunk:resource-cleanups branch Jul 23, 2017

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