Merge "Fix: create image from volume command" into stable/xena

3 files changed, 128 insertions, 3 deletions

diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py
index c1f46d2d..7c5b2ecf 100644
--- a/openstackclient/image/v2/image.py
+++ b/openstackclient/image/v2/image.py
@@ -21,6 +21,7 @@ import logging
 import os
 import sys
 
+from cinderclient import api_versions
 import openstack.cloud._utils
 from openstack.image import image_signer
 from osc_lib.api import utils as api_utils
@@ -484,14 +485,27 @@ class CreateImage(command.ShowOne):
                 volume_client.volumes,
                 parsed_args.volume,
             )
+            mv_kwargs = {}
+            if volume_client.api_version >= api_versions.APIVersion('3.1'):
+                mv_kwargs.update(
+                    visibility=kwargs.get('visibility', 'private'),
+                    protected=bool(parsed_args.protected)
+                )
+            else:
+                if kwargs.get('visibility') or parsed_args.protected:
+                    msg = _(
+                        '--os-volume-api-version 3.1 or greater is required '
+                        'to support the --public, --private, --community, '
+                        '--shared or --protected option.'
+                    )
+                    raise exceptions.CommandError(msg)
             response, body = volume_client.volumes.upload_to_image(
                 source_volume.id,
                 parsed_args.force,
                 parsed_args.name,
                 parsed_args.container_format,
                 parsed_args.disk_format,
-                visibility=kwargs.get('visibility', 'private'),
-                protected=True if parsed_args.protected else False
+                **mv_kwargs
             )
             info = body['os-volume_upload_image']
             try:
diff --git a/openstackclient/tests/unit/image/v2/test_image.py b/openstackclient/tests/unit/image/v2/test_image.py
index 35af6799..9c755ca9 100644
--- a/openstackclient/tests/unit/image/v2/test_image.py
+++ b/openstackclient/tests/unit/image/v2/test_image.py
@@ -19,6 +19,7 @@ import os
 import tempfile
 from unittest import mock
 
+from cinderclient import api_versions
 from openstack import exceptions as sdk_exceptions
 from osc_lib.cli import format_columns
 from osc_lib import exceptions
@@ -26,9 +27,10 @@ from osc_lib import exceptions
 from openstackclient.image.v2 import image
 from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes
 from openstackclient.tests.unit.image.v2 import fakes as image_fakes
+from openstackclient.tests.unit.volume.v3 import fakes as volume_fakes
 
 
-class TestImage(image_fakes.TestImagev2):
+class TestImage(image_fakes.TestImagev2, volume_fakes.TestVolume):
 
     def setUp(self):
         super(TestImage, self).setUp()
@@ -50,6 +52,13 @@ class TestImage(image_fakes.TestImagev2):
         self.project_mock.reset_mock()
         self.domain_mock = self.app.client_manager.identity.domains
         self.domain_mock.reset_mock()
+        self.volumes_mock = self.app.client_manager.volume.volumes
+        fake_body = {
+            'os-volume_upload_image':
+                {'volume_type': {'name': 'fake_type'}}}
+        self.volumes_mock.upload_to_image.return_value = (
+            200, fake_body)
+        self.volumes_mock.reset_mock()
 
     def setup_images_mock(self, count):
         images = image_fakes.FakeImage.create_images(count=count)
@@ -297,6 +306,101 @@ class TestImageCreate(TestImage):
             use_import=True
         )
 
+    @mock.patch('osc_lib.utils.find_resource')
+    @mock.patch('openstackclient.image.v2.image.get_data_file')
+    def test_image_create_from_volume(self, mock_get_data_f, mock_get_vol):
+
+        fake_vol_id = 'fake-volume-id'
+        mock_get_data_f.return_value = (None, None)
+
+        class FakeVolume:
+            id = fake_vol_id
+
+        mock_get_vol.return_value = FakeVolume()
+
+        arglist = [
+            '--volume', fake_vol_id,
+            self.new_image.name,
+        ]
+        verifylist = [
+            ('name', self.new_image.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.volumes_mock.upload_to_image.assert_called_with(
+            fake_vol_id,
+            False,
+            self.new_image.name,
+            'bare',
+            'raw'
+        )
+
+    @mock.patch('osc_lib.utils.find_resource')
+    @mock.patch('openstackclient.image.v2.image.get_data_file')
+    def test_image_create_from_volume_fail(self, mock_get_data_f,
+                                           mock_get_vol):
+
+        fake_vol_id = 'fake-volume-id'
+        mock_get_data_f.return_value = (None, None)
+
+        class FakeVolume:
+            id = fake_vol_id
+
+        mock_get_vol.return_value = FakeVolume()
+
+        arglist = [
+            '--volume', fake_vol_id,
+            self.new_image.name,
+            '--public'
+        ]
+        verifylist = [
+            ('name', self.new_image.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(exceptions.CommandError, self.cmd.take_action,
+                          parsed_args)
+
+    @mock.patch('osc_lib.utils.find_resource')
+    @mock.patch('openstackclient.image.v2.image.get_data_file')
+    def test_image_create_from_volume_v31(self, mock_get_data_f,
+                                          mock_get_vol):
+
+        self.app.client_manager.volume.api_version = (
+            api_versions.APIVersion('3.1'))
+
+        fake_vol_id = 'fake-volume-id'
+        mock_get_data_f.return_value = (None, None)
+
+        class FakeVolume:
+            id = fake_vol_id
+
+        mock_get_vol.return_value = FakeVolume()
+
+        arglist = [
+            '--volume', fake_vol_id,
+            self.new_image.name,
+            '--public'
+        ]
+        verifylist = [
+            ('name', self.new_image.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.volumes_mock.upload_to_image.assert_called_with(
+            fake_vol_id,
+            False,
+            self.new_image.name,
+            'bare',
+            'raw',
+            visibility='public',
+            protected=False
+        )
+
 
 class TestAddProjectToImage(TestImage):
 
diff --git a/releasenotes/notes/fix-image-create-from-volume-c573e553161605c4.yaml b/releasenotes/notes/fix-image-create-from-volume-c573e553161605c4.yaml
new file mode 100644
index 00000000..92fc7419
--- /dev/null
+++ b/releasenotes/notes/fix-image-create-from-volume-c573e553161605c4.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixed create image from volume command. If user wants to
+    pass ``visibility`` and ``protected`` fields, they need to
+    specify volume microversion 3.1 or greater by passing
+    ``os-volume-api-version 3.1`` with the command.