From 658ebecb84951fd8e418f15a5c8e2c8b1901b9d4 Mon Sep 17 00:00:00 2001 From: Prashanth Pai Date: Tue, 24 Feb 2015 14:40:25 +0530 Subject: Refactor volume initialization code * Validate inputs during initialization of Volume object. * Move glfs_new() and glfs_init() into mount() method. * Provide user consumable unmount() method. * Provide "mounted" property to Volume object. Users can now check state whether a volume is virtual mounted or not. Change-Id: Idc53ee7f9386ed995315bd5fb492a7d36f44875f Signed-off-by: Prashanth Pai --- gluster/exceptions.py | 18 +++++ gluster/gfapi.py | 129 +++++++++++++++++++++++++++--- test/functional/libgfapi-python-tests.py | 81 +++++++++++++++---- test/unit/gluster/test_gfapi.py | 130 +++++++++++++++++++++++++++++-- 4 files changed, 328 insertions(+), 30 deletions(-) create mode 100644 gluster/exceptions.py diff --git a/gluster/exceptions.py b/gluster/exceptions.py new file mode 100644 index 0000000..2bb4732 --- /dev/null +++ b/gluster/exceptions.py @@ -0,0 +1,18 @@ +# Copyright (c) 2012-2015 Red Hat, Inc. +# +# 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. + + +class LibgfapiException(Exception): + pass diff --git a/gluster/gfapi.py b/gluster/gfapi.py index 3554657..8a02831 100755 --- a/gluster/gfapi.py +++ b/gluster/gfapi.py @@ -14,6 +14,7 @@ import os import stat import errno from gluster import api +from gluster.exceptions import LibgfapiException class File(object): @@ -201,22 +202,132 @@ class Dir(object): class Volume(object): - def __init__(self, host, volid, proto="tcp", port=24007): + def __init__(self, host, volname, + proto="tcp", port=24007, log_file=None, log_level=7): + """ + Create a Volume object instance. + + :param host: Host with glusterd management daemon running. + :param volname: Name of GlusterFS volume to be mounted and used. + :param proto: Transport protocol to be used to connect to management + daemon. Permitted values are "tcp" and "rdma". + :param port: Port number where gluster management daemon is listening. + :param log_file: Path to log file. When this is set to None, a new + logfile will be created in default log directory + i.e /var/log/glusterfs + :param log_level: Integer specifying the degree of verbosity. + Higher the value, more verbose the logging. + + TODO: Provide an interface where user can specify volfile directly + instead of providing host and other details. This is helpful in cases + where user wants to load some non default xlator on client side. For + example, aux-gfid-mount or mount volume as read-only. + """ # Add a reference so the module-level variable "api" doesn't # get yanked out from under us (see comment above File def'n). self._api = api - self.fs = api.glfs_new(volid) - api.glfs_set_volfile_server(self.fs, proto, host, port) - def __del__(self): - self._api.glfs_fini(self.fs) - self._api = None + self._mounted = False + self.fs = None + self.log_file = log_file + self.log_level = log_level - def set_logging(self, path, level): - api.glfs_set_logging(self.fs, path, level) + if None in (volname, host): + # TODO: Validate host based on regex for IP/FQDN. + raise LibgfapiException("Host and Volume name should not be None.") + if proto not in ('tcp', 'rdma'): + raise LibgfapiException("Invalid protocol specified.") + if not isinstance(port, (int, long)): + raise LibgfapiException("Invalid port specified.") + + self.host = host + self.volname = volname + self.protocol = proto + self.port = port + + @property + def mounted(self): + return self._mounted def mount(self): - return api.glfs_init(self.fs) + """ + Mount a GlusterFS volume for use. + """ + if self.fs and self._mounted: + # Already mounted + return + + self.fs = api.glfs_new(self.volname) + if not self.fs: + raise LibgfapiException("glfs_new(%s) failed." % (self.volname)) + + ret = api.glfs_set_volfile_server(self.fs, self.protocol, + self.host, self.port) + if ret < 0: + # FIXME: For some reason, get_errno() is not able to capture + # proper errno. Until then.. + # https://bugzilla.redhat.com/show_bug.cgi?id=1196161 + raise LibgfapiException("glfs_set_volfile_server(%s, %s, %s, " + "%s) failed." % (self.fs, self.protocol, + self.host, self.port)) + + self.set_logging(self.log_file, self.log_level) + + if self.fs and not self._mounted: + ret = api.glfs_init(self.fs) + if ret < 0: + raise LibgfapiException("glfs_init(%s) failed." % (self.fs)) + else: + self._mounted = True + + def unmount(self): + """ + Unmount a mounted GlusterFS volume. + + Provides users a way to free resources instead of just waiting for + python garbage collector to call __del__() at some point later. + """ + if self.fs: + ret = self._api.glfs_fini(self.fs) + if ret < 0: + raise LibgfapiException("glfs_fini(%s) failed." % (self.fs)) + else: + # Succeeded. Protect against multiple unmount() calls. + self._mounted = False + self.fs = None + + def __del__(self): + try: + self.unmount() + except LibgfapiException: + pass + + def set_logging(self, log_file, log_level): + """ + Set logging parameters. Can be invoked either before or after + invoking mount(). + + When invoked before mount(), the preferred log file and log level + choices are recorded and then later enforced internally as part of + mount() + + When invoked at any point after mount(), the change in log file + and log level is instantaneous. + + :param log_file: Path of log file. + If set to "/dev/null", nothing will be logged. + If set to None, a new logfile will be created in + default log directory (/var/log/glusterfs) + :param log_level: Integer specifying the degree of verbosity. + Higher the value, more verbose the logging. + """ + if self.fs: + ret = api.glfs_set_logging(self.fs, self.log_file, self.log_level) + if ret < 0: + raise LibgfapiException("glfs_set_logging(%s, %s) failed." % + (self.log_file, self.log_level)) + self.log_file = log_file + self.log_level = log_level def chmod(self, path, mode): """ diff --git a/test/functional/libgfapi-python-tests.py b/test/functional/libgfapi-python-tests.py index 1519426..c0e760a 100644 --- a/test/functional/libgfapi-python-tests.py +++ b/test/functional/libgfapi-python-tests.py @@ -15,8 +15,10 @@ import types import errno from gluster import gfapi +from gluster.exceptions import LibgfapiException from test import get_test_config from ConfigParser import NoSectionError, NoOptionError +from uuid import uuid4 config = get_test_config() if config: @@ -42,14 +44,10 @@ class BinFileOpsTest(unittest.TestCase): @classmethod def setUpClass(cls): cls.vol = gfapi.Volume(HOST, VOLNAME) - cls.vol.set_logging("/dev/null", 7) ret = cls.vol.mount() if ret == 0: # Cleanup volume cls.vol.rmtree("/", ignore_errors=True) - else: - raise Exception("Initializing volume %s:%s failed." % - (HOST, VOLNAME)) @classmethod def tearDownClass(cls): @@ -80,14 +78,10 @@ class FileOpsTest(unittest.TestCase): @classmethod def setUpClass(cls): cls.vol = gfapi.Volume(HOST, VOLNAME) - cls.vol.set_logging("/dev/null", 7) ret = cls.vol.mount() if ret == 0: # Cleanup volume cls.vol.rmtree("/", ignore_errors=True) - else: - raise Exception("Initializing volume %s:%s failed." % - (HOST, VOLNAME)) @classmethod def tearDownClass(cls): @@ -253,15 +247,10 @@ class DirOpsTest(unittest.TestCase): @classmethod def setUpClass(cls): cls.vol = gfapi.Volume(HOST, VOLNAME) - cls.vol.set_logging("/dev/null", 7) - cls.vol.mount() ret = cls.vol.mount() if ret == 0: # Cleanup volume cls.vol.rmtree("/", ignore_errors=True) - else: - raise Exception("Initializing volume %s:%s failed." % - (HOST, VOLNAME)) cls.testfile = "testfile" @classmethod @@ -323,3 +312,69 @@ class DirOpsTest(unittest.TestCase): self.vol.rmtree(self.dir_path, True) self.assertRaises(OSError, self.vol.lstat, f) self.assertRaises(OSError, self.vol.lstat, self.dir_path) + + +class TestVolumeInit(unittest.TestCase): + + def test_mount_unmount_default(self): + # Create volume object instance + vol = gfapi.Volume(HOST, VOLNAME) + # Check attribute init + self.assertEqual(vol.log_file, None) + self.assertEqual(vol.log_level, 7) + self.assertEqual(vol.host, HOST) + self.assertEqual(vol.volname, VOLNAME) + self.assertEqual(vol.port, 24007) + self.assertFalse(vol.mounted) + # Mount the volume + vol.mount() + # Check mounted property + self.assertTrue(vol.mounted) + # Unmount the volume + vol.unmount() + # Check mounted property again + self.assertFalse(vol.mounted) + # Do a double unmount - should not crash or raise exception + vol.unmount() + self.assertFalse(vol.mounted) + # Do a double mount - should not crash or raise exception + vol.mount() + vol.mount() + self.assertTrue(vol.mounted) + # Unmount the volume + vol.unmount() + self.assertFalse(vol.mounted) + + def test_mount_err(self): + # Volume does not exist + fake_volname = str(uuid4().hex)[:10] + vol = gfapi.Volume(HOST, fake_volname) + self.assertRaises(LibgfapiException, vol.mount) + self.assertFalse(vol.mounted) + + # Invalid host - glfs_set_volfile_server will fail + fake_hostname = str(uuid4().hex)[:10] + vol = gfapi.Volume(fake_hostname, VOLNAME) + self.assertRaises(LibgfapiException, vol.mount) + self.assertFalse(vol.mounted) + + def test_set_logging(self): + # Create volume object instance + vol = gfapi.Volume(HOST, VOLNAME) + # Call set_logging before mount() + log_file = "/tmp/%s" % (uuid4().hex) + vol.set_logging(log_file, 7) + # Mount the volume + vol.mount() + self.assertTrue(vol.mounted) + self.assertEqual(vol.log_file, log_file) + self.assertEqual(vol.log_level, 7) + # Check that log has been created and exists + self.assertTrue(os.path.exists(log_file)) + # Change the logging after mounting + log_file2 = "/tmp/%s" % (uuid4().hex) + vol.set_logging(log_file2, 7) + self.assertEqual(vol.log_file, log_file2) + # Unmount the volume + vol.unmount() + self.assertFalse(vol.mounted) diff --git a/test/unit/gluster/test_gfapi.py b/test/unit/gluster/test_gfapi.py index dc470a1..8880727 100644 --- a/test/unit/gluster/test_gfapi.py +++ b/test/unit/gluster/test_gfapi.py @@ -17,6 +17,7 @@ import errno from gluster import gfapi from gluster import api +from gluster.exceptions import LibgfapiException from nose import SkipTest from mock import Mock, patch from contextlib import nested @@ -31,15 +32,23 @@ def _mock_glfs_closedir(fd): def _mock_glfs_new(volid): - return 2 + return 12345 + + +def _mock_glfs_init(fs): + return 0 def _mock_glfs_set_volfile_server(fs, proto, host, port): - return + return 0 def _mock_glfs_fini(fs): - return + return 0 + + +def _mock_glfs_set_logging(fs, log_file, log_level): + return 0 class TestFile(unittest.TestCase): @@ -183,10 +192,10 @@ class TestFile(unittest.TestCase): self.assertEqual(buflen, 12345) return buflen - for buflen in (-1,-2,-999): + for buflen in (-1, -2, -999): with patch("gluster.gfapi.api.glfs_read", _mock_glfs_read): with patch("gluster.gfapi.File.fgetsize", _mock_fgetsize): - b = self.fd.read(buflen) + self.fd.read(buflen) def test_write_success(self): mock_glfs_write = Mock() @@ -281,6 +290,9 @@ class TestVolume(unittest.TestCase): gluster.gfapi.api.glfs_set_volfile_server = \ _mock_glfs_set_volfile_server + cls._saved_glfs_init = gluster.gfapi.api.glfs_init + gluster.gfapi.api.glfs_init = _mock_glfs_init + cls._saved_glfs_fini = gluster.gfapi.api.glfs_fini gluster.gfapi.api.glfs_fini = _mock_glfs_fini @@ -289,7 +301,13 @@ class TestVolume(unittest.TestCase): cls._saved_glfs_closedir = gluster.gfapi.api.glfs_closedir gluster.gfapi.api.glfs_closedir = _mock_glfs_closedir + + cls._saved_glfs_set_logging = gluster.gfapi.api.glfs_set_logging + gluster.gfapi.api.glfs_set_logging = _mock_glfs_set_logging + cls.vol = gfapi.Volume("mockhost", "test") + cls.vol.fs = 12345 + cls.vol._mounted = True @classmethod def tearDownClass(cls): @@ -301,6 +319,101 @@ class TestVolume(unittest.TestCase): gluster.gfapi.api.glfs_close = cls._saved_glfs_close gluster.gfapi.api.glfs_closedir = cls._saved_glfs_closedir + def test_initialization_error(self): + self.assertRaises(LibgfapiException, gfapi.Volume, "host", None) + self.assertRaises(LibgfapiException, gfapi.Volume, None, "vol") + self.assertRaises(LibgfapiException, gfapi.Volume, None, None) + self.assertRaises(LibgfapiException, gfapi.Volume, "host", "vol", "ZZ") + self.assertRaises(LibgfapiException, gfapi.Volume, "host", "vol", + "tcp", "invalid_port") + + def test_initialization_success(self): + v = gfapi.Volume("host", "vol", "tcp", 9876) + self.assertEqual(v.host, "host") + self.assertEqual(v.volname, "vol") + self.assertEqual(v.protocol, "tcp") + self.assertEqual(v.port, 9876) + self.assertFalse(v.mounted) + + def test_mount_unmount_success(self): + v = gfapi.Volume("host", "vol") + v.mount() + self.assertTrue(v.mounted) + self.assertTrue(v.fs) + v.unmount() + self.assertFalse(v.mounted) + self.assertFalse(v.fs) + + def test_mount_multiple(self): + _m_glfs_new = Mock() + v = gfapi.Volume("host", "vol") + with patch("gluster.gfapi.api.glfs_new", _m_glfs_new): + # Mounting for first time + v.mount() + _m_glfs_new.assert_called_once_with("vol") + _m_glfs_new.reset_mock() + for i in range(0, 5): + v.mount() + self.assertFalse(_m_glfs_new.called) + self.assertTrue(v.mounted) + + def test_mount_error(self): + # glfs_new() failed + _m_glfs_new = Mock(return_value=None) + v = gfapi.Volume("host", "vol") + with patch("gluster.gfapi.api.glfs_new", _m_glfs_new): + self.assertRaises(LibgfapiException, v.mount) + self.assertFalse(v.fs) + self.assertFalse(v.mounted) + _m_glfs_new.assert_called_once_with("vol") + + # glfs_set_volfile_server() failed + _m_set_vol = Mock(return_value=-1) + v = gfapi.Volume("host", "vol") + with patch("gluster.gfapi.api.glfs_set_volfile_server", _m_set_vol): + self.assertRaises(LibgfapiException, v.mount) + self.assertFalse(v.mounted) + _m_glfs_new.assert_called_once_with("vol") + _m_set_vol.assert_called_once_with(v.fs, v.protocol, + v.host, v.port) + + # glfs_init() failed + _m_glfs_init = Mock(return_value=-1) + v = gfapi.Volume("host", "vol") + with patch("gluster.gfapi.api.glfs_init", _m_glfs_init): + self.assertRaises(LibgfapiException, v.mount) + self.assertFalse(v.mounted) + _m_glfs_init.assert_caled_once_with(v.fs) + + def test_unmount_error(self): + v = gfapi.Volume("host", "vol") + v.mount() + _m_glfs_fini = Mock(return_value=-1) + with patch("gluster.gfapi.api.glfs_fini", _m_glfs_fini): + self.assertRaises(LibgfapiException, v.unmount) + _m_glfs_fini.assert_called_once_with(v.fs) + # Should still be mounted as unmount failed. + self.assertTrue(v.mounted) + + def test_set_logging(self): + _m_set_logging = Mock() + + # Called after mount() + v = gfapi.Volume("host", "vol") + with patch("gluster.gfapi.api.glfs_set_logging", _m_set_logging): + v.mount() + v.set_logging("/path/whatever", 7) + self.assertEqual(v.log_file, "/path/whatever") + self.assertEqual(v.log_level, 7) + + def test_set_logging_err(self): + v = gfapi.Volume("host", "vol") + v.fs = 12345 + _m_set_logging = Mock(return_value=-1) + with patch("gluster.gfapi.api.glfs_set_logging", _m_set_logging): + self.assertRaises(LibgfapiException, v.set_logging, "/dev/null", 7) + _m_set_logging.assert_called_once_with(v.fs, None, 7) + def test_chmod_success(self): mock_glfs_chmod = Mock() mock_glfs_chmod.return_value = 0 @@ -339,7 +452,7 @@ class TestVolume(unittest.TestCase): with self.vol.open("file.txt", os.O_CREAT, 0644) as fd: self.assertTrue(isinstance(fd, gfapi.File)) self.assertEqual(mock_glfs_creat.call_count, 1) - mock_glfs_creat.assert_called_once_with(2, + mock_glfs_creat.assert_called_once_with(12345, "file.txt", os.O_CREAT, 0644) @@ -615,7 +728,7 @@ class TestVolume(unittest.TestCase): with self.vol.open("file.txt", os.O_WRONLY) as fd: self.assertTrue(isinstance(fd, gfapi.File)) self.assertEqual(mock_glfs_open.call_count, 1) - mock_glfs_open.assert_called_once_with(2, + mock_glfs_open.assert_called_once_with(12345, "file.txt", os.O_WRONLY) def test_open_with_statement_fail_exception(self): @@ -637,7 +750,8 @@ class TestVolume(unittest.TestCase): fd = self.vol.open("file.txt", os.O_WRONLY) self.assertTrue(isinstance(fd, gfapi.File)) self.assertEqual(mock_glfs_open.call_count, 1) - mock_glfs_open.assert_called_once_with(2, "file.txt", os.O_WRONLY) + mock_glfs_open.assert_called_once_with(12345, "file.txt", + os.O_WRONLY) def test_open_direct_fail_exception(self): mock_glfs_open = Mock() -- cgit