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

Proposed by Paul Larson on 2015-01-14
Status: Merged
Approved by: Paul Larson on 2015-01-16
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 on 2015-01-16
Celso Providelo (community) 2015-01-14 Approve on 2015-01-15
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.
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
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
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
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 on 2015-01-16

s/snappy/ubuntucore

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.

Paul Larson (pwlars) :
review: Approve

Preview Diff

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

Subscribers

People subscribed via source and target branches