Refactor image upload command

In preparation for supporting deploys on a glance-less undercloud,
this change refactors the upload command to move all glance
interacting logic into a glance adapter class. This will allow the
adapter abstraction to be used for an 'upload' adapter which only
copies files on the local disk.

This change also does the following:
- moves repeated update-or-upload logic to an adapter method, reducing
  the complexity of take_action
- cleans up the mock usage to consistently use the
  patch decorator - the current approach may be causing unit test
  failures in some environments
- removes the GlanceV1 implementation, since the V1 API was removed in
  Newton(!) this support is just a maintenance burden

Change-Id: Ie28dabaf07ed36adb3e8f07ca8c141f93a9d79cc
Blueprint: nova-less-deploy
Depends-On: https://review.opendev.org/711271
(cherry picked from commit c88da0a632)

lower-constraints.txt

@@ -52,7 +52,7 @@ MarkupSafe==1.0
 mccabe==0.2.1
 mistral-lib==0.3.0
 mistral==6.0.0
-mock==2.0.0
+mock==3.0.0
 monotonic==0.6
 msgpack-python==0.4.0
 munch==2.1.0

test-requirements.txt

@@ -6,7 +6,7 @@ hacking>=1.1.0,<1.2.0 # Apache-2.0
 coverage!=4.4,>=4.0 # Apache-2.0
 docutils>=0.11 # OSI-Approved Open Source, Public Domain
 fixtures>=3.0.0 # Apache-2.0/BSD
-mock>=2.0.0 # BSD
+mock>=3.0.0 # BSD
 stestr>=2.0.0 # Apache-2.0
 testtools>=2.2.0 # MIT
 requests-mock>=1.2.0 # Apache-2.0

tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py

@@ -18,6 +18,7 @@ import os
 
 from osc_lib import exceptions
 import tripleo_common.arch
+from tripleoclient.tests import base
 from tripleoclient.tests.fakes import FakeHandle
 from tripleoclient.tests.v1.test_plugin import TestPluginV1
 from tripleoclient.v1 import overcloud_image
@@ -114,7 +115,121 @@ class TestOvercloudImageBuild(TestPluginV1):
             images=None)
 
 
+class TestBaseClientAdapter(base.TestCommand):
+
+    def setUp(self):
+        super(TestBaseClientAdapter, self).setUp()
+        self.adapter = overcloud_image.BaseClientAdapter('/foo')
+
+    @mock.patch('os.path.isfile', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._files_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._copy_file', autospec=True)
+    def test_file_try_update_need_update(self,
+                                         mock_copy_file,
+                                         mock_files_changed,
+                                         mock_isfile):
+        mock_isfile.return_value = True
+        mock_files_changed.return_value = True
+
+        self.adapter.file_create_or_update('discimg', 'discimgprod')
+        mock_copy_file.assert_not_called()
+
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._files_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._copy_file', autospec=True)
+    def test_file_try_update_do_update(self,
+                                       mock_copy_file,
+                                       mock_files_changed):
+        mock_files_changed.return_value = True
+
+        self.update_existing = True
+        self.adapter.file_create_or_update('discimg', 'discimgprod')
+        mock_copy_file.assert_called_once_with(
+            self.adapter, 'discimg', 'discimgprod')
+
+
+class TestGlanceClientAdapter(TestPluginV1):
+
+    def setUp(self):
+        super(TestGlanceClientAdapter, self).setUp()
+        self.app.client_manager.image = mock.Mock()
+        self.app.client_manager.image.version = 2.0
+        self._arch = tripleo_common.arch.kernel_arch()
+        self.app.client_manager.image.images.create.return_value = (
+            mock.Mock(id=10, name='imgname',
+                      properties={'kernel_id': 10, 'ramdisk_id': 10,
+                                  'hw_architecture': self._arch},
+                      created_at='2015-07-31T14:37:22.000000'))
+        self.updated = []
+        self.adapter = overcloud_image.GlanceClientAdapter(
+            client=self.app.client_manager.image,
+            image_path='/foo',
+            updated=self.updated
+        )
+
+    @mock.patch('osc_lib.utils.find_resource')
+    def test_get_image_exists(self, mock_find_resource):
+        image_mock = mock.Mock(name='imagename')
+        mock_find_resource.return_value = image_mock
+        self.assertEqual(self.adapter._get_image('imagename'), image_mock)
+
+    @mock.patch('osc_lib.utils.find_resource')
+    def test_get_image_none(self, mock_find_resource):
+        mock_find_resource.side_effect = exceptions.CommandError('')
+        self.assertIsNone(self.adapter._get_image('noimagename'))
+
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    def test_image_try_update_no_exist(self, mock_get_image):
+        mock_get_image.return_value = None
+        self.assertFalse(self.adapter._image_try_update(
+            'name', 'fn'))
+        self.assertEqual([], self.updated)
+
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._image_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    def test_image_try_update_need_update(self,
+                                          mock_get_image,
+                                          mock_image_changed):
+        image_mock = mock.Mock(name='imagename')
+        mock_get_image.return_value = image_mock
+        mock_image_changed.return_value = True
+        self.assertEqual(
+            self.adapter._image_try_update('name', 'fn'),
+            image_mock
+        )
+        self.assertEqual([], self.updated)
+        self.app.client_manager.image.images.update.assert_not_called()
+
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._image_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    def test_image_try_update_do_update(self,
+                                        mock_get_image,
+                                        mock_image_changed):
+        image_mock = mock.Mock(name='imagename',
+                               created_at='2015-07-31T14:37:22.000000')
+        update_mock = mock.Mock(return_value=image_mock)
+        self.app.client_manager.image.images.update = update_mock
+        mock_get_image.return_value = image_mock
+        mock_image_changed.return_value = True
+        self.adapter.update_existing = True
+        self.assertEqual(
+            self.adapter._image_try_update('name', 'fn'),
+            None
+        )
+        self.assertEqual([image_mock.id], self.updated)
+        update_mock.assert_called_once()
+
+
 class TestUploadOvercloudImage(TestPluginV1):
+
     def setUp(self):
         super(TestUploadOvercloudImage, self).setUp()
 
@@ -128,53 +243,20 @@ class TestUploadOvercloudImage(TestPluginV1):
                       properties={'kernel_id': 10, 'ramdisk_id': 10,
                                   'hw_architecture': self._arch},
                       created_at='2015-07-31T14:37:22.000000'))
+        mock_cfe = mock.patch('tripleoclient.v1.overcloud_image.'
+                              'BaseClientAdapter.check_file_exists',
+                              autospec=True)
+        mock_cfe.start()
+        self.addCleanup(mock_cfe.stop)
+        mock_cfe.return_value = True
+
+        mock_rifp = mock.patch('tripleoclient.v1.overcloud_image.'
+                               'BaseClientAdapter.read_image_file_pointer',
+                               autospec=True)
+        mock_rifp.start()
+        self.addCleanup(mock_rifp.stop)
         self._file_handle = FakeHandle()
