Merge lp:~gholt/swift/autocreate into lp:~hudson-openstack/swift/trunk

[gholt]

Added account_autocreate mode where authorized requests to accounts that don't yet exist within Swift will cause those accounts to be automatically created. Also did a small refactor surrounding swift.common.utils.TRUE_VALUES.

[John Dickinson (notmyname)]

account_info doesn't retry or handle any results other than 200 to the autocreate PUT. Should it do more?

review: Needs Information

[gholt (gholt)]

Well, it's 2xx but yeah. Is there more you'd like it to do?

[gholt (gholt)]

Oh, I guess you mentioned retry. Not sure I'd want to implement retries on that since we don't really do that elsewhere in the proxy. But even if we don't retry, it seems like I should change non-2xx to cause a 5xx on the original request rather than a 404 or something.

[gholt (gholt)]

Okay, I made a failed account autocreate cause a 5xx now, which indicates the client should retry their request if desired.

[John Dickinson (notmyname)]

lgtm

review: Approve

[David Goetz (david-goetz)]

looks good

review: Approve

=== modified file 'doc/source/deployment_guide.rst'
--- doc/source/deployment_guide.rst	2011-05-26 02:24:12 +0000
+++ doc/source/deployment_guide.rst	2011-06-10 16:57:25 +0000
@@ -547,6 +547,10 @@
                                                node error limited
 allow_account_management      false            Whether account PUTs and DELETEs
                                                are even callable
+account_autocreate            false            If set to 'true' authorized
+                                               accounts that do not yet exist
+                                               within the Swift cluster will
+                                               be automatically created.
 ============================  ===============  =============================
 
 [tempauth]
 
=== modified file 'etc/proxy-server.conf-sample'
--- etc/proxy-server.conf-sample	2011-05-26 02:24:12 +0000
+++ etc/proxy-server.conf-sample	2011-06-10 16:57:25 +0000
@@ -40,6 +40,9 @@
 # If set to 'true' any authorized user may create and delete accounts; if
 # 'false' no one, even authorized, can.
 # allow_account_management = false
+# If set to 'true' authorized accounts that do not yet exist within the Swift
+# cluster will be automatically created.
+# account_autocreate = false
 
 [filter:tempauth]
 use = egg:swift#tempauth
 
=== modified file 'swift/common/bench.py'
--- swift/common/bench.py	2011-05-25 11:56:53 +0000
+++ swift/common/bench.py	2011-06-10 16:57:25 +0000
@@ -43,7 +43,7 @@
         self.user = conf.user
         self.key = conf.key
         self.auth_url = conf.auth
-        self.use_proxy = conf.use_proxy in TRUE_VALUES
+        self.use_proxy = conf.use_proxy.lower() in TRUE_VALUES
         if self.use_proxy:
             url, token = client.get_auth(self.auth_url, self.user, self.key)
             self.token = token
@@ -125,7 +125,7 @@
         self.logger = logger
         self.conf = conf
         self.names = []
-        self.delete = conf.delete in TRUE_VALUES
+        self.delete = conf.delete.lower() in TRUE_VALUES
         self.gets = int(conf.num_gets)
 
     def run(self):
 
=== modified file 'swift/common/daemon.py'
--- swift/common/daemon.py	2011-02-21 18:50:56 +0000
+++ swift/common/daemon.py	2011-06-10 16:57:25 +0000
@@ -75,7 +75,8 @@
                           log_name=kwargs.get('log_name'))
 
     # once on command line (i.e. daemonize=false) will over-ride config
-    once = once or conf.get('daemonize', 'true') not in utils.TRUE_VALUES
+    once = once or \
+            conf.get('daemonize', 'true').lower() not in utils.TRUE_VALUES
 
     # pre-configure logger
     if 'logger' in kwargs:
 
=== modified file 'swift/common/middleware/staticweb.py'
--- swift/common/middleware/staticweb.py	2011-05-26 02:24:12 +0000
+++ swift/common/middleware/staticweb.py	2011-06-10 16:57:25 +0000
@@ -270,7 +270,7 @@
         :param start_response: The original WSGI start_response hook.
         :param prefix: Any prefix desired for the container listing.
         """
-        if self._listings not in TRUE_VALUES:
+        if self._listings.lower() not in TRUE_VALUES:
             resp = HTTPNotFound()(env, self._start_response)
             return self._error_response(resp, env, start_response)
         tmp_env = self._get_escalated_env(env)
 
=== modified file 'swift/common/utils.py'
--- swift/common/utils.py	2011-04-20 19:54:28 +0000
+++ swift/common/utils.py	2011-06-10 16:57:25 +0000
@@ -72,7 +72,7 @@
         pass
 
 # Used when reading config values
-TRUE_VALUES = set(('true', '1', 'yes', 'True', 'Yes', 'on', 'On', 't', 'y'))
+TRUE_VALUES = set(('true', '1', 'yes', 'on', 't', 'y'))
 
 
 def validate_configuration():
 
=== modified file 'swift/proxy/server.py'
--- swift/proxy/server.py	2011-05-12 15:57:35 +0000
+++ swift/proxy/server.py	2011-06-10 16:57:25 +0000
@@ -41,8 +41,8 @@
 from webob import Request, Response
 
 from swift.common.ring import Ring
-from swift.common.utils import get_logger, normalize_timestamp, split_path, \
-    cache_from_env, ContextPool
+from swift.common.utils import cache_from_env, ContextPool, get_logger, \
+    normalize_timestamp, split_path, TRUE_VALUES
 from swift.common.bufferedhttp import http_connect
 from swift.common.constraints import check_metadata, check_object_creation, \
     check_utf8, CONTAINER_LISTING_LIMIT, MAX_ACCOUNT_NAME_LENGTH, \
@@ -338,7 +338,7 @@
         node['errors'] = self.app.error_suppression_limit + 1
         node['last_error'] = time.time()
 
-    def account_info(self, account):
+    def account_info(self, account, autocreate=False):
         """
         Get account information, and also verify that the account exists.
 
