Switch the deprecated "ironic" CLI to "latest" API version by default

The functional tests were updated to account for the initial state changed
to "enroll" and for new fields appearing in "show" and "update" responses.

Closes-Bug: #1671145
Change-Id: Ida18541fbbc8064868cac0accb6919de08e9f795

7 files changed, 183 insertions, 61 deletions

diff --git a/ironicclient/shell.py b/ironicclient/shell.py
index 25676f1..b1a08cb 100644
--- a/ironicclient/shell.py
+++ b/ironicclient/shell.py
@@ -38,14 +38,8 @@ from ironicclient.common import utils
 from ironicclient import exc
 
 
-LATEST_API_VERSION = ('1', 'latest')
-MISSING_VERSION_WARNING = (
-    "You are using the default API version of the 'ironic' command. "
-    "This is currently API version %s. In the future, the default will be "
-    "the latest API version understood by both API and CLI. You can preserve "
-    "the current behavior by passing the --ironic-api-version argument with "
-    "the desired version or using the IRONIC_API_VERSION environment variable."
-)
+LAST_KNOWN_API_VERSION = 34
+LATEST_VERSION = '1.{}'.format(LAST_KNOWN_API_VERSION)
 
 
 class IronicShell(object):
@@ -161,12 +155,10 @@ class IronicShell(object):
 
         parser.add_argument('--ironic-api-version',
                             default=cliutils.env('IRONIC_API_VERSION',
-                                                 default=None),
-                            help=_('Accepts 1.x (where "x" is microversion) '
-                                   'or "latest". Defaults to '
-                                   'env[IRONIC_API_VERSION] or %s. Starting '
-                                   'with the Queens release this will '
-                                   'default to "latest".') % http.DEFAULT_VER)
+                                                 default="latest"),
+                            help=_('Accepts 1.x (where "x" is microversion), '
+                                   '1 or "latest". Defaults to '
+                                   'env[IRONIC_API_VERSION] or "latest".'))
 
         parser.add_argument('--ironic_api_version',
                             help=argparse.SUPPRESS)
@@ -300,35 +292,31 @@ class IronicShell(object):
         print(' '.join(commands | options))
 
     def _check_version(self, api_version):
-        if api_version == 'latest':
-            return LATEST_API_VERSION
-        else:
-            if api_version is None:
-                print(MISSING_VERSION_WARNING % http.DEFAULT_VER,
-                      file=sys.stderr)
-                api_version = '1'
+        """Validate the supplied API (micro)version.
 
+        :param api_version: API version as a string ("1", "1.x" or "latest")
+        :returns: tuple (major version, version string)
+        """
+        if api_version in ('1', 'latest'):
+            return (1, LATEST_VERSION)
+        else:
             try:
                 versions = tuple(int(i) for i in api_version.split('.'))
             except ValueError:
                 versions = ()
-            if len(versions) == 1:
-                # Default value of ironic_api_version is '1'.
-                # If user not specify the value of api version, not passing
-                # headers at all.
-                os_ironic_api_version = None
-            elif len(versions) == 2:
-                os_ironic_api_version = api_version
-                # In the case of '1.0'
-                if versions[1] == 0:
-                    os_ironic_api_version = None
-            else:
+
+            if not versions or len(versions) > 2:
                 msg = _("The requested API version %(ver)s is an unexpected "
                         "format. Acceptable formats are 'X', 'X.Y', or the "
-                        "literal string '%(latest)s'."
-                        ) % {'ver': api_version, 'latest': 'latest'}
+                        "literal string 'latest'."
+                        ) % {'ver': api_version}
                 raise exc.CommandError(msg)
 
+            if versions == (1, 0):
+                os_ironic_api_version = None
+            else:
+                os_ironic_api_version = api_version
+
             api_major_version = versions[0]
             return (api_major_version, os_ironic_api_version)
 
@@ -422,6 +410,11 @@ class IronicShell(object):
             kwargs[key] = getattr(args, key)
         kwargs['os_ironic_api_version'] = os_ironic_api_version
         client = ironicclient.client.get_client(api_major_version, **kwargs)
+        if options.ironic_api_version in ('1', 'latest'):
+            # Allow negotiating a lower version, if the latest version
+            # supported by the client is higher than the latest version
+            # supported by the server.
+            client.http_client.api_version_select_state = 'default'
 
         try:
             args.func(client, args)
