Don't include plugins on 'copy-image' import
The import plugins were designed to interact with the image data or metadata before the image becomes active. It does not make sense to try to run them on 'copy-image' taskflow which is just copying the data between stores on active image. Co-Authored-By: Erno Kuvaja <jokke@usr.fi> Co-Authored-By: Abhishek Kekane <akekane@redhat.com> Closes-Bug: #1885725 Change-Id: I2df30aa26f96a0f210989758868b42bcb9d2d72f
This commit is contained in:
Erno Kuvaja
Dan Smith
parent
63d982588f
commit
b8332d9b6b
@ -435,8 +435,14 @@ def get_flow(**kwargs):
|
||||
|
||||
flow.add(_VerifyStaging(task_id, task_type, task_repo, file_uri))
|
||||
|
||||
for plugin in import_plugins.get_import_plugins(**kwargs):
|
||||
flow.add(plugin)
|
||||
# Note(jokke): The plugins were designed to act on the image data or
|
||||
# metadata during the import process before the image goes active. It
|
||||
# does not make sense to try to execute them during 'copy-image'.
|
||||
if import_method != 'copy-image':
|
||||
for plugin in import_plugins.get_import_plugins(**kwargs):
|
||||
flow.add(plugin)
|
||||
else:
|
||||
LOG.debug("Skipping plugins on 'copy-image' job.")
|
||||
|
||||
for idx, store in enumerate(stores, 1):
|
||||
set_active = (not all_stores_must_succeed) or (idx == len(stores))
|
||||
|
||||
@ -16,9 +16,16 @@
|
||||
|
||||
from unittest import mock
|
||||
|
||||
import glance_store as store
|
||||
from oslo_config import cfg
|
||||
from taskflow.patterns import linear_flow
|
||||
|
||||
import glance.async_
|
||||
from glance.async_.flows import api_image_import
|
||||
import glance.tests.utils as test_utils
|
||||
|
||||
CONF = cfg.CONF
|
||||
|
||||
|
||||
class TestTaskExecutor(test_utils.BaseTestCase):
|
||||
|
||||
@ -47,3 +54,146 @@ class TestTaskExecutor(test_utils.BaseTestCase):
|
||||
|
||||
# assert the call
|
||||
mock_run.assert_called_once_with(task_id, task_type)
|
||||
|
||||
|
||||
class TestImportTaskFlow(test_utils.BaseTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestImportTaskFlow, self).setUp()
|
||||
store.register_opts(CONF)
|
||||
self.config(default_store='file',
|
||||
stores=['file', 'http'],
|
||||
filesystem_store_datadir=self.test_dir,
|
||||
group="glance_store")
|
||||
self.config(enabled_import_methods=[
|
||||
'glance-direct', 'web-download', 'copy-image'])
|
||||
self.config(node_staging_uri='file:///tmp/staging')
|
||||
store.create_stores(CONF)
|
||||
self.base_flow = ['ConfigureStaging', 'ImportToStore',
|
||||
'DeleteFromFS', 'SaveImage',
|
||||
'CompleteTask']
|
||||
self.import_plugins = ['Convert_Image',
|
||||
'Decompress_Image',
|
||||
'InjectMetadataProperties']
|
||||
|
||||
def _get_flow(self, import_req=None):
|
||||
inputs = {
|
||||
'task_id': mock.MagicMock(),
|
||||
'task_type': mock.MagicMock(),
|
||||
'task_repo': mock.MagicMock(),
|
||||
'image_repo': mock.MagicMock(),
|
||||
'image_id': mock.MagicMock(),
|
||||
'import_req': import_req or mock.MagicMock()
|
||||
}
|
||||
flow = api_image_import.get_flow(**inputs)
|
||||
return flow
|
||||
|
||||
def _get_flow_tasks(self, flow):
|
||||
flow_comp = []
|
||||
for c, p in flow.iter_nodes():
|
||||
if isinstance(c, linear_flow.Flow):
|
||||
flow_comp += self._get_flow_tasks(c)
|
||||
else:
|
||||
name = str(c).split('-')
|
||||
if len(name) > 1:
|
||||
flow_comp.append(name[1])
|
||||
return flow_comp
|
||||
|
||||
def test_get_default_flow(self):
|
||||
# This test will ensure that without import plugins
|
||||
# and without internal plugins flow builds with the
|
||||
# base_flow components
|
||||
flow = self._get_flow()
|
||||
|
||||
flow_comp = self._get_flow_tasks(flow)
|
||||
# assert flow has 5 tasks
|
||||
self.assertEqual(5, len(flow_comp))
|
||||
for c in self.base_flow:
|
||||
self.assertIn(c, flow_comp)
|
||||
|
||||
def test_get_flow_web_download_enabled(self):
|
||||
# This test will ensure that without import plugins
|
||||
# and with web-download plugin flow builds with
|
||||
# base_flow components and '_WebDownload'
|
||||
import_req = {
|
||||
'method': {
|
||||
'name': 'web-download',
|
||||
'uri': 'http://cloud.foo/image.qcow2'
|
||||
}
|
||||
}
|
||||
|
||||
flow = self._get_flow(import_req=import_req)
|
||||
|
||||
flow_comp = self._get_flow_tasks(flow)
|
||||
# assert flow has 6 tasks
|
||||
self.assertEqual(6, len(flow_comp))
|
||||
for c in self.base_flow:
|
||||
self.assertIn(c, flow_comp)
|
||||
self.assertIn('WebDownload', flow_comp)
|
||||
|
||||
@mock.patch.object(store, 'get_store_from_store_identifier')
|
||||
def test_get_flow_copy_image_enabled(self, mock_store):
|
||||
# This test will ensure that without import plugins
|
||||
# and with copy-image plugin flow builds with
|
||||
# base_flow components and '_CopyImage'
|
||||
import_req = {
|
||||
'method': {
|
||||
'name': 'copy-image',
|
||||
'stores': ['fake-store']
|
||||
}
|
||||
}
|
||||
|
||||
mock_store.return_value = mock.Mock()
|
||||
flow = self._get_flow(import_req=import_req)
|
||||
|
||||
flow_comp = self._get_flow_tasks(flow)
|
||||
# assert flow has 6 tasks
|
||||
self.assertEqual(6, len(flow_comp))
|
||||
for c in self.base_flow:
|
||||
self.assertIn(c, flow_comp)
|
||||
self.assertIn('CopyImage', flow_comp)
|
||||
|
||||
def test_get_flow_with_all_plugins_enabled(self):
|
||||
# This test will ensure that flow includes import plugins
|
||||
# and base flow
|
||||
self.config(image_import_plugins=['image_conversion',
|
||||
'image_decompression',
|
||||
'inject_image_metadata'],
|
||||
group='image_import_opts')
|
||||
|
||||
flow = self._get_flow()
|
||||
|
||||
flow_comp = self._get_flow_tasks(flow)
|
||||
# assert flow has 8 tasks (base_flow + plugins)
|
||||
self.assertEqual(8, len(flow_comp))
|
||||
for c in self.base_flow:
|
||||
self.assertIn(c, flow_comp)
|
||||
for c in self.import_plugins:
|
||||
self.assertIn(c, flow_comp)
|
||||
|
||||
@mock.patch.object(store, 'get_store_from_store_identifier')
|
||||
def test_get_flow_copy_image_not_includes_import_plugins(
|
||||
self, mock_store):
|
||||
# This test will ensure that flow does not includes import
|
||||
# plugins as import method is copy image
|
||||
self.config(image_import_plugins=['image_conversion',
|
||||
'image_decompression',
|
||||
'inject_image_metadata'],
|
||||
group='image_import_opts')
|
||||
|
||||
mock_store.return_value = mock.Mock()
|
||||
import_req = {
|
||||
'method': {
|
||||
'name': 'copy-image',
|
||||
'stores': ['fake-store']
|
||||
}
|
||||
}
|
||||
|
||||
flow = self._get_flow(import_req=import_req)
|
||||
|
||||
flow_comp = self._get_flow_tasks(flow)
|
||||
# assert flow has 6 tasks
|
||||
self.assertEqual(6, len(flow_comp))
|
||||
for c in self.base_flow:
|
||||
self.assertIn(c, flow_comp)
|
||||
self.assertIn('CopyImage', flow_comp)
|
||||
|
||||
@ -0,0 +1,6 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
* Bug 1885725_: 'copy-image' import job should not run additional plugins
|
||||
|
||||
.. _1885725: https://code.launchpad.net/bugs/1885725
|
||||
Loading…
Reference in New Issue
Block a user