Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
provider/openstack: shorten hostnames #6380
Conversation
hoenirvili
suggested changes
Oct 5, 2016
Thanks again for submitting this PR, LGTM other than the coding style issue I just mentioned.
| @@ -56,7 +56,12 @@ func (env *Environ) cinderProvider() (*cinderProvider, error) { | ||
| if err != nil { | ||
| return nil, errors.Trace(err) | ||
| } | ||
| - return &cinderProvider{storageAdapter, env.name, env.uuid}, nil | ||
| + return &cinderProvider{ | ||
| + storageAdapter, |
hoenirvili
Oct 5, 2016
Contributor
Why omit here struct name fields? You used below the name fields why here omit them? Why not always use them when you are returning a struct or a ptr to struct? I think this way, in my opinion, are clearly specifying what values are attribute to, it's much more pleasing to the eye and more consistent (which is a good thing to have all around the juju codebase).
axw
Oct 5, 2016
Member
Heh, thanks for calling me out. I was just making the minimal change - will update to use field names.
hoenirvili
Oct 6, 2016
Contributor
Thanks again for the modifications, see you there in the next PR! Great work.
| @@ -77,7 +77,15 @@ var ( | ||
| func NewCinderVolumeSource(s OpenstackStorage) storage.VolumeSource { | ||
| const envName = "testenv" | ||
| modelUUID := testing.ModelTag.Id() | ||
| - return &cinderVolumeSource{s, envName, modelUUID} | ||
| + return &cinderVolumeSource{s, envName, modelUUID, fakeNamespace{}} |
| - return &cinderProvider{storageAdapter, env.name, env.uuid}, nil | ||
| + return &cinderProvider{ | ||
| + storageAdapter: storageAdapter, | ||
| + envName: env.name, |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
axw commentedOct 5, 2016
Shorten the hostnames we apply to instances
created by the OpenStack provider. We were
including the full model UUID, which is quite
long. Instead, we should include just the
suffix, and the model name.
When listing instances, don't do a prefix
filter on the model UUID. Instead, just look
for "juju-.*" and then match the model UUID
in the client code.
Example old hostname:
juju-fd943864-df2e-4da1-8e7d-5116a87d4e7c-machine-14
Example new hostname:
juju-df7591-controller-0
Fixes https://bugs.launchpad.net/juju/+bug/1615601
QA