-        self.cmd._read_image_file_pointer = mock.Mock(
+        mock_rifp.return_value = self._file_handle
-                return_value=self._file_handle)
-        self.cmd._check_file_exists = mock.Mock(return_value=True)
-
-    @mock.patch('osc_lib.utils.find_resource')
-    def test_get_image_exists(self, mock_find_resource):
-        image_mock = mock.Mock(name='imagename')
-        mock_find_resource.return_value = image_mock
-        self.assertEqual(self.cmd._get_image('imagename'), image_mock)
-
-    @mock.patch('osc_lib.utils.find_resource')
-    def test_get_image_none(self, mock_find_resource):
-        mock_find_resource.side_effect = exceptions.CommandError('')
-        self.assertIsNone(self.cmd._get_image('noimagename'))
-
-    def test_image_try_update_no_exist(self):
-        self.cmd._get_image = mock.Mock(return_value=None)
-        parsed_args = mock.Mock(update_existing=False)
-        self.assertFalse(self.cmd._image_try_update('name', 'fn', parsed_args))
-
-    def test_image_try_update_need_update(self):
-        image_mock = mock.Mock(name='imagename')
-        self.cmd._get_image = mock.Mock(return_value=image_mock)
-        self.cmd._image_changed = mock.Mock(return_value=True)
-        parsed_args = mock.Mock(update_existing=False)
-        self.assertEqual(self.cmd._image_try_update('name', 'fn', parsed_args),
-                         image_mock)
-        self.assertEqual(
-            0,
-            self.app.client_manager.image.images.update.call_count
-        )
-
-    def test_image_try_update_do_update(self):
-        image_mock = mock.Mock(name='imagename',
-                               created_at='2015-07-31T14:37:22.000000')
-        update_mock = mock.Mock(return_value=image_mock)
-        self.app.client_manager.image.images.update = update_mock
-        self.cmd._get_image = mock.Mock(return_value=image_mock)
-        self.cmd._image_changed = mock.Mock(return_value=True)
-        parsed_args = mock.Mock(update_existing=True)
-        self.assertEqual(self.cmd._image_try_update('name', 'fn', parsed_args),
-                         None)
-        self.assertEqual(
-            1,
-            update_mock.call_count
-        )
 
     @mock.patch.dict(os.environ, {'KEY': 'VALUE', 'OLD_KEY': 'OLD_VALUE'})
     def test_get_environment_var(self):
@@ -193,28 +275,6 @@ class TestUploadOvercloudImage(TestPluginV1):
                                                        'default-value',
                                                        deprecated=['OLD_KEY']))
 
-    @mock.patch('os.path.isfile', autospec=True)
-    def test_file_try_update_need_update(self, mock_isfile):
-        mock_isfile.return_value = True
-        self.cmd._files_changed = mock.Mock(return_value=True)
-        self.cmd._copy_file = mock.Mock()
-
-        self.cmd._file_create_or_update('discimg', 'discimgprod', False)
-        self.assertEqual(
-            0,
-            self.cmd._copy_file.call_count
-        )
-
-    def test_file_try_update_do_update(self):
-        self.cmd._files_changed = mock.Mock(return_value=True)
-        self.cmd._copy_file = mock.Mock()
-
-        self.cmd._file_create_or_update('discimg', 'discimgprod', True)
-        self.assertEqual(
-            1,
-            self.cmd._copy_file.call_count
-        )
-
     def test_platform_without_architecture_fail(self):
         parsed_args = self.check_parser(self.cmd, ['--platform', 'SNB'], [])
         self.assertRaises(exceptions.CommandError,
@@ -223,12 +283,16 @@ class TestUploadOvercloudImage(TestPluginV1):
 
     @mock.patch('os.path.isfile', autospec=True)
     @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_images_v2(self, mock_subprocess_call,
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    def test_overcloud_create_images_v2(self,
+                                        mock_get_image,
+                                        mock_subprocess_call,
                                         mock_isfile):
         parsed_args = self.check_parser(self.cmd, [], [])
         mock_isfile.return_value = False
 
-        self.cmd._get_image = mock.Mock(return_value=None)
+        mock_get_image.return_value = None
 
         self.cmd.take_action(parsed_args)
 
@@ -263,113 +327,36 @@ class TestUploadOvercloudImage(TestPluginV1):
                       '"/var/lib/ironic/httpboot/agent.ramdisk"', shell=True)
         ])
 
