Merge lp:~pwlars/uci-engine/snappy-imageimport into lp:uci-engine

Proposed by Paul Larson
Status: Merged
Approved by: Paul Larson
Approved revision: 923
Merged at revision: 922
Proposed branch: lp:~pwlars/uci-engine/snappy-imageimport
Merge into: lp:uci-engine
Diff against target: 139 lines (+127/-0)
2 files modified
ubuntucore-importimage/glanceimage.py (+86/-0)
ubuntucore-importimage/tests/test_glanceimage.py (+41/-0)
To merge this branch: bzr merge lp:~pwlars/uci-engine/snappy-imageimport
Reviewer Review Type Date Requested Status
Paul Larson Approve
Celso Providelo (community) Approve
Review via email: mp+246484@code.launchpad.net

Commit message

Add a cli/api to download a specified image and put it in glance.

Description of the change

This is just the simple tool to take an image, download it, and import it into glance.

To post a comment you must log in.
Revision history for this message
Celso Providelo (cprov) wrote :

Paul,

Thanks for working on this, the CLI does what is expected.

I am unsure about its location as an separated module. At least, we should s/snappy/ubuntucore/, but I don't like much the idea of creating a new module/directory for each CLI we planned, it is unnecessarily confusing and hard to find.

It needs ci_utils, so if we are going to fork/subprocess to it, we need `run-python` and the scripts could live in an agnostic common path, let's say bin/ or ubuntucore/bin/, because for now we deploy the whole uci-engine tree on nodes.

If we intend to import and use publish_image() inside workers, it should be a module (as the other ci-airline modules) and all CLIs could live in a module, let's say 'ubuntucore', which will eventually provide worker implementations for the services we need.

Any thoughts ?
[]

review: Needs Information
Revision history for this message
Paul Larson (pwlars) wrote :

> Paul,
>
> Thanks for working on this, the CLI does what is expected.
>
> I am unsure about its location as an separated module. At least, we should
> s/snappy/ubuntucore/, but I don't like much the idea of creating a new
> module/directory for each CLI we planned, it is unnecessarily confusing and
> hard to find.
Yes, I couldn't think of a better place to put it, but it's not just going to be the CLI. There's another task to create a worker that uses it, so I thought it might be more appropriate to give it it's own directory as we've done with every other service so far. Putting all of these related to snappy under a single directory would be fine I think too, but don't you think we should still logically separate them somehow since they are separate microservices?

Why should it be ubuntucore instead of snappy? I'm not 100% positive of this, but it seems that "snappy" is the new term for what we are testing, thought 99% of it was once called ubuntu core.

> It needs ci_utils, so if we are going to fork/subprocess to it, we need `run-
> python` and the scripts could live in an agnostic common path, let's say bin/
> or ubuntucore/bin/, because for now we deploy the whole uci-engine tree on
> nodes.
I figured I'd let the worker just import it and sort all that out. I'm open to suggestions, but I don't think we have any intention of running this from the command line in production. For testing purposes though, it's very easy to setup a venv, install ci-utils, and run this.

> If we intend to import and use publish_image() inside workers, it should be a
> module (as the other ci-airline modules) and all CLIs could live in a module,
> let's say 'ubuntucore', which will eventually provide worker implementations
> for the services we need.
Why wouldn't we put the cli/modules with the individual service rather than all together? Are you saying they should all live together, or just all under one root like I was talking about above?

review: Needs Information
Revision history for this message
Celso Providelo (cprov) wrote :

Paul,

I remember we agreeing that 'ubuntucore' was the right term to use for this story, since it's wider (and less fun) than 'snappy'. But my memory might be failing me.

Considering the imminent switch, I am retracting my concerns about having "image-importer" directory for hosting the CLI and TBD image_importer module for a rabbit worker. It is a standalone service and it matches the tree organisation.

review: Approve
Revision history for this message
Evan (ev) wrote :

On 15 January 2015 at 18:26, Celso Providelo
<email address hidden> wrote:
> I remember we agreeing that 'ubuntucore' was the right term to use for this story, since it's wider (and less fun) than 'snappy'. But my memory might be failing me.

The images we're testing are Ubuntu Core. The tests check that snappy
still functions.

923. By Paul Larson

s/snappy/ubuntucore

Revision history for this message
Paul Larson (pwlars) wrote :

Ok, in that case, I've changed the name to ubuntucore right now to take out the reference to snappy. It's easy enough to move around later to if we want to discuss changes to the layout. Going to go ahead and merge with that so that it's here for after we get done with the new sprint priorities.

