diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 355774c316..a6d645b9e9 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -17,6 +17,7 @@ import argparse import getpass +import io import logging import os import six @@ -296,7 +297,7 @@ class CreateServer(show.ShowOne): for f in parsed_args.file: dst, src = f.split('=', 1) try: - files[dst] = open(src) + files[dst] = io.open(src, 'rb') except IOError as e: raise exceptions.CommandError("Can't open '%s': %s" % (src, e)) @@ -313,7 +314,7 @@ class CreateServer(show.ShowOne): userdata = None if parsed_args.user_data: try: - userdata = open(parsed_args.user_data) + userdata = io.open(parsed_args.user_data) except IOError as e: msg = "Can't open '%s': %s" raise exceptions.CommandError(msg % (parsed_args.user_data, e)) @@ -368,7 +369,17 @@ class CreateServer(show.ShowOne): self.log.debug('boot_args: %s', boot_args) self.log.debug('boot_kwargs: %s', boot_kwargs) - server = compute_client.servers.create(*boot_args, **boot_kwargs) + + # Wrap the call to catch exceptions in order to close files + try: + server = compute_client.servers.create(*boot_args, **boot_kwargs) + finally: + # Clean up open files - make sure they are not strings + for f in files: + if hasattr(f, 'close'): + f.close() + if hasattr(userdata, 'close'): + userdata.close() if parsed_args.wait: if utils.wait_for_status( diff --git a/openstackclient/tests/compute/v2/fakes.py b/openstackclient/tests/compute/v2/fakes.py index 9a7964db0d..e5ed5cbd7d 100644 --- a/openstackclient/tests/compute/v2/fakes.py +++ b/openstackclient/tests/compute/v2/fakes.py @@ -26,6 +26,7 @@ server_name = 'waiter' SERVER = { 'id': server_id, 'name': server_name, + 'metadata': {}, } extension_name = 'Multinic' diff --git a/openstackclient/tests/compute/v2/test_server.py b/openstackclient/tests/compute/v2/test_server.py index a98cd15690..50de5c6ab7 100644 --- a/openstackclient/tests/compute/v2/test_server.py +++ b/openstackclient/tests/compute/v2/test_server.py @@ -14,11 +14,13 @@ # import copy +import mock from openstackclient.compute.v2 import server from openstackclient.tests.compute.v2 import fakes as compute_fakes from openstackclient.tests import fakes from openstackclient.tests.image.v2 import fakes as image_fakes +from openstackclient.tests import utils class TestServer(compute_fakes.TestComputev2): @@ -30,6 +32,10 @@ class TestServer(compute_fakes.TestComputev2): self.servers_mock = self.app.client_manager.compute.servers self.servers_mock.reset_mock() + # Get a shortcut to the ImageManager Mock + self.cimages_mock = self.app.client_manager.compute.images + self.cimages_mock.reset_mock() + # Get a shortcut to the FlavorManager Mock self.flavors_mock = self.app.client_manager.compute.flavors self.flavors_mock.reset_mock() @@ -39,6 +45,174 @@ class TestServer(compute_fakes.TestComputev2): self.images_mock.reset_mock() +class TestServerCreate(TestServer): + + def setUp(self): + super(TestServerCreate, self).setUp() + + self.servers_mock.create.return_value = fakes.FakeResource( + None, + copy.deepcopy(compute_fakes.SERVER), + loaded=True, + ) + new_server = fakes.FakeResource( + None, + copy.deepcopy(compute_fakes.SERVER), + loaded=True, + ) + new_server.__dict__['networks'] = {} + self.servers_mock.get.return_value = new_server + + self.image = fakes.FakeResource( + None, + copy.deepcopy(image_fakes.IMAGE), + loaded=True, + ) + self.cimages_mock.get.return_value = self.image + + self.flavor = fakes.FakeResource( + None, + copy.deepcopy(compute_fakes.FLAVOR), + loaded=True, + ) + self.flavors_mock.get.return_value = self.flavor + + # Get the command object to test + self.cmd = server.CreateServer(self.app, None) + + def test_server_create_no_options(self): + arglist = [ + compute_fakes.server_id, + ] + verifylist = [ + ('server_name', compute_fakes.server_id), + ] + try: + # Missing required args should bail here + self.check_parser(self.cmd, arglist, verifylist) + except utils.ParserException: + pass + + def test_server_create_minimal(self): + arglist = [ + '--image', 'image1', + '--flavor', 'flavor1', + compute_fakes.server_id, + ] + verifylist = [ + ('image', 'image1'), + ('flavor', 'flavor1'), + ('config_drive', False), + ('server_name', compute_fakes.server_id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + # DisplayCommandBase.take_action() returns two tuples + 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={}, + nics=[], + scheduler_hints={}, + config_drive=None, + ) + # ServerManager.create(name, image, flavor, **kwargs) + self.servers_mock.create.assert_called_with( + compute_fakes.server_id, + self.image, + self.flavor, + **kwargs + ) + + collist = ('addresses', 'flavor', 'id', 'image', 'name', 'properties') + self.assertEqual(columns, collist) + datalist = ( + '', + 'Large ()', + compute_fakes.server_id, + 'graven ()', + compute_fakes.server_name, + '', + ) + self.assertEqual(data, datalist) + + @mock.patch('openstackclient.compute.v2.server.io.open') + def test_server_create_userdata(self, mock_open): + mock_file = mock.MagicMock(name='File') + mock_open.return_value = mock_file + mock_open.read.return_value = '#!/bin/sh' + + arglist = [ + '--image', 'image1', + '--flavor', 'flavor1', + '--user-data', 'userdata.sh', + compute_fakes.server_id, + ] + verifylist = [ + ('image', 'image1'), + ('flavor', 'flavor1'), + ('user_data', 'userdata.sh'), + ('config_drive', False), + ('server_name', compute_fakes.server_id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + # DisplayCommandBase.take_action() returns two tuples + columns, data = self.cmd.take_action(parsed_args) + + # Ensure the userdata file is opened + mock_open.assert_called_with('userdata.sh') + + # Ensure the userdata file is closed + mock_file.close.assert_called() + + # Set expected values + kwargs = dict( + meta=None, + files={}, + reservation_id=None, + min_count=1, + max_count=1, + security_groups=[], + userdata=mock_file, + key_name=None, + availability_zone=None, + block_device_mapping={}, + nics=[], + scheduler_hints={}, + config_drive=None, + ) + # ServerManager.create(name, image, flavor, **kwargs) + self.servers_mock.create.assert_called_with( + compute_fakes.server_id, + self.image, + self.flavor, + **kwargs + ) + + collist = ('addresses', 'flavor', 'id', 'image', 'name', 'properties') + self.assertEqual(columns, collist) + datalist = ( + '', + 'Large ()', + compute_fakes.server_id, + 'graven ()', + compute_fakes.server_name, + '', + ) + self.assertEqual(data, datalist) + + class TestServerDelete(TestServer): def setUp(self): diff --git a/openstackclient/tests/utils.py b/openstackclient/tests/utils.py index 38d4725038..25f9852516 100644 --- a/openstackclient/tests/utils.py +++ b/openstackclient/tests/utils.py @@ -23,6 +23,10 @@ import testtools from openstackclient.tests import fakes +class ParserException(Exception): + pass + + class TestCase(testtools.TestCase): def setUp(self): testtools.TestCase.setUp(self) @@ -84,7 +88,7 @@ class TestCommand(TestCase): try: parsed_args = cmd_parser.parse_args(args) except SystemExit: - raise Exception("Argument parse failed") + raise ParserException("Argument parse failed") for av in verify_args: attr, value = av if attr: