Merge lp://qastaging/~cjohnston/ubuntu-ci-services-itself/get-swift-image into lp://qastaging/ubuntu-ci-services-itself

PASSED: Continuous integration, rev:404
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/440/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/440/rebuild

+class GetTicketImage(TestCaseWithGnupg):

I changed this in my branch earlier today and I think it got lost in the tons of changes after that: these tests don't check anything gnupg related, so I think we can use unittest.TestCase here.

> Because the image containers are presently set to be private, even though the
> artifact reference can be seen on the WebUI, it isn't possible for a user to
> download the image via the WebUI since they are missing the proper
> credentials. Because of this, we needed a way for the user to be able to
> download the image with the proper credentials.

Just to throw support behind this, I think it's entirely reasonable that we do not provide access to the images via the webui in phase 0 (and I think we discussed as much on IRC when planning where this feature would get implemented).

Once we're past phase 0, I want to see us put data store object creation behind a new intermediary service that manages the credentials. The CLI should just send the signed GPG content and this intermediary should validate the signature. No cloud credentials should change hands.

This is bug 1288710.

On 03/16/2014 06:33 PM, Chris Johnston wrote:
> +def get_image(args):
> + if args.ticket:
> + ticket = args.ticket
> + else:
> + ticket_status_base = utils.CI_URL + utils.TICKET_STATUS_BASE
> + url = ticket_status_base + '?current_workflow_step={}'.format(
> + ticket_states.TicketWorkflowStep.COMPLETED.value)
> + data = utils.get(url)

> + tickets = []
> + for o in data['objects']:
> + tickets.append(o[u'id'])
> + tickets = sorted(tickets, reverse=True)
> + ticket = tickets.pop(0)

You could do this in one line with:
   ticket = max([x['id'] for x in data['objects']])

If there are no completed tickets, does data['objects'] exist? Maybe to
make this super safe you do:
      ticket = max([x['id'] for x in data.get('objects', [])])

> +
> + data = _get_image_artifact(ticket)
> + ticket_id = str(ticket) + '-image'
> + ds = data_store.create_for_ticket(ticket_id=ticket_id,
> + auth_config=utils.AUTH_CONFIG,
> + public=False)
> + image = data['objects'][0][u'name']
> + local_path = os.path.join(utils.HOME, image)

I'm not a fan of this fixed download location. I'd want to specify via
the CLI and not have this throwing stuff under my $HOME. This might just
be me.

> + if os.path.exists(local_path):
> + print("{} already exists.".format(local_path))
> + else:
> + try:
> + download = ds.get_file(image)
> + fp = open(local_path, 'wb')
> + fp.write(download)

Is there a way we can have swift stream this to the file descriptor
rather than loading the ~200M into memory and writing it all to disk?

The mechanics of this look good and I was able to test that it works (from a ticket I submitted earlier today). I would prefer that it drop the image into my current dir or support an option to specify the location.

If there are significant issues with addressing Andy's comments, then I would strongly consider approving this as it does fill a functionality gap.

L60 I think the preferred syntax is; except Blah as b: rather than; except Blah, b:

L235 I prefer to name my test classes as TextXYZ so they stand out visually to me as tests.

I agree with @Andy regarding the download location and avoiding reading the entire file into memory.

Hrm, after a bit of research, swiftclient.client.get_object doesn't support streaming. So the data-store will need to be adjusted to handle streaming perhaps by using the url and using http streaming. I'd say that's for a later MP.

lets get this merged. and then fix the saving logic separately but soon.

r404 changes how we get the 'latest' completed ticket
r405 changes to require the user to specify where they want to download their image to

PASSED: Continuous integration, rev:405
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/448/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/448/rebuild

looks good. we should just get this merged i think. one note:

246 + self.image_path = os.path.join(
247 + utils.HOME, 'd62d3d9a-ac3d-11e3-847d-fa163eaf5928.img')
248 + self.addCleanup(self._remove_image)

you could use tempfile for that now so the test doesn't mess with $HOME