Revision history for this message
Paul Larson (pwlars) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== added directory 'ubuntucore-importimage'
=== added file 'ubuntucore-importimage/glanceimage.py'
--- ubuntucore-importimage/glanceimage.py 1970-01-01 00:00:00 +0000
+++ ubuntucore-importimage/glanceimage.py 2015-01-16 16:51:05 +0000
@@ -0,0 +1,86 @@
1#!/usr/bin/env python
2# Ubuntu CI Engine
3# Copyright 2015 Canonical Ltd.
4
5# This program is free software: you can redistribute it and/or modify it
6# under the terms of the GNU Affero General Public License version 3, as
7# published by the Free Software Foundation.
8
9# This program is distributed in the hope that it will be useful, but
10# WITHOUT ANY WARRANTY; without even the implied warranties of
11# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
12# PURPOSE. See the GNU Affero General Public License for more details.
13
14# You should have received a copy of the GNU Affero General Public License
15# along with this program. If not, see <http://www.gnu.org/licenses/>.
16
17import argparse
18import logging
19import os
20import requests
21import sys
22import time
23import urlparse
24
25import ci_utils
26from ci_utils import unit_config
27from ci_utils.image_store import ImageStore
28
29
30def _download(url, path, retry=3):
31 ''' Download from a url, retry up to 3 times by default '''
32 urlpath = urlparse.urlsplit(url).path
33 filename = os.path.basename(urlpath)
34 filename = os.path.join(path, filename)
35 try:
36 response = requests.get(url, stream=True)
37 response.raise_for_status()
38 with open(filename, 'wb') as f:
39 for chunk in response.iter_content(chunk_size=64 * 1024):
40 # Ignore keep-alives
41 if not chunk:
42 continue
43 f.write(chunk)
44 f.flush()
45 except (requests.ConnectionError, requests.HTTPError) as e:
46 if retry == 1:
47 raise(e)
48 logging.warn('Download failed, retrying in 30 sec')
49 time.sleep(30)
50 return _download(url, path, retry - 1)
51 return filename
52
53
54def publish_image(image_url, glance_filename):
55 glance = ImageStore(unit_config.get_auth_config())
56 image_list = [x.name for x in glance.glance.images.list()]
57 if glance_filename in image_list:
58 logging.info(
59 '{} is already in glance, exiting'.format(glance_filename))
60 sys.exit(0)
61 with ci_utils.TmpDir() as tmpdir:
62 logging.info('Downloading {}'.format(image_url))
63 local_copy = _download(image_url, tmpdir)
64 glance.image_create(glance_filename, local_copy)
65
66
67def _get_args():
68 parser = argparse.ArgumentParser(
69 description='Download an image and push it to glance')
70 parser.add_argument('-u', '--url', required=True,
71 help='URL of the image to download')
72 parser.add_argument('-n', '--name', required=True,
73 help='Name to store the image as in glance')
74 args = parser.parse_args()
75 return args
76
77
78def main():
79 logging.basicConfig(level=logging.INFO,
80 format="%(asctime)s %(levelname)s %(message)s")
81 args = _get_args()
82 publish_image(args.url, args.name)
83
84
85if __name__ == '__main__':
86 main()
087
=== added directory 'ubuntucore-importimage/tests'
=== added file 'ubuntucore-importimage/tests/__init__.py'
=== added file 'ubuntucore-importimage/tests/test_glanceimage.py'
--- ubuntucore-importimage/tests/test_glanceimage.py 1970-01-01 00:00:00 +0000
+++ ubuntucore-importimage/tests/test_glanceimage.py 2015-01-16 16:51:05 +0000
@@ -0,0 +1,41 @@
1import mock
2import unittest
3
4import glanceimage
5
6
7class TestDownload(unittest.TestCase):
8 def setUp(self):
9 super(TestDownload, self).setUp()
10 self.fake_url = 'http://somewhere.fake/snappy/disk1.img'
11
12 @mock.patch('glanceimage.ImageStore')
13 @mock.patch('glanceimage.requests')
14 @mock.patch('__builtin__.open')
15 def test_download(self, mock_open, mock_requests, mock_ImageStore):
16 '''Test that downloading a fake image url works properly'''
17 glance_filename = glanceimage._download(self.fake_url, 'imagename')
18 self.assertEqual('imagename/disk1.img', glance_filename)
19
20 @mock.patch('time.sleep')
21 @mock.patch('glanceimage.ImageStore')
22 @mock.patch('__builtin__.open')
23 def test_download_failed(self, mock_open, mock_ImageStore, mock_sleep):
24 '''Test that a faked url that isn't mocked returns ConnectionError'''
25 with self.assertRaises(glanceimage.requests.ConnectionError):
26 glanceimage._download(self.fake_url, 'imagename')
27
28class TestPublishImage(unittest.TestCase):
29 @mock.patch('glanceclient.v1.images.ImageManager.list')
30 @mock.patch('glanceimage._download')
31 def test_image_already_exists(self, mock_download, mock_list):
32 '''Make sure we don't try to get the image if it already exists'''
33 from collections import namedtuple
34 image = namedtuple('image', ['name'])
35 imagelist = image('fakename')
36 mock_list.return_value = [imagelist]
37
38 with self.assertRaises(SystemExit) as e:
39 glanceimage.publish_image('fakeurl', 'fakename')
40 self.assertEqual(e.exception.code, 0)
41 self.assertFalse(mock_download.called)

Subscribers

People subscribed via source and target branches