juju bug 1756135, improve handling of service catalog urls #62
Conversation
version discovery. Provide coverage to additional OpenStack deployments. Add Trace to client logging.
| @@ -118,6 +118,30 @@ func (s *localLiveSuite) makeServiceURLTests() []makeServiceURLTest { | |||
| success: false, | |||
| err: "no endpoints known for service type: no-such-service", | |||
| }, | |||
| { | |||
There was a problem hiding this comment.
Are these completely representative of everything we saw? I'm thinking of the double version in the URL, like https://10.235.28.252/volumev2/v1/
client/client.go
Outdated
| logger.Tracef("MakeServiceURL: %s",returnURL) | ||
| } | ||
| } () | ||
|
|
There was a problem hiding this comment.
Might be nice to include the URL you are falling back to in logger.Warningf("falling back to catalogue service URL").
There was a problem hiding this comment.
We do print out the catalogue near the top of bootstrap... and the url will be printed too if trace is enable.
client/apiversion.go
Outdated
| } | ||
| return nil, err | ||
| if err := c.sendRequest("GET", getVersionURL, c.Token(), requestData); err != nil { | ||
| logger.Debugf("API version discovery failed: %v", err) |
There was a problem hiding this comment.
This was warn -- I think we should keep it on warn. We would want every user to see this.
client/apiversion.go
Outdated
| // some hrefURL.Path contain more than the version, with | ||
| // overlap on the apiURLVersionInfo.rootURL | ||
| if strings.HasPrefix(match, "/"+versionURL.Path) { | ||
| versionURL.Path = "/" |
There was a problem hiding this comment.
Maybe I'm being verbose, but trace logging for this?
| for i, p := range pathParts { | ||
| if _, err := parseVersion(p); err == nil { | ||
| found = true | ||
| if i == 0 { |
There was a problem hiding this comment.
Debug statements would be useful here for each condition - if we have to do this again, it would be nice to understand how juju parsed it.
There was a problem hiding this comment.
Can you also provide an example URL or format for each case?
There was a problem hiding this comment.
added logger.tracef and additional comments
|
|
during api version discovery. Provide coverage to additional OpenStack deployments.
Add Trace to client logging.
Tested against the on-site config, openstack and rackspace, and additional unit tests.