From f15352f8613d73b003abc5fe4da9ff23229895c8 Mon Sep 17 00:00:00 2001
From: Steve Martinelli <s.martinelli@gmail.com>
Date: Wed, 9 Nov 2016 16:01:33 -0500
Subject: [PATCH] clean up image choices and help text

Use choices for image set and image create commands, this aligns
with our use of choices in networking commands.

Also update the help text to match that of the networking
commands, where we iterate through the options.

Related-Bug: 1635518
Change-Id: Ib4c66b06e07f1d4e5bfe1b74053f2215cccad890
---
 doc/source/command-objects/image.rst              |  6 ++++--
 openstackclient/image/v1/image.py                 | 15 ++++++++-------
 openstackclient/image/v2/image.py                 | 12 ++++++++----
 openstackclient/tests/unit/image/v1/test_image.py |  6 +++---
 openstackclient/tests/unit/image/v2/test_image.py | 14 +++++++-------
 5 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/doc/source/command-objects/image.rst b/doc/source/command-objects/image.rst
index 842eab8d8a..5184f44698 100644
--- a/doc/source/command-objects/image.rst
+++ b/doc/source/command-objects/image.rst
@@ -78,7 +78,8 @@ Create/upload an image
 
 .. option:: --disk-format <disk-format>
 
-    Image disk format (default: raw)
+    Image disk format. The supported options are: ami, ari, aki, vhd, vmdk,
+    raw, qcow2, vhdx, vdi, and iso. The default format is: raw
 
 .. option:: --size <size>
 
@@ -337,7 +338,8 @@ Set image properties
 
 .. option:: --disk-format <disk-format>
 
-    Image disk format (default: raw)
+    Image disk format. The supported options are: ami, ari, aki, vhd, vmdk,
+    raw, qcow2, vhdx, vdi, and iso.
 
 .. option:: --size <size>
 
diff --git a/openstackclient/image/v1/image.py b/openstackclient/image/v1/image.py
index cf389d17b7..0d54e339e1 100644
--- a/openstackclient/image/v1/image.py
+++ b/openstackclient/image/v1/image.py
@@ -38,6 +38,8 @@ from openstackclient.i18n import _
 
 DEFAULT_CONTAINER_FORMAT = 'bare'
 DEFAULT_DISK_FORMAT = 'raw'
+DISK_CHOICES = ["ami", "ari", "aki", "vhd", "vmdk", "raw", "qcow2", "vhdx",
+                "vdi", "iso"]
 
 
 LOG = logging.getLogger(__name__)
@@ -89,8 +91,9 @@ class CreateImage(command.ShowOne):
             "--disk-format",
             default=DEFAULT_DISK_FORMAT,
             metavar="<disk-format>",
-            help=_("Image disk format "
-                   "(default: %s)") % DEFAULT_DISK_FORMAT,
+            choices=DISK_CHOICES,
+            help=_("Image disk format. The supported options are: %s. "
+                   "The default format is: raw") % ', '.join(DISK_CHOICES)
         )
         parser.add_argument(
             "--size",
@@ -502,14 +505,12 @@ class SetImage(command.Command):
             container_choices,
             choices=container_choices
         )
