[lxc-devel] [nova-lxd/master] Code tidyness
zulcss on Github
lxc-bot at linuxcontainers.org
Mon May 30 22:20:27 UTC 2016
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 713 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160530/8448d3a0/attachment.bin>
-------------- next part --------------
From b6fa96898f13bba09eb6b7bc5a99a1ec1535538f Mon Sep 17 00:00:00 2001
From: Chuck Short <chuck.short at canonical.com>
Date: Mon, 30 May 2016 17:38:16 -0400
Subject: [PATCH 1/2] Remove dead crufty unused code
Remove dead unused code.
Signed-off-by: Chuck Short <chuck.short at canonical.com>
---
nova/virt/lxd/session.py | 20 --------------------
nova/virt/lxd/utils.py | 33 ++++-----------------------------
2 files changed, 4 insertions(+), 49 deletions(-)
diff --git a/nova/virt/lxd/session.py b/nova/virt/lxd/session.py
index 3d05e9c..f62ea37 100644
--- a/nova/virt/lxd/session.py
+++ b/nova/virt/lxd/session.py
@@ -40,26 +40,6 @@
CONF = nova.conf.CONF
LOG = logging.getLogger(__name__)
-
-def mount_filesystem(self, dev_path, dir_path):
- try:
- _out, err = utils.execute('mount',
- '-t', 'ext4',
- dev_path, dir_path, run_as_root=True)
- except processutils.ProcessExecutionError as e:
- err = six.text_type(e)
- return err
-
-
-def umount_filesystem(self, dir_path):
- try:
- _out, err = utils.execute('umount',
- dir_path, run_as_root=True)
- except processutils.ProcessExecutionError as e:
- err = six.text_type(e)
- return err
-
-
class LXDAPISession(object):
"""The session to invoke the LXD API session."""
diff --git a/nova/virt/lxd/utils.py b/nova/virt/lxd/utils.py
index 7ad822f..5b1cc20 100644
--- a/nova/virt/lxd/utils.py
+++ b/nova/virt/lxd/utils.py
@@ -41,14 +41,6 @@ def get_container_manifest_image(self, image_meta):
return os.path.join(self.base_dir,
'%s-manifest.tar' % image_meta.id)
- def get_container_metadata(self, image_meta):
- return os.path.join(self.base_dir,
- '%s-lxd.tar.xz' % image_meta.id)
-
- def get_container_rootfsImg(self, image_meta):
- return os.path.join(self.base_dir,
- '%s-root.tar.gz' % image_meta.id)
-
def get_container_configdrive(self, instance):
return os.path.join(CONF.instances_path,
instance,
@@ -70,24 +62,7 @@ def get_container_rootfs(self, instance):
'rootfs')
def get_container_rescue(self, instance):
- if self.is_lvm(instance):
- return os.path.join(CONF.lxd.root_dir,
- 'containers',
- instance)
- else:
- return os.path.join(CONF.lxd.root_dir,
- 'containers',
- instance,
- 'rootfs')
-
- def get_container_lvm(self, instance):
- return '%s/%s.lv' % (self.get_container_dir(instance),
- instance)
-
- def is_lvm(self, instance):
- try:
- if os.path.exists(os.readlink(
- self.get_container_lvm(instance))):
- return True
- except Exception:
- return False
+ return os.path.join(CONF.lxd.root_dir,
+ 'containers',
+ instance,
+ 'rootfs')
From b693e68ac8ef24a9bde94b4f9928f74dd64eb882 Mon Sep 17 00:00:00 2001
From: Chuck Short <chuck.short at canonical.com>
Date: Mon, 30 May 2016 18:07:37 -0400
Subject: [PATCH 2/2] Remove container_defined method usage
In preperation of pylx2.0 emove container_defined method
usage in container operations and pylxd interaction.
Since pylxd 2.0 raises an exception if the container
is not found. So remove contaner_defined unless it
was absolutely necessary.
Also update unit tests to reflect this change.
Signed-off-by: Chuck Short <chuck.short at canonical.com>
---
nova/tests/unit/virt/lxd/test_session.py | 98 +++++++-------------------------
nova/virt/lxd/operations.py | 11 ----
nova/virt/lxd/session.py | 38 -------------
3 files changed, 21 insertions(+), 126 deletions(-)
diff --git a/nova/tests/unit/virt/lxd/test_session.py b/nova/tests/unit/virt/lxd/test_session.py
index e1ff300..96df8f2 100644
--- a/nova/tests/unit/virt/lxd/test_session.py
+++ b/nova/tests/unit/virt/lxd/test_session.py
@@ -84,17 +84,14 @@ def test_container_update(self):
self.assertEqual((200, fake_api.fake_container_config()),
self.session.container_update(config, instance))
calls = [
- mock.call.container_defined(instance.name),
mock.call.container_update(instance.name, config)]
self.assertEqual(calls, self.ml.method_calls)
@stubs.annotated_data(
- ('api_fail', True, lxd_exceptions.APIError('Fake', 500),
+ ('api_fail', lxd_exceptions.APIError('Fake', 500),
exception.NovaException),
- ('missing_container', False, None,
- exception.InstanceNotFound)
)
- def test_container_update_fail(self, tag, container_defined, side_effect,
+ def test_container_update_fail(self, tag, side_effect,
expected):
"""
container_update will fail if the container is not found, or the
@@ -103,18 +100,11 @@ def test_container_update_fail(self, tag, container_defined, side_effect,
"""
config = mock.Mock()
instance = stubs._fake_instance()
- if container_defined:
- self.ml.container_defined.return_value = container_defined
- self.ml.container_update.side_effect = (
- lxd_exceptions.APIError('Fake', 500))
- self.assertRaises(
- expected,
- self.session.container_update, config, instance)
- if not container_defined:
- self.ml.container_defined.return_value = container_defined
- self.assertRaises(
- expected,
- self.session.container_update, config, instance)
+ self.ml.container_update.side_effect = (
+ lxd_exceptions.APIError('Fake', 500))
+ self.assertRaises(
+ expected,
+ self.session.container_update, config, instance)
@stubs.annotated_data(
('running', True),
@@ -266,46 +256,16 @@ def test_container_start(self, tag, defined, side_effect=None):
Verify that the correct pyLXD calls are made.
"""
instance = stubs._fake_instance()
- self.ml.container_defined.return_value = defined
self.ml.container_start.return_value = side_effect
self.assertEqual(None,
self.session.container_start(instance.name,
instance))
- calls = [mock.call.container_defined(instance.name),
- mock.call.container_start(instance.name, -1),
+ calls = [mock.call.container_start(instance.name, -1),
mock.call.wait_container_operation(
'/1.0/operation/1234', 200, -1)]
self.assertEqual(calls, self.ml.method_calls)
@stubs.annotated_data(
- ('container_missing', False,
- exception.InstanceNotFound),
- ('api_error', True,
- exception.NovaException,
- lxd_exceptions.APIError('Fake', 500)),
- )
- def test_container_start_fail(self, tag, container_defined,
- expected, side_effect=None):
- """
- container_start starts a container on a given LXD host.
- Veify that an exception.InstanceNotFound when the container
- is not found on an LXD host. Raises an exception.NovaException
- when there is an APIError.
- """
- instance = stubs._fake_instance()
- if container_defined:
- self.ml.container_defined.return_value = container_defined
- self.ml.container_start.side_effect = side_effect
- self.assertRaises(expected,
- self.session.container_start,
- instance.name, instance)
- if not container_defined:
- self.ml.container_defined.return_value = container_defined
- self.assertRaises(expected,
- self.session.container_start, instance.name,
- instance)
-
- @stubs.annotated_data(
('1', (200, fake_api.fake_operation_info_ok()))
)
def test_container_stop(self, tag, side_effect):
@@ -319,8 +279,7 @@ def test_container_stop(self, tag, side_effect):
self.assertEqual(None,
self.session.container_stop(instance.name,
instance))
- calls = [mock.call.container_defined(instance.name),
- mock.call.container_stop(instance.name, -1),
+ calls = [mock.call.container_stop(instance.name, -1),
mock.call.wait_container_operation(
'/1.0/operation/1234', 200, -1)]
self.assertEqual(calls, self.ml.method_calls)
@@ -353,8 +312,7 @@ def test_continer_reboot(self, tag, side_effect):
self.ml.container_reboot.return_value = side_effect
self.assertEqual(None,
self.session.container_reboot(instance))
- calls = [mock.call.container_defined(instance.name),
- mock.call.container_reboot(instance.name, -1),
+ calls = [mock.call.container_reboot(instance.name, -1),
mock.call.wait_container_operation(
'/1.0/operation/1234', 200, -1)]
self.assertEqual(calls, self.ml.method_calls)
@@ -375,38 +333,26 @@ def test_container_reboot_fail(self, tag, side_effect, expected):
self.session.container_reboot, instance)
@stubs.annotated_data(
- ('exists', True, (200, fake_api.fake_operation_info_ok())),
- ('missing', False, (200, fake_api.fake_operation_info_ok()))
+ ('exists', (200, fake_api.fake_operation_info_ok())),
)
- def test_container_destroy(self, tag, container_defined, side_effect):
+ def test_container_destroy(self, tag, side_effect):
"""
container_destroy delete a container from the LXD Host. Check
that the approiate pylxd calls are made.
"""
instance = stubs._fake_instance()
- if container_defined:
- self.ml.container_defined.return_value = container_defined
- self.ml.container_stop.return_value = side_effect
- self.ml.container_destroy.return_value = side_effect
- self.assertEqual(None,
- self.session.container_destroy(instance.name,
- instance))
- calls = [mock.call.container_defined(instance.name),
- mock.call.container_defined(instance.name),
- mock.call.container_stop(instance.name, -1),
- mock.call.wait_container_operation(
- '/1.0/operation/1234', 200, -1),
+ self.ml.container_stop.return_value = side_effect
+ self.ml.container_destroy.return_value = side_effect
+ self.assertEqual(None,
+ self.session.container_destroy(instance.name,
+ instance))
+ calls = [mock.call.container_stop(instance.name, -1),
+ mock.call.wait_container_operation(
+ '/1.0/operation/1234', 200, -1),
mock.call.container_destroy(instance.name),
mock.call.wait_container_operation(
'/1.0/operation/1234', 200, -1)]
- self.assertEqual(calls, self.ml.method_calls)
- if not container_defined:
- self.ml.container_defined.return_value = container_defined
- self.assertEqual(None,
- self.session.container_destroy(instance.name,
- instance))
- calls = [mock.call.container_defined(instance.name)]
- self.assertEqual(calls, self.ml.method_calls)
+ self.assertEqual(calls, self.ml.method_calls)
@stubs.annotated_data(
('fail_to_stop', True, 'fail_stop',
@@ -429,7 +375,6 @@ def test_container_destroy_fail(self, tag, container_defined,
self.session.container_destroy, instance.name,
instance)
if test_type == 'fail_destroy':
- self.ml.container_defined.return_value = container_defined
self.ml.container_stop.return_value = \
(200, fake_api.fake_operation_info_ok())
self.ml.container_destroy.side_effect = side_effect
@@ -488,7 +433,6 @@ def test_container_unpause(self, tag, side_effect):
self.session.container_unpause(instance.name,
instance))
calls = [
- mock.call.container_defined(instance.name),
mock.call.container_resume(instance.name, -1),
mock.call.wait_container_operation(
'/1.0/operation/1234', 200, -1)]
diff --git a/nova/virt/lxd/operations.py b/nova/virt/lxd/operations.py
index 84fb80c..362b48c 100644
--- a/nova/virt/lxd/operations.py
+++ b/nova/virt/lxd/operations.py
@@ -474,10 +474,6 @@ def rescue(self, context, instance, network_info, image_meta,
"""
LOG.debug('rescue called for instance', instance=instance)
try:
- if not self.session.container_defined(instance.name, instance):
- msg = _('Unable to find instance')
- raise exception.NovaException(msg)
-
# Step 1 - Stop the old container
self.session.container_stop(instance.name, instance)
@@ -513,10 +509,6 @@ def unrescue(self, instance, network_info):
"""
LOG.debug('unrescue called for instance', instance=instance)
try:
- if not self.session.container_defined(instance.name, instance):
- msg = _('Unable to find instance')
- raise exception.NovaException(msg)
-
# Step 1 - Destory the rescue instance.
self.session.container_destroy(instance.name,
instance)
@@ -581,9 +573,6 @@ def get_info(self, instance):
"""
LOG.debug('get_info called for instance', instance=instance)
try:
- if not self.session.container_defined(instance.name, instance):
- return hardware.InstanceInfo(state=power_state.NOSTATE)
-
container_state = self.session.container_state(instance)
return hardware.InstanceInfo(state=container_state['state'],
max_mem_kb=container_state['max_mem'],
diff --git a/nova/virt/lxd/session.py b/nova/virt/lxd/session.py
index f62ea37..58ab87e 100644
--- a/nova/virt/lxd/session.py
+++ b/nova/virt/lxd/session.py
@@ -107,9 +107,6 @@ def container_update(self, config, instance):
LOG.debug('container_update called fo instance', instance=instance)
try:
client = self.get_session()
- if not self.container_defined(instance.name, instance):
- msg = _('Instance is not found: %s') % instance.name
- raise exception.InstanceNotFound(msg)
return client.container_update(instance.name,
config)
@@ -161,8 +158,6 @@ def container_state(self, instance):
max_mem = 0
client = self.get_session()
- if not self.container_defined(instance.name, instance):
- return
(state, data) = client.container_state(instance.name)
state = constants.LXD_POWER_STATES[data['metadata']['status_code']]
@@ -195,10 +190,6 @@ def container_config(self, instance):
"""
LOG.debug('container_config called for instance', instance=instance)
try:
- if not self.container_defined(instance.name, instance):
- msg = _('Instance is not found %s') % instance.name
- raise exception.InstanceNotFound(msg)
-
client = self.get_session()
return client.get_container_config(instance.name)
except lxd_exceptions.APIError as ex:
@@ -222,10 +213,6 @@ def container_info(self, instance):
"""
LOG.debug('container_info called for instance', instance=instance)
try:
- if not self.container_defined(instance.name, instance):
- msg = _('Instance is not found %s') % instance.name
- raise exception.InstanceNotFound(msg)
-
client = self.get_session()
return client.container_info(instance.name)
except lxd_exceptions.APIError as ex:
@@ -280,12 +267,6 @@ def container_start(self, instance_name, instance):
# Start the container
client = self.get_session()
- # (chuck): Something wicked could happen between
- # container
- if not self.container_defined(instance_name, instance):
- msg = _('Instance is not found %s ') % instance.name
- raise exception.InstanceNotFound(msg)
-
(state, data) = client.container_start(instance_name,
CONF.lxd.timeout)
self.operation_wait(data.get('operation'), instance)
@@ -314,10 +295,6 @@ def container_stop(self, instance_name, instance):
"""
LOG.debug('container_stop called for instance', instance=instance)
try:
- if not self.container_defined(instance_name, instance):
- msg = _('Instance is not found %s') % instance.name
- raise exception.InstanceNotFound(msg)
-
LOG.info(_LI('Stopping instance %(instance)s with'
' %(image)s'), {'instance': instance.name,
'image': instance.image_ref})
@@ -350,10 +327,6 @@ def container_reboot(self, instance):
"""
LOG.debug('container_reboot called for instance', instance=instance)
try:
- if not self.container_defined(instance.name, instance):
- msg = _('Instance is not found %s') % instance.name
- raise exception.InstanceNotFound(msg)
-
LOG.info(_LI('Rebooting instance %(instance)s with'
' %(image)s'), {'instance': instance.name,
'image': instance.image_ref})
@@ -388,9 +361,6 @@ def container_destroy(self, instance_name, instance):
"""
LOG.debug('container_destroy for instance', instance=instance)
try:
- if not self.container_defined(instance_name, instance):
- return
-
LOG.info(_LI('Destroying instance %(instance)s with'
' %(image)s'), {'instance': instance.name,
'image': instance.image_ref})
@@ -425,10 +395,6 @@ def container_pause(self, instance_name, instance):
"""
LOG.debug('container_paused called for instance', instance=instance)
try:
- if not self.container_defined(instance_name, instance):
- msg = _('Instance is not found %s') % instance_name
- raise exception.InstanceNotFound(msg)
-
LOG.info(_LI('Pausing instance %(instance)s with'
' %(image)s'), {'instance': instance_name,
'image': instance.image_ref})
@@ -463,10 +429,6 @@ def container_unpause(self, instance_name, instance):
"""
LOG.debug('container_unpause called for instance', instance=instance)
try:
- if not self.container_defined(instance_name, instance):
- msg = _('Instance is not found %s') % instance_name
- raise exception.InstanceNotFound(msg)
-
LOG.info(_LI('Unpausing instance %(instance)s with'
' %(image)s'), {'instance': instance.name,
'image': instance.image_ref})
More information about the lxc-devel
mailing list