openstack/swift
Code Issues Proposed changes

Move parse search logic outside from builder

Browse Source 
Dramatic part of RingBuilder.search_devs which parse a complex format
of a search device string moved to the swift-ring-builder script.
Instead, the search_devs has a simple interface to search devices.

blueprint argparse-in-swift-ring-builder

Change-Id: If3dd77b297b474fb9a058e4693fef2dfb11fca3d
This commit is contained in:
Ilya Kharin 2013-05-16 16:58:03 +04:00
parent f57628a4ae
commit 43bf568f48
5 changed files with 215 additions and 156 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
bin/swift-ring-builder
View File

@ -28,6 +28,7 @@ from swift.common import exceptions
from swift.common.ring import RingBuilder from swift.common.ring import RingBuilder
from swift.common.ring.builder import MAX_BALANCE from swift.common.ring.builder import MAX_BALANCE
from swift.common.utils import lock_parent_directory from swift.common.utils import lock_parent_directory
from swift.common.ring.utils import parse_search_value
MAJOR_VERSION = 1 MAJOR_VERSION = 1
MINOR_VERSION = 3 MINOR_VERSION = 3
@ -134,9 +135,9 @@ swift-ring-builder <builder_file> search <search-value>
if len(argv) < 4: if len(argv) < 4:
print Commands.search.__doc__.strip() print Commands.search.__doc__.strip()
print print
print builder.search_devs.__doc__.strip() print parse_search_value.__doc__.strip()
exit(EXIT_ERROR) exit(EXIT_ERROR)
devs = builder.search_devs(argv[3]) devs = builder.search_devs(parse_search_value(argv[3]))
if not devs: if not devs:
print 'No matching devices found' print 'No matching devices found'
exit(EXIT_ERROR) exit(EXIT_ERROR)
@ -175,11 +176,11 @@ swift-ring-builder <builder_file> list_parts <search-value> [<search-value>] ..
if len(argv) < 4: if len(argv) < 4:
print Commands.list_parts.__doc__.strip() print Commands.list_parts.__doc__.strip()
print print
print builder.search_devs.__doc__.strip() print parse_search_value.__doc__.strip()
exit(EXIT_ERROR) exit(EXIT_ERROR)
devs = [] devs = []
for arg in argv[3:]: for arg in argv[3:]:
devs.extend(builder.search_devs(arg) or []) devs.extend(builder.search_devs(parse_search_value(arg)) or [])
if not devs: if not devs:
print 'No matching devices found' print 'No matching devices found'
exit(EXIT_ERROR) exit(EXIT_ERROR)
@ -329,15 +330,13 @@ swift-ring-builder <builder_file> add
(dev['id'], dev['ip'], dev['port'], dev['device']) (dev['id'], dev['ip'], dev['port'], dev['device'])
print "The on-disk ring builder is unchanged.\n" print "The on-disk ring builder is unchanged.\n"
exit(EXIT_ERROR) exit(EXIT_ERROR)
dev_params = {'region': region, 'zone': zone, 'ip': ip,
builder.add_dev({'region': region, 'zone': zone, 'ip': ip,
'port': port, 'replication_ip': replication_ip, 'port': port, 'replication_ip': replication_ip,
'replication_port': replication_port, 'replication_port': replication_port,
'device': device_name, 'weight': weight, 'device': device_name, 'weight': weight,
'meta': meta}) 'meta': meta}
new_dev = builder.search_devs( builder.add_dev(dev_params)
'r%dz%d-%s:%s/%s' % new_dev = builder.search_devs(dev_params)[0]
(region, zone, ip, port, device_name))[0]
print('Device %s with %s weight got id %s' % print('Device %s with %s weight got id %s' %
(format_device(new_dev), weight, new_dev['id'])) (format_device(new_dev), weight, new_dev['id']))
builder.save(argv[1]) builder.save(argv[1])
@ -355,13 +354,13 @@ swift-ring-builder <builder_file> set_weight <search-value> <weight>
if len(argv) < 5 or len(argv) % 2 != 1: if len(argv) < 5 or len(argv) % 2 != 1:
print Commands.set_weight.__doc__.strip() print Commands.set_weight.__doc__.strip()
print print
print builder.search_devs.__doc__.strip() print parse_search_value.__doc__.strip()
exit(EXIT_ERROR) exit(EXIT_ERROR)
devs_and_weights = izip(islice(argv, 3, len(argv), 2), devs_and_weights = izip(islice(argv, 3, len(argv), 2),
islice(argv, 4, len(argv), 2)) islice(argv, 4, len(argv), 2))
for devstr, weightstr in devs_and_weights: for devstr, weightstr in devs_and_weights:
devs = builder.search_devs(devstr) devs = builder.search_devs(parse_search_value(devstr))
weight = float(weightstr) weight = float(weightstr)
if not devs: if not devs:
print("Search value \"%s\" matched 0 devices.\n" print("Search value \"%s\" matched 0 devices.\n"
@ -402,14 +401,14 @@ swift-ring-builder <builder_file> set_info
if len(argv) < 5 or len(argv) % 2 != 1: if len(argv) < 5 or len(argv) % 2 != 1:
print Commands.set_info.__doc__.strip() print Commands.set_info.__doc__.strip()
print print
print builder.search_devs.__doc__.strip() print parse_search_value.__doc__.strip()
exit(EXIT_ERROR) exit(EXIT_ERROR)
searches_and_changes = izip(islice(argv, 3, len(argv), 2), searches_and_changes = izip(islice(argv, 3, len(argv), 2),
islice(argv, 4, len(argv), 2)) islice(argv, 4, len(argv), 2))
for search_value, change_value in searches_and_changes: for search_value, change_value in searches_and_changes:
devs = builder.search_devs(search_value) devs = builder.search_devs(parse_search_value(search_value))
change = [] change = []
if len(change_value) and change_value[0].isdigit(): if len(change_value) and change_value[0].isdigit():
i = 1 i = 1
@ -514,11 +513,11 @@ swift-ring-builder <builder_file> remove <search-value> [search-value ...]
if len(argv) < 4: if len(argv) < 4:
print Commands.remove.__doc__.strip() print Commands.remove.__doc__.strip()
print print
print builder.search_devs.__doc__.strip() print parse_search_value.__doc__.strip()
exit(EXIT_ERROR) exit(EXIT_ERROR)
for search_value in argv[3:]: for search_value in argv[3:]:
devs = builder.search_devs(search_value) devs = builder.search_devs(parse_search_value(search_value))
if not devs: if not devs:
print("Search value \"%s\" matched 0 devices.\n" print("Search value \"%s\" matched 0 devices.\n"
"The on-disk ring builder is unchanged." % search_value) "The on-disk ring builder is unchanged." % search_value)
@ -716,7 +715,7 @@ if __name__ == '__main__':
for cmd in cmds: for cmd in cmds:
print Commands.__dict__[cmd].__doc__.strip() print Commands.__dict__[cmd].__doc__.strip()
print print
print RingBuilder.search_devs.__doc__.strip() print parse_search_value.__doc__.strip()
print print
for line in wrap(' '.join(cmds), 79, initial_indent='Quick list: ', for line in wrap(' '.join(cmds), 79, initial_indent='Quick list: ',
subsequent_indent=' '): subsequent_indent=' '):

130
swift/common/ring/builder.py
View File

@ -1007,128 +1007,26 @@ class RingBuilder(object):
""" """
pickle.dump(self.to_dict(), open(builder_file, 'wb'), protocol=2) pickle.dump(self.to_dict(), open(builder_file, 'wb'), protocol=2)
def search_devs(self, search_value): def search_devs(self, search_values):
"""Search devices by parameters.
:param search_values: a dictionary with search values to filter
devices, supported parameters are id,
region, zone, ip, port, replication_ip,
replication_port, device, weight, meta
:returns: list of device dicts
""" """
The <search-value> can be of the form::
d<device_id>r<region>z<zone>-<ip>:<port>[R<r_ip>:<r_port>]/
<device_name>_<meta>
Where <r_ip> and <r_port> are replication ip and port.
Any part is optional, but you must include at least one part.
Examples::
d74 Matches the device id 74
r4 Matches devices in region 4
z1 Matches devices in zone 1
z1-1.2.3.4 Matches devices in zone 1 with the ip 1.2.3.4
1.2.3.4 Matches devices in any zone with the ip 1.2.3.4
z1:5678 Matches devices in zone 1 using port 5678
:5678 Matches devices that use port 5678
R5.6.7.8 Matches devices that use replication ip 5.6.7.8
R:5678 Matches devices that use replication port 5678
1.2.3.4R5.6.7.8 Matches devices that use ip 1.2.3.4 and replication ip
5.6.7.8
/sdb1 Matches devices with the device name sdb1
_shiny Matches devices with shiny in the meta data
_"snet: 5.6.7.8" Matches devices with snet: 5.6.7.8 in the meta data
[::1] Matches devices in any zone with the ip ::1
z1-[::1]:5678 Matches devices in zone 1 with ip ::1 and port 5678
Most specific example::
d74r4z1-1.2.3.4:5678/sdb1_"snet: 5.6.7.8"
Nerd explanation:
All items require their single character prefix except the ip, in which
case the - is optional unless the device id or zone is also included.
"""
orig_search_value = search_value
match = []
if search_value.startswith('d'):
i = 1
while i < len(search_value) and search_value[i].isdigit():
i += 1
match.append(('id', int(search_value[1:i])))
search_value = search_value[i:]
if search_value.startswith('r'):
i = 1
while i < len(search_value) and search_value[i].isdigit():
i += 1
match.append(('region', int(search_value[1:i])))
search_value = search_value[i:]
if search_value.startswith('z'):
i = 1
while i < len(search_value) and search_value[i].isdigit():
i += 1
match.append(('zone', int(search_value[1:i])))
search_value = search_value[i:]
if search_value.startswith('-'):
search_value = search_value[1:]
if len(search_value) and search_value[0].isdigit():
i = 1
while i < len(search_value) and search_value[i] in '0123456789.':
i += 1
match.append(('ip', search_value[:i]))
search_value = search_value[i:]
elif len(search_value) and search_value[0] == '[':
i = 1
while i < len(search_value) and search_value[i] != ']':
i += 1
i += 1
match.append(('ip', search_value[:i].lstrip('[').rstrip(']')))
search_value = search_value[i:]
if search_value.startswith(':'):
i = 1
while i < len(search_value) and search_value[i].isdigit():
i += 1
match.append(('port', int(search_value[1:i])))
search_value = search_value[i:]
# replication parameters
if search_value.startswith('R'):
search_value = search_value[1:]
if len(search_value) and search_value[0].isdigit():
i = 1
while (i < len(search_value) and
search_value[i] in '0123456789.'):
i += 1
match.append(('replication_ip', search_value[:i]))
search_value = search_value[i:]
elif len(search_value) and search_value[0] == '[':
i = 1
while i < len(search_value) and search_value[i] != ']':
i += 1
i += 1
match.append(('replication_ip',
search_value[:i].lstrip('[').rstrip(']')))
search_value = search_value[i:]
if search_value.startswith(':'):
i = 1
while i < len(search_value) and search_value[i].isdigit():
i += 1
match.append(('replication_port', int(search_value[1:i])))
search_value = search_value[i:]
if search_value.startswith('/'):
i = 1
while i < len(search_value) and search_value[i] != '_':
i += 1
match.append(('device', search_value[1:i]))
search_value = search_value[i:]
if search_value.startswith('_'):
match.append(('meta', search_value[1:]))
search_value = ''
if search_value:
raise ValueError('Invalid <search-value>: %s' %
repr(orig_search_value))
matched_devs = [] matched_devs = []
for dev in self.devs: for dev in self.devs:
if not dev: if not dev:
continue continue
matched = True matched = True
for key, value in match: for key in ('id', 'region', 'zone', 'ip', 'port', 'replication_ip',
'replication_port', 'device', 'weight', 'meta'):
if key in search_values:
value = search_values.get(key)
if value is not None:
if key == 'meta': if key == 'meta':
if value not in dev.get(key): if value not in dev.get(key):
matched = False matched = False

131
swift/common/ring/utils.py
View File

@ -1,3 +1,17 @@
# Copyright (c) 2010-2013 OpenStack, LLC.
#
# 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.
from collections import defaultdict from collections import defaultdict
@ -109,3 +123,120 @@ def build_tier_tree(devices):
else: else:
tier2children[()].add(tier) tier2children[()].add(tier)
return tier2children return tier2children
def parse_search_value(search_value):
"""The <search-value> can be of the form::
d<device_id>r<region>z<zone>-<ip>:<port>[R<r_ip>:<r_port>]/
<device_name>_<meta>
Where <r_ip> and <r_port> are replication ip and port.
Any part is optional, but you must include at least one part.
Examples::
d74 Matches the device id 74
r4 Matches devices in region 4
z1 Matches devices in zone 1
z1-1.2.3.4 Matches devices in zone 1 with the ip 1.2.3.4
1.2.3.4 Matches devices in any zone with the ip 1.2.3.4
z1:5678 Matches devices in zone 1 using port 5678
:5678 Matches devices that use port 5678
R5.6.7.8 Matches devices that use replication ip 5.6.7.8
R:5678 Matches devices that use replication port 5678
1.2.3.4R5.6.7.8 Matches devices that use ip 1.2.3.4 and replication ip
5.6.7.8
/sdb1 Matches devices with the device name sdb1
_shiny Matches devices with shiny in the meta data
_"snet: 5.6.7.8" Matches devices with snet: 5.6.7.8 in the meta data
[::1] Matches devices in any zone with the ip ::1
z1-[::1]:5678 Matches devices in zone 1 with ip ::1 and port 5678
Most specific example::
d74r4z1-1.2.3.4:5678/sdb1_"snet: 5.6.7.8"
Nerd explanation:
All items require their single character prefix except the ip, in which
case the - is optional unless the device id or zone is also included.
"""
orig_search_value = search_value
match = {}
if search_value.startswith('d'):
i = 1
while i < len(search_value) and search_value[i].isdigit():
i += 1
match['id'] = int(search_value[1:i])
search_value = search_value[i:]
if search_value.startswith('r'):
i = 1
while i < len(search_value) and search_value[i].isdigit():
i += 1
match['region'] = int(search_value[1:i])
search_value = search_value[i:]
if search_value.startswith('z'):
i = 1
while i < len(search_value) and search_value[i].isdigit():
i += 1
match['zone'] = int(search_value[1:i])
search_value = search_value[i:]
if search_value.startswith('-'):
search_value = search_value[1:]
if len(search_value) and search_value[0].isdigit():
i = 1
while i < len(search_value) and search_value[i] in '0123456789.':
i += 1
match['ip'] = search_value[:i]
search_value = search_value[i:]
elif len(search_value) and search_value[0] == '[':
i = 1
while i < len(search_value) and search_value[i] != ']':
i += 1
i += 1
match['ip'] = search_value[:i].lstrip('[').rstrip(']')
search_value = search_value[i:]
if search_value.startswith(':'):
i = 1
while i < len(search_value) and search_value[i].isdigit():
i += 1
match['port'] = int(search_value[1:i])
search_value = search_value[i:]
# replication parameters
if search_value.startswith('R'):
search_value = search_value[1:]
if len(search_value) and search_value[0].isdigit():
i = 1
while (i < len(search_value) and
search_value[i] in '0123456789.'):
i += 1
match['replication_ip'] = search_value[:i]
search_value = search_value[i:]
elif len(search_value) and search_value[0] == '[':
i = 1
while i < len(search_value) and search_value[i] != ']':
i += 1
i += 1
match['replication_ip'] = search_value[:i].lstrip('[').rstrip(']')
search_value = search_value[i:]
if search_value.startswith(':'):
i = 1
while i < len(search_value) and search_value[i].isdigit():
i += 1
match['replication_port'] = int(search_value[1:i])
search_value = search_value[i:]
if search_value.startswith('/'):
i = 1
while i < len(search_value) and search_value[i] != '_':
i += 1
match['device'] = search_value[1:i]
search_value = search_value[i:]
if search_value.startswith('_'):
match['meta'] = search_value[1:]
search_value = ''
if search_value:
raise ValueError('Invalid <search-value>: %s' %
repr(orig_search_value))
return match

29
test/unit/common/ring/test_builder.py
View File

@ -778,32 +778,33 @@ class TestRingBuilder(unittest.TestCase):
for d in devs: for d in devs:
rb.add_dev(d) rb.add_dev(d)
rb.rebalance() rb.rebalance()
res = rb.search_devs('r0') res = rb.search_devs({'region': 0})
self.assertEquals(res, [devs[0], devs[1]]) self.assertEquals(res, [devs[0], devs[1]])
res = rb.search_devs('r1') res = rb.search_devs({'region': 1})
self.assertEquals(res, [devs[2], devs[3]]) self.assertEquals(res, [devs[2], devs[3]])
res = rb.search_devs('r1z2') res = rb.search_devs({'region': 1, 'zone': 2})
self.assertEquals(res, [devs[2]]) self.assertEquals(res, [devs[2]])
res = rb.search_devs('d1') res = rb.search_devs({'id': 1})
self.assertEquals(res, [devs[1]]) self.assertEquals(res, [devs[1]])
res = rb.search_devs('z1') res = rb.search_devs({'zone': 1})
self.assertEquals(res, [devs[1]]) self.assertEquals(res, [devs[1]])
res = rb.search_devs('-127.0.0.1') res = rb.search_devs({'ip': '127.0.0.1'})
self.assertEquals(res, [devs[1]]) self.assertEquals(res, [devs[1]])
res = rb.search_devs('-[127.0.0.1]:10001') res = rb.search_devs({'ip': '127.0.0.1', 'port': 10001})
self.assertEquals(res, [devs[1]]) self.assertEquals(res, [devs[1]])
res = rb.search_devs(':10001') res = rb.search_devs({'port': 10001})
self.assertEquals(res, [devs[1]]) self.assertEquals(res, [devs[1]])
res = rb.search_devs('R127.0.0.10') res = rb.search_devs({'replication_ip': '127.0.0.10'})
self.assertEquals(res, [devs[4]]) self.assertEquals(res, [devs[4]])
res = rb.search_devs('R[127.0.0.10]:20000') res = rb.search_devs({'replication_ip': '127.0.0.10',
'replication_port': 20000})
self.assertEquals(res, [devs[4]]) self.assertEquals(res, [devs[4]])
res = rb.search_devs('R:20000') res = rb.search_devs({'replication_port': 20000})
self.assertEquals(res, [devs[4]]) self.assertEquals(res, [devs[4]])
res = rb.search_devs('/sdb1') res = rb.search_devs({'device': 'sdb1'})
self.assertEquals(res, [devs[1]])
res = rb.search_devs({'meta': 'meta1'})
self.assertEquals(res, [devs[1]]) self.assertEquals(res, [devs[1]])
res = rb.search_devs('_meta1')
self.assertRaises(ValueError, rb.search_devs, 'OMGPONIES')
def test_validate(self): def test_validate(self):
rb = ring.RingBuilder(8, 3, 1) rb = ring.RingBuilder(8, 3, 1)

32
test/unit/common/ring/test_utils.py
View File

@ -15,7 +15,8 @@
import unittest import unittest
from swift.common.ring.utils import build_tier_tree, tiers_for_dev from swift.common.ring.utils import (build_tier_tree, tiers_for_dev,
parse_search_value)
class TestUtils(unittest.TestCase): class TestUtils(unittest.TestCase):
@ -89,6 +90,35 @@ class TestUtils(unittest.TestCase):
(1, 2, '192.168.2.2:6000', 10), (1, 2, '192.168.2.2:6000', 10),
(1, 2, '192.168.2.2:6000', 11)])) (1, 2, '192.168.2.2:6000', 11)]))
def test_parse_search_value(self):
res = parse_search_value('r0')
self.assertEqual(res, {'region': 0})
res = parse_search_value('r1')
self.assertEqual(res, {'region': 1})
res = parse_search_value('r1z2')
self.assertEqual(res, {'region': 1, 'zone': 2})
res = parse_search_value('d1')
self.assertEqual(res, {'id': 1})
res = parse_search_value('z1')
self.assertEqual(res, {'zone': 1})
res = parse_search_value('-127.0.0.1')
self.assertEqual(res, {'ip': '127.0.0.1'})
res = parse_search_value('-[127.0.0.1]:10001')
self.assertEqual(res, {'ip': '127.0.0.1', 'port': 10001})
res = parse_search_value(':10001')
self.assertEqual(res, {'port': 10001})
res = parse_search_value('R127.0.0.10')
self.assertEqual(res, {'replication_ip': '127.0.0.10'})
res = parse_search_value('R[127.0.0.10]:20000')
self.assertEqual(res, {'replication_ip': '127.0.0.10', 'replication_port': 20000})
res = parse_search_value('R:20000')
self.assertEqual(res, {'replication_port': 20000})
res = parse_search_value('/sdb1')
self.assertEqual(res, {'device': 'sdb1'})
res = parse_search_value('_meta1')
self.assertEqual(res, {'meta': 'meta1'})
self.assertRaises(ValueError, parse_search_value, 'OMGPONIES')
if __name__ == '__main__': if __name__ == '__main__':
unittest.main() unittest.main()