Add actions to blacklist osd-devices

The blacklist actions allow for adding and removing devices
to a unit-local list of devices to be skipped during osd
initialization. This list will be used to override the
application level, and thereby deployment wide, 'osd-devices'
configuration option on a individual unit basis.

The pre-existing list-disk action is extended to return
list of blacklisted devices under the 'blacklist' key.

Change-Id: I28a3c5d6076fb496dead3fe3387d9bbbbe9ec083
Closes-Bug: #1730267
This commit is contained in:
Frode Nordahl 2017-11-06 15:03:24 +01:00
parent e2c41c215c
commit c4d4e42c1a
10 changed files with 386 additions and 2 deletions

View File

@ -48,3 +48,39 @@ add-disk:
description: The name of the bucket in Ceph to add these devices into
required:
- osd-devices
blacklist-add-disk:
description: |
Add disk(s) to blacklist. Blacklisted disks will not be
initialized for use with Ceph even if listed in the application
level osd-devices configuration option.
.
The current blacklist can be viewed with list-disks action.
.
NOTE: This action and blacklist will not have any effect on
already initialized disks.
params:
osd-devices:
type: string
description: |
A space-separated list of devices to add to blacklist.
.
Each element should be a absolute path to a device node or filesystem
directory (the latter is supported for ceph >= 0.56.6).
.
Example: '/dev/vdb /var/tmp/test-osd'
required:
- osd-devices
blacklist-remove-disk:
description: Remove disk(s) from blacklist.
params:
osd-devices:
type: string
description: |
A space-separated list of devices to remove from blacklist.
.
Each element should be a existing entry in the units blacklist.
Use list-disks action to list current blacklist entries.
.
Example: '/dev/vdb /var/tmp/test-osd'
required:
- osd-devices

1
actions/blacklist-add-disk Symbolic link
View File

@ -0,0 +1 @@
blacklist.py

View File

@ -0,0 +1 @@
blacklist.py

102
actions/blacklist.py Executable file
View File

@ -0,0 +1,102 @@
#!/usr/bin/env python
#
# Copyright 2017 Canonical Ltd
#
# 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.
import os
import sys
sys.path.append('hooks')
import charmhelpers.core.hookenv as hookenv
import charmhelpers.core.unitdata as unitdata
BLACKLIST_KEY = 'osd-blacklist'
class Error(Exception):
def __init__(self, message):
self.message = message
def __str__(self):
return repr(self.message)
def get_devices():
"""Parse 'osd-devices' action parameter, returns list."""
devices = []
for path in hookenv.action_get('osd-devices').split(' '):
path = path.strip()
if not os.path.isabs(path):
raise Error('{}: Not absolute path.'.format(path))
devices.append(path)
return devices
def blacklist_add():
"""
Add devices given in 'osd-devices' action parameter to
unit-local devices blacklist.
"""
db = unitdata.kv()
blacklist = db.get(BLACKLIST_KEY, [])
for device in get_devices():
if not os.path.exists(device):
raise Error('{}: No such file or directory.'.format(device))
if device not in blacklist:
blacklist.append(device)
db.set(BLACKLIST_KEY, blacklist)
db.flush()
def blacklist_remove():
"""
Remove devices given in 'osd-devices' action parameter from
unit-local devices blacklist.
"""
db = unitdata.kv()
blacklist = db.get(BLACKLIST_KEY, [])
for device in get_devices():
try:
blacklist.remove(device)
except ValueError:
raise Error('{}: Device not in blacklist.'.format(device))
db.set(BLACKLIST_KEY, blacklist)
db.flush()
# A dictionary of all the defined actions to callables
ACTIONS = {
"blacklist-add-disk": blacklist_add,
"blacklist-remove-disk": blacklist_remove,
}
def main(args):
"""Main program"""
action_name = os.path.basename(args[0])
try:
action = ACTIONS[action_name]
except KeyError:
return "Action {} undefined".format(action_name)
else:
try:
action()
except Exception as e:
hookenv.action_fail("Action {} failed: {}"
"".format(action_name, str(e)))
if __name__ == "__main__":
sys.exit(main(sys.argv))

View File

@ -29,7 +29,10 @@ sys.path.append('lib/')
import charmhelpers.core.hookenv as hookenv
import ceph.utils
import utils
if __name__ == '__main__':
hookenv.action_set({
'disks': ceph.utils.unmounted_disks()})
'disks': ceph.utils.unmounted_disks(),
'blacklist': utils.get_blacklist(),
})

View File

@ -66,6 +66,7 @@ from utils import (
is_unit_paused_set,
get_public_addr,
get_cluster_addr,
get_blacklist,
)
from charmhelpers.contrib.openstack.alternatives import install_alternative
from charmhelpers.contrib.network.ip import (
@ -441,7 +442,11 @@ def get_devices():
# their block device paths to the list.
storage_ids = storage_list('osd-devices')
devices.extend((storage_get('location', s) for s in storage_ids))
return devices
# Filter out any devices in the action managed unit-local device blacklist
return filter(
lambda device: device not in get_blacklist(), devices
)
def get_journal_devices():
@ -451,6 +456,11 @@ def get_journal_devices():
devices = []
storage_ids = storage_list('osd-journals')
devices.extend((storage_get('location', s) for s in storage_ids))
# Filter out any devices in the action managed unit-local device blacklist
devices = filter(
lambda device: device not in get_blacklist(), devices
)
devices = filter(os.path.exists, devices)
return set(devices)

View File

@ -207,3 +207,9 @@ def is_unit_paused_set():
return not(not(kv.get('unit-paused')))
except:
return False
def get_blacklist():
"""Get blacklist stored in the local kv() store"""
db = unitdata.kv()
return db.get('osd-blacklist', [])

View File

@ -672,3 +672,52 @@ class CephOsdBasicDeployment(OpenStackAmuletDeployment):
assert u.wait_on_action(action_id), "Resume action failed."
assert u.status_get(sentry_unit)[0] == "active"
u.log.debug('OK')
def test_911_blacklist(self):
"""The blacklist actions execute and behave as expected. """
u.log.debug('Checking blacklist-add-disk and'
'blacklist-remove-disk actions...')
sentry_unit = self.ceph_osd_sentry
assert u.status_get(sentry_unit)[0] == "active"
# Attempt to add device with non-absolute path should fail
action_id = u.run_action(sentry_unit,
"blacklist-add-disk",
params={"osd-devices": "vda"})
assert not u.wait_on_action(action_id), "completed"
assert u.status_get(sentry_unit)[0] == "active"
# Attempt to add device with non-existent path should fail
action_id = u.run_action(sentry_unit,
"blacklist-add-disk",
params={"osd-devices": "/non-existent"})
assert not u.wait_on_action(action_id), "completed"
assert u.status_get(sentry_unit)[0] == "active"
# Attempt to add device with existent path should succeed
action_id = u.run_action(sentry_unit,
"blacklist-add-disk",
params={"osd-devices": "/dev/vda"})
assert u.wait_on_action(action_id), "completed"
assert u.status_get(sentry_unit)[0] == "active"
# Attempt to remove listed device should always succeed
action_id = u.run_action(sentry_unit,
"blacklist-remove-disk",
params={"osd-devices": "/dev/vda"})
assert u.wait_on_action(action_id), "completed"
assert u.status_get(sentry_unit)[0] == "active"
u.log.debug('OK')
def test_912_list_disks(self):
"""The list-disks action execute. """
u.log.debug('Checking list-disks action...')
sentry_unit = self.ceph_osd_sentry
assert u.status_get(sentry_unit)[0] == "active"
action_id = u.run_action(sentry_unit, "list-disks")
assert u.wait_on_action(action_id), "completed"
assert u.status_get(sentry_unit)[0] == "active"
u.log.debug('OK')

View File

@ -0,0 +1,141 @@
# Copyright 2017 Canonical Ltd
#
# 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.
import mock
from charmhelpers.core import hookenv
from actions import blacklist
from test_utils import CharmTestCase
class BlacklistActionTests(CharmTestCase):
def setUp(self):
super(BlacklistActionTests, self).setUp(
blacklist, [])
@mock.patch('os.path.isabs')
@mock.patch('os.path.exists')
@mock.patch('charmhelpers.core.unitdata.kv')
@mock.patch('charmhelpers.core.hookenv.action_get')
def test_add_disk(self, _action_get, _kv, _exists, _isabs):
"""Add device with absolute and existent path succeeds"""
_action_get.return_value = '/dev/vda'
_kv.return_value = _kv
_kv.get.return_value = []
_exists.return_value = True
_isabs.return_value = True
blacklist.blacklist_add()
_exists.assert_called()
_isabs.assert_called()
_kv.get.assert_called_with('osd-blacklist', [])
_kv.set.assert_called_with('osd-blacklist', ['/dev/vda'])
_kv.flush.assert_called()
@mock.patch('os.path.isabs')
@mock.patch('os.path.exists')
@mock.patch('charmhelpers.core.unitdata.kv')
@mock.patch('charmhelpers.core.hookenv.action_get')
def test_add_disk_nonexistent(self, _action_get, _kv, _exists, _isabs):
"""Add device with non-existent path raises exception"""
_action_get.return_value = '/dev/vda'
_kv.return_value = _kv
_kv.get.return_value = []
_exists.return_value = False
_isabs.return_value = True
with self.assertRaises(blacklist.Error):
blacklist.blacklist_add()
_isabs.assert_called()
_exists.assert_called()
_kv.get.assert_called_with('osd-blacklist', [])
assert not _kv.set.called
assert not _kv.flush.called
@mock.patch('os.path.isabs')
@mock.patch('os.path.exists')
@mock.patch('charmhelpers.core.unitdata.kv')
@mock.patch('charmhelpers.core.hookenv.action_get')
def test_add_disk_nonabsolute(self, _action_get, _kv, _exists, _isabs):
"""Add device with non-absolute path raises exception"""
_action_get.return_value = 'vda'
_kv.return_value = _kv
_kv.get.return_value = []
_exists.return_value = True
_isabs.return_value = False
with self.assertRaises(blacklist.Error):
blacklist.blacklist_add()
_isabs.assert_called()
_kv.get.assert_called_with('osd-blacklist', [])
assert not _exists.called
assert not _kv.set.called
assert not _kv.flush.called
@mock.patch('charmhelpers.core.unitdata.kv')
@mock.patch('charmhelpers.core.hookenv.action_get')
def test_remove_disk(self, _action_get, _kv):
"""Remove action succeeds, and regardless of existence of device"""
_action_get.return_value = '/nonexistent2'
_kv.return_value = _kv
_kv.get.return_value = ['/nonexistent1', '/nonexistent2']
blacklist.blacklist_remove()
_kv.get.assert_called_with('osd-blacklist', [])
_kv.set.assert_called_with('osd-blacklist', ['/nonexistent1'])
_kv.flush.assert_called()
@mock.patch('charmhelpers.core.unitdata.kv')
@mock.patch('charmhelpers.core.hookenv.action_get')
def test_remove_disk_nonlisted(self, _action_get, _kv):
"""Remove action raises on removal of device not in list"""
_action_get.return_value = '/nonexistent3'
_kv.return_value = _kv
_kv.get.return_value = ['/nonexistent1', '/nonexistent2']
with self.assertRaises(blacklist.Error):
blacklist.blacklist_remove()
_kv.get.assert_called_with('osd-blacklist', [])
assert not _kv.set.called
assert not _kv.flush.called
class MainTestCase(CharmTestCase):
def setUp(self):
super(MainTestCase, self).setUp(hookenv, ["action_fail"])
def test_invokes_action(self):
dummy_calls = []
def dummy_action():
dummy_calls.append(True)
with mock.patch.dict(blacklist.ACTIONS, {"foo": dummy_action}):
blacklist.main(["foo"])
self.assertEqual(dummy_calls, [True])
def test_unknown_action(self):
"""Unknown actions aren't a traceback."""
exit_string = blacklist.main(["foo"])
self.assertEqual("Action foo undefined", exit_string)
def test_failing_action(self):
"""Actions which traceback trigger action_fail() calls."""
dummy_calls = []
self.action_fail.side_effect = dummy_calls.append
def dummy_action():
raise ValueError("uh oh")
with mock.patch.dict(blacklist.ACTIONS, {"foo": dummy_action}):
blacklist.main(["foo"])
self.assertEqual(dummy_calls, ["Action foo failed: uh oh"])

View File

@ -339,3 +339,38 @@ class CephHooksTestCase(unittest.TestCase):
mock_storage_list.assert_called()
mock_os_path_exists.assert_called()
self.assertEqual(devices, set(['/dev/vda', '/dev/vdb']))
@patch.object(ceph_hooks, 'get_blacklist')
@patch.object(ceph_hooks, 'storage_list')
@patch.object(ceph_hooks, 'config')
def test_get_devices_blacklist(self, mock_config, mock_storage_list,
mock_get_blacklist):
'''Devices returned as expected when blacklist in effect'''
config = {'osd-devices': '/dev/vda /dev/vdb'}
mock_config.side_effect = lambda key: config[key]
mock_storage_list.return_value = []
mock_get_blacklist.return_value = ['/dev/vda']
devices = ceph_hooks.get_devices()
mock_storage_list.assert_called()
mock_get_blacklist.assert_called()
self.assertEqual(devices, ['/dev/vdb'])
@patch('os.path.exists')
@patch.object(ceph_hooks, 'get_blacklist')
@patch.object(ceph_hooks, 'storage_list')
@patch.object(ceph_hooks, 'config')
def test_get_journal_devices_blacklist(self, mock_config,
mock_storage_list,
mock_get_blacklist,
mock_os_path_exists):
'''Devices returned as expected when blacklist in effect'''
config = {'osd-journal': '/dev/vda /dev/vdb'}
mock_config.side_effect = lambda key: config[key]
mock_storage_list.return_value = []
mock_get_blacklist.return_value = ['/dev/vda']
mock_os_path_exists.return_value = True
devices = ceph_hooks.get_journal_devices()
mock_storage_list.assert_called()
mock_os_path_exists.assert_called()
mock_get_blacklist.assert_called()
self.assertEqual(devices, set(['/dev/vdb']))