From da7eda66e96a72e7f51a9c6bebb4722503cc9662 Mon Sep 17 00:00:00 2001
From: Stephen Finucane <stephenfin@redhat.com>
Date: Wed, 10 Jul 2024 12:05:06 +0100
Subject: [PATCH] quota: Default network quotas to not force

The existing default behavior has been deprecated for over a 18 months
(change I25828a3d68e2e900f498e17a0d01fb70be77548e). It's time for a new
default.

Change-Id: Iaf4fa931dcbf16c22933f63629c6a4d443ac5310
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
---
 openstackclient/common/quota.py               | 47 +++++++++----------
 .../tests/functional/common/test_quota.py     |  4 +-
 .../tests/unit/common/test_quota.py           |  4 +-
 ...ota-no-force-default-0975bdf15655070c.yaml |  6 +++
 4 files changed, 33 insertions(+), 28 deletions(-)
 create mode 100644 releasenotes/notes/network-quota-no-force-default-0975bdf15655070c.yaml

diff --git a/openstackclient/common/quota.py b/openstackclient/common/quota.py
index abbbe3cdb0..50324f865a 100644
--- a/openstackclient/common/quota.py
+++ b/openstackclient/common/quota.py
@@ -20,6 +20,7 @@ import logging
 import sys
 
 from osc_lib.command import command
+from osc_lib import exceptions
 from osc_lib import utils
 
 from openstackclient.i18n import _
@@ -506,22 +507,19 @@ class SetQuota(common.NetDetectionMixin, command.Command):
             '--force',
             action='store_true',
             dest='force',
-            # TODO(stephenfin): Change the default to False in Z or later
-            default=None,
+            default=False,
             help=_(
-                'Force quota update (only supported by compute and network) '
-                '(default for network)'
+                'Force quota update (only supported by compute and network)'
             ),
         )
         force_group.add_argument(
             '--no-force',
             action='store_false',
             dest='force',
-            default=None,
+            default=False,
             help=_(
                 'Do not force quota update '
-                '(only supported by compute and network) '
-                '(default for compute)'
+                '(only supported by compute and network) (default)'
             ),
         )
         # kept here for backwards compatibility/to keep the neutron folks happy
@@ -529,7 +527,7 @@ class SetQuota(common.NetDetectionMixin, command.Command):
             '--check-limit',
             action='store_false',
             dest='force',
-            default=None,
+            default=False,
             help=argparse.SUPPRESS,
         )
         return parser
@@ -545,6 +543,12 @@ class SetQuota(common.NetDetectionMixin, command.Command):
             )
             self.log.warning(msg)
 
+        if (
+            parsed_args.quota_class or parsed_args.default
+        ) and parsed_args.force:
+            msg = _('--force cannot be used with --class or --default')
+            raise exceptions.CommandError(msg)
+
         compute_client = self.app.client_manager.compute
         volume_client = self.app.client_manager.volume
 
@@ -554,7 +558,7 @@ class SetQuota(common.NetDetectionMixin, command.Command):
             if value is not None:
                 compute_kwargs[k] = value
 
-        if parsed_args.force is not None:
+        if parsed_args.force is True:
             compute_kwargs['force'] = parsed_args.force
 
         volume_kwargs = {}
@@ -566,22 +570,6 @@ class SetQuota(common.NetDetectionMixin, command.Command):
                 volume_kwargs[k] = value
 
         network_kwargs = {}
-        if parsed_args.force is True:
-            # Unlike compute, network doesn't provide a simple boolean option.
-            # Instead, it provides two options: 'force' and 'check_limit'
-            # (a.k.a. 'not force')
-            network_kwargs['force'] = True
-        elif parsed_args.force is False:
-            network_kwargs['check_limit'] = True
-        else:
-            msg = _(
-                "This command currently defaults to '--force' when modifying "
-                "network quotas. This behavior will change in a future "
-                "release. Consider explicitly providing '--force' or "
-                "'--no-force' options to avoid changes in behavior."
-            )
-            self.log.warning(msg)
-
         if self.app.client_manager.is_network_endpoint_enabled():
             for k, v in NETWORK_QUOTAS.items():
                 value = getattr(parsed_args, k, None)
