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

Proposed by gholt
Status: Merged
Approved by: David Goetz
Approved revision: 311
Merged at revision: 308
Proposed branch: lp:~gholt/swift/autocreate
Merge into: lp:~hudson-openstack/swift/trunk
Diff against target: 334 lines (+92/-21)
10 files modified
doc/source/deployment_guide.rst (+4/-0)
etc/proxy-server.conf-sample (+3/-0)
swift/common/bench.py (+2/-2)
swift/common/daemon.py (+2/-1)
swift/common/middleware/staticweb.py (+1/-1)
swift/common/utils.py (+1/-1)
swift/proxy/server.py (+29/-12)
swift/stats/log_uploader.py (+1/-1)
test/unit/common/test_utils.py (+4/-0)
test/unit/proxy/test_server.py (+45/-3)
To merge this branch: bzr merge lp:~gholt/swift/autocreate
Reviewer Review Type Date Requested Status
David Goetz (community) Approve
John Dickinson Approve
Review via email: mp+64206@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
John Dickinson (notmyname) wrote :

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

review: Needs Information
Revision history for this message
gholt (gholt) wrote :

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

Revision history for this message
gholt (gholt) wrote :

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.

Revision history for this message
gholt (gholt) wrote :

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

Revision history for this message
John Dickinson (notmyname) wrote :

lgtm

review: Approve
Revision history for this message
David Goetz (david-goetz) wrote :

