Skip to content
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

gce provider: fix daily image URL #4902

Merged
merged 3 commits into from Apr 3, 2016
Merged

Conversation

tych0
Copy link
Contributor

@tych0 tych0 commented Mar 28, 2016

Signed-off-by: Tycho Andersen tycho.andersen@canonical.com

Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
@tych0
Copy link
Contributor Author

tych0 commented Mar 28, 2016

For some context,

2016-03-28 10:40:06 rcj tych0, ah, I see the problem
2016-03-28 10:40:23 rcj image is in ubuntu-os-cloud-devel project, not ubuntu-os-cloud
2016-03-28 10:41:21 --> wolverineav (~wolverine@c-50-136-241-122.hsd1.ca.comcast.net) has joined #juju
2016-03-28 10:41:48 tych0 rcj: ok, what does that mean? :)
2016-03-28 10:45:33 rcj tych0, well, to use the gcloud cli as an example, this would launch that image... gcloud compute instances create "gce-rcj-x2" --zone us-central1-c --machine-type g1-small --network "default" --boot-disk-size 10 --boot-disk-type "pd-ssd" --boot-disk-device-name "gce-rcj-x1" --image /ubuntu-os-cloud-devel/daily-ubuntu-1604-xenial-v20160326
2016-03-28 10:46:08 rcj tych0, the paste you had showed the image in /ubuntu-os-cloud/ which is where we put release images only, not daily.
2016-03-28 10:46:11 --> jog (~jog@198.0.45.75) has joined #juju
2016-03-28 10:47:00 tych0 rcj: so is it fair to say that,
2016-03-28 10:47:05 tych0 ubuntuImageBasePath  = "projects/ubuntu-os-cloud/global/images/"
2016-03-28 10:47:12 tych0 we need another constant like that that looks like,
2016-03-28 10:47:29 tych0 ubuntuDailyImageBasePath = "projects/ubuntu-os-cloud-devel/global/images/"
2016-03-28 10:47:30 tych0 ?
2016-03-28 10:47:46 rcj yes

@@ -202,7 +202,7 @@ var getDisksTests = []struct {

func (s *environBrokerSuite) TestGetDisks(c *gc.C) {
for _, test := range getDisksTests {
diskSpecs, err := gce.GetDisks(s.spec, s.StartInstArgs.Constraints, test.Series, "32f7d570-5bac-4b72-b169-250c24a94b2b")
diskSpecs, err := gce.GetDisks(s.spec, s.StartInstArgs.Constraints, test.Series, "32f7d570-5bac-4b72-b169-250c24a94b2b", false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty clear that we need to add another test, or maybe another scenario permutation about how we handle Daily images.
Possibly another test higher up that we notice that "daily" images actually get them from another source? But definitely one at this level that the URL changes if you ask for dailies.

@jameinel
Copy link
Member

The change looks fine, but we need some tests so that other people coming to this can understand what it should be doing.

Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
@tych0
Copy link
Contributor Author

tych0 commented Mar 29, 2016

Thanks, I added the test.

@jameinel
Copy link
Member

jameinel commented Apr 1, 2016

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Apr 1, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot
Copy link
Collaborator

jujubot commented Apr 1, 2016

Build failed: Generating tarball failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/7146

Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
@jameinel
Copy link
Member

jameinel commented Apr 3, 2016

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Apr 3, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 1ec2456 into juju:master Apr 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants