openstack
/
charms.ceph
Code Issues Proposed changes

Fixup python 3 compatibility for subprocess calls 

check_output needs to be decoded in py3 environments; provide
a helper wrapper function todo this + update unit tests to deal
with this slightly different approach to UTF-8 decoding.

Change-Id: I95c4fce0d5cc997ba19f22b611cf99fbd5b17dd1
This commit is contained in:
James Page 2017-03-10 13:38:51 +00:00
parent c7a2595fab
commit 8e58c99777
4 changed files with 61 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
ceph/__init__.py
View File

@ -62,6 +62,8 @@ from charmhelpers.contrib.storage.linux.utils import (
from charmhelpers.contrib.openstack.utils import (
get_os_codename_install_source)
from ceph.ceph_helpers import check_output
CEPH_BASE_DIR = os.path.join(os.sep, 'var', 'lib', 'ceph')
OSD_BASE_DIR = os.path.join(CEPH_BASE_DIR, 'osd')
HDPARM_FILE = os.path.join(os.sep, 'etc', 'hdparm.conf')
@ -179,7 +181,7 @@ def tune_nic(network_interface):
try:
# Apply the settings
log("Applying sysctl settings", level=DEBUG)
subprocess.check_output(["sysctl", "-p", sysctl_file])
check_output(["sysctl", "-p", sysctl_file])
except subprocess.CalledProcessError as err:
log('sysctl -p {} failed with error {}'.format(sysctl_file,
err.output),
@ -318,9 +320,9 @@ def set_hdd_read_ahead(dev_name, read_ahead_sectors=256):
log('Setting read ahead to {} for device {}'.format(
read_ahead_sectors,
dev_name))
subprocess.check_output(['hdparm',
'-a{}'.format(read_ahead_sectors),
dev_name])
check_output(['hdparm',
'-a{}'.format(read_ahead_sectors),
dev_name])
except subprocess.CalledProcessError as e:
log('hdparm failed with error: {}'.format(e.output),
level=ERROR)
@ -333,7 +335,7 @@ def get_block_uuid(block_dev):
:return: The UUID of the device or None on Error.
"""
try:
block_info = subprocess.check_output(
block_info = check_output(
['blkid', '-o', 'export', block_dev])
for tag in block_info.split('\n'):
parts = tag.split('=')
@ -482,7 +484,7 @@ def get_osd_weight(osd_id):
Also raises CalledProcessError if our ceph command fails
"""
try:
tree = subprocess.check_output(
tree = check_output(
['ceph', 'osd', 'tree', '--format=json'])
try:
json_tree = json.loads(tree)
@ -509,7 +511,7 @@ def get_osd_tree(service):
Also raises CalledProcessError if our ceph command fails
"""
try:
tree = subprocess.check_output(
tree = check_output(
['ceph', '--id', service,
'osd', 'tree', '--format=json'])
try:
@ -685,7 +687,7 @@ def is_quorum():
]
if os.path.exists(asok):
try:
result = json.loads(subprocess.check_output(cmd))
result = json.loads(check_output(cmd))
except subprocess.CalledProcessError:
return False
except ValueError:
@ -712,7 +714,7 @@ def is_leader():
]
if os.path.exists(asok):
try:
result = json.loads(subprocess.check_output(cmd))
result = json.loads(check_output(cmd))
except subprocess.CalledProcessError:
return False
except ValueError:
@ -819,7 +821,7 @@ def replace_osd(dead_osd_number,
# Drop this osd out of the cluster. This will begin a
# rebalance operation
status_set('maintenance', 'Removing osd {}'.format(dead_osd_number))
subprocess.check_output([
check_output([
'ceph',
'--id',
'osd-upgrade',
@ -830,8 +832,8 @@ def replace_osd(dead_osd_number,
if systemd():
service_stop('ceph-osd@{}'.format(dead_osd_number))
else:
subprocess.check_output(['stop', 'ceph-osd', 'id={}'.format(
dead_osd_number)]),
check_output(['stop', 'ceph-osd', 'id={}'.format(
dead_osd_number)])
# umount if still mounted
ret = umount(mount_point)
if ret < 0:
@ -839,20 +841,20 @@ def replace_osd(dead_osd_number,
mount_point, os.strerror(ret)))
# Clean up the old mount point
shutil.rmtree(mount_point)
subprocess.check_output([
check_output([
'ceph',
'--id',
'osd-upgrade',
'osd', 'crush', 'remove',
'osd.{}'.format(dead_osd_number)])
# Revoke the OSDs access keys
subprocess.check_output([
check_output([
'ceph',
'--id',
'osd-upgrade',
'auth', 'del',
'osd.{}'.format(dead_osd_number)])
subprocess.check_output([
check_output([
'ceph',
'--id',
'osd-upgrade',
@ -871,7 +873,7 @@ def replace_osd(dead_osd_number,
def is_osd_disk(dev):
try:
info = subprocess.check_output(['sgdisk', '-i', '1', dev])
info = check_output(['sgdisk', '-i', '1', dev])
info = info.split("\n") # IGNORE:E1103
for line in info:
for ptype in CEPH_PARTITIONS:
@ -952,7 +954,7 @@ def generate_monitor_secret():
'--name=mon.',
'--gen-key'
]
res = subprocess.check_output(cmd)
res = check_output(cmd)
return "{}==".format(res.split('=')[1].strip())
@ -1100,8 +1102,8 @@ def create_named_keyring(entity, name, caps=None):
]
for subsystem, subcaps in caps.items():
cmd.extend([subsystem, '; '.join(subcaps)])
log("Calling subprocess.check_output: {}".format(cmd), level=DEBUG)
return parse_key(subprocess.check_output(cmd).strip()) # IGNORE:E1103
log("Calling check_output: {}".format(cmd), level=DEBUG)
return parse_key(check_output(cmd).strip()) # IGNORE:E1103
def get_upgrade_key():
@ -1118,7 +1120,7 @@ def get_named_key(name, caps=None, pool_list=None):
"""
try:
# Does the key already exist?
output = subprocess.check_output(
output = check_output(
[
'sudo',
'-u', ceph_user(),
@ -1158,8 +1160,8 @@ def get_named_key(name, caps=None, pool_list=None):
pools = " ".join(['pool={0}'.format(i) for i in pool_list])
subcaps[0] = subcaps[0] + " " + pools
cmd.extend([subsystem, '; '.join(subcaps)])
log("Calling subprocess.check_output: {}".format(cmd), level=DEBUG)
return parse_key(subprocess.check_output(cmd).strip()) # IGNORE:E1103
log("Calling check_output: {}".format(cmd), level=DEBUG)
return parse_key(check_output(cmd).strip()) # IGNORE:E1103
def upgrade_key_caps(key, caps):
@ -1251,7 +1253,7 @@ def maybe_zap_journal(journal_dev):
def get_partitions(dev):
cmd = ['partx', '--raw', '--noheadings', dev]
try:
out = subprocess.check_output(cmd).splitlines()
out = check_output(cmd).splitlines()
log("get partitions: {}".format(out), level=DEBUG)
return out
except subprocess.CalledProcessError as e:
@ -1361,7 +1363,7 @@ def get_running_osds():
"""Returns a list of the pids of the current running OSD daemons"""
cmd = ['pgrep', 'ceph-osd']
try:
result = subprocess.check_output(cmd)
result = check_output(cmd)
return result.split()
except subprocess.CalledProcessError:
return []
@ -1377,9 +1379,9 @@ def get_cephfs(service):
# This command wasn't introduced until 0.86 ceph
return []
try:
output = subprocess.check_output(["ceph",
'--id', service,
"fs", "ls"])
output = check_output(["ceph",
'--id', service,
"fs", "ls"])
if not output:
return []
"""
@ -1881,7 +1883,7 @@ def list_pools(service):
"""
try:
pool_list = []
pools = subprocess.check_output(['rados', '--id', service, 'lspools'])
pools = check_output(['rados', '--id', service, 'lspools'])
for pool in pools.splitlines():
pool_list.append(pool)
return pool_list

31
ceph/ceph_helpers.py
View File

@ -36,7 +36,11 @@ import uuid
import re
import subprocess
from subprocess import (check_call, check_output, CalledProcessError, )
from subprocess import (
check_call,
check_output as s_check_output,
CalledProcessError,
)
from charmhelpers.core.hookenv import (config,
local_unit,
relation_get,
@ -111,6 +115,15 @@ DEFAULT_POOL_WEIGHT = 10.0
LEGACY_PG_COUNT = 200
def check_output(*args, **kwargs):
'''
Helper wrapper for py2/3 compat with subprocess.check_output
@returns str: UTF-8 decoded representation of output
'''
return s_check_output(*args, **kwargs).decode('UTF-8')
def validator(value, valid_type, valid_range=None):
"""
Used to validate these: http://docs.ceph.com/docs/master/rados/operations/
@ -188,7 +201,7 @@ class Crushmap(object):
stdout=subprocess.PIPE)
return subprocess.check_output(
('crushtool', '-d', '-'),
stdin=crush.stdout).decode('utf-8')
stdin=crush.stdout)
except Exception as e:
log("load_crushmap error: {}".format(e))
raise "Failed to read Crushmap"
@ -565,7 +578,8 @@ def monitor_key_delete(service, key):
:param key: six.string_types. The key to delete.
"""
try:
check_output(['ceph', '--id', service, 'config-key', 'del', str(key)])
check_output(['ceph', '--id', service,
'config-key', 'del', str(key)])
except CalledProcessError as e:
log("Monitor config-key put failed with message: {}".format(e.output))
raise
@ -867,8 +881,7 @@ def get_cache_mode(service, pool_name):
def pool_exists(service, name):
"""Check to see if a RADOS pool already exists."""
try:
out = check_output(['rados', '--id', service, 'lspools']).decode(
'UTF-8')
out = check_output(['rados', '--id', service, 'lspools'])
except CalledProcessError:
return False
@ -882,7 +895,7 @@ def get_osds(service):
version = ceph_version()
if version and version >= '0.56':
return json.loads(check_output(['ceph', '--id', service, 'osd', 'ls',
'--format=json']).decode('UTF-8'))
'--format=json']))
return None
@ -900,7 +913,7 @@ def rbd_exists(service, pool, rbd_img):
"""Check to see if a RADOS block device exists."""
try:
out = check_output(['rbd', 'list', '--id', service, '--pool', pool
]).decode('UTF-8')
])
except CalledProcessError:
return False
@ -1025,7 +1038,7 @@ def configure(service, key, auth, use_syslog):
def image_mapped(name):
"""Determine whether a RADOS block device is mapped locally."""
try:
out = check_output(['rbd', 'showmapped']).decode('UTF-8')
out = check_output(['rbd', 'showmapped'])
except CalledProcessError:
return False
@ -1212,7 +1225,7 @@ def ceph_version():
"""Retrieve the local version of ceph."""
if os.path.exists('/usr/bin/ceph'):
cmd = ['ceph', '-v']
output = check_output(cmd).decode('US-ASCII')
output = check_output(cmd)
output = output.split()
if len(output) > 3:
return output[2]

6
unit_tests/test_ceph.py
View File

@ -38,7 +38,7 @@ class CephTestCase(unittest.TestCase):
def setUp(self):
super(CephTestCase, self).setUp()
@mock.patch('subprocess.check_output')
@mock.patch.object(ceph, 'check_output')
def test_get_osd_weight(self, output):
"""It gives an OSD's weight"""
output.return_value = """{
@ -107,7 +107,7 @@ class CephTestCase(unittest.TestCase):
def test_get_named_key_with_pool(self):
with mock.patch.object(ceph, "ceph_user", return_value="ceph"):
with mock.patch.object(ceph.subprocess, "check_output") \
with mock.patch.object(ceph, "check_output") \
as subprocess:
with mock.patch.object(ceph.socket, "gethostname",
return_value="osd001"):
@ -129,7 +129,7 @@ class CephTestCase(unittest.TestCase):
def test_get_named_key(self):
with mock.patch.object(ceph, "ceph_user", return_value="ceph"):
with mock.patch.object(ceph.subprocess, "check_output") \
with mock.patch.object(ceph, "check_output") \
as subprocess:
subprocess.side_effect = [
CalledProcessError(0, 0, 0),

12
unit_tests/test_ceph_helpers.py
View File

@ -19,19 +19,19 @@ if not six.PY3:
else:
builtin_open = 'builtins.open'
LS_POOLS = b"""
LS_POOLS = """
images
volumes
rbd
"""
LS_RBDS = b"""
LS_RBDS = """
rbd1
rbd2
rbd3
"""
IMG_MAP = b"""
IMG_MAP = """
bar
baz
"""
@ -408,7 +408,7 @@ class CephUtilsTests(unittest.TestCase):
@patch.object(ceph_utils, 'ceph_version')
def test_get_osds(self, version):
version.return_value = '0.56.2'
self.check_output.return_value = json.dumps([1, 2, 3]).encode('UTF-8')
self.check_output.return_value = json.dumps([1, 2, 3])
self.assertEquals(ceph_utils.get_osds('test'), [1, 2, 3])
@patch.object(ceph_utils, 'ceph_version')
@ -419,7 +419,7 @@ class CephUtilsTests(unittest.TestCase):
@patch.object(ceph_utils, 'ceph_version')
def test_get_osds_none(self, version):
version.return_value = '0.56.2'
self.check_output.return_value = json.dumps(None).encode('UTF-8')
self.check_output.return_value = json.dumps(None)
self.assertEquals(ceph_utils.get_osds('test'), None)
@patch.object(ceph_utils, 'get_osds')
@ -917,7 +917,7 @@ class CephUtilsTests(unittest.TestCase):
def test_ceph_version_ok(self, path, output):
path.return_value = True
output.return_value = \
b'ceph version 0.67.4 (ad85b8bfafea6232d64cb7ba76a8b6e8252fa0c7)'
'ceph version 0.67.4 (ad85b8bfafea6232d64cb7ba76a8b6e8252fa0c7)'
self.assertEquals(ceph_utils.ceph_version(), '0.67.4')
@patch.object(ceph_utils.Pool, 'get_pgs')