Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
migrate cloud-credentials #6548
Conversation
QAjuju bootstrap aws source juju switch source // wait until ubuntu deployed fully juju logout |
babbageclunk
approved these changes
Nov 8, 2016
A couple of minor suggestions and comments, otherwise looks good.
| @@ -49,7 +49,7 @@ build: godeps | ||
| go build $(PROJECT)/... | ||
| check: godeps | ||
| - go test -v -test.timeout=1200s $(PROJECT)/... -check.v | ||
| + go test -v -test.timeout=1500s $(PROJECT)/... -check.v |
babbageclunk
Nov 8, 2016
Member
Is this because the tests in general have been taking longer, or just slow on your machine?
howbazaar
Nov 14, 2016
Owner
Some of the QA tests were timing out close to 1200s if the machine was under load. This just gives it a little longer.
| @@ -66,6 +66,30 @@ func (*ModelSerializationSuite) TestUpdateConfig(c *gc.C) { | ||
| }) | ||
| } | ||
| +func (*ModelSerializationSuite) TestCloudCredentails(c *gc.C) { |
| + if creds := model.CloudCredential(); creds != nil { | ||
| + // Need to add credential or make sure an existing credential | ||
| + // matches. | ||
| + // TODO: there really should be a way to create a cloud credential |
babbageclunk
Nov 8, 2016
Member
This comment confused me - I guess you mean to create the cloud credential ID? Or to create a tag from (cloud, owner, name).
| + if !names.IsValidCloudCredential(credID) { | ||
| + return nil, nil, errors.Errorf("model credentails id not valid: %q", credID) | ||
| + } | ||
| + credTag := names.NewCloudCredentialTag(credID) |
babbageclunk
Nov 8, 2016
Member
Really this should take the three values rather than one. (Although not in this change.)
howbazaar
Nov 14, 2016
Owner
Yes, this is what I meant by the comment. Added some details to the comment.
| + credTag := names.NewCloudCredentialTag(credID) | ||
| + | ||
| + existingCreds, err := st.CloudCredential(credTag) | ||
| + if err != nil { |
babbageclunk
Nov 8, 2016
Member
This would be less nesty as
if errors.IsNotFound(err) {
// new case
} else if err != nil {
return nil, nil, errors.Trace(err)
} else {
// existing case
}Or just check err != nil && !errors.IsNotFound(err) first so the error return's early.
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
howbazaar commentedNov 8, 2016
Export the cloud credentials as part of the model description. When importing, the credentials
either have to match or not exist on the target.