Given the way image building is likely to work, it is far simpler to
just use HHMM as a sequence number, instead of the XX in the current
implementation.
Also added ARCH to the filename.
The current name is in the format:
yymmdd_hhmm_release-flavor-arch
Added testing/.cache to .gitignore
unittests work
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
yymmdd_hhmm_release-flavor-arch
According to the code, the format is yymmdd_hhmm-release-flavor-arch (dash/underscore swapped). If that's intended, it's fine.
| libtaskotron/ext/disposable/vm.py | ||
|---|---|---|
| 202–203 | Without the f it looks like it requires a number, not an arbitrary string. I would add it back to make this clear, that does not influence how we use that in our deployment. (It would be also nice to add value types to docstrings, but in this case, it would still be a bit confusing). | |
| testing/test_vm.py | ||
| 15–19 | What if we did import testcloud.exceptions as tce and then used tce.TestcloudImageError in code? Would that eliminate the errors? | |
According to the code, the format is yymmdd_hhmm-release-flavor-arch (dash/underscore swapped). If that's intended, it's fine.
Good catch.
I'll also probably add the 'fedora' string there somewhere, although I'm not sure we'll be building images of something-else-than-fedora any time soon, but we can definitely make it future-proof-y.
| Path | Packages | |||
|---|---|---|---|---|
| M | .gitignore (3 lines) | |||
| M | libtaskotron/ext/disposable/vm.py (31 lines) | |||
| M | testing/test_vm.py (39 lines) |
| Commit | Tree | Parents | Author | Summary | Date |
|---|---|---|---|---|---|
| 419b4df747e1 | d3c95c9e6d97 | 86e57406bd5b | Josef Skladanka | Changed the filename structure of disposable images (Show More…) | Dec 16 2015, 10:28 AM |
| 86e57406bd5b | 2a46b5532768 | 226a55a5f5f2 | Josef Skladanka | Added testing/.cache to .gitignore | Dec 16 2015, 10:27 AM |
| Show All 11 Lines | |||||
| 12 | 12 | | |||
| 13 | # vim swap files | 13 | # vim swap files | ||
| 14 | .*.swp | 14 | .*.swp | ||
| 15 | 15 | | |||
| 16 | # test coverage | 16 | # test coverage | ||
| 17 | /.coverage | 17 | /.coverage | ||
| 18 | /htmlcov/ | 18 | /htmlcov/ | ||
| 19 | 19 | | |||
| 20 | # pytest cache | ||||
| 21 | /testing/.cache | ||||
| 22 | | ||||
| 20 | # Sphinx docs | 23 | # Sphinx docs | ||
| 21 | /docs/build/ | 24 | /docs/build/ | ||
| 22 | /docs/source/directives/ | 25 | /docs/source/directives/ | ||
| 23 | 26 | | |||
| 24 | # virtualenv | 27 | # virtualenv | ||
| 25 | /env_taskotron/ | 28 | /env_taskotron/ | ||
| 26 | 29 | | |||
| 27 | # local configuration files | 30 | # local configuration files | ||
| 28 | /conf/taskotron.yaml | 31 | /conf/taskotron.yaml | ||
| 29 | /conf/yumrepoinfo.conf | 32 | /conf/yumrepoinfo.conf | ||
| Show First 20 Lines • Show All 190 Lines • ▼ Show 20 Line(s) | 186 | def get_all_filenames(cls, imagesdir=None): | |||
|---|---|---|---|---|---|
| 191 | """ | 191 | """ | ||
| 192 | 192 | | |||
| 193 | if not imagesdir: | 193 | if not imagesdir: | ||
| 194 | imagesdir = config.get_config().imagesdir | 194 | imagesdir = config.get_config().imagesdir | ||
| 195 | 195 | | |||
| 196 | return os.listdir(imagesdir) | 196 | return os.listdir(imagesdir) | ||
| 197 | 197 | | |||
| 198 | @classmethod | 198 | @classmethod | ||
| 199 | def get_latest(cls, release, template, imagesdir=None): | 199 | def get_latest(cls, release, flavor, arch='x86_64', imagesdir=None): | ||
| 200 | """Search for the most recent image available on the system. | 200 | """Search for the most recent image available on the system. | ||
| 201 | 201 | | |||
| 202 | :param release: release of the image (e.g. ``f23``) | 202 | :param release: release of the image (e.g. ``23``) | ||
| 203 | :param template: template of the image (e.g. ``minimal``) | 203 | :param flavor: flavor of the image (e.g. ``minimal``) | ||
| 204 | :param imagesdir: absolute path to directory containing the images, path from config is | 204 | :param imagesdir: absolute path to directory containing the images, path from config is | ||
| 205 | used if None | 205 | used if None | ||
| 206 | :param arch: architecture of the image | ||||
| 206 | 207 | | |||
| 207 | :return: testcloud.image.Image instance | 208 | :return: testcloud.image.Image instance | ||
| 208 | :raises TaskotronImageError: if no such image for given release and template was found | 209 | :raises TaskotronImageError: if no such image for given release and flavor was found | ||
| 209 | """ | 210 | """ | ||
| 210 | 211 | | |||
| 211 | latest_metadata = cls.get_latest_metadata(release, template, imagesdir) | 212 | latest_metadata = cls.get_latest_metadata(release, flavor, arch, imagesdir) | ||
| 212 | 213 | | |||
| 213 | if not latest_metadata: | 214 | if not latest_metadata: | ||
| 214 | raise exc.TaskotronImageNotFoundError('No such image for release %s and template %s.' % | 215 | raise exc.TaskotronImageNotFoundError('No such image for release %s and flavor %s.' % | ||
| 215 | (release, template)) | 216 | (release, flavor)) | ||
| 216 | else: | 217 | else: | ||
| 217 | return image.Image("file://" + os.path.join(imagesdir, latest_metadata['filename'])) | 218 | return image.Image("file://" + os.path.join(imagesdir, latest_metadata['filename'])) | ||
| 218 | 219 | | |||
| 219 | @classmethod | 220 | @classmethod | ||
| 220 | def get_latest_metadata(cls, release, template, imagesdir=None): | 221 | def get_latest_metadata(cls, release, flavor, arch='x86_64', imagesdir=None): | ||
| 221 | """Search for the most recent image available on the system. | 222 | """Search for the most recent image available on the system. | ||
| 222 | 223 | | |||
| 223 | :param release: release of the image (e.g. ``f23``) | 224 | :param release: release of the image (e.g. ``23``) | ||
| 224 | :param template: template of the image (e.g. ``minimal``) | 225 | :param flavor: flavor of the image (e.g. ``minimal``) | ||
| 225 | :param imagesdir: absolute path to directory containing the images, path from config is | 226 | :param imagesdir: absolute path to directory containing the images, path from config is | ||
| 226 | used if None | 227 | used if None | ||
| 228 | :param arch: arch of the image (e.g. 'x86_64') | ||||
| 227 | 229 | | |||
| 228 | :return: metadata of the most recent image | 230 | :return: metadata of the most recent image | ||
| 229 | :rtype: dict {'date': str, 'version': str, 'release': str, 'filename': str} | 231 | :rtype: dict {'date': str, 'version': str, 'release': str, 'arch': str, 'filename': str} | ||
| 230 | """ | 232 | """ | ||
| 231 | 233 | | |||
| 232 | pattern = re.compile(r'^([0-9]{8})_([0-9]{2})-(.*?)-(.*?)\.qcow2$') | 234 | pattern = re.compile(r'^([0-9]{8})_([0-9]{4})-fedora-(.*?)-(.*?)-(.*?)\.qcow2$') | ||
| 233 | 235 | | |||
| 234 | images = [] | 236 | images = [] | ||
| 235 | for filename in cls.get_all_filenames(imagesdir): | 237 | for filename in cls.get_all_filenames(imagesdir): | ||
| 236 | m = pattern.match(filename) | 238 | m = pattern.match(filename) | ||
| 237 | if m: | 239 | if m: | ||
| 238 | images.append({'date': m.group(1), | 240 | images.append({'date': m.group(1), | ||
| 239 | 'version': m.group(2), | 241 | 'version': m.group(2), | ||
| 240 | 'release': m.group(3), | 242 | 'release': m.group(3), | ||
| 241 | 'template': m.group(4), | 243 | 'flavor': m.group(4), | ||
| 244 | 'arch': m.group(5), | ||||
| 242 | 'filename': filename}) | 245 | 'filename': filename}) | ||
| 243 | 246 | | |||
| 244 | filtered = [i for i in images if i['release'] == release and i['template'] == template] | 247 | filtered = [i for i in images if i['release'] == release and i['flavor'] == flavor and i['arch'] == arch] | ||
| 245 | if not filtered: | 248 | if not filtered: | ||
| 246 | return None | 249 | return None | ||
| 247 | else: | 250 | else: | ||
| 248 | return sorted(filtered, key=lambda i: int(i['date']+i['version']))[-1] | 251 | return sorted(filtered, key=lambda i: int(i['date']+i['version']))[-1] | ||
| 1 | # -*- coding: utf-8 -*- | 1 | # -*- coding: utf-8 -*- | ||
|---|---|---|---|---|---|
| 2 | # Copyright 2009-2015, Red Hat, Inc. | 2 | # Copyright 2009-2015, Red Hat, Inc. | ||
| 3 | # License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> | 3 | # License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> | ||
| 4 | # See the LICENSE file for more details on Licensing | 4 | # See the LICENSE file for more details on Licensing | ||
| 5 | 5 | | |||
| 6 | import pytest | 6 | import pytest | ||
| 7 | from mock import Mock, MagicMock | 7 | from mock import Mock, MagicMock | ||
| 8 | from dingus import Dingus | 8 | from dingus import Dingus | ||
| 9 | 9 | | |||
| 10 | from libtaskotron.ext.disposable import vm | 10 | from libtaskotron.ext.disposable import vm | ||
| 11 | from libtaskotron import exceptions as exc | 11 | from libtaskotron import exceptions as exc | ||
| 12 | 12 | | |||
| 13 | from testcloud import image, instance | 13 | from testcloud import image, instance | ||
| 14 | from testcloud.exceptions import TestcloudImageError, TestcloudInstanceError | 14 | import testcloud.exceptions as tce | ||
| 15 | 15 | | |||
| 16 | 16 | | |||
| 17 | class TestvmImagePrepare(object): | 17 | class TestvmImagePrepare(object): | ||
| 18 | '''Test libtaskotron.vm._prepare_image''' | 18 | '''Test libtaskotron.vm._prepare_image''' | ||
| 19 | 19 | | |||
| 20 | def setup_method(self, method): | 20 | def setup_method(self, method): | ||
| 21 | self.ref_uuid = '1234-5678-9012' | 21 | self.ref_uuid = '1234-5678-9012' | ||
| 22 | 22 | | |||
| 23 | def should_raise_on_failure(self, monkeypatch): | 23 | def should_raise_on_failure(self, monkeypatch): | ||
| 24 | stub_image = Mock(side_effect=TestcloudImageError()) | 24 | stub_image = Mock(side_effect=tce.TestcloudImageError()) | ||
| 25 | 25 | | |||
| 26 | monkeypatch.setattr(image, 'Image', stub_image) | 26 | monkeypatch.setattr(image, 'Image', stub_image) | ||
| 27 | 27 | | |||
| 28 | test_vm = vm.TestCloudMachine(self.ref_uuid) | 28 | test_vm = vm.TestCloudMachine(self.ref_uuid) | ||
| 29 | 29 | | |||
| 30 | with pytest.raises(exc.TaskotronRemoteError): | 30 | with pytest.raises(exc.TaskotronRemoteError): | ||
| 31 | test_vm._prepare_image() | 31 | test_vm._prepare_image() | ||
| 32 | 32 | | |||
| Show All 37 Lines | 69 | def should_bequiet_on_success(self): | |||
| 70 | stub_instance = Mock() | 70 | stub_instance = Mock() | ||
| 71 | 71 | | |||
| 72 | test_vm = vm.TestCloudMachine(self.ref_uuid) | 72 | test_vm = vm.TestCloudMachine(self.ref_uuid) | ||
| 73 | 73 | | |||
| 74 | test_vm._stop_instance(stub_instance) | 74 | test_vm._stop_instance(stub_instance) | ||
| 75 | 75 | | |||
| 76 | def should_raise_on_failure(self): | 76 | def should_raise_on_failure(self): | ||
| 77 | stub_instance = Mock() | 77 | stub_instance = Mock() | ||
| 78 | stub_instance.stop.side_effect = TestcloudInstanceError() | 78 | stub_instance.stop.side_effect = tce.TestcloudInstanceError() | ||
| 79 | 79 | | |||
| 80 | test_vm = vm.TestCloudMachine(self.ref_uuid) | 80 | test_vm = vm.TestCloudMachine(self.ref_uuid) | ||
| 81 | 81 | | |||
| 82 | with pytest.raises(exc.TaskotronRemoteError): | 82 | with pytest.raises(exc.TaskotronRemoteError): | ||
| 83 | test_vm._stop_instance(stub_instance) | 83 | test_vm._stop_instance(stub_instance) | ||
| 84 | 84 | | |||
| 85 | 85 | | |||
| 86 | class TestvmDestroy(object): | 86 | class TestvmDestroy(object): | ||
| 87 | '''Tests for the libtaskotron.vm.destroy() method''' | 87 | '''Tests for the libtaskotron.vm.destroy() method''' | ||
| 88 | 88 | | |||
| 89 | def setup_method(self, method): | 89 | def setup_method(self, method): | ||
| 90 | self.ref_uuid = '1234-5678-9012' | 90 | self.ref_uuid = '1234-5678-9012' | ||
| 91 | 91 | | |||
| 92 | def should_bequiet_on_success(self): | 92 | def should_bequiet_on_success(self): | ||
| 93 | stub_instance = Mock() | 93 | stub_instance = Mock() | ||
| 94 | 94 | | |||
| 95 | test_vm = vm.TestCloudMachine(self.ref_uuid) | 95 | test_vm = vm.TestCloudMachine(self.ref_uuid) | ||
| 96 | 96 | | |||
| 97 | test_vm._destroy_instance(stub_instance) | 97 | test_vm._destroy_instance(stub_instance) | ||
| 98 | 98 | | |||
| 99 | def should_raise_on_failure(self): | 99 | def should_raise_on_failure(self): | ||
| 100 | stub_instance = Mock() | 100 | stub_instance = Mock() | ||
| 101 | stub_instance.destroy.side_effect = TestcloudInstanceError() | 101 | stub_instance.destroy.side_effect = tce.TestcloudInstanceError() | ||
| 102 | 102 | | |||
| 103 | test_vm = vm.TestCloudMachine(self.ref_uuid) | 103 | test_vm = vm.TestCloudMachine(self.ref_uuid) | ||
| 104 | 104 | | |||
| 105 | with pytest.raises(exc.TaskotronRemoteError): | 105 | with pytest.raises(exc.TaskotronRemoteError): | ||
| 106 | test_vm._destroy_instance(stub_instance) | 106 | test_vm._destroy_instance(stub_instance) | ||
| 107 | 107 | | |||
| 108 | 108 | | |||
| 109 | class TestvmExistingInstances(object): | 109 | class TestvmExistingInstances(object): | ||
| ▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Line(s) | 157 | with pytest.raises(exc.TaskotronRemoteError): | |||
| 158 | test_vm._check_existing_instance(False) | 158 | test_vm._check_existing_instance(False) | ||
| 159 | 159 | | |||
| 160 | 160 | | |||
| 161 | @pytest.mark.usefixtures("setup") | 161 | @pytest.mark.usefixtures("setup") | ||
| 162 | class TestImageLatest: | 162 | class TestImageLatest: | ||
| 163 | 163 | | |||
| 164 | @pytest.fixture | 164 | @pytest.fixture | ||
| 165 | def setup(self, monkeypatch): | 165 | def setup(self, monkeypatch): | ||
| 166 | self.images = ['20151008_04-f23-cloud.qcow2', | 166 | self.images = ['20151008_0427-fedora-23-cloud-x86_64.qcow2', | ||
| 167 | 'ubuntu-fabulous-fedora.qcow2', | 167 | 'ubuntu-fabulous-fedora.qcow2', | ||
| 168 | '20151008_02-f23-minimal.qcow2', | 168 | '20151008_0223-fedora-23-minimal-i386.qcow2', | ||
| 169 | '20151105_01-server.qcow2', | 169 | '20151105_0100-server-x86_64.qcow2', | ||
| 170 | 'el_capitan.iso', | 170 | 'el_capitan.iso', | ||
| 171 | '20151008_01-server.qcow2', | 171 | '20151008_0100-server.qcow2', | ||
| 172 | '20151007_05-f23-minimal.qcow2', | 172 | '20151007_0516-fedora-23-minimal-x86_64.qcow2', | ||
| 173 | '20151007_0516-fedora-rawhide-minimal-x86_64.qcow2', | ||||
| 173 | 'windows10.raw', | 174 | 'windows10.raw', | ||
| 174 | '20151008_03-f23-minimal.qcow2'] | 175 | '20151008_0315-fedora-23-minimal-i386.qcow2', | ||
| 176 | '20151008_0315-fedora-23-minimal-x86_64.qcow2'] | ||||
| 175 | self.stub_list_images = Dingus() | 177 | self.stub_list_images = Dingus() | ||
| 176 | monkeypatch.setattr(vm.Image, 'get_all_filenames', self.stub_list_images) | 178 | monkeypatch.setattr(vm.Image, 'get_all_filenames', self.stub_list_images) | ||
| 177 | 179 | | |||
| 178 | def test_dir_populated(self): | 180 | def test_dir_populated(self): | ||
| 179 | self.stub_list_images.return_value = self.images | 181 | self.stub_list_images.return_value = self.images | ||
| 180 | 182 | | |||
| 181 | assert vm.Image.get_latest_metadata('f23', 'minimal', '')['filename'] ==\ | 183 | assert vm.Image.get_latest_metadata('23', 'minimal', 'i386', '')['filename'] ==\ | ||
| 182 | '20151008_03-f23-minimal.qcow2' | 184 | '20151008_0315-fedora-23-minimal-i386.qcow2' | ||
| 185 | assert vm.Image.get_latest_metadata('23', 'minimal', 'x86_64', '')['filename'] ==\ | ||||
| 186 | '20151008_0315-fedora-23-minimal-x86_64.qcow2' | ||||
| 187 | | ||||
| 183 | 188 | | |||
| 184 | def test_dir_populated_single(self): | 189 | def test_dir_populated_single(self): | ||
| 185 | self.stub_list_images.return_value = self.images | 190 | self.stub_list_images.return_value = self.images | ||
| 186 | 191 | | |||
| 187 | assert vm.Image.get_latest_metadata('f23', 'cloud', '')['filename'] ==\ | 192 | assert vm.Image.get_latest_metadata('23', 'cloud', 'x86_64', '')['filename'] ==\ | ||
| 188 | '20151008_04-f23-cloud.qcow2' | 193 | '20151008_0427-fedora-23-cloud-x86_64.qcow2' | ||
| 194 | assert vm.Image.get_latest_metadata('rawhide', 'minimal', 'x86_64', '')['filename'] ==\ | ||||
| 195 | '20151007_0516-fedora-rawhide-minimal-x86_64.qcow2' | ||||
| 189 | 196 | | |||
| 190 | def test_dir_empty(self): | 197 | def test_dir_empty(self): | ||
| 191 | self.stub_list_images.return_value = [] | 198 | self.stub_list_images.return_value = [] | ||
| 192 | 199 | | |||
| 193 | assert vm.Image.get_latest_metadata('f23', 'minimal', '') is None | 200 | assert vm.Image.get_latest_metadata('23', 'minimal', '') is None | ||
| 194 | 201 | | |||
| 195 | def test_no_such_image(self): | 202 | def test_no_such_image(self): | ||
| 196 | self.stub_list_images.return_value = self.images | 203 | self.stub_list_images.return_value = self.images | ||
| 197 | 204 | | |||
| 198 | assert vm.Image.get_latest_metadata('f69', 'ultimate', '') is None | 205 | assert vm.Image.get_latest_metadata('69', 'ultimate', '') is None | ||
Without the f it looks like it requires a number, not an arbitrary string. I would add it back to make this clear, that does not influence how we use that in our deployment. (It would be also nice to add value types to docstrings, but in this case, it would still be a bit confusing).