From 4b261e179e447a26012608a69da253bf7ecaafd1 Mon Sep 17 00:00:00 2001 From: Kallebe Monteiro Date: Fri, 6 Mar 2015 16:03:39 -0300 Subject: [PATCH] Fix SAN generic driver ssh whitespaced commands SAN driver was sending commands over ssh with whitespaces between every character resulting in commands like "u n a m e - s" instead of "uname -s" (example from bug report). As the bug reporter noted, the problem was that there were two joins in the process of mounting the command. The first one got the tuple and joined into a string. The second one joined a whitespace between each character of the string. Additionally, this behavior made utils.ssh_injection a bad check, because it expected a list of args used in 'for arg in cmd_list' and what it got was a single string. This resulted in the check being made over the single characters instead of each arg. All this is to explain why I chose to remove the first join instead of the second one. An unit test for this issue was created. SAN driver had no unit test. Change-Id: I8e8624f2b82c26522936adf05e637a7bbe34a069 Closes-Bug: #1422604 --- cinder/tests/test_san.py | 55 ++++++++++++++++++++++++++++++++ cinder/volume/drivers/san/san.py | 7 ++-- 2 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 cinder/tests/test_san.py diff --git a/cinder/tests/test_san.py b/cinder/tests/test_san.py new file mode 100644 index 000000000..951690177 --- /dev/null +++ b/cinder/tests/test_san.py @@ -0,0 +1,55 @@ +# Copyright 2015 OpenStack Foundation +# +# 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 mock + +from cinder import test +from cinder.volume import configuration +from cinder.volume.drivers.san import san + + +class SanDriverTestCase(test.TestCase): + """Tests for SAN driver""" + + def __init__(self, *args, **kwargs): + super(SanDriverTestCase, self).__init__(*args, **kwargs) + + def setUp(self): + super(SanDriverTestCase, self).setUp() + self.configuration = mock.Mock(spec=configuration.Configuration) + self.configuration.san_is_local = False + self.configuration.san_ip = "10.0.0.1" + self.configuration.san_login = "admin" + self.configuration.san_password = "password" + self.configuration.san_ssh_port = 22 + self.configuration.san_thin_provision = True + self.configuration.san_private_key = 'private_key' + self.configuration.ssh_min_pool_conn = 1 + self.configuration.ssh_max_pool_conn = 5 + self.configuration.ssh_conn_timeout = 30 + + @mock.patch.object(san.processutils, 'ssh_execute') + @mock.patch.object(san.ssh_utils, 'SSHPool') + @mock.patch.object(san.utils, 'check_ssh_injection') + def test_ssh_formatted_command(self, mock_check_ssh_injection, + mock_ssh_pool, mock_ssh_execute): + driver = san.SanDriver(configuration=self.configuration) + cmd_list = ['uname', '-s'] + expected_cmd = 'uname -s' + driver.san_execute(*cmd_list) + # get the same used mocked item from the pool + with driver.sshpool.item() as ssh_item: + mock_ssh_execute.assert_called_with(ssh_item, expected_cmd, + check_exit_code=None) diff --git a/cinder/volume/drivers/san/san.py b/cinder/volume/drivers/san/san.py index f5adc86b8..cd564998f 100644 --- a/cinder/volume/drivers/san/san.py +++ b/cinder/volume/drivers/san/san.py @@ -16,7 +16,7 @@ Default Driver for san-stored volumes. The unique thing about a SAN is that we don't expect that we can run the volume -controller on the SAN hardware. We expect to access it over SSH or some API. +controller on the SAN hardware. We expect to access it over SSH or some API. """ import random @@ -24,11 +24,11 @@ import random from eventlet import greenthread from oslo_concurrency import processutils from oslo_config import cfg +from oslo_log import log as logging from oslo_utils import excutils from cinder import exception from cinder.i18n import _, _LE -from cinder.openstack.common import log as logging from cinder import ssh_utils from cinder import utils from cinder.volume import driver @@ -98,8 +98,7 @@ class SanDriver(driver.VolumeDriver): return utils.execute(*cmd, **kwargs) else: check_exit_code = kwargs.pop('check_exit_code', None) - command = ' '.join(cmd) - return self._run_ssh(command, check_exit_code) + return self._run_ssh(cmd, check_exit_code) def _run_ssh(self, cmd_list, check_exit_code=True, attempts=1): utils.check_ssh_injection(cmd_list) -- 2.45.2