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
(cherry picked from commit b8332d9b6b
)
This commit is contained in:
parent
72510f9ce9
commit
2df2792fee
@ -435,8 +435,14 @@ def get_flow(**kwargs):
|
|||||||
|
|
||||||
flow.add(_VerifyStaging(task_id, task_type, task_repo, file_uri))
|
flow.add(_VerifyStaging(task_id, task_type, task_repo, file_uri))
|
||||||
|
|
||||||
for plugin in import_plugins.get_import_plugins(**kwargs):
|
# Note(jokke): The plugins were designed to act on the image data or
|
||||||
flow.add(plugin)
|
# 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):
|
for idx, store in enumerate(stores, 1):
|
||||||
set_active = (not all_stores_must_succeed) or (idx == len(stores))
|
set_active = (not all_stores_must_succeed) or (idx == len(stores))
|
||||||
|
@ -16,9 +16,16 @@
|
|||||||
|
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
|
import glance_store as store
|
||||||
|
from oslo_config import cfg
|
||||||
|
from taskflow.patterns import linear_flow
|
||||||
|
|
||||||
import glance.async_
|
import glance.async_
|
||||||
|
from glance.async_.flows import api_image_import
|
||||||
import glance.tests.utils as test_utils
|
import glance.tests.utils as test_utils
|
||||||
|
|
||||||
|
CONF = cfg.CONF
|
||||||
|
|
||||||
|
|
||||||
class TestTaskExecutor(test_utils.BaseTestCase):
|
class TestTaskExecutor(test_utils.BaseTestCase):
|
||||||
|
|
||||||
@ -47,3 +54,146 @@ class TestTaskExecutor(test_utils.BaseTestCase):
|
|||||||
|
|
||||||
# assert the call
|
# assert the call
|
||||||
mock_run.assert_called_once_with(task_id, task_type)
|
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