From 2410b616bba2d68914c2c00e95b2f7c30f391e1b Mon Sep 17 00:00:00 2001 From: Christian Schwede Date: Wed, 20 Jul 2016 09:51:24 +0000 Subject: [PATCH] Fix swiftdir option and usage of storage policy aliases If swift-recon/swift-get-nodes/swift-object-info is used with the swiftdir option they will read rings from the given directory; however they are still using /etc/swift/swift.conf to find the policies on the current node. This makes it impossible to maintain a local swift.conf copy (if you don't have write access to /etc/swift) or check multiple clusters from the same node. Until now swift-recon was also not usable with storage policy aliases, this patch fixes this as well. Closes-Bug: 1577582 Closes-Bug: 1604707 Closes-Bug: 1617951 Co-Authored-By: Alistair Coles Co-Authored-By: Thiago da Silva Change-Id: I13188d42ec19e32e4420739eacd1e5b454af2ae3 --- bin/swift-get-nodes | 6 +++++ bin/swift-object-info | 5 ++++ swift/cli/recon.py | 13 ++++++--- swift/common/storage_policy.py | 12 ++++++--- swift/common/utils.py | 23 ++++++++++++++++ test/functional/__init__.py | 24 ++++++----------- test/unit/cli/test_recon.py | 31 ++++++++++++++++++--- test/unit/common/test_storage_policy.py | 2 +- test/unit/common/test_utils.py | 36 ++++++++++++++++++++++++- 9 files changed, 123 insertions(+), 29 deletions(-) diff --git a/bin/swift-get-nodes b/bin/swift-get-nodes index e1744274f3..3d944bf563 100755 --- a/bin/swift-get-nodes +++ b/bin/swift-get-nodes @@ -19,6 +19,8 @@ from optparse import OptionParser from os.path import basename from swift.common.ring import Ring +from swift.common.storage_policy import reload_storage_policies +from swift.common.utils import set_swift_dir from swift.cli.info import (parse_get_node_args, print_item_locations, InfoSystemExit) @@ -51,6 +53,10 @@ if __name__ == '__main__': parser.add_option('-d', '--swift-dir', default='/etc/swift', dest='swift_dir', help='Path to swift directory') options, args = parser.parse_args() + + if set_swift_dir(options.swift_dir): + reload_storage_policies() + try: ring_path, args = parse_get_node_args(options, args) except InfoSystemExit as e: diff --git a/bin/swift-object-info b/bin/swift-object-info index c625b47bb8..fc8a13e4b7 100755 --- a/bin/swift-object-info +++ b/bin/swift-object-info @@ -17,6 +17,8 @@ import sys from optparse import OptionParser +from swift.common.storage_policy import reload_storage_policies +from swift.common.utils import set_swift_dir from swift.cli.info import print_obj, InfoSystemExit @@ -38,6 +40,9 @@ if __name__ == '__main__': if len(args) != 1: sys.exit(parser.print_help()) + if set_swift_dir(options.swift_dir): + reload_storage_policies() + try: print_obj(*args, **vars(options)) except InfoSystemExit: diff --git a/swift/cli/recon.py b/swift/cli/recon.py index 202e449535..d370cc65e0 100644 --- a/swift/cli/recon.py +++ b/swift/cli/recon.py @@ -18,11 +18,13 @@ from __future__ import print_function from eventlet.green import socket +from six import string_types from six.moves.urllib.parse import urlparse -from swift.common.utils import SWIFT_CONF_FILE, md5_hash_for_file +from swift.common.utils import ( + SWIFT_CONF_FILE, md5_hash_for_file, set_swift_dir) from swift.common.ring import Ring -from swift.common.storage_policy import POLICIES +from swift.common.storage_policy import POLICIES, reload_storage_policies import eventlet import json import optparse @@ -916,7 +918,9 @@ class SwiftRecon(object): if self.server_type == 'object': ring_names = [p.ring_name for p in POLICIES if ( p.name == policy or not policy or ( - policy.isdigit() and int(policy) == int(p)))] + policy.isdigit() and int(policy) == int(p) or + (isinstance(policy, string_types) + and policy in p.aliases)))] else: ring_names = [self.server_type] @@ -1013,6 +1017,9 @@ class SwiftRecon(object): server_types = ['object'] swift_dir = options.swiftdir + if set_swift_dir(swift_dir): + reload_storage_policies() + self.verbose = options.verbose self.suppress_errors = options.suppress self.timeout = options.timeout diff --git a/swift/common/storage_policy.py b/swift/common/storage_policy.py index b1eac2de2a..36a9fde0d3 100644 --- a/swift/common/storage_policy.py +++ b/swift/common/storage_policy.py @@ -20,10 +20,10 @@ import textwrap import six from six.moves.configparser import ConfigParser from swift.common.utils import ( - config_true_value, SWIFT_CONF_FILE, whataremyips, list_from_csv, + config_true_value, quorum_size, whataremyips, list_from_csv, config_positive_int_value) from swift.common.ring import Ring, RingData -from swift.common.utils import quorum_size +from swift.common import utils from swift.common.exceptions import RingLoadError from pyeclib.ec_iface import ECDriver, ECDriverError, VALID_EC_TYPES @@ -925,15 +925,19 @@ class StoragePolicySingleton(object): def reload_storage_policies(): """ Reload POLICIES from ``swift.conf``. + + :param swift_conf_dir: non-default directory to read swift.conf from + This is by default /etc/swift/swift.conf. If given, + it will also trigger a re-validation of swift.conf """ global _POLICIES policy_conf = ConfigParser() - policy_conf.read(SWIFT_CONF_FILE) + policy_conf.read(utils.SWIFT_CONF_FILE) try: _POLICIES = parse_storage_policies(policy_conf) except PolicyError as e: raise SystemExit('ERROR: Invalid Storage Policy Configuration ' - 'in %s (%s)' % (SWIFT_CONF_FILE, e)) + 'in %s (%s)' % (utils.SWIFT_CONF_FILE, e)) # parse configuration and setup singleton diff --git a/swift/common/utils.py b/swift/common/utils.py index 1c06598f37..5882b77e2b 100644 --- a/swift/common/utils.py +++ b/swift/common/utils.py @@ -187,6 +187,29 @@ class InvalidHashPathConfigError(ValueError): "swift_hash_path_prefix are missing from %s" % SWIFT_CONF_FILE +def set_swift_dir(swift_dir): + """ + Sets the directory from which swift config files will be read. If the given + directory differs from that already set then the swift.conf file in the new + directory will be validated and storage policies will be reloaded from the + new swift.conf file. + + :param swift_dir: non-default directory to read swift.conf from + """ + global HASH_PATH_SUFFIX + global HASH_PATH_PREFIX + global SWIFT_CONF_FILE + if (swift_dir is not None and + swift_dir != os.path.dirname(SWIFT_CONF_FILE)): + SWIFT_CONF_FILE = os.path.join( + swift_dir, os.path.basename(SWIFT_CONF_FILE)) + HASH_PATH_PREFIX = '' + HASH_PATH_SUFFIX = '' + validate_configuration() + return True + return False + + def validate_hash_conf(): global HASH_PATH_SUFFIX global HASH_PATH_PREFIX diff --git a/test/functional/__init__.py b/test/functional/__init__.py index 5a90d6c229..3e3c4486d7 100644 --- a/test/functional/__init__.py +++ b/test/functional/__init__.py @@ -39,6 +39,7 @@ from six.moves.http_client import HTTPException from swift.common.middleware.memcache import MemcacheMiddleware from swift.common.storage_policy import parse_storage_policies, PolicyError +from swift.common.utils import set_swift_dir from test import get_config, listen_zero from test.functional.swift_test_client import Account, Connection, Container, \ @@ -106,9 +107,6 @@ skip, skip2, skip3, skip_service_tokens, skip_if_no_reseller_admin = \ orig_collate = '' insecure = False -orig_hash_path_suff_pref = ('', '') -orig_swift_conf_name = None - in_process = False _testdir = _test_servers = _test_coros = _test_socks = None policy_specified = None @@ -413,6 +411,7 @@ def in_process_setup(the_object_server=object_server): utils.mkdirs(os.path.join(_testdir, 'sdc1', 'tmp')) swift_conf = _in_process_setup_swift_conf(swift_conf_src, _testdir) + _info('prepared swift.conf: %s' % swift_conf) # Call the associated method for the value of # 'SWIFT_TEST_IN_PROCESS_CONF_LOADER', if one exists @@ -437,12 +436,11 @@ def in_process_setup(the_object_server=object_server): obj_sockets = _in_process_setup_ring(swift_conf, conf_src_dir, _testdir) - global orig_swift_conf_name - orig_swift_conf_name = utils.SWIFT_CONF_FILE - utils.SWIFT_CONF_FILE = swift_conf - constraints.reload_constraints() - storage_policy.SWIFT_CONF_FILE = swift_conf - storage_policy.reload_storage_policies() + # load new swift.conf file + if set_swift_dir(os.path.dirname(swift_conf)): + constraints.reload_constraints() + storage_policy.reload_storage_policies() + global config if constraints.SWIFT_CONSTRAINTS_LOADED: # Use the swift constraints that are loaded for the test framework @@ -453,9 +451,6 @@ def in_process_setup(the_object_server=object_server): else: # In-process swift constraints were not loaded, somethings wrong raise SkipTest - global orig_hash_path_suff_pref - orig_hash_path_suff_pref = utils.HASH_PATH_PREFIX, utils.HASH_PATH_SUFFIX - utils.validate_hash_conf() global _test_socks _test_socks = [] @@ -918,10 +913,7 @@ def teardown_package(): rmtree(os.path.dirname(_testdir)) except Exception: pass - utils.HASH_PATH_PREFIX, utils.HASH_PATH_SUFFIX = \ - orig_hash_path_suff_pref - utils.SWIFT_CONF_FILE = orig_swift_conf_name - constraints.reload_constraints() + reset_globals() diff --git a/test/unit/cli/test_recon.py b/test/unit/cli/test_recon.py index bdec8d83c8..f1e3c13664 100644 --- a/test/unit/cli/test_recon.py +++ b/test/unit/cli/test_recon.py @@ -16,11 +16,13 @@ import json import mock import os +import random import re import tempfile import time import unittest import shutil +import string import sys import six @@ -147,6 +149,7 @@ class TestScout(unittest.TestCase): @patch_policies class TestRecon(unittest.TestCase): def setUp(self, *_args, **_kwargs): + self.swift_conf_file = utils.SWIFT_CONF_FILE self.recon_instance = recon.SwiftRecon() self.swift_dir = tempfile.mkdtemp() self.ring_name = POLICIES.legacy.ring_name @@ -156,10 +159,24 @@ class TestRecon(unittest.TestCase): self.tmpfile_name2 = os.path.join( self.swift_dir, self.ring_name2 + '.ring.gz') - utils.HASH_PATH_SUFFIX = 'endcap' - utils.HASH_PATH_PREFIX = 'startcap' + swift_conf = os.path.join(self.swift_dir, 'swift.conf') + self.policy_name = ''.join(random.sample(string.letters, 20)) + with open(swift_conf, "wb") as sc: + sc.write(''' +[swift-hash] +swift_hash_path_suffix = changeme + +[storage-policy:0] +name = default +default = yes + +[storage-policy:1] +name = unu +aliases = %s +''' % self.policy_name) def tearDown(self, *_args, **_kwargs): + utils.SWIFT_CONF_FILE = self.swift_conf_file shutil.rmtree(self.swift_dir, ignore_errors=True) def _make_object_rings(self): @@ -590,7 +607,7 @@ class TestRecon(unittest.TestCase): self.assertEqual(expected, discovered_hosts) - def test_main_object_hosts_default_unu(self): + def _test_main_object_hosts_policy_name(self, policy_name='unu'): self._make_object_rings() discovered_hosts = set() @@ -602,7 +619,7 @@ class TestRecon(unittest.TestCase): with mock.patch.object(sys, 'argv', [ "prog", "object", "--swiftdir=%s" % self.swift_dir, - "--validate-servers", '--policy=unu']): + "--validate-servers", '--policy', policy_name]): self.recon_instance.main() @@ -612,6 +629,12 @@ class TestRecon(unittest.TestCase): ]) self.assertEqual(expected, discovered_hosts) + def test_main_object_hosts_default_unu(self): + self._test_main_object_hosts_policy_name() + + def test_main_object_hosts_default_alias(self): + self._test_main_object_hosts_policy_name(self.policy_name) + def test_main_object_hosts_default_invalid(self): self._make_object_rings() stdout = StringIO() diff --git a/test/unit/common/test_storage_policy.py b/test/unit/common/test_storage_policy.py index e40af6e507..615b346fe1 100644 --- a/test/unit/common/test_storage_policy.py +++ b/test/unit/common/test_storage_policy.py @@ -1051,7 +1051,7 @@ class TestStoragePolicies(unittest.TestCase): with NamedTemporaryFile() as f: conf.write(f) f.flush() - with mock.patch('swift.common.storage_policy.SWIFT_CONF_FILE', + with mock.patch('swift.common.utils.SWIFT_CONF_FILE', new=f.name): try: reload_storage_policies() diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index ea788c61a7..0a4843aa17 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -31,6 +31,7 @@ import mock import random import re import socket +import string import sys import json import math @@ -61,9 +62,11 @@ from swift.common.exceptions import Timeout, MessageTimeout, \ ConnectionTimeout, LockTimeout, ReplicationLockTimeout, \ MimeInvalid from swift.common import utils -from swift.common.utils import is_valid_ip, is_valid_ipv4, is_valid_ipv6 +from swift.common.utils import is_valid_ip, is_valid_ipv4, is_valid_ipv6, \ + set_swift_dir from swift.common.container_sync_realms import ContainerSyncRealms from swift.common.header_key_dict import HeaderKeyDict +from swift.common.storage_policy import POLICIES, reload_storage_policies from swift.common.swob import Request, Response from test.unit import FakeLogger, requires_o_tmpfile_support @@ -6186,5 +6189,36 @@ class TestHashForFileFunction(unittest.TestCase): self.fail('Some data did not compute expected hash:\n' + '\n'.join(failures)) + +class TestSetSwiftDir(unittest.TestCase): + def setUp(self): + self.swift_dir = tempfile.mkdtemp() + self.swift_conf = os.path.join(self.swift_dir, 'swift.conf') + self.policy_name = ''.join(random.sample(string.letters, 20)) + with open(self.swift_conf, "wb") as sc: + sc.write(''' +[swift-hash] +swift_hash_path_suffix = changeme + +[storage-policy:0] +name = default +default = yes + +[storage-policy:1] +name = %s +''' % self.policy_name) + + def tearDown(self): + shutil.rmtree(self.swift_dir, ignore_errors=True) + + def test_set_swift_dir(self): + set_swift_dir(None) + reload_storage_policies() + self.assertIsNone(POLICIES.get_by_name(self.policy_name)) + + set_swift_dir(self.swift_dir) + reload_storage_policies() + self.assertIsNotNone(POLICIES.get_by_name(self.policy_name)) + if __name__ == '__main__': unittest.main()