From c4bcebb1b38cba2ecf50f635794dc8543293d8c1 Mon Sep 17 00:00:00 2001 From: Ivan Kolodyazhny Date: Tue, 10 May 2016 15:18:06 +0300 Subject: [PATCH] Attach/detach features implementation Supported protocols: iSCSI Current version is tested only on Linux hosts This patch introduces new CLI commands 'local-attach' and 'local-detach'. Depends-On: I5a10574cb91794f978eda2aa2e2e65dc92ca56b4 Change-Id: Ibd7616273342ff27628fb4abf48dd16847b2a636 --- README.rst | 6 +- brick_cinderclient_ext/__init__.py | 72 ++++++++++++ brick_cinderclient_ext/brick_utils.py | 12 ++ brick_cinderclient_ext/client.py | 46 +++++++- .../tests/unit/test_brick_client.py | 55 ++++++--- .../tests/unit/test_volume_actions.py | 108 ++++++++++++++++++ brick_cinderclient_ext/volume_actions.py | 101 ++++++++++++++++ test-requirements.txt | 3 +- 8 files changed, 382 insertions(+), 21 deletions(-) create mode 100644 brick_cinderclient_ext/tests/unit/test_volume_actions.py create mode 100644 brick_cinderclient_ext/volume_actions.py diff --git a/README.rst b/README.rst index c8ff5b6..2c3b0b0 100644 --- a/README.rst +++ b/README.rst @@ -19,9 +19,9 @@ Requires dependency:: Optional dependencies based on Cinder driver's protocol:: -* open-iscsi - for volume attachment via iSCSI -* ceph-common - for volume attachment via iSCSI (Ceph) -* nfs-common - for volume attachment using NFS protocol +* open-iscsi, udev - for volume attachment via iSCSI + +NOTE (e0ne): current version is tested only on Linux hosts For any other information, refer to the parent projects, Cinder and python-cinderclient:: diff --git a/brick_cinderclient_ext/__init__.py b/brick_cinderclient_ext/__init__.py index 89fd10f..80c78e2 100644 --- a/brick_cinderclient_ext/__init__.py +++ b/brick_cinderclient_ext/__init__.py @@ -18,7 +18,9 @@ Command-line interface to the os-brick. """ from __future__ import print_function +import json import logging +import socket import pbr.version @@ -60,6 +62,76 @@ def do_get_connector(client, args): utils.print_dict(connector) +@utils.arg('identifier', + metavar='', + help=VOLUME_ID_HELP_MESSAGE) +@utils.service_type('volumev2') +@utils.arg('--hostname', + metavar='', + default=socket.gethostname(), + help='hostname') +@utils.arg('--mountpoint', + metavar='', + default=None, + help='mountpoint') +@utils.arg('--mode', + metavar='', + default='rw', + help='mode') +@utils.arg('--multipath', + metavar='', + default=False, + help=MULTIPATH_HELP_MESSAGE) +@utils.arg('--enforce_multipath', + metavar='', + default=False, + help=ENFORCE_MULTIPATH_HELP_MESSAGE) +@utils.service_type('volumev2') +def do_local_attach(client, args): + hostname = args.hostname + volume = args.identifier + brickclient = brick_client.Client(client) + device_info = brickclient.attach(volume, + hostname, + args.mountpoint, + args.mode, + args.multipath, + args.enforce_multipath) + + utils.print_dict(device_info) + + +@utils.arg('identifier', + metavar='', + help=VOLUME_ID_HELP_MESSAGE) +@utils.arg('--attachment_uuid', + metavar='', + default=None, + help='The uuid of the volume attachment.') +@utils.arg('--multipath', + metavar='', + default=False, + help=MULTIPATH_HELP_MESSAGE) +@utils.arg('--enforce_multipath', + metavar='', + default=False, + help=ENFORCE_MULTIPATH_HELP_MESSAGE) +@utils.arg('--device_info', + metavar='', + default=None, + help='The device_info is returned from connect_volume.') +@utils.service_type('volumev2') +def do_local_detach(client, args): + volume = args.identifier + brickclient = brick_client.Client(client) + device_info = None + if args.device_info: + device_info = json.joads(args.device_info) + + brickclient.detach(volume, args.attachment_uuid, args.multipath, + args.enforce_multipath, device_info) + + @utils.arg('identifier', metavar='', help=VOLUME_ID_HELP_MESSAGE) diff --git a/brick_cinderclient_ext/brick_utils.py b/brick_cinderclient_ext/brick_utils.py index 3e81cf3..b0584ab 100644 --- a/brick_cinderclient_ext/brick_utils.py +++ b/brick_cinderclient_ext/brick_utils.py @@ -18,6 +18,7 @@ import os import socket from cinderclient import exceptions +from oslo_concurrency import processutils def get_my_ip(): @@ -43,3 +44,14 @@ def require_root(f): "This command requies root permissions.") return f(*args, **kwargs) return wrapper + + +def safe_execute(cmd): + try: + processutils.execute(*cmd, root_helper=get_root_helper(), + run_as_root=True) + except processutils.ProcessExecutionError as e: + print('Command "{0}" execution returned {1} exit code:'.format( + e.cmd, e.exit_code)) + print('Stderr: {0}'.format(e.stderr)) + print('Stdout: {0}'.format(e.stdout)) diff --git a/brick_cinderclient_ext/client.py b/brick_cinderclient_ext/client.py index f05b987..7e6d3d7 100644 --- a/brick_cinderclient_ext/client.py +++ b/brick_cinderclient_ext/client.py @@ -13,11 +13,11 @@ from __future__ import print_function from cinderclient import exceptions - from os_brick.initiator import connector from oslo_concurrency import processutils from brick_cinderclient_ext import brick_utils +from brick_cinderclient_ext import volume_actions as actions class Client(object): @@ -50,9 +50,51 @@ class Client(object): brick_utils.get_root_helper(), brick_utils.get_my_ip(), multipath=multipath, - enforce_multipath=(enforce_multipath)) + enforce_multipath=(enforce_multipath), + execute=processutils.execute) return conn_prop + def attach(self, volume_id, hostname, mountpoint=None, mode='rw', + multipath=False, enforce_multipath=False): + + # Reserve volume before attachment + with actions.Reserve(self.volumes_client, volume_id) as cmd: + cmd.reserve() + + with actions.InitializeConnection( + self.volumes_client, volume_id) as cmd: + connection = cmd.initialize(self, multipath, enforce_multipath) + + with actions.VerifyProtocol(self.volumes_client, volume_id) as cmd: + cmd.verify(connection['driver_volume_type']) + + with actions.ConnectVolume(self.volumes_client, volume_id) as cmd: + brick_connector = self._brick_get_connector( + connection['driver_volume_type'], is_local=True) + device_info = cmd.connect(brick_connector, + connection['data'], + mountpoint, mode, hostname) + return device_info + + def detach(self, volume_id, attachment_uuid=None, multipath=False, + enforce_multipath=False, device_info=None): + + with actions.BeginDetach(self.volumes_client, volume_id) as cmd: + cmd.reserve() + + with actions.InitializeConnectionForDetach( + self.volumes_client, volume_id) as cmd: + connection = cmd.initialize(self, multipath, enforce_multipath) + + brick_connector = self._brick_get_connector( + connection['driver_volume_type'], is_local=True) + + with actions.DisconnectVolume(self.volumes_client, volume_id) as cmd: + cmd.disconnect(brick_connector, connection['data'], device_info) + + with actions.DetachVolume(self.volumes_client, volume_id) as cmd: + cmd.detach(self, attachment_uuid, multipath, enforce_multipath) + def get_volume_paths(self, volume_id, use_multipath=False): """Gets volume paths on the system for a specific volume.""" conn_props = self.get_connector(multipath=use_multipath) diff --git a/brick_cinderclient_ext/tests/unit/test_brick_client.py b/brick_cinderclient_ext/tests/unit/test_brick_client.py index b5c9666..06ec730 100644 --- a/brick_cinderclient_ext/tests/unit/test_brick_client.py +++ b/brick_cinderclient_ext/tests/unit/test_brick_client.py @@ -26,19 +26,6 @@ class TestBrickClient(base.BaseTestCase): self.hostname = 'hostname' self.client = client.Client() - @mock.patch('brick_cinderclient_ext.brick_utils.get_my_ip') - @mock.patch('brick_cinderclient_ext.brick_utils.get_root_helper') - @mock.patch('os_brick.initiator.connector.get_connector_properties') - def test_get_connector(self, mock_connector, mock_root_helper, - mock_my_ip): - mock_root_helper.return_value = 'root-helper' - mock_my_ip.return_value = '1.0.0.0' - - self.client.get_connector() - mock_connector.assert_called_with('root-helper', '1.0.0.0', - enforce_multipath=False, - multipath=False) - def _init_fake_cinderclient(self, protocol): # Init fake cinderclient self.mock_vc = mock.MagicMock() @@ -65,18 +52,56 @@ class TestBrickClient(base.BaseTestCase): return conn_props, mock_connect + @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('brick_cinderclient_ext.brick_utils.get_my_ip') + @mock.patch('brick_cinderclient_ext.brick_utils.get_root_helper') + @mock.patch('os_brick.initiator.connector.get_connector_properties') + def test_get_connector(self, mock_connector, mock_root_helper, + mock_my_ip, mock_execute): + mock_root_helper.return_value = 'root-helper' + mock_my_ip.return_value = '1.0.0.0' + + self.client.get_connector() + mock_connector.assert_called_with('root-helper', '1.0.0.0', + enforce_multipath=False, + multipath=False, + execute=mock_execute) + + @mock.patch('oslo_concurrency.processutils.execute') @mock.patch('brick_cinderclient_ext.brick_utils.get_my_ip') @mock.patch('brick_cinderclient_ext.brick_utils.get_root_helper') @mock.patch('os_brick.initiator.connector.get_connector_properties') def test_get_connector_with_multipath(self, mock_connector, - mock_root_helper, mock_my_ip): + mock_root_helper, mock_my_ip, + mock_execute): mock_root_helper.return_value = 'root-helper' mock_my_ip.return_value = '1.0.0.0' self.client.get_connector(True, True) mock_connector.assert_called_with('root-helper', '1.0.0.0', enforce_multipath=True, - multipath=True) + multipath=True, + execute=mock_execute) + + @mock.patch('os_brick.initiator.connector.get_connector_properties') + def test_attach_iscsi(self, mock_conn_prop): + connection = self._init_fake_cinderclient('iscsi') + conn_props, mock_connect = self._init_fake_os_brick(mock_conn_prop) + + self.client.attach(self.volume_id, self.hostname) + self.mock_vc.volumes.initialize_connection.assert_called_with( + self.volume_id, conn_props) + mock_connect.connect_volume.assert_called_with(connection['data']) + + @mock.patch('os_brick.initiator.connector.get_connector_properties') + def test_detach_iscsi(self, mock_conn_prop): + connection = self._init_fake_cinderclient('iscsi') + conn_props, m_connect = self._init_fake_os_brick(mock_conn_prop) + + self.client.detach(self.volume_id) + self.mock_vc.volumes.initialize_connection.assert_called_with( + self.volume_id, conn_props) + m_connect.disconnect_volume.assert_called_with(connection['data'], {}) @mock.patch('os_brick.initiator.connector.get_connector_properties') def test_get_volume_paths(self, mock_conn_prop): diff --git a/brick_cinderclient_ext/tests/unit/test_volume_actions.py b/brick_cinderclient_ext/tests/unit/test_volume_actions.py new file mode 100644 index 0000000..6b84f7a --- /dev/null +++ b/brick_cinderclient_ext/tests/unit/test_volume_actions.py @@ -0,0 +1,108 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import ddt +import mock + +from cinderclient import exceptions as cinder_exceptions +from os_brick import exception +from oslotest import base + +from brick_cinderclient_ext import volume_actions + + +@ddt.ddt +class TestVolumeActions(base.BaseTestCase): + def setUp(self): + super(TestVolumeActions, self).setUp() + self.volume_id = '3d96b134-75bd-492b-8372-330455cae38f' + self.brick_client = mock.Mock() + self.v_client = mock.Mock() + self.command_args = [self.v_client, self.volume_id] + + def test_reserve(self): + with volume_actions.Reserve(*self.command_args) as cmd: + cmd.reserve() + + self.v_client.volumes.reserve.assert_called_once_with(self.volume_id) + + def test_reserve_failed(self): + self.v_client.volumes.reserve.side_effect = ( + cinder_exceptions.BadRequest(400)) + try: + with volume_actions.Reserve(*self.command_args) as cmd: + cmd.reserve() + except cinder_exceptions.BadRequest: + self.v_client.volumes.unreserve.assert_called_once_with( + self.volume_id) + + self.v_client.volumes.reserve.assert_called_once_with(self.volume_id) + + def test_initialize_connection(self): + self.brick_client.get_connector.return_value = None + with volume_actions.InitializeConnection(*self.command_args) as cmd: + cmd.initialize(self.brick_client, False, False) + + self.brick_client.get_connector.assert_called_once_with(False, False) + self.v_client.volumes.initialize_connection.assert_called_once_with( + self.volume_id, None) + + @ddt.data('iscsi', 'iSCSI', 'ISCSI') + def test_verify_protocol(self, protocol): + with volume_actions.VerifyProtocol(*self.command_args) as cmd: + # NOTE(e0ne): veryfy that no exception is rased + cmd.verify(protocol) + + def test_verify_protocol_failed(self): + try: + with volume_actions.VerifyProtocol(*self.command_args) as cmd: + cmd.verify('protocol') + except exception.ProtocolNotSupported: + self.v_client.volumes.unreserve.assert_called_once_with( + self.volume_id) + + def test_connect_volume(self): + connector = mock.Mock() + connector.connect_volume.return_value = {'device': 'info'} + with volume_actions.ConnectVolume(*self.command_args) as cmd: + cmd.connect(connector, + 'connection_data', 'mountpoint', 'mode', 'hostname') + + connector.connect_volume.assert_called_once_with('connection_data') + self.v_client.volumes.attach.assert_called_once_with( + self.volume_id, + instance_uuid=None, mountpoint='mountpoint', mode='mode', + host_name='hostname') + + @ddt.data((None, {}), ('connection_data', 'connection_data')) + @ddt.unpack + def test_disconnect_no_device_info(self, command_arg, connector_arg): + connector = mock.Mock() + with volume_actions.DisconnectVolume(*self.command_args) as cmd: + cmd.disconnect(connector, 'connection_data', command_arg) + + connector.disconnect_volume.assert_called_once_with('connection_data', + connector_arg) + + def test_detach(self): + brick_client = mock.Mock() + brick_client.get_connector.return_value = 'connector' + with volume_actions.DetachVolume(*self.command_args) as cmd: + cmd.detach(brick_client, 'attachment_uuid', + 'multipath', 'enforce_multipath') + + brick_client.get_connector.assert_called_once_with('multipath', + 'enforce_multipath') + self.v_client.volumes.terminate_connection.assert_called_once_with( + self.volume_id, 'connector') + self.v_client.volumes.detach.assert_called_once_with( + self.volume_id, 'attachment_uuid') diff --git a/brick_cinderclient_ext/volume_actions.py b/brick_cinderclient_ext/volume_actions.py new file mode 100644 index 0000000..546e1c4 --- /dev/null +++ b/brick_cinderclient_ext/volume_actions.py @@ -0,0 +1,101 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from os_brick import exception +from os_brick.initiator import connector + + +class VolumeAction(object): + def __init__(self, volumes_client, volume_id): + self.volumes_client = volumes_client + self.volume_id = volume_id + + def __enter__(self): + return self + + def __exit__(self, type, value, traceback): + if traceback: + self.volumes_client.volumes.unreserve(self.volume_id) + return False + return True + + +class Reserve(VolumeAction): + def reserve(self): + self.volumes_client.volumes.reserve(self.volume_id) + + +class InitializeConnection(VolumeAction): + def initialize(self, brick_client, multipath, enforce_multipath): + conn_prop = brick_client.get_connector(multipath, enforce_multipath) + return self.volumes_client.volumes.initialize_connection( + self.volume_id, conn_prop) + + +class VerifyProtocol(VolumeAction): + # NOTE(e0ne): iSCSI drivers works without issues, RBD and NFS don't + # work. Drivers with other protocols are not tested yet. + SUPPORTED_PROCOTOLS = [connector.ISCSI] + + def verify(self, protocol): + protocol = protocol.upper() + + # NOTE(e0ne): iSCSI drivers works without issues, RBD and NFS don't + # work. Drivers with other protocols are not tested yet. + if protocol not in VerifyProtocol.SUPPORTED_PROCOTOLS: + raise exception.ProtocolNotSupported(protocol=protocol) + + +class ConnectVolume(VolumeAction): + def connect(self, brick_connector, connection_data, + mountpoint, mode, hostname): + device_info = brick_connector.connect_volume(connection_data) + + self.volumes_client.volumes.attach(self.volume_id, instance_uuid=None, + mountpoint=mountpoint, + mode=mode, + host_name=hostname) + return device_info + + +class VolumeDetachAction(VolumeAction): + def __exit__(self, type, value, traceback): + if traceback: + self.volumes_client.volumes.roll_detaching(self.volume_id) + return False + return True + + +class BeginDetach(VolumeDetachAction): + def reserve(self): + self.volumes_client.volumes.begin_detaching(self.volume_id) + + +class InitializeConnectionForDetach(InitializeConnection, VolumeDetachAction): + pass + + +class DisconnectVolume(VolumeDetachAction): + def disconnect(self, brick_connector, connection_data, device_info): + device_info = device_info or {} + + brick_connector.disconnect_volume(connection_data, device_info) + + +class DetachVolume(VolumeDetachAction): + def detach(self, brick_client, + attachment_uuid, multipath, enforce_multipath): + conn_prop = brick_client.get_connector(multipath, enforce_multipath) + + self.volumes_client.volumes.terminate_connection(self.volume_id, + conn_prop) + self.volumes_client.volumes.detach(self.volume_id, attachment_uuid) diff --git a/test-requirements.txt b/test-requirements.txt index 4ee60cc..b33de3c 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -5,10 +5,11 @@ hacking<0.11,>=0.10.0 coverage>=3.6 # Apache-2.0 +ddt>=1.0.1 # MIT python-subunit>=0.0.18 # Apache-2.0/BSD sphinx!=1.2.0,!=1.3b1,<1.3,>=1.1.2 # BSD oslosphinx!=3.4.0,>=2.5.0 # Apache-2.0 oslotest>=1.10.0 # Apache-2.0 testrepository>=0.0.18 # Apache-2.0/BSD testscenarios>=0.4 # Apache-2.0/BSD -testtools>=1.4.0 # MIT +testtools>=1.4.0 # MIT \ No newline at end of file