From ab0b1fe885ee0a210a58008b631521025be7f3eb Mon Sep 17 00:00:00 2001
From: Stephen Finucane <sfinucan@redhat.com>
Date: Tue, 13 Oct 2020 15:35:21 +0100
Subject: [PATCH] Validate 'server group create --policy' option

We were documenting that some of these policies were only supported with
specific microversions, however, we weren't actually enforcing that,
leading to a poor user experience. Correct this.

Change-Id: Ic3c555226a220efd9b0f27edffccf6c4c95c2747
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
---
 openstackclient/compute/v2/server_group.py    | 25 ++++++++---
 .../unit/compute/v2/test_server_group.py      | 43 +++++++++++++++++++
 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/openstackclient/compute/v2/server_group.py b/openstackclient/compute/v2/server_group.py
index 1af6e28dc7..a33632446c 100644
--- a/openstackclient/compute/v2/server_group.py
+++ b/openstackclient/compute/v2/server_group.py
@@ -56,12 +56,18 @@ class CreateServerGroup(command.ShowOne):
         parser.add_argument(
             '--policy',
             metavar='<policy>',
+            choices=[
+                'affinity',
+                'anti-affinity',
+                'soft-affinity',
+                'soft-anti-affinity',
+            ],
             default='affinity',
-            help=_("Add a policy to <name> "
-                   "('affinity' or 'anti-affinity', "
-                   "defaults to 'affinity'). Specify --os-compute-api-version "
-                   "2.15 or higher for the 'soft-affinity' or "
-                   "'soft-anti-affinity' policy.")
+            help=_(
+                "Add a policy to <name> "
+                "Specify --os-compute-api-version 2.15 or higher for the "
+                "'soft-affinity' or 'soft-anti-affinity' policy."
+            )
         )
         return parser
 
@@ -69,9 +75,18 @@ class CreateServerGroup(command.ShowOne):
         compute_client = self.app.client_manager.compute
         info = {}
 
+        if parsed_args.policy in ('soft-affinity', 'soft-anti-affinity'):
+            if compute_client.api_version < api_versions.APIVersion('2.15'):
+                msg = _(
+                    '--os-compute-api-version 2.15 or greater is required to '
+                    'support the %s policy'
+                )
+                raise exceptions.CommandError(msg % parsed_args.policy)
+
         policy_arg = {'policies': [parsed_args.policy]}
         if compute_client.api_version >= api_versions.APIVersion("2.64"):
             policy_arg = {'policy': parsed_args.policy}
+
         server_group = compute_client.server_groups.create(
             name=parsed_args.name, **policy_arg)
 
diff --git a/openstackclient/tests/unit/compute/v2/test_server_group.py b/openstackclient/tests/unit/compute/v2/test_server_group.py
index 359cd2bd02..bf0ea0ba45 100644
--- a/openstackclient/tests/unit/compute/v2/test_server_group.py
+++ b/openstackclient/tests/unit/compute/v2/test_server_group.py
@@ -90,6 +90,28 @@ class TestServerGroupCreate(TestServerGroup):
         self.cmd = server_group.CreateServerGroup(self.app, None)
 
     def test_server_group_create(self):
+        arglist = [
+            '--policy', 'anti-affinity',
+            'affinity_group',
+        ]
+        verifylist = [
+            ('policy', 'anti-affinity'),
+            ('name', 'affinity_group'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+        self.server_groups_mock.create.assert_called_once_with(
+            name=parsed_args.name,
+            policies=[parsed_args.policy],
+        )
+
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.data, data)
+
+    def test_server_group_create_with_soft_policies(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.15')
+
         arglist = [
             '--policy', 'soft-anti-affinity',
             'affinity_group',
@@ -108,6 +130,27 @@ class TestServerGroupCreate(TestServerGroup):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)
 
+    def test_server_group_create_with_soft_policies_pre_v215(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.14')
+
+        arglist = [
+            '--policy', 'soft-anti-affinity',
+            'affinity_group',
+        ]
+        verifylist = [
+            ('policy', 'soft-anti-affinity'),
+            ('name', 'affinity_group'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        ex = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+        self.assertIn(
+            '--os-compute-api-version 2.15 or greater is required',
+            str(ex))
+
     def test_server_group_create_v264(self):
         self.app.client_manager.compute.api_version = api_versions.APIVersion(
             '2.64')