@@ -593,6 +581,15 @@ class SetQuota(common.NetDetectionMixin, command.Command):
                 if value is not None:
                     compute_kwargs[k] = value
 
+        if network_kwargs:
+            if parsed_args.force is True:
+                # Unlike compute, network doesn't provide a simple boolean
+                # option. Instead, it provides two options: 'force' and
+                # 'check_limit' (a.k.a. 'not force')
+                network_kwargs['force'] = True
+            else:
+                network_kwargs['check_limit'] = True
+
         if parsed_args.quota_class or parsed_args.default:
             if compute_kwargs:
                 compute_client.quota_classes.update(
diff --git a/openstackclient/tests/functional/common/test_quota.py b/openstackclient/tests/functional/common/test_quota.py
index e46d237dbe..8aa93a9294 100644
--- a/openstackclient/tests/functional/common/test_quota.py
+++ b/openstackclient/tests/functional/common/test_quota.py
@@ -137,7 +137,7 @@ class QuotaTests(base.TestCase):
     def _restore_quota_limit(self, resource, limit, project):
         self.openstack(f'quota set --{resource} {limit} {project}')
 
-    def test_quota_set_network_with_no_force(self):
+    def test_quota_set_network(self):
         if not self.haz_network:
             self.skipTest('No Network service present')
         if not self.is_extension_enabled('quota-check-limit'):
@@ -172,7 +172,7 @@ class QuotaTests(base.TestCase):
         self.assertRaises(
             exceptions.CommandFailed,
             self.openstack,
-            'quota set --networks 1 --no-force ' + self.PROJECT_NAME,
+            'quota set --networks 1 ' + self.PROJECT_NAME,
         )
 
     def test_quota_set_network_with_force(self):
diff --git a/openstackclient/tests/unit/common/test_quota.py b/openstackclient/tests/unit/common/test_quota.py
index 78c84e163c..be85d27b2a 100644
--- a/openstackclient/tests/unit/common/test_quota.py
+++ b/openstackclient/tests/unit/common/test_quota.py
@@ -522,6 +522,7 @@ class TestQuotaSet(TestQuota):
             ('security_groups', compute_fakes.secgroup_num),
             ('server_groups', compute_fakes.servgroup_num),
             ('server_group_members', compute_fakes.servgroup_members_num),
+            ('force', False),
             ('project', self.projects[0].name),
         ]
         self.app.client_manager.network_endpoint_enabled = False
@@ -682,12 +683,14 @@ class TestQuotaSet(TestQuota):
             ('router', network_fakes.QUOTA['router']),
             ('rbac_policy', network_fakes.QUOTA['rbac_policy']),
             ('port', network_fakes.QUOTA['port']),
+            ('force', False),
             ('project', self.projects[0].name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
         kwargs = {
+            'check_limit': True,
             'subnet': network_fakes.QUOTA['subnet'],
             'network': network_fakes.QUOTA['network'],
             'floatingip': network_fakes.QUOTA['floatingip'],
@@ -948,7 +951,6 @@ class TestQuotaSet(TestQuota):
 
         kwargs_compute = {
             'cores': compute_fakes.core_num,
-            'force': False,
         }
         kwargs_volume = {
             'volumes': volume_fakes.QUOTA['volumes'],
diff --git a/releasenotes/notes/network-quota-no-force-default-0975bdf15655070c.yaml b/releasenotes/notes/network-quota-no-force-default-0975bdf15655070c.yaml
new file mode 100644
index 0000000000..e1a1359053
--- /dev/null
+++ b/releasenotes/notes/network-quota-no-force-default-0975bdf15655070c.yaml
@@ -0,0 +1,6 @@
+---
+upgrade:
+  - The ``openstack quota set`` command previously defaulted to ``--force``
+    behavior for network quotas. This behavior has now changed and the command
+    now defaults to ``--no-force`` behavior. Users should specify the
+    ``--force`` option if they wish to retain previous behavior.