Default --nic to 'auto' if creating a server with >= 2.37

Compute API version >= 2.37 requires a 'networks' value in
the server create request. The novaclient CLI defaults this
to 'auto' if not specified, but the novaclient ServerManager.create
python API binding code does not, as it wants clients to be explicit.

For the purposes of the OSC CLI, we should follow suit and if the
user is requesting OS_COMPUTE_API_VERSION>=2.37 without specific
nics, we should just default to 'auto'.

Change-Id: Ib760c55e31209223338a4086ff1f4fee88dc6959
Closes-Bug: #1750395

4 files changed, 68 insertions, 7 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 85c20aee..3341fbfe 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -22,6 +22,7 @@ import logging
 import os
 import sys
 
+from novaclient import api_versions
 from novaclient.v2 import servers
 from osc_lib.cli import parseractions
 from osc_lib.command import command
@@ -754,9 +755,14 @@ class CreateServer(command.ShowOne):
                     raise exceptions.CommandError(msg)
                 nics = nics[0]
         else:
-            # Default to empty list if nothing was specified, let nova side to
-            # decide the default behavior.
-            nics = []
+            # Compute API version >= 2.37 requires a value, so default to
+            # 'auto' to maintain legacy behavior if a nic wasn't specified.
+            if compute_client.api_version >= api_versions.APIVersion('2.37'):
+                nics = 'auto'
+            else:
+                # Default to empty list if nothing was specified, let nova
+                # side to decide the default behavior.
+                nics = []
 
         # Check security group exist and convert ID to name
         security_group_names = []
diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py
index 0b29fe5f..13fdfa06 100644
--- a/openstackclient/tests/functional/compute/v2/test_server.py
+++ b/openstackclient/tests/functional/compute/v2/test_server.py
@@ -586,7 +586,9 @@ class ServerTests(common.ComputeTestCase):
                 server_name
             )
         except exceptions.CommandFailed as e:
-            self.assertIn('nics are required after microversion 2.36',
-                          e.stderr)
-        else:
-            self.fail('CommandFailed should be raised.')
+            # If we got here, it shouldn't be because a nics value wasn't
+            # provided to the server; it is likely due to something else in
+            # the functional tests like there being multiple available
+            # networks and the test didn't specify a specific network.
+            self.assertNotIn('nics are required after microversion 2.36',
+                             e.stderr)
diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py
index 1ec71785..1d97cecb 100644
--- a/openstackclient/tests/unit/compute/v2/fakes.py
+++ b/openstackclient/tests/unit/compute/v2/fakes.py
@@ -17,6 +17,7 @@ import copy
 import uuid
 
 import mock
+from novaclient import api_versions
 
 from openstackclient.api import compute_v2
 from openstackclient.tests.unit import fakes
@@ -198,6 +199,8 @@ class FakeComputev2Client(object):
 
         self.management_url = kwargs['endpoint']
 
+        self.api_version = api_versions.APIVersion('2.1')
+
 
 class TestComputev2(utils.TestCommand):
 
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 87c9a985..8f82e7cd 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -18,6 +18,7 @@ import getpass
 
 import mock
 from mock import call
+from novaclient import api_versions
 from osc_lib import exceptions
 from osc_lib import utils as common_utils
 from oslo_utils import timeutils
@@ -844,6 +845,55 @@ class TestServerCreate(TestServer):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.datalist(), data)
 
+    def test_server_create_with_auto_network_default_v2_37(self):
+        """Tests creating a server without specifying --nic using 2.37."""
+        arglist = [
+            '--image', 'image1',
+            '--flavor', 'flavor1',
+            self.new_server.name,
+        ]
+        verifylist = [
+            ('image', 'image1'),
+            ('flavor', 'flavor1'),
+            ('config_drive', False),
+            ('server_name', self.new_server.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # Since check_parser doesn't handle compute global options like
+        # --os-compute-api-version, we have to mock the construction of
+        # the novaclient client object with our own APIVersion.
+        with mock.patch.object(self.app.client_manager.compute, 'api_version',
+                               api_versions.APIVersion('2.37')):
+            columns, data = self.cmd.take_action(parsed_args)
+
+        # Set expected values
+        kwargs = dict(
+            meta=None,
+            files={},
+            reservation_id=None,
+            min_count=1,
+            max_count=1,
+            security_groups=[],
+            userdata=None,
+            key_name=None,
+            availability_zone=None,
+            block_device_mapping_v2=[],
+            nics='auto',
+            scheduler_hints={},
+            config_drive=None,
+        )
+        # ServerManager.create(name, image, flavor, **kwargs)
+        self.servers_mock.create.assert_called_with(
+            self.new_server.name,
+            self.image,
+            self.flavor,
+            **kwargs
+        )
+
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.datalist(), data)
+
     def test_server_create_with_none_network(self):
         arglist = [
             '--image', 'image1',