Merge "Option '--env' should be required for listing OpenStack configs"

fuelclient/cli/actions/openstack_config.py

@@ -55,16 +55,14 @@ class OpenstackConfigAction(Action):
             ('execute', self.execute)
         )
 
+    @check_all('env')
     def list(self, params):
         """List all available configurations:
             fuel openstack-config --list --env 1
             fuel openstack-config --list --env 1 --node 1
             fuel openstack-config --list --env 1 --deleted
         """
-        filters = {}
-
-        if 'env' in params:
-            filters['cluster_id'] = params.env
+        filters = {'cluster_id': params.env}
 
         if 'deleted' in params:
             filters['is_active'] = int(not params.deleted)

fuelclient/commands/openstack_config.py

@@ -21,10 +21,10 @@ class OpenstackConfigMixin(object):
     entity_name = 'openstack-config'
 
     @staticmethod
-    def add_env_arg(parser, required=False):
+    def add_env_arg(parser):
         parser.add_argument(
             '-e', '--env',
-            type=int, required=required,
+            type=int, required=True,
             help='Environment ID.')
 
     @staticmethod
@@ -116,7 +116,7 @@ class OpenstackConfigUpload(OpenstackConfigMixin, base.BaseCommand):
     def get_parser(self, prog_name):
         parser = super(OpenstackConfigUpload, self).get_parser(prog_name)
 
-        self.add_env_arg(parser, required=True)
+        self.add_env_arg(parser)
         self.add_node_id_arg(parser)
         self.add_node_role_arg(parser)
         self.add_file_arg(parser)
@@ -140,7 +140,7 @@ class OpenstackConfigExecute(OpenstackConfigMixin, base.BaseCommand):
     def get_parser(self, prog_name):
         parser = super(OpenstackConfigExecute, self).get_parser(prog_name)
 
-        self.add_env_arg(parser, required=True)
+        self.add_env_arg(parser)
         self.add_node_id_arg(parser)
         self.add_node_role_arg(parser)
 

fuelclient/tests/unit/v1/test_openstack_config.py

@@ -113,6 +113,14 @@ class TestOpenstackConfigActions(base.UnitTestCase):
                       '--node', '42', '--list'])
         self.assertTrue(m_get.called)
 
+    @mock.patch('sys.stderr')
+    def test_config_list_fail(self, m_stderr):
+        self.assertRaises(
+            SystemExit,
+            self.execute, ['fuel', 'openstack-config', '--list'])
+        m_stderr.write.assert_called_once_with(
+            '"--env" required!\n')
+
     def test_config_delete(self):
         m_del = self.m_request.delete(
             '/api/v1/openstack-config/42/', json={})

fuelclient/tests/unit/v2/cli/test_openstack_config.py

@@ -54,6 +54,13 @@ class TestOpenstackConfig(test_engine.BaseCLITest):
                              'node_role': None, 'is_active': True}
         )
 
+    @mock.patch('sys.stderr')
+    def test_config_list_for_cluster_fail(self, mocked_stderr):
+        self.assertRaises(SystemExit,
+                          self.exec_command, 'openstack-config list')
+        self.assertIn('-e/--env',
+                      mocked_stderr.write.call_args_list[-1][0][0])
+
     def test_config_upload(self):
         self.m_client.upload.return_value = 'config.yaml'
 
@@ -93,8 +100,6 @@ class TestOpenstackConfig(test_engine.BaseCLITest):
     @mock.patch('sys.stderr')
     def test_config_download_fail(self, mocked_stderr):
         cmd = 'openstack-config download 1'
-        self.assertRaises(SystemExit, self.exec_command, cmd)
-
         self.assertRaises(SystemExit, self.exec_command, cmd)
         self.assertIn('-f/--file',
                       mocked_stderr.write.call_args_list[-1][0][0])