@@ -353,7 +353,7 @@
             result_code = self.app.memcache.get(cache_key)
             if result_code == 200:
                 return partition, nodes
-            elif result_code == 404:
+            elif result_code == 404 and not autocreate:
                 return None, None
         result_code = 0
         attempts_left = self.app.account_ring.replica_count
@@ -386,6 +386,17 @@
             except (Exception, TimeoutError):
                 self.exception_occurred(node, _('Account'),
                     _('Trying to get account info for %s') % path)
+        if result_code == 404 and autocreate:
+            if len(account) > MAX_ACCOUNT_NAME_LENGTH:
+                return None, None
+            headers = {'X-Timestamp': normalize_timestamp(time.time()),
+                       'X-Trans-Id': self.trans_id}
+            resp = self.make_requests(Request.blank('/v1' + path),
+                self.app.account_ring, partition, 'PUT',
+                path, [headers] * len(nodes))
+            if resp.status_int // 100 != 2:
+                raise Exception('Could not autocreate account %r' % path)
+            result_code = 200
         if self.app.memcache and result_code in (200, 404):
             if result_code == 200:
                 cache_timeout = self.app.recheck_account_existence
@@ -397,7 +408,7 @@
             return partition, nodes
         return None, None
 
-    def container_info(self, account, container):
+    def container_info(self, account, container, account_autocreate=False):
         """
         Get container information and thusly verify container existance.
         This will also make a call to account_info to verify that the
@@ -423,7 +434,7 @@
                     return partition, nodes, read_acl, write_acl
                 elif status == 404:
                     return None, None, None, None
-        if not self.account_info(account)[1]:
+        if not self.account_info(account, autocreate=account_autocreate)[1]:
             return None, None, None, None
         result_code = 0
         read_acl = None
@@ -854,7 +865,8 @@
         if error_response:
             return error_response
         container_partition, containers, _junk, req.acl = \
-            self.container_info(self.account_name, self.container_name)
+            self.container_info(self.account_name, self.container_name,
+                account_autocreate=self.app.account_autocreate)
         if 'swift.authorize' in req.environ:
             aresp = req.environ['swift.authorize'](req)
             if aresp:
@@ -911,7 +923,8 @@
     def PUT(self, req):
         """HTTP PUT request handler."""
         container_partition, containers, _junk, req.acl = \
-            self.container_info(self.account_name, self.container_name)
+            self.container_info(self.account_name, self.container_name,
+                account_autocreate=self.app.account_autocreate)
         if 'swift.authorize' in req.environ:
             aresp = req.environ['swift.authorize'](req)
             if aresp:
@@ -1219,7 +1232,8 @@
             resp.body = 'Container name length of %d longer than %d' % \
                         (len(self.container_name), MAX_CONTAINER_NAME_LENGTH)
             return resp
-        account_partition, accounts = self.account_info(self.account_name)
+        account_partition, accounts = self.account_info(self.account_name,
+            autocreate=self.app.account_autocreate)
         if not accounts:
             return HTTPNotFound(request=req)
         container_partition, containers = self.app.container_ring.get_nodes(
@@ -1249,7 +1263,8 @@
             self.clean_acls(req) or check_metadata(req, 'container')
         if error_response:
             return error_response
-        account_partition, accounts = self.account_info(self.account_name)
+        account_partition, accounts = self.account_info(self.account_name,
+            autocreate=self.app.account_autocreate)
         if not accounts:
             return HTTPNotFound(request=req)
         container_partition, containers = self.app.container_ring.get_nodes(
@@ -1391,7 +1406,7 @@
         self.put_queue_depth = int(conf.get('put_queue_depth', 10))
         self.object_chunk_size = int(conf.get('object_chunk_size', 65536))
         self.client_chunk_size = int(conf.get('client_chunk_size', 65536))
-        self.log_headers = conf.get('log_headers') == 'True'
+        self.log_headers = conf.get('log_headers', 'no').lower() in TRUE_VALUES
         self.error_suppression_interval = \
             int(conf.get('error_suppression_interval', 60))
         self.error_suppression_limit = \
@@ -1401,7 +1416,7 @@
         self.recheck_account_existence = \
             int(conf.get('recheck_account_existence', 60))
         self.allow_account_management = \
-            conf.get('allow_account_management', 'false').lower() == 'true'
+            conf.get('allow_account_management', 'no').lower() in TRUE_VALUES
         self.resellers_conf = ConfigParser()
         self.resellers_conf.read(os.path.join(swift_dir, 'resellers.conf'))
         self.object_ring = object_ring or \
@@ -1413,6 +1428,8 @@
         self.memcache = memcache
         mimetypes.init(mimetypes.knownfiles +
                        [os.path.join(swift_dir, 'mime.types')])
+        self.account_autocreate = \
+            conf.get('account_autocreate', 'no').lower() in TRUE_VALUES
 
     def get_controller(self, path):
         """
 
