openstack
/
tripleo-heat-templates
Code Issues Proposed changes

Avoid failing on deleted file 

Avoid failing on deleted file as sometimes file might get
deleted while the script run.  Log the exception instead for
troubleshooting purposes.

Change-Id: I733cec2b34ef0bd0780ba5b0520127b911505e1b
This commit is contained in:
David Hill 2020-06-05 14:12:31 -04:00 committed by Oliver Walsh
parent d58efb58e0
commit 6c3c8b41de
2 changed files with 120 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
container_config_scripts/nova_statedir_ownership.py
View File

@ -36,14 +36,22 @@ class PathManager(object):
"""Helper class to manipulate ownership of a given path""" """Helper class to manipulate ownership of a given path"""
def __init__(self, path): def __init__(self, path):
self.path = path self.path = path
self.uid = None
self.gid = None
self.is_dir = None
self.secontext = None
self._update() self._update()
def _update(self): def _update(self):
statinfo = os.stat(self.path) try:
self.is_dir = stat.S_ISDIR(statinfo.st_mode) statinfo = os.stat(self.path)
self.uid = statinfo.st_uid self.is_dir = stat.S_ISDIR(statinfo.st_mode)
self.gid = statinfo.st_gid self.uid = statinfo.st_uid
self.secontext = selinux.lgetfilecon(self.path)[1] self.gid = statinfo.st_gid
self.secontext = selinux.lgetfilecon(self.path)[1]
except Exception:
LOG.exception('Could not update metadata for %s', self.path)
raise
def __str__(self): def __str__(self):
return "uid: {} gid: {} path: {}{}".format( return "uid: {} gid: {} path: {}{}".format(
@ -79,6 +87,7 @@ class PathManager(object):
except Exception: except Exception:
LOG.exception('Could not change ownership of %s: ', LOG.exception('Could not change ownership of %s: ',
self.path) self.path)
raise
else: else:
LOG.info('Ownership of %s already %d:%d', LOG.info('Ownership of %s already %d:%d',
self.path, self.path,
@ -102,7 +111,7 @@ class PathManager(object):
else: else:
raise raise
except Exception: except Exception:
LOG.error('Could not set selinux context of %s to %s:', LOG.exception('Could not set selinux context of %s to %s:',
self.path, context) self.path, context)
raise raise
@ -167,26 +176,33 @@ class NovaStatedirOwnershipManager(object):
if pathname == self.upgrade_marker_path: if pathname == self.upgrade_marker_path:
continue continue
pathinfo = PathManager(pathname) try:
LOG.info("Checking %s", pathinfo) pathinfo = PathManager(pathname)
if pathinfo.is_dir: LOG.info("Checking %s", pathinfo)
# Always chown the directories if pathinfo.is_dir:
pathinfo.chown(self.target_uid, self.target_gid) # Always chown the directories
chcon_r = chcon pathinfo.chown(self.target_uid, self.target_gid)
if chcon: chcon_r = chcon
chcon_r = pathinfo.chcon(self.target_secontext) if chcon:
self._walk(pathname, chcon_r) chcon_r = pathinfo.chcon(self.target_secontext)
elif self.id_change: self._walk(pathname, chcon_r)
# Only chown files if it's an upgrade and the file is owned by elif self.id_change:
# the host nova uid/gid # Only chown files if it's an upgrade and the file is owned by
pathinfo.chown( # the host nova uid/gid
self.target_uid if pathinfo.uid == self.previous_uid pathinfo.chown(
else pathinfo.uid, self.target_uid if pathinfo.uid == self.previous_uid
self.target_gid if pathinfo.gid == self.previous_gid else pathinfo.uid,
else pathinfo.gid self.target_gid if pathinfo.gid == self.previous_gid
) else pathinfo.gid
if chcon: )
pathinfo.chcon(self.target_secontext) if chcon:
pathinfo.chcon(self.target_secontext)
except Exception:
# Likely to have been caused by external systems
# interacting with this directory tree,
# especially on NFS e.g snapshot dirs.
# Just ignore it and continue on to the next entry
continue
def run(self): def run(self):
LOG.info('Applying nova statedir ownership') LOG.info('Applying nova statedir ownership')

85
container_config_scripts/tests/test_nova_statedir_ownership.py
View File

@ -16,6 +16,7 @@
from unittest import mock from unittest import mock
import contextlib import contextlib
from os import stat as orig_stat
import six import six
import stat import stat
import sys import sys
@ -75,6 +76,39 @@ def generate_testtree1(nova_uid, nova_gid):
st_gid=nova_gid), st_gid=nova_gid),
'nfs': False, 'nfs': False,
}, },
'/var/lib/nova/instances/removeddir': {
'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid,
st_gid=nova_gid),
'nfs': False,
'removed_when': 'listdir'
},
'/var/lib/nova/instances/removedfile': {
'stat': FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=0,
st_gid=nova_gid),
'nfs': False,
'removed_when': 'lgetfilecon'
},
'/var/lib/nova/instances/removedfile2': {
'stat': FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=0,
st_gid=nova_gid),
'nfs': False,
'removed_when': 'lsetfilecon'
},
'/var/lib/nova/instances/removedfile3': {
'nfs': False,
'removed_when': 'stat'
},
'/var/lib/nova/instances/removeddir2': {
'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid,
st_gid=nova_gid),
'nfs': False,
'removed_when': 'lsetfilecon'
},
'/var/lib/nova/instances/foo': { '/var/lib/nova/instances/foo': {
'stat': FakeStatInfo(st_mode=stat.S_IFDIR, 'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid, st_uid=nova_uid,
@ -93,6 +127,20 @@ def generate_testtree1(nova_uid, nova_gid):
st_gid=nova_gid), st_gid=nova_gid),
'nfs': True, 'nfs': True,
}, },
'/var/lib/nova/instances/foo/removeddir': {
'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid,
st_gid=nova_gid),
'nfs': True,
'removed_when': 'listdir'
},
'/var/lib/nova/instances/foo/removeddir2': {
'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=0,
st_gid=nova_gid),
'nfs': True,
'removed_when': 'chown'
},
'/var/lib/nova/instances/foo/abc': { '/var/lib/nova/instances/foo/abc': {
'stat': FakeStatInfo(st_mode=stat.S_IFREG, 'stat': FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=0, st_uid=0,
@ -121,14 +169,25 @@ def generate_testtree2(marker_uid, marker_gid, *args, **kwargs):
return tree return tree
def check_removed(path, op, testtree):
if op == testtree.get(path, {}).get('removed_when', ''):
raise OSError(2, 'No such file or directory: ' + path)
def generate_fake_stat(testtree): def generate_fake_stat(testtree):
def fake_stat(path): def fake_stat(path):
return testtree.get(path, {}).get('stat') check_removed(path, 'stat', testtree)
if path.startswith('/var'):
return testtree.get(path, {}).get('stat')
else:
# Tracebacks need to use the real stat
return orig_stat(path)
return fake_stat return fake_stat
def generate_fake_chown(testtree): def generate_fake_chown(testtree):
def fake_chown(path, uid, gid): def fake_chown(path, uid, gid):
check_removed(path, 'chown', testtree)
if uid != -1: if uid != -1:
testtree[path]['stat'].st_uid = uid testtree[path]['stat'].st_uid = uid
if gid != -1: if gid != -1:
@ -138,12 +197,14 @@ def generate_fake_chown(testtree):
def generate_fake_exists(testtree): def generate_fake_exists(testtree):
def fake_exists(path): def fake_exists(path):
check_removed(path, 'exists', testtree)
return path in testtree return path in testtree
return fake_exists return fake_exists
def generate_fake_listdir(testtree): def generate_fake_listdir(testtree):
def fake_listdir(path): def fake_listdir(path):
check_removed(path, 'listdir', testtree)
path_parts = path.split('/') path_parts = path.split('/')
for entry in testtree: for entry in testtree:
entry_parts = entry.split('/') entry_parts = entry.split('/')
@ -155,16 +216,21 @@ def generate_fake_listdir(testtree):
def generate_fake_unlink(testtree): def generate_fake_unlink(testtree):
def fake_unlink(path): def fake_unlink(path):
check_removed(path, 'unlink', testtree)
del testtree[path] del testtree[path]
return fake_unlink return fake_unlink
def generate_fake_lgetfilecon(testtree):
def fake_lgetfilecon(path):
check_removed(path, 'lgetfilecon', testtree)
def generate_fake_lsetfilecon(testtree): def generate_fake_lsetfilecon(testtree):
def fake_lsetfilecon(path, context): def fake_lsetfilecon(path, context):
check_removed(path, 'lsetfilecon', testtree)
if testtree[path]['nfs']: if testtree[path]['nfs']:
e = OSError('Operation not supported') raise OSError(95, 'Operation not supported')
e.errno = 95
raise e
@contextlib.contextmanager @contextlib.contextmanager
@ -175,6 +241,7 @@ def fake_testtree(testtree):
fake_listdir = generate_fake_listdir(testtree) fake_listdir = generate_fake_listdir(testtree)
fake_unlink = generate_fake_unlink(testtree) fake_unlink = generate_fake_unlink(testtree)
fake_lsetfilecon = generate_fake_lsetfilecon(testtree) fake_lsetfilecon = generate_fake_lsetfilecon(testtree)
fake_lgetfilecon = generate_fake_lgetfilecon(testtree)
with mock.patch('os.chown', with mock.patch('os.chown',
side_effect=fake_chown) as fake_chown: side_effect=fake_chown) as fake_chown:
with mock.patch('os.path.exists', with mock.patch('os.path.exists',
@ -191,7 +258,8 @@ def fake_testtree(testtree):
) as fake_unlink: ) as fake_unlink:
with mock.patch( with mock.patch(
'selinux.lgetfilecon', 'selinux.lgetfilecon',
return_value=[10, 'newcontext'], side_effect=fake_lgetfilecon,
return_value=[10, 'newcontext']
) as fake_lgetfilecon: ) as fake_lgetfilecon:
with mock.patch( with mock.patch(
'selinux.lsetfilecon', 'selinux.lsetfilecon',
@ -276,7 +344,7 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):
with fake_testtree(testtree) as (fake_chown, _, _, _, _, _, fake_lsetfilecon): with fake_testtree(testtree) as (fake_chown, _, _, _, _, _, fake_lsetfilecon):
NovaStatedirOwnershipManager('/var/lib/nova').run() NovaStatedirOwnershipManager('/var/lib/nova').run()
fake_chown.assert_not_called() fake_chown.assert_called_once_with('/var/lib/nova/instances/foo/removeddir2', 100, -1)
fake_lsetfilecon.assert_any_call('/var/lib/nova', 'newcontext') fake_lsetfilecon.assert_any_call('/var/lib/nova', 'newcontext')
fake_lsetfilecon.assert_any_call('/var/lib/nova/instances/foo', 'newcontext') fake_lsetfilecon.assert_any_call('/var/lib/nova/instances/foo', 'newcontext')
chcon_paths = [x[0][0] for x in fake_lsetfilecon.call_args_list] chcon_paths = [x[0][0] for x in fake_lsetfilecon.call_args_list]
@ -290,7 +358,7 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):
with fake_testtree(testtree) as (fake_chown, _, _, _, fake_unlink, _, _): with fake_testtree(testtree) as (fake_chown, _, _, _, fake_unlink, _, _):
NovaStatedirOwnershipManager('/var/lib/nova').run() NovaStatedirOwnershipManager('/var/lib/nova').run()
fake_chown.assert_not_called() fake_chown.assert_called_once_with('/var/lib/nova/instances/foo/removeddir2', 100, -1)
fake_unlink.assert_called_with('/var/lib/nova/upgrade_marker') fake_unlink.assert_called_with('/var/lib/nova/upgrade_marker')
def test_upgrade_marker_id_change(self): def test_upgrade_marker_id_change(self):
@ -310,6 +378,9 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):
if k == '/var/lib/_nova_secontext': if k == '/var/lib/_nova_secontext':
# Ignore, outside tree # Ignore, outside tree
continue continue
if testtree[k].get('removed_when', False):
# Ignore, deleted
continue
v = v['stat'] v = v['stat']
if v.st_uid == other_uid or v.st_gid == other_gid: if v.st_uid == other_uid or v.st_gid == other_gid:
expected_changes[k] = ( expected_changes[k] = (