diff --git a/ironicclient/tests/functional/base.py b/ironicclient/tests/functional/base.py
index ab0aefa..7d0a693 100644
--- a/ironicclient/tests/functional/base.py
+++ b/ironicclient/tests/functional/base.py
@@ -216,7 +216,9 @@ class FunctionalTestBase(base.ClientTestBase):
 
         if utils.get_object(node_list, node_id):
             node_show = self.show_node(node_id)
-            if node_show['provision_state'] != 'available':
+            if node_show['provision_state'] not in ('available',
+                                                    'manageable',
+                                                    'enroll'):
                 self.ironic('node-set-provision-state',
                             params='{0} deleted'.format(node_id))
             if node_show['power_state'] not in ('None', 'off'):
diff --git a/ironicclient/tests/functional/test_help_msg.py b/ironicclient/tests/functional/test_help_msg.py
index 07b094b..39bde41 100644
--- a/ironicclient/tests/functional/test_help_msg.py
+++ b/ironicclient/tests/functional/test_help_msg.py
@@ -67,11 +67,3 @@ class IronicClientHelp(base.FunctionalTestBase):
         self.assertIn(caption, output)
         for string in subcommands:
             self.assertIn(string, output)
-
-    def test_warning_on_api_version(self):
-        result = self._ironic('help', merge_stderr=True)
-        self.assertIn('You are using the default API version', result)
-
-        result = self._ironic('help', flags='--ironic-api-version 1.9',
-                              merge_stderr=True)
-        self.assertNotIn('You are using the default API version', result)
diff --git a/ironicclient/tests/functional/test_json_response.py b/ironicclient/tests/functional/test_json_response.py
index ad1240f..dc3fda8 100644
--- a/ironicclient/tests/functional/test_json_response.py
+++ b/ironicclient/tests/functional/test_json_response.py
@@ -48,10 +48,10 @@ class TestNodeJsonResponse(base.FunctionalTestBase):
             "uuid": {"type": "string"},
             "console_enabled": {"type": "boolean"},
             "target_provision_state": {"type": ["string", "null"]},
-            "raid_config": {"type": "string"},
+            "raid_config": {"type": "object"},
             "provision_updated_at": {"type": ["string", "null"]},
             "maintenance": {"type": "boolean"},
-            "target_raid_config": {"type": "string"},
+            "target_raid_config": {"type": "object"},
             "inspection_started_at": {"type": ["string", "null"]},
             "inspection_finished_at": {"type": ["string", "null"]},
             "power_state": {"type": ["string", "null"]},
@@ -65,8 +65,12 @@ class TestNodeJsonResponse(base.FunctionalTestBase):
             "driver_internal_info": {"type": "object"},
             "chassis_uuid": {"type": ["string", "null"]},
             "instance_info": {"type": "object"}
-            }
-        }
+        },
+        "patternProperties": {
+            ".*_interface$": {"type": ["string", "null"]}
+        },
+        "additionalProperties": True
+    }
 
     def setUp(self):
         super(TestNodeJsonResponse, self).setUp()
diff --git a/ironicclient/tests/functional/test_node.py b/ironicclient/tests/functional/test_node.py
index a512b7f..5466225 100644
--- a/ironicclient/tests/functional/test_node.py
+++ b/ironicclient/tests/functional/test_node.py
@@ -161,18 +161,21 @@ class NodeSanityTestIronicClient(base.FunctionalTestBase):
         """Test steps:
 
         1) create node
-        2) check that provision state is 'available'
+        2) check that provision state is 'enroll'
         3) set new provision state to the node
         4) check that provision state has been updated successfully
         """
         node_show = self.show_node(self.node['uuid'])
 
-        self.assertEqual('available', node_show['provision_state'])
+        self.assertEqual('enroll', node_show['provision_state'])
 