=== modified file 'swift/stats/log_uploader.py'
--- swift/stats/log_uploader.py	2011-05-24 20:51:07 +0000
+++ swift/stats/log_uploader.py	2011-06-10 16:57:25 +0000
@@ -69,7 +69,7 @@
         self.internal_proxy = InternalProxy(proxy_server_conf)
         self.new_log_cutoff = int(cutoff or
                                   uploader_conf.get('new_log_cutoff', '7200'))
-        self.unlink_log = uploader_conf.get('unlink_log', 'True').lower() in \
+        self.unlink_log = uploader_conf.get('unlink_log', 'true').lower() in \
                 utils.TRUE_VALUES
         self.filename_pattern = regex or \
             uploader_conf.get('source_filename_pattern',
 
=== modified file 'test/unit/common/test_utils.py'
--- test/unit/common/test_utils.py	2011-03-30 20:04:15 +0000
+++ test/unit/common/test_utils.py	2011-06-10 16:57:25 +0000
@@ -768,6 +768,10 @@
         self.assertEquals(utils.human_readable(1237940039285380274899124224),
                           '1024Yi')
 
+    def test_TRUE_VALUES(self):
+        for v in utils.TRUE_VALUES:
+            self.assertEquals(v, v.lower())
+
 
 if __name__ == '__main__':
     unittest.main()
 
=== modified file 'test/unit/proxy/test_server.py'
--- test/unit/proxy/test_server.py	2011-05-09 20:21:34 +0000
+++ test/unit/proxy/test_server.py	2011-06-10 16:57:25 +0000
@@ -393,6 +393,48 @@
             test(404, 507, 503)
             test(503, 503, 503)
 
+    def test_account_info_account_autocreate(self):
+        with save_globals():
+            self.memcache.store = {}
+            proxy_server.http_connect = \
+                fake_http_connect(404, 404, 404, 201, 201, 201)
+            partition, nodes = \
+                self.controller.account_info(self.account, autocreate=False)
+            self.check_account_info_return(partition, nodes, is_none=True)
+
+            self.memcache.store = {}
+            proxy_server.http_connect = \
+                fake_http_connect(404, 404, 404, 201, 201, 201)
+            partition, nodes = \
+                self.controller.account_info(self.account)
+            self.check_account_info_return(partition, nodes, is_none=True)
+
+            self.memcache.store = {}
+            proxy_server.http_connect = \
+                fake_http_connect(404, 404, 404, 201, 201, 201)
+            partition, nodes = \
+                self.controller.account_info(self.account, autocreate=True)
+            self.check_account_info_return(partition, nodes)
+
+            self.memcache.store = {}
+            proxy_server.http_connect = \
+                fake_http_connect(404, 404, 404, 503, 201, 201)
+            partition, nodes = \
+                self.controller.account_info(self.account, autocreate=True)
+            self.check_account_info_return(partition, nodes)
+
+            self.memcache.store = {}
+            proxy_server.http_connect = \
+                fake_http_connect(404, 404, 404, 503, 201, 503)
+            exc = None
+            try:
+                partition, nodes = \
+                    self.controller.account_info(self.account, autocreate=True)
+            except Exception, err:
+                exc = err
+            self.assertEquals(str(exc),
+                              "Could not autocreate account '/some_account'")
+
     def check_container_info_return(self, ret, is_none=False):
         if is_none:
             partition, nodes, read_acl, write_acl = None, None, None, None
@@ -406,7 +448,7 @@
         self.assertEqual(write_acl, ret[3])
 
     def test_container_info_invalid_account(self):
-        def account_info(self, account):
+        def account_info(self, account, autocreate=False):
             return None, None
 
         with save_globals():
@@ -417,7 +459,7 @@
 
     # tests if 200 is cached and used
     def test_container_info_200(self):
-        def account_info(self, account):
+        def account_info(self, account, autocreate=False):
             return True, True
 
         with save_globals():
@@ -443,7 +485,7 @@
 
     # tests if 404 is cached and used
     def test_container_info_404(self):
-        def account_info(self, account):
+        def account_info(self, account, autocreate=False):
             return True, True
 
         with save_globals():