Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
migrations: Implement AdoptResources for the openstack provider #6943
Conversation
babbageclunk
added some commits
Feb 7, 2017
| + if err != nil { | ||
| + return errors.Trace(err) | ||
| + } | ||
| + // I'm not sure about this - it works at the moment because |
babbageclunk
Feb 8, 2017
Member
Oops - I meant to pull this out and replace it with a comment on github. Anyway, I'll pull it out once I know the answer.
| @@ -21,6 +21,11 @@ import ( | ||
| "github.com/juju/juju/network" | ||
| ) | ||
| +var ( | ||
| + validUUID = `[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}` | ||
| + ExtractGroupControllerRe = regexp.MustCompile(`^(?P<prefix>juju-)(?P<controllerUUID>` + validUUID + `)(?P<suffix>-.*)$`) |
axw
Feb 8, 2017
Member
Please don't export the regexp itself. The pattern would be OK exported as a constant, but this thing is mutable.
| @@ -49,6 +54,10 @@ type Firewaller interface { | ||
| // DeleteGroups deletes the security groups with the specified names. | ||
| DeleteGroups(names ...string) error | ||
| + // Implementations are expected to update all of the security |
axw
Feb 8, 2017
Member
I realise the function below doesn't follow the pattern, but please stick to the idiom of:
"UpdateGroupController updates all of the security groups for this model to refer to the specified controller."
I think the intent might be a bit clearer by appending something like ", such that DeleteAllControllerGroups will remove them only when called with the specified controller ID."
| + if err != nil { | ||
| + return errors.Trace(err) | ||
| + } | ||
| + // I'm not sure about this - it works at the moment because |
|
!!chittychitty!! |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
babbageclunk commentedFeb 8, 2017
Description of change
The AdoptResources environ method will be used at the end of a model migration to update the controller tag for all of the resources used by the model, so that if the source controller is subsequently destroyed then these resources won't be cleaned up with it.
In the openstack provider we tag volumes and instances with the controller UUID. Metadata for these can be updated using the API - I've added the SetVolumeMetadata method to the goose Cinder client (dependency updated here). Security groups don't have tags, so the controller is included as a component of the group name. The group names for the current model are all rewritten with the new controller UUID.
QA steps
None yet - this method isn't called from the migration master yet.
Documentation changes
N/A
Bug reference
This is part of the fix for https://bugs.launchpad.net/juju/+bug/1648063