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 <alistairncoles@gmail.com>
Co-Authored-By: Thiago da Silva <thiago@redhat.com>
Change-Id: I13188d42ec19e32e4420739eacd1e5b454af2ae3

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:

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:

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

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

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

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
+    # load new swift.conf file
-    orig_swift_conf_name = utils.SWIFT_CONF_FILE
+    if set_swift_dir(os.path.dirname(swift_conf)):
-    utils.SWIFT_CONF_FILE = swift_conf
+        constraints.reload_constraints()
-    constraints.reload_constraints()
+        storage_policy.reload_storage_policies()
-    storage_policy.SWIFT_CONF_FILE = swift_conf
+
-    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()
 
 

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'
+        swift_conf = os.path.join(self.swift_dir, 'swift.conf')
-        utils.HASH_PATH_PREFIX = 'startcap'
+        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()

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()

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()