looks good

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'doc/source/deployment_guide.rst'
2--- doc/source/deployment_guide.rst 2011-05-26 02:24:12 +0000
3+++ doc/source/deployment_guide.rst 2011-06-10 16:57:25 +0000
4@@ -547,6 +547,10 @@
5 node error limited
6 allow_account_management false Whether account PUTs and DELETEs
7 are even callable
8+account_autocreate false If set to 'true' authorized
9+ accounts that do not yet exist
10+ within the Swift cluster will
11+ be automatically created.
12 ============================ =============== =============================
13
14 [tempauth]
15
16=== modified file 'etc/proxy-server.conf-sample'
17--- etc/proxy-server.conf-sample 2011-05-26 02:24:12 +0000
18+++ etc/proxy-server.conf-sample 2011-06-10 16:57:25 +0000
19@@ -40,6 +40,9 @@
20 # If set to 'true' any authorized user may create and delete accounts; if
21 # 'false' no one, even authorized, can.
22 # allow_account_management = false
23+# If set to 'true' authorized accounts that do not yet exist within the Swift
24+# cluster will be automatically created.
25+# account_autocreate = false
26
27 [filter:tempauth]
28 use = egg:swift#tempauth
29
30=== modified file 'swift/common/bench.py'
31--- swift/common/bench.py 2011-05-25 11:56:53 +0000
32+++ swift/common/bench.py 2011-06-10 16:57:25 +0000
33@@ -43,7 +43,7 @@
34 self.user = conf.user
35 self.key = conf.key
36 self.auth_url = conf.auth
37- self.use_proxy = conf.use_proxy in TRUE_VALUES
38+ self.use_proxy = conf.use_proxy.lower() in TRUE_VALUES
39 if self.use_proxy:
40 url, token = client.get_auth(self.auth_url, self.user, self.key)
41 self.token = token
42@@ -125,7 +125,7 @@
43 self.logger = logger
44 self.conf = conf
45 self.names = []
46- self.delete = conf.delete in TRUE_VALUES
47+ self.delete = conf.delete.lower() in TRUE_VALUES
48 self.gets = int(conf.num_gets)
49
50 def run(self):
51
52=== modified file 'swift/common/daemon.py'
53--- swift/common/daemon.py 2011-02-21 18:50:56 +0000
54+++ swift/common/daemon.py 2011-06-10 16:57:25 +0000
55@@ -75,7 +75,8 @@
56 log_name=kwargs.get('log_name'))
57
58 # once on command line (i.e. daemonize=false) will over-ride config
59- once = once or conf.get('daemonize', 'true') not in utils.TRUE_VALUES
60+ once = once or \
61+ conf.get('daemonize', 'true').lower() not in utils.TRUE_VALUES
62
63 # pre-configure logger
64 if 'logger' in kwargs:
65
66=== modified file 'swift/common/middleware/staticweb.py'
67--- swift/common/middleware/staticweb.py 2011-05-26 02:24:12 +0000
68+++ swift/common/middleware/staticweb.py 2011-06-10 16:57:25 +0000
69@@ -270,7 +270,7 @@
70 :param start_response: The original WSGI start_response hook.
71 :param prefix: Any prefix desired for the container listing.
72 """
73- if self._listings not in TRUE_VALUES:
74+ if self._listings.lower() not in TRUE_VALUES:
75 resp = HTTPNotFound()(env, self._start_response)
76 return self._error_response(resp, env, start_response)
77 tmp_env = self._get_escalated_env(env)
78
79=== modified file 'swift/common/utils.py'
80--- swift/common/utils.py 2011-04-20 19:54:28 +0000
81+++ swift/common/utils.py 2011-06-10 16:57:25 +0000
82@@ -72,7 +72,7 @@
83 pass
84
85 # Used when reading config values
86-TRUE_VALUES = set(('true', '1', 'yes', 'True', 'Yes', 'on', 'On', 't', 'y'))
87+TRUE_VALUES = set(('true', '1', 'yes', 'on', 't', 'y'))
88
89
90 def validate_configuration():
91
92=== modified file 'swift/proxy/server.py'
93--- swift/proxy/server.py 2011-05-12 15:57:35 +0000
94+++ swift/proxy/server.py 2011-06-10 16:57:25 +0000
95@@ -41,8 +41,8 @@
96 from webob import Request, Response
97
98 from swift.common.ring import Ring
99-from swift.common.utils import get_logger, normalize_timestamp, split_path, \
100- cache_from_env, ContextPool
101+from swift.common.utils import cache_from_env, ContextPool, get_logger, \
102+ normalize_timestamp, split_path, TRUE_VALUES
103 from swift.common.bufferedhttp import http_connect
104 from swift.common.constraints import check_metadata, check_object_creation, \
105 check_utf8, CONTAINER_LISTING_LIMIT, MAX_ACCOUNT_NAME_LENGTH, \
106@@ -338,7 +338,7 @@
107 node['errors'] = self.app.error_suppression_limit + 1
108 node['last_error'] = time.time()
109
110- def account_info(self, account):
111+ def account_info(self, account, autocreate=False):
112 """
113 Get account information, and also verify that the account exists.
114
115@@ -353,7 +353,7 @@
116 result_code = self.app.memcache.get(cache_key)
117 if result_code == 200:
118 return partition, nodes
119- elif result_code == 404:
120+ elif result_code == 404 and not autocreate:
121 return None, None
122 result_code = 0
123 attempts_left = self.app.account_ring.replica_count
124@@ -386,6 +386,17 @@
125 except (Exception, TimeoutError):
126 self.exception_occurred(node, _('Account'),
127 _('Trying to get account info for %s') % path)
128+ if result_code == 404 and autocreate:
129+ if len(account) > MAX_ACCOUNT_NAME_LENGTH:
130+ return None, None
131+ headers = {'X-Timestamp': normalize_timestamp(time.time()),
132+ 'X-Trans-Id': self.trans_id}
133+ resp = self.make_requests(Request.blank('/v1' + path),
134+ self.app.account_ring, partition, 'PUT',
135+ path, [headers] * len(nodes))
136+ if resp.status_int // 100 != 2:
137+ raise Exception('Could not autocreate account %r' % path)
138+ result_code = 200
139 if self.app.memcache and result_code in (200, 404):
140 if result_code == 200:
141 cache_timeout = self.app.recheck_account_existence
142@@ -397,7 +408,7 @@
143 return partition, nodes
144 return None, None
145
146- def container_info(self, account, container):
147+ def container_info(self, account, container, account_autocreate=False):
148 """
149 Get container information and thusly verify container existance.
150 This will also make a call to account_info to verify that the
151@@ -423,7 +434,7 @@
152 return partition, nodes, read_acl, write_acl
153 elif status == 404:
154 return None, None, None, None
155- if not self.account_info(account)[1]:
156+ if not self.account_info(account, autocreate=account_autocreate)[1]:
157 return None, None, None, None
158 result_code = 0
159 read_acl = None
160@@ -854,7 +865,8 @@
161 if error_response:
162 return error_response
163 container_partition, containers, _junk, req.acl = \
164- self.container_info(self.account_name, self.container_name)
165+ self.container_info(self.account_name, self.container_name,
166+ account_autocreate=self.app.account_autocreate)
167 if 'swift.authorize' in req.environ:
168 aresp = req.environ['swift.authorize'](req)
169 if aresp:
170@@ -911,7 +923,8 @@
171 def PUT(self, req):
172 """HTTP PUT request handler."""
173 container_partition, containers, _junk, req.acl = \
174- self.container_info(self.account_name, self.container_name)
175+ self.container_info(self.account_name, self.container_name,
176+ account_autocreate=self.app.account_autocreate)
177 if 'swift.authorize' in req.environ:
178 aresp = req.environ['swift.authorize'](req)
179 if aresp:
180@@ -1219,7 +1232,8 @@
181 resp.body = 'Container name length of %d longer than %d' % \
182 (len(self.container_name), MAX_CONTAINER_NAME_LENGTH)
183 return resp
184- account_partition, accounts = self.account_info(self.account_name)
185+ account_partition, accounts = self.account_info(self.account_name,
186+ autocreate=self.app.account_autocreate)
187 if not accounts:
188 return HTTPNotFound(request=req)
189 container_partition, containers = self.app.container_ring.get_nodes(
190@@ -1249,7 +1263,8 @@
191 self.clean_acls(req) or check_metadata(req, 'container')
192 if error_response:
193 return error_response
194- account_partition, accounts = self.account_info(self.account_name)
195+ account_partition, accounts = self.account_info(self.account_name,
196+ autocreate=self.app.account_autocreate)
197 if not accounts:
198 return HTTPNotFound(request=req)
199 container_partition, containers = self.app.container_ring.get_nodes(
200@@ -1391,7 +1406,7 @@
201 self.put_queue_depth = int(conf.get('put_queue_depth', 10))
202 self.object_chunk_size = int(conf.get('object_chunk_size', 65536))
203 self.client_chunk_size = int(conf.get('client_chunk_size', 65536))
204- self.log_headers = conf.get('log_headers') == 'True'
205+ self.log_headers = conf.get('log_headers', 'no').lower() in TRUE_VALUES
206 self.error_suppression_interval = \
207 int(conf.get('error_suppression_interval', 60))
208 self.error_suppression_limit = \
209@@ -1401,7 +1416,7 @@
210 self.recheck_account_existence = \
211 int(conf.get('recheck_account_existence', 60))
212 self.allow_account_management = \
213- conf.get('allow_account_management', 'false').lower() == 'true'
214+ conf.get('allow_account_management', 'no').lower() in TRUE_VALUES
215 self.resellers_conf = ConfigParser()
216 self.resellers_conf.read(os.path.join(swift_dir, 'resellers.conf'))
217 self.object_ring = object_ring or \
218@@ -1413,6 +1428,8 @@
219 self.memcache = memcache
220 mimetypes.init(mimetypes.knownfiles +
221 [os.path.join(swift_dir, 'mime.types')])
222+ self.account_autocreate = \
223+ conf.get('account_autocreate', 'no').lower() in TRUE_VALUES
224
225 def get_controller(self, path):
226 """
227
228=== modified file 'swift/stats/log_uploader.py'
229--- swift/stats/log_uploader.py 2011-05-24 20:51:07 +0000
230+++ swift/stats/log_uploader.py 2011-06-10 16:57:25 +0000
231@@ -69,7 +69,7 @@
232 self.internal_proxy = InternalProxy(proxy_server_conf)
233 self.new_log_cutoff = int(cutoff or
234 uploader_conf.get('new_log_cutoff', '7200'))
235- self.unlink_log = uploader_conf.get('unlink_log', 'True').lower() in \
236+ self.unlink_log = uploader_conf.get('unlink_log', 'true').lower() in \
237 utils.TRUE_VALUES
238 self.filename_pattern = regex or \
239 uploader_conf.get('source_filename_pattern',
240
241=== modified file 'test/unit/common/test_utils.py'
242--- test/unit/common/test_utils.py 2011-03-30 20:04:15 +0000
243+++ test/unit/common/test_utils.py 2011-06-10 16:57:25 +0000
244@@ -768,6 +768,10 @@
245 self.assertEquals(utils.human_readable(1237940039285380274899124224),
246 '1024Yi')
247
248+ def test_TRUE_VALUES(self):
249+ for v in utils.TRUE_VALUES:
250+ self.assertEquals(v, v.lower())
251+
252
253 if __name__ == '__main__':
254 unittest.main()
255
256=== modified file 'test/unit/proxy/test_server.py'
257--- test/unit/proxy/test_server.py 2011-05-09 20:21:34 +0000
258+++ test/unit/proxy/test_server.py 2011-06-10 16:57:25 +0000
259@@ -393,6 +393,48 @@
260 test(404, 507, 503)
261 test(503, 503, 503)
262
263+ def test_account_info_account_autocreate(self):
264+ with save_globals():
265+ self.memcache.store = {}
266+ proxy_server.http_connect = \
267+ fake_http_connect(404, 404, 404, 201, 201, 201)
268+ partition, nodes = \
269+ self.controller.account_info(self.account, autocreate=False)
270+ self.check_account_info_return(partition, nodes, is_none=True)
271+
272+ self.memcache.store = {}
273+ proxy_server.http_connect = \
274+ fake_http_connect(404, 404, 404, 201, 201, 201)
275+ partition, nodes = \
276+ self.controller.account_info(self.account)
277+ self.check_account_info_return(partition, nodes, is_none=True)
278+
279+ self.memcache.store = {}
280+ proxy_server.http_connect = \
281+ fake_http_connect(404, 404, 404, 201, 201, 201)
282+ partition, nodes = \
283+ self.controller.account_info(self.account, autocreate=True)
284+ self.check_account_info_return(partition, nodes)
285+
286+ self.memcache.store = {}
287+ proxy_server.http_connect = \
288+ fake_http_connect(404, 404, 404, 503, 201, 201)
289+ partition, nodes = \
290+ self.controller.account_info(self.account, autocreate=True)
291+ self.check_account_info_return(partition, nodes)
292+
293+ self.memcache.store = {}
294+ proxy_server.http_connect = \
295+ fake_http_connect(404, 404, 404, 503, 201, 503)
296+ exc = None
297+ try:
298+ partition, nodes = \
299+ self.controller.account_info(self.account, autocreate=True)
300+ except Exception, err:
301+ exc = err
302+ self.assertEquals(str(exc),
303+ "Could not autocreate account '/some_account'")
304+
305 def check_container_info_return(self, ret, is_none=False):
306 if is_none:
307 partition, nodes, read_acl, write_acl = None, None, None, None
308@@ -406,7 +448,7 @@
309 self.assertEqual(write_acl, ret[3])
310
311 def test_container_info_invalid_account(self):
312- def account_info(self, account):
313+ def account_info(self, account, autocreate=False):
314 return None, None
315
316 with save_globals():
317@@ -417,7 +459,7 @@
318
319 # tests if 200 is cached and used
320 def test_container_info_200(self):
321- def account_info(self, account):
322+ def account_info(self, account, autocreate=False):
323 return True, True
324
325 with save_globals():
326@@ -443,7 +485,7 @@
327
328 # tests if 404 is cached and used
329 def test_container_info_404(self):
330- def account_info(self, account):
331+ def account_info(self, account, autocreate=False):
332 return True, True
333
334 with save_globals():