openstack
/
charm-ceph-osd
Code Issues Proposed changes

Don't clobber apparmor profiles. 

The charm attempts to avoid restarts of the ceph-osd processes
by asssessing whether new or changed apparmor profiles have
been installed.

The charm always copies apparmor files from the charm to
/etc/apparmor.d which overwrites any local changes made to the
profile - specifically if the profile is in complain mode.

As the content of the profile then changes the restart_on_change
decorator then fires the aa_profile_changed function which
switches the profile back into enforce mode.

This change only overwrites apparmor profiles in the event
that the hash of the source file in the charm changes; if it
does then the current profile mode is re-asserted to ensure
that complain mode deployments don't switch to enforce by
mistake.

Change-Id: I8f8cbf17af6219bd9fbdcf71a7000cba4c63f3f3
Closes-Bug: 1783373
Co-Authored-By: James Page <james.page@ubuntu.com>
This commit is contained in:
Pete Vander Giessen 2019-02-15 20:11:29 +00:00 committed by James Page
parent 63160a3956
commit 7b242ebbc3
2 changed files with 29 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
hooks/ceph_hooks.py
View File

@ -53,12 +53,12 @@ from charmhelpers.core.host import (
is_container,
lsb_release,
mkdir,
restart_on_change,
service_reload,
service_restart,
umount,
write_file,
CompareHostReleases,
file_hash,
)
from charmhelpers.fetch import (
add_source,
@ -193,25 +193,33 @@ def aa_profile_changed(service_name='ceph-osd-all'):
service_restart(service_name)
@restart_on_change({
'/etc/apparmor.d/usr.bin.ceph-osd': ['ceph-osd-all']},
restart_functions={'ceph-osd-all': aa_profile_changed})
def copy_profile_into_place():
"""
Copy the apparmor profiles included with the charm
into the /etc/apparmor.d directory.
"""
new_install = False
apparmor_dir = os.path.join(os.sep,
'etc',
'apparmor.d')
File are only copied if they have changed at source
to avoid overwriting any aa-complain mode flags set
:returns: flag indicating if any profiles where newly
installed or changed
:rtype: boolean
"""
db = kv()
changes = False
apparmor_dir = os.path.join(os.sep, 'etc', 'apparmor.d')
for x in glob.glob('files/apparmor/*'):
if not os.path.exists(os.path.join(apparmor_dir,
os.path.basename(x))):
new_install = True
shutil.copy(x, apparmor_dir)
return new_install
db_key = 'hash:{}'.format(x)
new_hash = file_hash(x)
previous_hash = db.get(db_key)
if new_hash != previous_hash:
log('Installing apparmor profile for {}'
.format(os.path.basename(x)))
shutil.copy(x, apparmor_dir)
db.set(db_key, new_hash)
db.flush()
changes = True
return changes
class CephOsdAppArmorContext(AppArmorContext):
@ -261,9 +269,12 @@ def install_apparmor_profile():
based on current setting of 'aa-profile-mode'
configuration option.
"""
log('Installing apparmor profile for ceph-osd')
new_install = copy_profile_into_place()
if new_install or config().changed('aa-profile-mode'):
changes = copy_profile_into_place()
# NOTE(jamespage): If any profiles where changed or
# freshly installed then force
# re-assertion of the current profile mode
# to avoid complain->enforce side effects
if changes or config().changed('aa-profile-mode'):
aa_context = CephOsdAppArmorContext()
aa_context.setup_aa_profile()
aa_profile_changed()

2
unit_tests/test_ceph_hooks.py
View File

@ -323,8 +323,8 @@ class CephHooksTestCase(unittest.TestCase):
m_aa_context.setup_aa_profile.assert_called()
mock_copy_profile_into_place.assert_called()
m_config.changed.assert_called_with('aa-profile-mode')
mock_service_restart.assert_called_with('ceph-osd-all')
m_config.changed.assert_called_with('aa-profile-mode')
mock_service_reload.assert_called_with('apparmor')
@patch.object(ceph_hooks, 'ceph')