Update group deletion to work with controller UUID #49

Merged
merged 2 commits into from Oct 5, 2017

Conversation

Projects
None yet
2 participants
Member

babbageclunk commented Oct 4, 2017

The workaround for the problem with AdoptResources means that security
groups that need to be deleted now are named
juju---0, so handle that in the downgrade
step. Also remove the groups from any instances before trying to delete
them.

Includes a small tweak to limit status history migrated to 20 items.

babbageclunk added some commits Oct 3, 2017

Update group deletion to work with controller UUID
The workaround for the problem with AdoptResources means that security
groups that need to be deleted now are named
juju-<controlleruuid>-<modeluuid>-0, so handle that in the downgrade
step. Also remove the groups from any instances before trying to delete
them.
Restrict status history items to 20 for each key
Having them unlimited was causing OOMs when serialising the model.

@babbageclunk babbageclunk requested a review from howbazaar Oct 4, 2017

func (e *environ) downgradeGroups(modelUUID string) error {
client := e.nova()
+ groupPrefixRe, err := regexp.Compile(securityGroupPrefix + modelUUID)
@howbazaar

howbazaar Oct 5, 2017

Owner

It seems weird that you are adding a modelUUID here. You have two UUIDs to match?

@babbageclunk

babbageclunk Oct 5, 2017

Member

In this case I want to match any group that has the right model UUID, but I don't know the controller UUID (so match it with a regex). It's possible in the abort code higher up that we couldn't connect to the controller to find it out, but since the abort command is trying to undo as much as possible regardless of errors in an unrelated task, we still want to clean up these groups.

func (e *exporter) statusHistoryArgs(globalKey string) []description.StatusArgs {
history := e.statusHistory[globalKey]
result := make([]description.StatusArgs, len(history))
e.logger.Debugf("found %d status history docs for %s", len(history), globalKey)
+ if len(history) > maxStatusHistoryEntries {
@howbazaar

howbazaar Oct 5, 2017

Owner

Are we grabbing the right 20 here?

@babbageclunk

babbageclunk Oct 5, 2017

Member

Yup - it's coming in reverse-updated order, so the first 20 will be the most recent.

@babbageclunk babbageclunk merged commit 1c4951e into juju:master Oct 5, 2017

@babbageclunk babbageclunk deleted the babbageclunk:delete-groups-on-abort branch Oct 5, 2017

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