-        self.set_node_provision_state(self.node['uuid'], 'active')
-        node_show = self.show_node(self.node['uuid'])
-
-        self.assertEqual('active', node_show['provision_state'])
+        for verb, target in [('manage', 'manageable'),
+                             ('provide', 'available'),
+                             ('active', 'active'),
+                             ('deleted', 'available')]:
+            self.set_node_provision_state(self.node['uuid'], verb)
+            node_show = self.show_node(self.node['uuid'])
+            self.assertEqual(target, node_show['provision_state'])
 
     def test_node_validate(self):
         """Test steps:
diff --git a/ironicclient/tests/unit/test_shell.py b/ironicclient/tests/unit/test_shell.py
index 29865a7..38d0de9 100644
--- a/ironicclient/tests/unit/test_shell.py
+++ b/ironicclient/tests/unit/test_shell.py
@@ -174,7 +174,8 @@ class ShellTest(utils.BaseTestCase):
             'os_cert': None, 'os_key': None,
             'max_retries': http.DEFAULT_MAX_RETRIES,
             'retry_interval': http.DEFAULT_RETRY_INTERVAL,
-            'os_ironic_api_version': None, 'timeout': 600, 'insecure': False
+            'os_ironic_api_version': ironic_shell.LATEST_VERSION,
+            'timeout': 600, 'insecure': False
         }
         mock_client.assert_called_once_with(1, **expected_kwargs)
         # Make sure we are actually prompted.
@@ -203,7 +204,8 @@ class ShellTest(utils.BaseTestCase):
             'os_endpoint_type': '', 'os_cacert': None, 'os_cert': None,
             'os_key': None, 'max_retries': http.DEFAULT_MAX_RETRIES,
             'retry_interval': http.DEFAULT_RETRY_INTERVAL,
-            'os_ironic_api_version': None, 'timeout': 600, 'insecure': False
+            'os_ironic_api_version': ironic_shell.LATEST_VERSION,
+            'timeout': 600, 'insecure': False
         }
         mock_client.assert_called_once_with(1, **expected_kwargs)
         self.assertFalse(mock_getpass.called)
@@ -254,17 +256,118 @@ class ShellTest(utils.BaseTestCase):
         err = self.shell('--ironic-api-version latest help')[1]
         self.assertIn('The "ironic" CLI is deprecated', err)
 
-        self.assertRaises(exc.CommandError,
-                          self.shell, '--ironic-api-version 1.2.1 help')
+        err = self.shell('--ironic-api-version 1 help')[1]
+        self.assertIn('The "ironic" CLI is deprecated', err)
 
     def test_invalid_ironic_api_version(self):
         self.assertRaises(exceptions.UnsupportedVersion,
                           self.shell, '--ironic-api-version 0.8 help')
+        self.assertRaises(exc.CommandError,
+                          self.shell, '--ironic-api-version 1.2.1 help')
 
-    def test_warning_on_no_version(self):
-        err = self.shell('help')[1]
-        self.assertIn('You are using the default API version', err)
-        self.assertIn('The "ironic" CLI is deprecated', err)
+    @mock.patch.object(client, 'get_client', autospec=True,
+                       side_effect=keystone_exc.ConnectFailure)
+    def test_api_version_in_env(self, mock_client):
+        env = dict(IRONIC_API_VERSION='1.10', **FAKE_ENV)
+        self.make_env(environ_dict=env)
+        # We will get a ConnectFailure because there is no keystone.
+        self.assertRaises(keystone_exc.ConnectFailure,
+                          self.shell, 'node-list')
+        expected_kwargs = {
+            'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'],
+            'os_tenant_id': '', 'os_tenant_name': '',
+            'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '',
+            'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'],
+            'os_auth_token': '', 'os_project_id': '',
+            'os_project_name': FAKE_ENV['OS_PROJECT_NAME'],
+            'os_project_domain_id': '',
+            'os_project_domain_name': '', 'os_region_name': '',
+            'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None,
+            'os_cert': None, 'os_key': None,
+            'max_retries': http.DEFAULT_MAX_RETRIES,
+            'retry_interval': http.DEFAULT_RETRY_INTERVAL,
+            'os_ironic_api_version': '1.10',
+            'timeout': 600, 'insecure': False
+        }
+        mock_client.assert_called_once_with(1, **expected_kwargs)
+
+    @mock.patch.object(client, 'get_client', autospec=True,
+                       side_effect=keystone_exc.ConnectFailure)
+    def test_api_version_v1_in_env(self, mock_client):
+        env = dict(IRONIC_API_VERSION='1', **FAKE_ENV)
+        self.make_env(environ_dict=env)
+        # We will get a ConnectFailure because there is no keystone.
+        self.assertRaises(keystone_exc.ConnectFailure,
+                          self.shell, 'node-list')
+        expected_kwargs = {
+            'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'],
+            'os_tenant_id': '', 'os_tenant_name': '',
+            'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '',
+            'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'],
+            'os_auth_token': '', 'os_project_id': '',
+            'os_project_name': FAKE_ENV['OS_PROJECT_NAME'],
+            'os_project_domain_id': '',
+            'os_project_domain_name': '', 'os_region_name': '',
+            'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None,
+            'os_cert': None, 'os_key': None,
+            'max_retries': http.DEFAULT_MAX_RETRIES,
+            'retry_interval': http.DEFAULT_RETRY_INTERVAL,
+            'os_ironic_api_version': ironic_shell.LATEST_VERSION,
+            'timeout': 600, 'insecure': False
+        }
+        mock_client.assert_called_once_with(1, **expected_kwargs)
+
+    @mock.patch.object(client, 'get_client', autospec=True,
+                       side_effect=keystone_exc.ConnectFailure)
+    def test_api_version_in_args(self, mock_client):
+        env = dict(IRONIC_API_VERSION='1.10', **FAKE_ENV)
+        self.make_env(environ_dict=env)
+        # We will get a ConnectFailure because there is no keystone.
+        self.assertRaises(keystone_exc.ConnectFailure,
+                          self.shell, '--ironic-api-version 1.11 node-list')
+        expected_kwargs = {
+            'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'],
+            'os_tenant_id': '', 'os_tenant_name': '',
+            'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '',
+            'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'],
+            'os_auth_token': '', 'os_project_id': '',
+            'os_project_name': FAKE_ENV['OS_PROJECT_NAME'],
+            'os_project_domain_id': '',
+            'os_project_domain_name': '', 'os_region_name': '',
+            'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None,
+            'os_cert': None, 'os_key': None,
+            'max_retries': http.DEFAULT_MAX_RETRIES,
+            'retry_interval': http.DEFAULT_RETRY_INTERVAL,
+            'os_ironic_api_version': '1.11',
+            'timeout': 600, 'insecure': False
+        }
+        mock_client.assert_called_once_with(1, **expected_kwargs)
+
+    @mock.patch.object(client, 'get_client', autospec=True,
+                       side_effect=keystone_exc.ConnectFailure)
+    def test_api_version_v1_in_args(self, mock_client):
+        env = dict(IRONIC_API_VERSION='1.10', **FAKE_ENV)
+        self.make_env(environ_dict=env)
+        # We will get a ConnectFailure because there is no keystone.
+        self.assertRaises(keystone_exc.ConnectFailure,
+                          self.shell, '--ironic-api-version 1 node-list')
+        expected_kwargs = {
+            'ironic_url': '', 'os_auth_url': FAKE_ENV['OS_AUTH_URL'],
+            'os_tenant_id': '', 'os_tenant_name': '',
+            'os_username': FAKE_ENV['OS_USERNAME'], 'os_user_domain_id': '',
+            'os_user_domain_name': '', 'os_password': FAKE_ENV['OS_PASSWORD'],
+            'os_auth_token': '', 'os_project_id': '',
+            'os_project_name': FAKE_ENV['OS_PROJECT_NAME'],
+            'os_project_domain_id': '',
+            'os_project_domain_name': '', 'os_region_name': '',
+            'os_service_type': '', 'os_endpoint_type': '', 'os_cacert': None,
+            'os_cert': None, 'os_key': None,
+            'max_retries': http.DEFAULT_MAX_RETRIES,
+            'retry_interval': http.DEFAULT_RETRY_INTERVAL,
+            'os_ironic_api_version': ironic_shell.LATEST_VERSION,
+            'timeout': 600, 'insecure': False
+        }
+        mock_client.assert_called_once_with(1, **expected_kwargs)
 
 
 class TestCase(testtools.TestCase):
diff --git a/releasenotes/notes/ironic-cli-version-a5cdec73d585444d.yaml b/releasenotes/notes/ironic-cli-version-a5cdec73d585444d.yaml
new file mode 100644
index 0000000..22edecb
--- /dev/null
+++ b/releasenotes/notes/ironic-cli-version-a5cdec73d585444d.yaml
@@ -0,0 +1,25 @@
+---
+upgrade:
+  - |
+    The default API version for the ``ironic`` command is now "latest", which
+    is the maximum version understood by both the client and the server.
+    This change makes the CLI automatically pull in new features and changes
+    (including potentially breaking), when talking to new servers.
+
+    Scripts that rely on some specific API behavior should set the
+    ``IRONIC_API_VERSION`` environment variable or use the
+    ``--ironic-api-version`` CLI argument.
+
+    .. note:: This change does not affect the Python API.
+features:
+  - |
+    The ``ironic`` command now supports the specification of API version ``1``.
+    The actual version used will be the maximum 1.x version understood by both
+    the client and the server.  Thus, it is currently identical to the
+    ``latest`` value.
+fixes:
+  - |
+    Users of the ``ironic`` command no longer have to specify an explicit
+    API version to use the latest features. The default API version is now
+    "latest", which is the maximum version understood by both the client
+    and the server.