-        disk_choices = ["ami", "ari", "aki", "vhd", "vmdk", "raw", "qcow2",
-                        "vhdx", "vdi", "iso"]
         parser.add_argument(
             "--disk-format",
             metavar="<disk-format>",
-            help=_("Disk format of image. Acceptable formats: %s") %
-            disk_choices,
-            choices=disk_choices
+            choices=DISK_CHOICES,
+            help=_("Image disk format. The supported options are: %s.") %
+            ', '.join(DISK_CHOICES)
         )
         parser.add_argument(
             "--size",
diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py
index 657277c0dc..4031952bc4 100644
--- a/openstackclient/image/v2/image.py
+++ b/openstackclient/image/v2/image.py
@@ -32,6 +32,8 @@ from openstackclient.identity import common
 
 DEFAULT_CONTAINER_FORMAT = 'bare'
 DEFAULT_DISK_FORMAT = 'raw'
+DISK_CHOICES = ["ami", "ari", "aki", "vhd", "vmdk", "raw", "qcow2", "vhdx",
+                "vdi", "iso"]
 
 
 LOG = logging.getLogger(__name__)
@@ -140,9 +142,10 @@ class CreateImage(command.ShowOne):
         parser.add_argument(
             "--disk-format",
             default=DEFAULT_DISK_FORMAT,
+            choices=DISK_CHOICES,
             metavar="<disk-format>",
-            help=_("Image disk format "
-                   "(default: %s)") % DEFAULT_DISK_FORMAT,
+            help=_("Image disk format. The supported options are: %s. "
+                   "The default format is: raw") % ', '.join(DISK_CHOICES)
         )
         parser.add_argument(
             "--min-disk",
@@ -650,8 +653,9 @@ class SetImage(command.Command):
         parser.add_argument(
             "--disk-format",
             metavar="<disk-format>",
-            help=_("Image disk format "
-                   "(default: %s)") % DEFAULT_DISK_FORMAT,
+            choices=DISK_CHOICES,
+            help=_("Image disk format. The supported options are: %s") %
+            ', '.join(DISK_CHOICES)
         )
         protected_group = parser.add_mutually_exclusive_group()
         protected_group.add_argument(
diff --git a/openstackclient/tests/unit/image/v1/test_image.py b/openstackclient/tests/unit/image/v1/test_image.py
index aef74f0467..036c833651 100644
--- a/openstackclient/tests/unit/image/v1/test_image.py
+++ b/openstackclient/tests/unit/image/v1/test_image.py
@@ -116,7 +116,7 @@ class TestImageCreate(TestImage):
         self.images_mock.configure_mock(**mock_exception)
         arglist = [
             '--container-format', 'ovf',
-            '--disk-format', 'fs',
+            '--disk-format', 'ami',
             '--min-disk', '10',
             '--min-ram', '4',
             '--protected',
@@ -126,7 +126,7 @@ class TestImageCreate(TestImage):
         ]
         verifylist = [
             ('container_format', 'ovf'),
-            ('disk_format', 'fs'),
+            ('disk_format', 'ami'),
             ('min_disk', 10),
             ('min_ram', 4),
             ('protected', True),
@@ -147,7 +147,7 @@ class TestImageCreate(TestImage):
         self.images_mock.create.assert_called_with(
             name=self.new_image.name,
             container_format='ovf',
-            disk_format='fs',
+            disk_format='ami',
             min_disk=10,
             min_ram=4,
             protected=True,
diff --git a/openstackclient/tests/unit/image/v2/test_image.py b/openstackclient/tests/unit/image/v2/test_image.py
index ebc9c3a759..2f2212e4c3 100644
--- a/openstackclient/tests/unit/image/v2/test_image.py
+++ b/openstackclient/tests/unit/image/v2/test_image.py
@@ -130,7 +130,7 @@ class TestImageCreate(TestImage):
         self.images_mock.configure_mock(**mock_exception)
         arglist = [
             '--container-format', 'ovf',
-            '--disk-format', 'fs',
+            '--disk-format', 'ami',
             '--min-disk', '10',
             '--min-ram', '4',
             ('--protected'
@@ -143,7 +143,7 @@ class TestImageCreate(TestImage):
         ]
         verifylist = [
             ('container_format', 'ovf'),
-            ('disk_format', 'fs'),
+            ('disk_format', 'ami'),
             ('min_disk', 10),
             ('min_ram', 4),
             ('protected', self.new_image.protected),
@@ -165,7 +165,7 @@ class TestImageCreate(TestImage):
         self.images_mock.create.assert_called_with(
             name=self.new_image.name,
             container_format='ovf',
-            disk_format='fs',
+            disk_format='ami',
             min_disk=10,
             min_ram=4,
             owner=self.project.id,
@@ -193,7 +193,7 @@ class TestImageCreate(TestImage):
 
         arglist = [
             '--container-format', 'ovf',
-            '--disk-format', 'fs',
+            '--disk-format', 'ami',
             '--min-disk', '10',
             '--min-ram', '4',
             '--owner', 'unexist_owner',
@@ -203,7 +203,7 @@ class TestImageCreate(TestImage):
         ]
         verifylist = [
             ('container_format', 'ovf'),
-            ('disk_format', 'fs'),
+            ('disk_format', 'ami'),
             ('min_disk', 10),
             ('min_ram', 4),
             ('owner', 'unexist_owner'),
@@ -227,7 +227,7 @@ class TestImageCreate(TestImage):
 
         arglist = [
             '--container-format', 'ovf',
-            '--disk-format', 'fs',
+            '--disk-format', 'ami',
             '--min-disk', '10',
             '--min-ram', '4',
             '--protected',
@@ -237,7 +237,7 @@ class TestImageCreate(TestImage):
         ]
         verifylist = [
             ('container_format', 'ovf'),
-            ('disk_format', 'fs'),
+            ('disk_format', 'ami'),
             ('min_disk', 10),
             ('min_ram', 4),
             ('protected', True),