-    @mock.patch('os.path.isfile', autospec=True)
-    @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_images_v1(self, mock_subprocess_call,
-                                        mock_isfile):
-        parsed_args = self.check_parser(self.cmd, [], [])
-        mock_isfile.return_value = False
-
-        self.cmd._get_image = mock.Mock(return_value=None)
-        self.app.client_manager.image.version = 1.0
-
-        self.cmd.take_action(parsed_args)
-
-        self.assertEqual(
-            0,
-            self.app.client_manager.image.images.delete.call_count
-        )
-        self.assertEqual(
-            3,
-            self.app.client_manager.image.images.create.call_count
-        )
-        self.app.client_manager.image.images.create.assert_has_calls([
-            mock.call(properties={'hw_architecture': self._arch},
-                      data=self._file_handle,
-                      name='overcloud-full-vmlinuz',
-                      disk_format='aki',
-                      is_public=True),
-            mock.call(properties={'hw_architecture': self._arch},
-                      data=self._file_handle,
-                      name='overcloud-full-initrd',
-                      disk_format='ari',
-                      is_public=True),
-            mock.call(properties={'kernel_id': 10, 'ramdisk_id': 10,
-                                  'hw_architecture': self._arch},
-                      name='overcloud-full',
-                      data=self._file_handle,
-                      container_format='bare',
-                      disk_format='qcow2',
-                      is_public=True),
-        ])
-
-    @mock.patch('os.path.isfile', autospec=True)
-    @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_images_with_arch_v1(self, mock_subprocess_call,
-                                                  mock_isfile):
-        parsed_args = self.check_parser(self.cmd, ['--arch', 'x86_64'], [])
-        mock_isfile.return_value = False
-
-        self.cmd._get_image = mock.Mock(return_value=None)
-        self.app.client_manager.image.version = 1.0
-
-        self.cmd.take_action(parsed_args)
-
-        self.assertEqual(
-            0,
-            self.app.client_manager.image.images.delete.call_count
-        )
-        self.assertEqual(
-            3,
-            self.app.client_manager.image.images.create.call_count
-        )
-        self.app.client_manager.image.images.create.assert_has_calls([
-            mock.call(properties={'hw_architecture': 'x86_64'},
-                      data=self._file_handle,
-                      name='x86_64-overcloud-full-vmlinuz',
-                      disk_format='aki',
-                      is_public=True),
-            mock.call(properties={'hw_architecture': 'x86_64'},
-                      data=self._file_handle,
-                      name='x86_64-overcloud-full-initrd',
-                      disk_format='ari',
-                      is_public=True),
-            mock.call(properties={'hw_architecture': 'x86_64',
-                                  'kernel_id': 10, 'ramdisk_id': 10},
-                      name='x86_64-overcloud-full',
-                      data=self._file_handle,
-                      container_format='bare',
-                      disk_format='qcow2',
-                      is_public=True),
-        ])
-
-        self.assertEqual(mock_subprocess_call.call_count, 2)
-        mock_subprocess_call.assert_has_calls([
-            mock.call('sudo cp -f "./ironic-python-agent.kernel" '
-                      '"/var/lib/ironic/httpboot/agent.kernel"', shell=True),
-            mock.call('sudo cp -f "./ironic-python-agent.initramfs" '
-                      '"/var/lib/ironic/httpboot/agent.ramdisk"', shell=True)
-        ])
-
     @mock.patch('os.path.isfile')
     @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_images_image_path(self, mock_subprocess_call,
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._image_try_update', autospec=True)
+    def test_overcloud_create_images_image_path(self,
+                                                mock_image_try_update,
+                                                mock_get_image,
+                                                mock_subprocess_call,
                                                 mock_isfile):
         parsed_args = self.check_parser(self.cmd,
                                         ['--image-path', '/foo'],
                                         [])
-        self.cmd._image_try_update = mock.Mock()
+        mock_get_image.return_value = None
+        mock_image_try_update.return_value = None
         mock_isfile.return_value = False
 
         self.cmd.take_action(parsed_args)
 
-        self.cmd._image_try_update.assert_has_calls([
+        self.cmd.adapter._image_try_update.assert_has_calls([
-            mock.call('overcloud-full-vmlinuz', '/foo/overcloud-full.vmlinuz',
+            mock.call(self.cmd.adapter,
-                      mock.ANY),
+                      'overcloud-full-vmlinuz',
-            mock.call('overcloud-full-initrd', '/foo/overcloud-full.initrd',
+                      '/foo/overcloud-full.vmlinuz'),
-                      mock.ANY),
+            mock.call(self.cmd.adapter,
-            mock.call('overcloud-full', '/foo/overcloud-full.qcow2',
+                      'overcloud-full-initrd',
-                      mock.ANY),
+                      '/foo/overcloud-full.initrd'),
+            mock.call(self.cmd.adapter,
+                      'overcloud-full',
+                      '/foo/overcloud-full.qcow2'),
         ])
         mock_subprocess_call.assert_has_calls([
             mock.call('sudo cp -f "/foo/ironic-python-agent.kernel" '
@@ -380,17 +367,27 @@ class TestUploadOvercloudImage(TestPluginV1):
 
     @mock.patch('os.path.isfile', autospec=True)
     @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_noupdate_images(self, mock_subprocess_call,
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._image_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._files_changed', autospec=True)
+    def test_overcloud_create_noupdate_images(self,
+                                              mock_files_changed,
+                                              mock_image_changed,
+                                              mock_get_image,
+                                              mock_subprocess_call,
                                               mock_isfile):
         parsed_args = self.check_parser(self.cmd, [], [])
         mock_isfile.return_value = True
-        self.cmd._files_changed = mock.Mock(return_value=True)
+        mock_files_changed.return_value = True
 
         existing_image = mock.Mock(id=10, name='imgname',
                                    properties={'kernel_id': 10,
                                                'ramdisk_id': 10})
-        self.cmd._get_image = mock.Mock(return_value=existing_image)
+        mock_get_image.return_value = existing_image
-        self.cmd._image_changed = mock.Mock(return_value=True)
+        mock_image_changed.return_value = True
 
         self.cmd.take_action(parsed_args)
 
@@ -411,16 +408,26 @@ class TestUploadOvercloudImage(TestPluginV1):
         self.assertFalse(self.cmd.updated)
 
     @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_update_images(self, mock_subprocess_call):
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._image_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._files_changed', autospec=True)
+    def test_overcloud_create_update_images(self,
+                                            mock_files_changed,
+                                            mock_image_changed,
+                                            mock_get_image,
+                                            mock_subprocess_call):
         parsed_args = self.check_parser(self.cmd, ['--update-existing'], [])
-        self.cmd._files_changed = mock.Mock(return_value=True)
+        mock_files_changed.return_value = True
 
         existing_image = mock.Mock(id=10, name='imgname',
                                    properties={'kernel_id': 10,
                                                'ramdisk_id': 10},
                                    created_at='2015-07-31T14:37:22.000000')
-        self.cmd._get_image = mock.Mock(return_value=existing_image)
+        mock_get_image.return_value = existing_image
-        self.cmd._image_changed = mock.Mock(return_value=True)
+        mock_image_changed.return_value = True
         self.app.client_manager.image.images.update.return_value = (
             existing_image)
 
@@ -443,6 +450,7 @@ class TestUploadOvercloudImage(TestPluginV1):
 
 
 class TestUploadOvercloudImageFull(TestPluginV1):
+
     def setUp(self):
         super(TestUploadOvercloudImageFull, self).setUp()
 
@@ -455,19 +463,33 @@ class TestUploadOvercloudImageFull(TestPluginV1):
             mock.Mock(id=10, name='imgname',
                       properties={'hw_architecture': self._arch},
                       created_at='2015-07-31T14:37:22.000000'))
+        mock_cfe = mock.patch('tripleoclient.v1.overcloud_image.'
+                              'BaseClientAdapter.check_file_exists',
+                              autospec=True)
+        mock_cfe.start()
+        self.addCleanup(mock_cfe.stop)
+        mock_cfe.return_value = True
+
+        mock_rifp = mock.patch('tripleoclient.v1.overcloud_image.'
+                               'BaseClientAdapter.read_image_file_pointer',
+                               autospec=True)
+        mock_rifp.start()
+        self.addCleanup(mock_rifp.stop)
         self._file_handle = FakeHandle()
-        self.cmd._read_image_file_pointer = mock.Mock(
+        mock_rifp.return_value = self._file_handle
-                return_value=self._file_handle)
-        self.cmd._check_file_exists = mock.Mock(return_value=True)
 
     @mock.patch('os.path.isfile', autospec=True)
     @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_images(self, mock_subprocess_call,
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    def test_overcloud_create_images(self,
+                                     mock_get_image,
+                                     mock_subprocess_call,
                                      mock_isfile):
         parsed_args = self.check_parser(self.cmd, ['--whole-disk'], [])
         mock_isfile.return_value = False
 
-        self.cmd._get_image = mock.Mock(return_value=None)
+        mock_get_image.return_value = None
 
         self.cmd.take_action(parsed_args)
 
@@ -501,14 +523,18 @@ class TestUploadOvercloudImageFull(TestPluginV1):
 
     @mock.patch('os.path.isfile', autospec=True)
     @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_images_with_arch(self, mock_subprocess_call,
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    def test_overcloud_create_images_with_arch(self,
+                                               mock_get_image,
+                                               mock_subprocess_call,
                                                mock_isfile):
         parsed_args = self.check_parser(self.cmd,
-                                        ['--whole-disk', '--arch', 'x86_64'],
+                                        ['--whole-disk', '--arch', 'ppc64le'],
                                         [])
         mock_isfile.return_value = False
 
-        self.cmd._get_image = mock.Mock(return_value=None)
+        mock_get_image.return_value = None
 
         self.cmd.take_action(parsed_args)
 
@@ -522,14 +548,14 @@ class TestUploadOvercloudImageFull(TestPluginV1):
         )
 
         self.app.client_manager.image.images.create.assert_has_calls([
-            mock.call(name='x86_64-overcloud-full',
+            mock.call(name='ppc64le-overcloud-full',
                       disk_format='qcow2',
                       container_format='bare',
                       visibility='public'),
         ])
 
         self.app.client_manager.image.images.update.assert_has_calls([
-            mock.call(mock.ANY, hw_architecture='x86_64'),
+            mock.call(mock.ANY, hw_architecture='ppc64le'),
         ])
         self.assertEqual(mock_subprocess_call.call_count, 2)
         mock_subprocess_call.assert_has_calls([
@@ -541,16 +567,26 @@ class TestUploadOvercloudImageFull(TestPluginV1):
 
     @mock.patch('os.path.isfile', autospec=True)
     @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_noupdate_images(self, mock_subprocess_call,
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._image_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._files_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    def test_overcloud_create_noupdate_images(self, mock_get_image,
+                                              mock_files_changed,
+                                              mock_image_changed,
+                                              mock_subprocess_call,
                                               mock_isfile):
         parsed_args = self.check_parser(self.cmd, ['--whole-disk'], [])
         mock_isfile.return_value = True
-        self.cmd._files_changed = mock.Mock(return_value=True)
+        mock_files_changed.return_value = True
 
         existing_image = mock.Mock(id=10, name='imgname',
                                    properties={'hw_architecture': self._arch})
-        self.cmd._get_image = mock.Mock(return_value=existing_image)
+        mock_get_image.return_value = existing_image
         self.cmd._image_changed = mock.Mock(return_value=True)
+        mock_image_changed.return_value = True
 
         self.cmd.take_action(parsed_args)
 
@@ -570,16 +606,25 @@ class TestUploadOvercloudImageFull(TestPluginV1):
         self.assertEqual(mock_subprocess_call.call_count, 0)
 
     @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_update_images(self, mock_subprocess_call):
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._image_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._files_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    def test_overcloud_create_update_images(self, mock_get_image,
+                                            mock_files_changed,
+                                            mock_image_changed,
+                                            mock_subprocess_call):
         parsed_args = self.check_parser(
             self.cmd, ['--update-existing', '--whole-disk'], [])
-        self.cmd._files_changed = mock.Mock(return_value=True)
+        mock_files_changed.return_value = True
 
         existing_image = mock.Mock(id=10, name='imgname',
                                    properties={'hw_architecture': self._arch},
                                    created_at='2015-07-31T14:37:22.000000')
-        self.cmd._get_image = mock.Mock(return_value=existing_image)
+        mock_get_image.return_value = existing_image
-        self.cmd._image_changed = mock.Mock(return_value=True)
+        mock_image_changed.return_value = True
         self.app.client_manager.image.images.update.return_value = (
             existing_image)
 
@@ -617,26 +662,38 @@ class TestUploadOvercloudImageFullMultiArch(TestPluginV1):
         self.app.client_manager.image = mock.Mock()
         self.app.client_manager.image.version = 2.0
         # NOTE(tonyb): This is a little fragile.  It works because
-        # GlanceV2ClientAdapter.upload_image() calls
+        # GlanceClientAdapter._upload_image() calls
         # self.client.images.create() and self.client.images.get() once each
         # call so this way we always create() and get() the same mocked "image"
         self.app.client_manager.image.images.create.side_effect = self.images
         self.app.client_manager.image.images.get.side_effect = self.images
+
+        mock_cfe = mock.patch('tripleoclient.v1.overcloud_image.'
+                              'BaseClientAdapter.check_file_exists',
+                              autospec=True)
+        mock_cfe.start()
+        self.addCleanup(mock_cfe.stop)
+        mock_cfe.return_value = True
+
+        mock_rifp = mock.patch('tripleoclient.v1.overcloud_image.'
+                               'BaseClientAdapter.read_image_file_pointer',
+                               autospec=True)
+        mock_rifp.start()
+        self.addCleanup(mock_rifp.stop)
         self._file_handle = FakeHandle()
-        self.cmd._read_image_file_pointer = mock.Mock(
+        mock_rifp.return_value = self._file_handle
-                return_value=self._file_handle)
-        self.cmd._check_file_exists = mock.Mock(return_value=True)
 
     @mock.patch('tripleo_common.arch.kernel_arch', autospec=True,
                 return_value='x86_64')
     @mock.patch('os.path.isfile', autospec=True)
     @mock.patch('subprocess.check_call', autospec=True)
-    def test_overcloud_create_images_with_arch(self, mock_subprocess_call,
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
+    def test_overcloud_create_images_with_arch(self, mock_get_image,
+                                               mock_subprocess_call,
                                                mock_isfile, mock_arch):
         mock_isfile.return_value = False
-
+        mock_get_image.return_value = None
-        self.cmd._get_image = mock.Mock(return_value=None)
-        mock.patch
 
         parsed_args = self.check_parser(self.cmd,
                                         ['--whole-disk'],
@@ -689,14 +746,15 @@ class TestUploadOvercloudImageFullMultiArch(TestPluginV1):
                 return_value='x86_64')
     @mock.patch('os.path.isfile', autospec=True)
     @mock.patch('subprocess.check_call', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'GlanceClientAdapter._get_image', autospec=True)
     def test_overcloud_create_images_with_arch_and_pltform(self,
+                                                           mock_get_image,
                                                            mock_subprocess,
                                                            mock_isfile,
                                                            mock_arch):
         mock_isfile.return_value = False
-
+        mock_get_image.return_value = None
-        self.cmd._get_image = mock.Mock(return_value=None)
-        mock.patch
 
         parsed_args = self.check_parser(self.cmd,
                                         ['--whole-disk'],
@@ -763,6 +821,7 @@ class TestUploadOvercloudImageFullMultiArch(TestPluginV1):
 
 
 class TestUploadOnlyExisting(TestPluginV1):
+
     def setUp(self):
         super(TestUploadOnlyExisting, self).setUp()
 
@@ -773,95 +832,155 @@ class TestUploadOnlyExisting(TestPluginV1):
         self.app.client_manager.image.images.create.return_value = (
             mock.Mock(id=10, name='imgname', properties={},
                       created_at='2015-07-31T14:37:22.000000'))
-        self.cmd._check_file_exists = mock.Mock()
+        mock_cfe = mock.patch('tripleoclient.v1.overcloud_image.'
+                              'BaseClientAdapter.check_file_exists',
+                              autospec=True)
+        mock_cfe.start()
+        self.addCleanup(mock_cfe.stop)
+        mock_cfe.return_value = True
+
+        mock_rifp = mock.patch('tripleoclient.v1.overcloud_image.'
+                               'BaseClientAdapter.read_image_file_pointer',
+                               autospec=True)
+        mock_rifp.start()
+        self.addCleanup(mock_rifp.stop)
         self._file_handle = FakeHandle()
-        self.cmd._read_image_file_pointer = mock.Mock(
+        mock_rifp.return_value = self._file_handle
-                return_value=self._file_handle)
 
     @mock.patch('subprocess.check_call', autospec=True)
     @mock.patch('os.path.isfile', autospec=True)
-    def test_overcloud_upload_just_ipa_wholedisk(
+    @mock.patch('tripleoclient.v1.overcloud_image.'
-            self, mock_isfile_call, mock_subprocess_call):
+                'GlanceClientAdapter._image_try_update', autospec=True)
-        self.cmd._image_changed = mock.Mock(return_value=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
-        self.cmd._image_try_update = mock.Mock(return_value=None)
+                'GlanceClientAdapter._image_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._files_changed', autospec=True)
+    def test_overcloud_upload_just_ipa_wholedisk(self,
+                                                 mock_files_changed,
+                                                 mock_image_changed,
+                                                 mock_image_try_update,
+                                                 mock_isfile_call,
+                                                 mock_subprocess_call):
+        mock_image_changed.return_value = True
+        mock_image_try_update.return_value = None
 
         parsed_args = self.check_parser(
             self.cmd, ['--whole-disk', '--image-type=ironic-python-agent'], [])
 
-        self.cmd._files_changed = mock.Mock(return_value=True)
+        mock_files_changed.return_value = True
         self.cmd.take_action(parsed_args)
 
         # ensure check_file_exists has not been called
-        self.assertItemsEqual(self.cmd._check_file_exists.call_args_list,
+        self.assertItemsEqual(
-                              [mock.call('./ironic-python-agent.initramfs'),
+            self.cmd.adapter.check_file_exists.call_args_list,
-                               mock.call('./ironic-python-agent.kernel')])
+            [mock.call(self.cmd.adapter, './ironic-python-agent.initramfs'),
+             mock.call(self.cmd.adapter, './ironic-python-agent.kernel')])
 
-        self.assertFalse(self.cmd._image_try_update.called)
+        self.assertFalse(mock_image_try_update.called)
 
     @mock.patch('subprocess.check_call', autospec=True)
     @mock.patch('os.path.isfile', autospec=True)
-    def test_overcloud_upload_just_os_wholedisk(
+    @mock.patch('tripleoclient.v1.overcloud_image.'
-            self, mock_isfile_call, mock_subprocess_call):
+                'GlanceClientAdapter._image_try_update', autospec=True)
-        self.cmd._image_changed = mock.Mock(return_value=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
-        self.cmd._image_try_update = mock.Mock(return_value=None)
+                'GlanceClientAdapter._image_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._files_changed', autospec=True)
+    def test_overcloud_upload_just_os_wholedisk(self,
+                                                mock_files_changed,
+                                                mock_image_changed,
+                                                mock_image_try_update,
+                                                mock_isfile_call,
+                                                mock_subprocess_call):
+        mock_image_changed.return_value = True
+        mock_image_try_update.return_value = None
 
         parsed_args = self.check_parser(
             self.cmd, ['--whole-disk', '--image-type=os'], [])
 
-        self.cmd._files_changed = mock.Mock(return_value=True)
+        mock_files_changed.return_value = True
         self.cmd.take_action(parsed_args)
 
         # ensure check_file_exists has been called just with ipa
-        self.assertItemsEqual(self.cmd._check_file_exists.call_args_list,
+        self.cmd.adapter.check_file_exists.assert_called_once_with(
-                              [mock.call('./overcloud-full.qcow2')])
+            self.cmd.adapter, './overcloud-full.qcow2')
 
         # ensure try_update has been called just with ipa
-        files = []
+        mock_image_try_update.assert_called_once_with(
-        for item in self.cmd._image_try_update.call_args_list:
+            self.cmd.adapter,
-            files.append(item[0][1])
+            'overcloud-full',
-        self.assertEqual(files, ['./overcloud-full.qcow2'])
+            './overcloud-full.qcow2'
+        )
 
     @mock.patch('subprocess.check_call', autospec=True)
     @mock.patch('os.path.isfile', autospec=True)
-    def test_overcloud_upload_just_ipa(
+    @mock.patch('tripleoclient.v1.overcloud_image.'
-            self, mock_isfile_call, mock_subprocess_call):
+                'GlanceClientAdapter._image_try_update', autospec=True)
-        self.cmd._image_changed = mock.Mock(return_value=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
-        self.cmd._image_try_update = mock.Mock(return_value=None)
+                'GlanceClientAdapter._image_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._files_changed', autospec=True)
+    def test_overcloud_upload_just_ipa(self,
+                                       mock_files_changed,
+                                       mock_image_changed,
+                                       mock_image_try_update,
+                                       mock_isfile_call,
+                                       mock_subprocess_call):
+        mock_image_changed.return_value = True
+        mock_image_try_update.return_value = None
 
         parsed_args = self.check_parser(
             self.cmd, ['--image-type=ironic-python-agent'], [])
 
-        self.cmd._files_changed = mock.Mock(return_value=True)
+        mock_files_changed.return_value = True
         self.cmd.take_action(parsed_args)
 
         # ensure check_file_exists has been called just with ipa
-        self.assertItemsEqual(self.cmd._check_file_exists.call_args_list,
+        self.assertItemsEqual(
-                              [mock.call('./ironic-python-agent.initramfs'),
+            self.cmd.adapter.check_file_exists.call_args_list,
-                               mock.call('./ironic-python-agent.kernel')])
+            [mock.call(self.cmd.adapter, './ironic-python-agent.initramfs'),
+             mock.call(self.cmd.adapter, './ironic-python-agent.kernel')]
+        )
 
-        self.assertFalse(self.cmd._image_try_update.called)
+        self.assertFalse(mock_image_try_update.called)
 
     @mock.patch('subprocess.check_call', autospec=True)
     @mock.patch('os.path.isfile', autospec=True)
-    def test_overcloud_upload_just_os(
+    @mock.patch('tripleoclient.v1.overcloud_image.'
-            self, mock_isfile_call, mock_subprocess_call):
+                'GlanceClientAdapter._image_try_update', autospec=True)
-        self.cmd._image_changed = mock.Mock(return_value=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
-        self.cmd._image_try_update = mock.Mock(return_value=None)
+                'GlanceClientAdapter._image_changed', autospec=True)
+    @mock.patch('tripleoclient.v1.overcloud_image.'
+                'BaseClientAdapter._files_changed', autospec=True)
+    def test_overcloud_upload_just_os(self,
+                                      mock_files_changed,
+                                      mock_image_changed,
+                                      mock_image_try_update,
+                                      mock_isfile_call,
+                                      mock_subprocess_call):
+        mock_image_changed.return_value = True
+        mock_image_try_update.return_value = None
 
         parsed_args = self.check_parser(
             self.cmd, ['--image-type=os'], [])
 
-        self.cmd._files_changed = mock.Mock(return_value=True)
+        mock_files_changed.return_value = True
         self.cmd.take_action(parsed_args)
 
         # ensure check_file_exists has been called just with ipa
-        self.assertItemsEqual(self.cmd._check_file_exists.call_args_list,
+        self.assertItemsEqual(
-                              [mock.call('./overcloud-full.qcow2')])
+            self.cmd.adapter.check_file_exists.call_args_list,
+            [mock.call(self.cmd.adapter, './overcloud-full.qcow2')])
 
         # ensure try_update has been called just with ipa
-        files = []
+        mock_image_try_update.assert_has_calls([
-        for item in self.cmd._image_try_update.call_args_list:
+            mock.call(self.cmd.adapter,
-            files.append(item[0][1])
+                      'overcloud-full-vmlinuz',
-        self.assertEqual(files, ['./overcloud-full.vmlinuz',
+                      './overcloud-full.vmlinuz'),
-                                 './overcloud-full.initrd',
+            mock.call(self.cmd.adapter,
-                                 './overcloud-full.qcow2'])
+                      'overcloud-full-initrd',
+                      './overcloud-full.initrd'),
+            mock.call(self.cmd.adapter,
+                      'overcloud-full',
+                      './overcloud-full.qcow2'),
+        ])

tripleoclient/utils.py

@@ -1530,31 +1530,37 @@ def _name_helper(basename, arch=None, platform=None, use_subdir=False):
     return basename
 
 
-def overcloud_kernel(basename, arch=None, platform=None):
+def overcloud_kernel(basename, arch=None, platform=None,
+                     use_subdir=False):
     return (_name_helper('%s-vmlinuz' % basename, arch=arch,
-                         platform=platform),
+                         platform=platform, use_subdir=use_subdir),
             '.vmlinuz')
 
 
-def overcloud_ramdisk(basename, arch=None, platform=None):
+def overcloud_ramdisk(basename, arch=None, platform=None,
+                      use_subdir=False):
     return (_name_helper('%s-initrd' % basename, arch=arch,
-                         platform=platform),
+                         platform=platform, use_subdir=use_subdir),
             '.initrd')
 
 
-def overcloud_image(basename, arch=None, platform=None):
+def overcloud_image(basename, arch=None, platform=None,
-    return (_name_helper(basename, arch=arch, platform=platform),
+                    use_subdir=False):
+    return (_name_helper(basename, arch=arch, platform=platform,
+                         use_subdir=use_subdir),
             '.qcow2')
 
 
-def deploy_kernel(arch=None, platform=None):
+def deploy_kernel(basename='agent', arch=None, platform=None,
-    return _name_helper('agent', arch=arch, platform=platform,
+                  use_subdir=True):
-                        use_subdir=True) + '.kernel'
+    return _name_helper(basename, arch=arch, platform=platform,
+                        use_subdir=use_subdir) + '.kernel'
 
 
-def deploy_ramdisk(arch=None, platform=None):
+def deploy_ramdisk(basename='agent', arch=None, platform=None,
-    return _name_helper('agent', arch=arch, platform=platform,
+                   use_subdir=True):
-                        use_subdir=True) + '.ramdisk'
+    return _name_helper(basename, arch=arch, platform=platform,
+                        use_subdir=use_subdir) + '.ramdisk'
 
 
 def _candidate_files(node, call):

tripleoclient/v1/overcloud_image.py

@@ -15,6 +15,7 @@
 
 from __future__ import print_function
 
+import abc
 import logging
 import os
 import re
@@ -95,61 +96,79 @@ class BuildOvercloudImage(command.Command):
         manager.build()
 
 
-class GlanceBaseClientAdapter(object):
+class BaseClientAdapter(object):
-    def __init__(self, client):
+
+    log = logging.getLogger(__name__ + ".BaseClientAdapter")
+
+    def __init__(self, image_path, progress=False,
+                 update_existing=False, updated=None):
+        self.progress = progress
+        self.image_path = image_path
+        self.update_existing = update_existing
+        self.updated = updated
+
+    @abc.abstractmethod
+    def get_image_property(self, image, prop):
+        pass
+
+    @abc.abstractmethod
+    def update_or_upload(self, image_name, properties, names_func,
+                         arch, platform=None,
+                         disk_format='qcow2', container_format='bare'):
+        pass
+
+    def _copy_file(self, src, dest):
+        subprocess.check_call('sudo cp -f "{0}" "{1}"'.format(src, dest),
+                              shell=True)
+
+    def _files_changed(self, filepath1, filepath2):
+        return (plugin_utils.file_checksum(filepath1) !=
+                plugin_utils.file_checksum(filepath2))
+
+    def file_create_or_update(self, src_file, dest_file):
+        if os.path.isfile(dest_file):
+            if self._files_changed(src_file, dest_file):
+                if self.update_existing:
+                    self._copy_file(src_file, dest_file)
+                else:
+                    print('Image file "%s" already exists and can be updated'
+                          ' with --update-existing.' % dest_file)
+            else:
+                print('Image file "%s" is up-to-date, skipping.' % dest_file)
+        else:
+            self._copy_file(src_file, dest_file)
+
+    def check_file_exists(self, file_path):
+        if not os.path.isfile(file_path):
+            raise exceptions.CommandError(
+                'Required file "%s" does not exist.' % file_path
+            )
+
+    def read_image_file_pointer(self, filepath):
+        self.check_file_exists(filepath)
+        file_descriptor = open(filepath, 'rb')
+
+        if self.progress:
+            file_descriptor = VerboseFileWrapper(file_descriptor)
+
+        return file_descriptor
+
+
+class GlanceClientAdapter(BaseClientAdapter):
+
+    def __init__(self, client, **kwargs):
+        super(GlanceClientAdapter, self).__init__(**kwargs)
         self.client = client
 
-    def print_image_info(self, image):
+    def _print_image_info(self, image):
         table = PrettyTable(['ID', 'Name', 'Disk Format', 'Size', 'Status'])
         table.add_row([image.id, image.name, image.disk_format, image.size,
                        image.status])
         print(table, file=sys.stdout)
 
-
-class GlanceV1ClientAdapter(GlanceBaseClientAdapter):
-    def upload_image(self, *args, **kwargs):
-        image = self.client.images.create(*args, **kwargs)
-
-        print('Image "%s" was uploaded.' % image.name, file=sys.stdout)
-        self.print_image_info(image)
-        return image
-
-    def get_image_property(self, image, prop):
-        return image.properties[prop]
-
-
-class GlanceV2ClientAdapter(GlanceBaseClientAdapter):
-    def upload_image(self, *args, **kwargs):
-        is_public = kwargs.pop('is_public')
-        data = kwargs.pop('data')
-        properties = kwargs.pop('properties', None)
-        kwargs['visibility'] = 'public' if is_public else 'private'
-        kwargs.setdefault('container_format', 'bare')
-
-        image = self.client.images.create(*args, **kwargs)
-
-        self.client.images.upload(image.id, image_data=data)
-        if properties:
-            self.client.images.update(image.id, **properties)
-        # Refresh image info
-        image = self.client.images.get(image.id)
-
-        print('Image "%s" was uploaded.' % image.name, file=sys.stdout)
-        self.print_image_info(image)
-        return image
-
-    def get_image_property(self, image, prop):
-        return getattr(image, prop)
-
-
-class UploadOvercloudImage(command.Command):
-    """Create overcloud glance images from existing image files."""
-    log = logging.getLogger(__name__ + ".UploadOvercloudImage")
-
     def _get_image(self, name):
         try:
-            image = utils.find_resource(self.app.client_manager.image.images,
+            image = utils.find_resource(self.client.images, name)
-                                        name)
         except exceptions.CommandError as e:
             # TODO(maufart): enhance error detection, when python-glanceclient
             # starts provide it https://bugs.launchpad.net/glance/+bug/1480156
@@ -164,43 +183,22 @@ class UploadOvercloudImage(command.Command):
                 return None
         return image
 
-    def _image_changed(self, name, filename):
+    def _image_changed(self, image, filename):
-        image = utils.find_resource(self.app.client_manager.image.images,
-                                    name)
         return image.checksum != plugin_utils.file_checksum(filename)
 
-    def _check_file_exists(self, file_path):
+    def _image_try_update(self, image_name, image_file):
-        if not os.path.isfile(file_path):
-            raise exceptions.CommandError(
-                'Required file "%s" does not exist.' % file_path
-            )
-
-    def _read_image_file_pointer(self, dirname, filename):
-        filepath = os.path.join(dirname, filename)
-        self._check_file_exists(filepath)
-        file_descriptor = open(filepath, 'rb')
-
-        if self._progress:
-            file_descriptor = VerboseFileWrapper(file_descriptor)
-
-        return file_descriptor
-
-    def _copy_file(self, src, dest):
-        subprocess.check_call('sudo cp -f "{0}" "{1}"'.format(src, dest),
-                              shell=True)
-
-    def _image_try_update(self, image_name, image_file, parsed_args):
         image = self._get_image(image_name)
         if image:
-            if self._image_changed(image_name, image_file):
+            if self._image_changed(image, image_file):
-                if parsed_args.update_existing:
+                if self.update_existing:
-                    self.app.client_manager.image.images.update(
+                    self.client.images.update(
                         image.id,
                         name='%s_%s' % (image.name, re.sub(r'[\-:\.]|(0+$)',
                                                            '',
                                                            image.created_at))
                     )
-                    self.updated = True
+                    if self.updated is not None:
+                        self.updated.append(image.id)
                     return None
                 else:
                     print('Image "%s" already exists and can be updated'
@@ -212,28 +210,66 @@ class UploadOvercloudImage(command.Command):
         else:
             return None
 
-    def _files_changed(self, filepath1, filepath2):
+    def _upload_image(self, name, data, properties=None, visibility='public',
-        return (plugin_utils.file_checksum(filepath1) !=
+                      disk_format='qcow2', container_format='bare'):
-                plugin_utils.file_checksum(filepath2))
 
-    def _file_create_or_update(self, src_file, dest_file, update_existing):
+        image = self.client.images.create(
-        if os.path.isfile(dest_file):
+            name=name,
-            if self._files_changed(src_file, dest_file):
+            visibility=visibility,
-                if update_existing:
+            disk_format=disk_format,
-                    self._copy_file(src_file, dest_file)
+            container_format=container_format
-                else:
+        )
-                    print('Image file "%s" already exists and can be updated'
-                          ' with --update-existing.' % dest_file)
-            else:
-                print('Image file "%s" is up-to-date, skipping.' % dest_file)
-        else:
-            self._copy_file(src_file, dest_file)
 
-    def _get_glance_client_adaptor(self):
+        self.client.images.upload(image.id, image_data=data)
-        if self.app.client_manager.image.version >= 2.0:
+        if properties:
-            return GlanceV2ClientAdapter(self.app.client_manager.image)
+            self.client.images.update(image.id, **properties)
-        else:
+        # Refresh image info
-            return GlanceV1ClientAdapter(self.app.client_manager.image)
+        image = self.client.images.get(image.id)
+
+        print('Image "%s" was uploaded.' % image.name, file=sys.stdout)
+        self._print_image_info(image)
+        return image
+
+    def get_image_property(self, image, prop):
+        return getattr(image, prop)
+
+    def update_or_upload(self, image_name, properties, names_func,
+                         arch, platform=None,
+                         disk_format='qcow2', container_format='bare'):
+
+        if arch == 'x86_64' and platform is None:
+            arch = None
+
+        (glance_name, extension) = names_func(
+                image_name, arch=arch, platform=platform)
+
+        file_path = os.path.join(self.image_path, image_name + extension)
+
+        updated_image = self._image_try_update(glance_name, file_path)
+        if updated_image:
+            return updated_image
+
+        with self.read_image_file_pointer(file_path) as data:
+            return self._upload_image(
+                    name=glance_name,
+                    disk_format=disk_format,
+                    container_format=container_format,
+                    properties=properties,
+                    data=data)
+
+
+class UploadOvercloudImage(command.Command):
+    """Make existing image files available for overcloud deployment."""
+    log = logging.getLogger(__name__ + ".UploadOvercloudImage")
+
+    def _get_client_adapter(self, parsed_args):
+        return GlanceClientAdapter(
+            self.app.client_manager.image,
+            progress=parsed_args.progress,
+            image_path=parsed_args.image_path,
+            update_existing=parsed_args.update_existing,
+            updated=self.updated
+        )
 
     def _get_environment_var(self, envvar, default, deprecated=[]):
         for env_key in deprecated:
@@ -293,7 +329,7 @@ class UploadOvercloudImage(command.Command):
             "--architecture",
             help=_("Architecture type for these images, "
                    "\'x86_64\', \'i386\' and \'ppc64le\' "
-                   "are common options.  This option should match at least "
+                   "are common options. This option should match at least "
                    "one \'arch\' value in instackenv.json"),
         )
         parser.add_argument(
@@ -322,9 +358,8 @@ class UploadOvercloudImage(command.Command):
 
     def take_action(self, parsed_args):
         self.log.debug("take_action(%s)" % parsed_args)
-        glance_client_adaptor = self._get_glance_client_adaptor()
+        self.updated = []
-        self.updated = False
+        self.adapter = self._get_client_adapter(parsed_args)
-        self._progress = parsed_args.progress
 
         if parsed_args.platform and not parsed_args.architecture:
             raise exceptions.CommandError('You supplied a platform (%s) '
@@ -348,12 +383,12 @@ class UploadOvercloudImage(command.Command):
             overcloud_image_type = 'partition'
 
         for image in image_files:
-            self._check_file_exists(os.path.join(parsed_args.image_path,
+            self.adapter.check_file_exists(
-                                                 image))
+                os.path.join(parsed_args.image_path, image))
 
         image_name = parsed_args.os_image_name.split('.')[0]
 
-        self.log.debug("uploading %s overcloud images to glance" %
+        self.log.debug("uploading %s overcloud images " %
                        overcloud_image_type)
 
         properties = {}
@@ -369,69 +404,38 @@ class UploadOvercloudImage(command.Command):
         if parsed_args.image_type is None or parsed_args.image_type == 'os':
             # vmlinuz and initrd only need to be uploaded for a partition image
             if not parsed_args.whole_disk:
-                (oc_vmlinuz_name,
+                kernel = self.adapter.update_or_upload(
-                 oc_vmlinuz_extension) = plugin_utils.overcloud_kernel(
+                    image_name=image_name,
-                     image_name, arch=arch, platform=platform)
+                    properties=properties,
-                oc_vmlinuz_file = os.path.join(parsed_args.image_path,
+                    names_func=plugin_utils.overcloud_kernel,
-                                               image_name +
+                    arch=arch,
-                                               oc_vmlinuz_extension)
+                    platform=platform,
-                with self._read_image_file_pointer(
+                    disk_format='aki'
-                        parsed_args.image_path, oc_vmlinuz_file) as data:
+                )
-                    kernel = (self._image_try_update(oc_vmlinuz_name,
-                                                     oc_vmlinuz_file,
-                                                     parsed_args) or
-                              glance_client_adaptor.upload_image(
-                                  name=oc_vmlinuz_name,
-                                  is_public=True,
-                                  disk_format='aki',
-                                  properties=properties,
-                                  data=data
-                    ))
 
-                (oc_initrd_name,
+                ramdisk = self.adapter.update_or_upload(
-                 oc_initrd_extension) = plugin_utils.overcloud_ramdisk(
+                    image_name=image_name,
-                     image_name, arch=arch, platform=platform)
+                    properties=properties,
-                oc_initrd_file = os.path.join(parsed_args.image_path,
+                    names_func=plugin_utils.overcloud_ramdisk,
-                                              image_name +
+                    arch=arch,
-                                              oc_initrd_extension)
+                    platform=platform,
-                with self._read_image_file_pointer(
+                    disk_format='ari'
-                        parsed_args.image_path, oc_initrd_file) as data:
+                )
-                    ramdisk = (self._image_try_update(oc_initrd_name,
-                                                      oc_initrd_file,
-                                                      parsed_args) or
-                               glance_client_adaptor.upload_image(
-                                   name=oc_initrd_name,
-                                   is_public=True,
-                                   disk_format='ari',
-                                   properties=properties,
-                                   data=data
-                    ))
 
-                (oc_name,
+                overcloud_image = self.adapter.update_or_upload(
-                 oc_extension) = plugin_utils.overcloud_image(
+                    image_name=image_name,
-                     image_name, arch=arch, platform=platform)
+                    properties=dict(
-                oc_file = os.path.join(parsed_args.image_path,
+                        {'kernel_id': kernel.id,
-                                       image_name +
+                         'ramdisk_id': ramdisk.id},
-                                       oc_extension)
+                        **properties),
-                with self._read_image_file_pointer(
+                    names_func=plugin_utils.overcloud_image,
-                        parsed_args.image_path, oc_file) as data:
+                    arch=arch,
-                    overcloud_image = (self._image_try_update(oc_name, oc_file,
+                    platform=platform
-                                                              parsed_args) or
+                )
-                                       glance_client_adaptor.upload_image(
-                                           name=oc_name,
-                                           is_public=True,
-                                           disk_format='qcow2',
-                                           container_format='bare',
-                                           properties=dict(
-                                               {'kernel_id': kernel.id,
-                                                'ramdisk_id': ramdisk.id},
-                                               **properties),
-                                           data=data
-                    ))
 
-                img_kernel_id = glance_client_adaptor.get_image_property(
+                img_kernel_id = self.adapter.get_image_property(
                     overcloud_image, 'kernel_id')
-                img_ramdisk_id = glance_client_adaptor.get_image_property(
+                img_ramdisk_id = self.adapter.get_image_property(
                     overcloud_image, 'ramdisk_id')
                 # check overcloud image links
                 if (img_kernel_id != kernel.id or
@@ -442,46 +446,33 @@ class UploadOvercloudImage(command.Command):
                                    'manually.')
 
             else:
-                (oc_name,
+                overcloud_image = self.adapter.update_or_upload(
-                 oc_extension) = plugin_utils.overcloud_image(
+                    image_name=image_name,
-                     image_name, arch=arch, platform=platform)
+                    properties=properties,
-                oc_file = os.path.join(parsed_args.image_path,
+                    names_func=plugin_utils.overcloud_image,
-                                       image_name +
+                    arch=arch,
-                                       oc_extension)
+                    platform=platform
-                with self._read_image_file_pointer(
+                )
-                        parsed_args.image_path, oc_file) as data:
-                    overcloud_image = (self._image_try_update(oc_name, oc_file,
-                                                              parsed_args) or
-                                       glance_client_adaptor.upload_image(
-                                           name=oc_name,
-                                           is_public=True,
-                                           disk_format='qcow2',
-                                           container_format='bare',
-                                           properties=properties,
-                                           data=data
-                    ))
 
-            self.log.debug("uploading bm images to glance")
+            self.log.debug("uploading bm images")
 
         if parsed_args.image_type is None or \
                 parsed_args.image_type == 'ironic-python-agent':
             self.log.debug("copy agent images to HTTP BOOT dir")
 
-            self._file_create_or_update(
+            self.adapter.file_create_or_update(
                 os.path.join(parsed_args.image_path,
                              '%s.kernel' % parsed_args.ipa_name),
-                os.path.join(parsed_args.http_boot, 'agent.kernel'),
+                os.path.join(parsed_args.http_boot, 'agent.kernel')
-                parsed_args.update_existing
             )
 
-            self._file_create_or_update(
+            self.adapter.file_create_or_update(
                 os.path.join(parsed_args.image_path,
                              '%s.initramfs' % parsed_args.ipa_name),
-                os.path.join(parsed_args.http_boot, 'agent.ramdisk'),
+                os.path.join(parsed_args.http_boot, 'agent.ramdisk')
-                parsed_args.update_existing
             )
 
         if self.updated:
-            print('Some images have been updated in Glance, make sure to '
+            print('%s images have been updated, make sure to '
                   'rerun\n\topenstack overcloud node configure\nto reflect '
-                  'the changes on the nodes')
+                  'the changes on the nodes' % len(self.updated))