New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New API call, Controller.HostedModelConfig #6315
Conversation
Needs QA steps. In particular, have you tested an rc1 client with an rc2 server and vice versa? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My primary concern is about lack of QA steps as per my other comment.
return api.MakeCloudSpec(result.Result) | ||
} | ||
|
||
func (api *CloudSpecAPI) MakeCloudSpec(pSpec *params.CloudSpec) (environs.CloudSpec, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docstring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
// HostedModelConfig returns all the information that the client needs | ||
// in order to connect directly with the host model's provider and destroy | ||
// it directly. | ||
func (s *ControllerAPI) HostedModelConfig() (params.HostedModelConfigResults, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This confused me a bit at first. The name made me think it would return the config for one hosted model, but it doesn't take any args. How about calling it HostedModelConfigs
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK.
@@ -22,6 +22,24 @@ type ModelConfigResults struct { | |||
Config map[string]ConfigValue `json:"config"` | |||
} | |||
|
|||
// HostedModelConfig contains the model config and the cloud spec | |||
// for the model, both things that the CLI needs to talk directly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/the CLI/a client/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no QA steps for this branch because it is only adding a facade call. The next branch has the QA steps.
// HostedModelConfig returns all the information that the client needs | ||
// in order to connect directly with the host model's provider and destroy | ||
// it directly. | ||
func (s *ControllerAPI) HostedModelConfig() (params.HostedModelConfigResults, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK.
@@ -22,6 +22,24 @@ type ModelConfigResults struct { | |||
Config map[string]ConfigValue `json:"config"` | |||
} | |||
|
|||
// HostedModelConfig contains the model config and the cloud spec | |||
// for the model, both things that the CLI needs to talk directly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
return api.MakeCloudSpec(result.Result) | ||
} | ||
|
||
func (api *CloudSpecAPI) MakeCloudSpec(pSpec *params.CloudSpec) (environs.CloudSpec, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
This new API call is used in an upcoming PR that adds a timeout to the kill-controller command. Once the timeout is hit, the client will attempt to destroy all the models by talking to the provider directly. To do this, it needs to have the model's config and cloud config in order to be able to instantiate an Environ.
The facade version is not bumped as it is no danger to the client. If the client is newer than the server, which is possible for early RCs, then a timeout of 5 minutes will happen before the provider is invoked directly. The call will fail with NotImplemented, so will cause no further damage. kill-controller was written with the ability to call it more than once, so running the command again is expected behaviour.