Browse Source

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
(cherry picked from commit 6c3c8b41de)
changes/06/748306/1
David Hill 3 months ago
committed by Oliver Walsh
parent
commit
eddbc4b2c4
2 changed files with 120 additions and 33 deletions
  1. +42
    -26
      container_config_scripts/nova_statedir_ownership.py
  2. +78
    -7
      container_config_scripts/tests/test_nova_statedir_ownership.py

+ 42
- 26
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"""
def __init__(self, path):
self.path = path
self.uid = None
self.gid = None
self.is_dir = None
self.secontext = None
self._update()

def _update(self):
statinfo = os.stat(self.path)
self.is_dir = stat.S_ISDIR(statinfo.st_mode)
self.uid = statinfo.st_uid
self.gid = statinfo.st_gid
self.secontext = selinux.lgetfilecon(self.path)[1]
try:
statinfo = os.stat(self.path)
self.is_dir = stat.S_ISDIR(statinfo.st_mode)
self.uid = statinfo.st_uid
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):
return "uid: {} gid: {} path: {}{}".format(
@@ -79,6 +87,7 @@ class PathManager(object):
except Exception:
LOG.exception('Could not change ownership of %s: ',
self.path)
raise
else:
LOG.info('Ownership of %s already %d:%d',
self.path,
@@ -102,7 +111,7 @@ class PathManager(object):
else:
raise
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)
raise

@@ -167,26 +176,33 @@ class NovaStatedirOwnershipManager(object):
if pathname == self.upgrade_marker_path:
continue

pathinfo = PathManager(pathname)
LOG.info("Checking %s", pathinfo)
if pathinfo.is_dir:
# Always chown the directories
pathinfo.chown(self.target_uid, self.target_gid)
chcon_r = chcon
if chcon:
chcon_r = pathinfo.chcon(self.target_secontext)
self._walk(pathname, chcon_r)
elif self.id_change:
# Only chown files if it's an upgrade and the file is owned by
# the host nova uid/gid
pathinfo.chown(
self.target_uid if pathinfo.uid == self.previous_uid
else pathinfo.uid,
self.target_gid if pathinfo.gid == self.previous_gid
else pathinfo.gid
)
if chcon:
pathinfo.chcon(self.target_secontext)
try:
pathinfo = PathManager(pathname)
LOG.info("Checking %s", pathinfo)
if pathinfo.is_dir:
# Always chown the directories
pathinfo.chown(self.target_uid, self.target_gid)
chcon_r = chcon
if chcon:
chcon_r = pathinfo.chcon(self.target_secontext)
self._walk(pathname, chcon_r)
elif self.id_change:
# Only chown files if it's an upgrade and the file is owned by
# the host nova uid/gid
pathinfo.chown(
self.target_uid if pathinfo.uid == self.previous_uid
else pathinfo.uid,
self.target_gid if pathinfo.gid == self.previous_gid
else pathinfo.gid
)
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):
LOG.info('Applying nova statedir ownership')


+ 78
- 7
container_config_scripts/tests/test_nova_statedir_ownership.py View File

@@ -15,6 +15,7 @@

import contextlib
import mock
from os import stat as orig_stat
import six
import stat
import sys
@@ -74,6 +75,39 @@ def generate_testtree1(nova_uid, nova_gid):
st_gid=nova_gid),
'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': {
'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid,
@@ -92,6 +126,20 @@ def generate_testtree1(nova_uid, nova_gid):
st_gid=nova_gid),
'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': {
'stat': FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=0,
@@ -120,14 +168,25 @@ def generate_testtree2(marker_uid, marker_gid, *args, **kwargs):
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 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


def generate_fake_chown(testtree):
def fake_chown(path, uid, gid):
check_removed(path, 'chown', testtree)
if uid != -1:
testtree[path]['stat'].st_uid = uid
if gid != -1:
@@ -137,12 +196,14 @@ def generate_fake_chown(testtree):

def generate_fake_exists(testtree):
def fake_exists(path):
check_removed(path, 'exists', testtree)
return path in testtree
return fake_exists


def generate_fake_listdir(testtree):
def fake_listdir(path):
check_removed(path, 'listdir', testtree)
path_parts = path.split('/')
for entry in testtree:
entry_parts = entry.split('/')
@@ -154,16 +215,21 @@ def generate_fake_listdir(testtree):

def generate_fake_unlink(testtree):
def fake_unlink(path):
check_removed(path, 'unlink', testtree)
del testtree[path]
return fake_unlink


def generate_fake_lgetfilecon(testtree):
def fake_lgetfilecon(path):
check_removed(path, 'lgetfilecon', testtree)


def generate_fake_lsetfilecon(testtree):
def fake_lsetfilecon(path, context):
check_removed(path, 'lsetfilecon', testtree)
if testtree[path]['nfs']:
e = OSError('Operation not supported')
e.errno = 95
raise e
raise OSError(95, 'Operation not supported')


@contextlib.contextmanager
@@ -174,6 +240,7 @@ def fake_testtree(testtree):
fake_listdir = generate_fake_listdir(testtree)
fake_unlink = generate_fake_unlink(testtree)
fake_lsetfilecon = generate_fake_lsetfilecon(testtree)
fake_lgetfilecon = generate_fake_lgetfilecon(testtree)
with mock.patch('os.chown',
side_effect=fake_chown) as fake_chown:
with mock.patch('os.path.exists',
@@ -190,7 +257,8 @@ def fake_testtree(testtree):
) as fake_unlink:
with mock.patch(
'selinux.lgetfilecon',
return_value=[10, 'newcontext'],
side_effect=fake_lgetfilecon,
return_value=[10, 'newcontext']
) as fake_lgetfilecon:
with mock.patch(
'selinux.lsetfilecon',
@@ -275,7 +343,7 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):

with fake_testtree(testtree) as (fake_chown, _, _, _, _, _, fake_lsetfilecon):
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/instances/foo', 'newcontext')
chcon_paths = [x[0][0] for x in fake_lsetfilecon.call_args_list]
@@ -289,7 +357,7 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):

with fake_testtree(testtree) as (fake_chown, _, _, _, fake_unlink, _, _):
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')

def test_upgrade_marker_id_change(self):
@@ -309,6 +377,9 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):
if k == '/var/lib/_nova_secontext':
# Ignore, outside tree
continue
if testtree[k].get('removed_when', False):
# Ignore, deleted
continue
v = v['stat']
if v.st_uid == other_uid or v.st_gid == other_gid:
expected_changes[k] = (


Loading…
Cancel
Save