Pass ImageStream to ubuntu-cloudimage-query command. #4480
Conversation
| @@ -1005,7 +1005,8 @@ func (a *MachineAgent) updateSupportedContainers( | |||
| // adds additional fields, this fails. | |||
| container.ImageURLGetterConfig{ | |||
| st.Addr(), envUUID.Id(), []byte(agentConfig.CACert()), | |||
| cfg.CloudImageBaseURL(), container.ImageDownloadURL, | |||
| cfg.CloudImageBaseURL(), cfg.ImageStream(), | |||
There was a problem hiding this comment.
This is sufficiently lengthy now to justify putting the field names explicitly:
EnvUUID: envUUID.Id(),
CloudimgBaseUrl: cfg.CloudImageBaseURL()
...
|
@wallyworld does this change seem ok to you? |
| @@ -66,7 +67,7 @@ func (ug *imageURLGetter) CACert() []byte { | |||
|
|
|||
| // ImageDownloadURL determines the public URL which can be used to obtain an | |||
| // image blob with the specified parameters. | |||
| func ImageDownloadURL(kind instance.ContainerType, series, arch, cloudimgBaseUrl string) (string, error) { | |||
| func ImageDownloadURL(kind instance.ContainerType, series, arch, cloudimgBaseUrl, cloudimgStream string) (string, error) { | |||
There was a problem hiding this comment.
please put the stream param after arch
func ImageDownloadURL(kind instance.ContainerType, series, arch, cloudimgStream, cloudimgBaseUrl string) (string, error) {
|
Thanks for the great improvement. It looks good but is missing tests. We need test coverage before being able to land. |
|
Thank you for this contribution \o/ I have addressed the concerns of the reviewers and updated broken tests. |
|
I am landing proposal copied form this one to ensure the functionality is in the next 1.25 release. Thank you for suggestion! Feel free to disregard my previous merge request as well as my pull request. |
…-125 Passing stream to image download (fix from xnox). This is a fix that was originally proposed by xnox (#4480). Re-capped to make it into 1.25.4 before deadline. Xnox: This enables using daily stream in juju local provider, for example on s390x. Addressed review comments: Fixed tests. Moved stream to follow arch. Named properties for construction. (Review request: http://reviews.vapour.ws/r/4059/)
|
+1 excellent. |
|
Merged via #4619 |
This enables using daily stream in juju local provider, for example on s390x.