openstack
/
tripleo-heat-templates
Code Issues Proposed changes

Merge "Tolerate NFS exports in /var/lib/nova when selinux relabelling" into stable/train

This commit is contained in:
Zuul 2020-04-01 00:22:19 +00:00 committed by Gerrit Code Review
parent 0b2d0de99d eea3ff0e15
commit 9948955b21
7 changed files with 180 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
bindep.txt
View File

@ -2,3 +2,9 @@
# see https://docs.openstack.org/infra/bindep/ for additional information.
libssl-dev [platform:dpkg]
openssl-devel [platform:rpm]
# For SELinux
libselinux-python [platform:rpm !platform:rhel-8 !platform:centos-8]
libsemanage-python [platform:redhat !platform:rhel-8 !platform:centos-8]
libselinux-python3 [platform:rpm !platform:rhel-7 !platform:centos-7]
libsemanage-python3 [platform:redhat !platform:rhel-7 !platform:centos-7]

48
container_config_scripts/nova_statedir_ownership.py
View File

@ -17,6 +17,7 @@ from __future__ import print_function
import logging
import os
import pwd
import selinux
import stat
import sys
@ -42,6 +43,7 @@ class PathManager(object):
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]
def __str__(self):
return "uid: {} gid: {} path: {}{}".format(
@ -83,6 +85,27 @@ class PathManager(object):
uid,
gid)
def chcon(self, context):
# If dir returns whether to recusively set context
try:
try:
selinux.lsetfilecon(self.path, context)
LOG.info('Setting selinux context of %s to %s',
self.path, context)
return True
except OSError as e:
if self.is_dir and e.errno == 95:
# Operation not supported, assume NFS mount and skip
LOG.info('Setting selinux context not supported for %s',
self.path)
return False
else:
raise
except Exception:
LOG.error('Could not set selinux context of %s to %s:',
self.path, context)
raise
class NovaStatedirOwnershipManager(object):
"""Class to manipulate the ownership of the nova statedir (/var/lib/nova).
@ -106,17 +129,19 @@ class NovaStatedirOwnershipManager(object):
docker nova uid/gid is not known in this context).
"""
def __init__(self, statedir, upgrade_marker='upgrade_marker',
nova_user='nova'):
nova_user='nova', secontext_marker='../_nova_secontext'):
self.statedir = statedir
self.nova_user = nova_user
self.upgrade_marker_path = os.path.join(statedir, upgrade_marker)
self.secontext_marker_path = os.path.normpath(os.path.join(statedir, secontext_marker))
self.upgrade = os.path.exists(self.upgrade_marker_path)
self.target_uid, self.target_gid = self._get_nova_ids()
self.previous_uid, self.previous_gid = self._get_previous_nova_ids()
self.id_change = (self.target_uid, self.target_gid) != \
(self.previous_uid, self.previous_gid)
self.target_secontext = self._get_secontext()
def _get_nova_ids(self):
nova_uid, nova_gid = pwd.getpwnam(self.nova_user)[2:4]
@ -129,7 +154,13 @@ class NovaStatedirOwnershipManager(object):
else:
return self._get_nova_ids()
def _walk(self, top):
def _get_secontext(self):
if os.path.exists(self.secontext_marker_path):
return selinux.lgetfilecon(self.secontext_marker_path)[1]
else:
return None
def _walk(self, top, chcon=True):
for f in os.listdir(top):
pathname = os.path.join(top, f)
@ -141,7 +172,10 @@ class NovaStatedirOwnershipManager(object):
if pathinfo.is_dir:
# Always chown the directories
pathinfo.chown(self.target_uid, self.target_gid)
self._walk(pathname)
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
@ -151,6 +185,8 @@ class NovaStatedirOwnershipManager(object):
self.target_gid if pathinfo.gid == self.previous_gid
else pathinfo.gid
)
if chcon:
pathinfo.chcon(self.target_secontext)
def run(self):
LOG.info('Applying nova statedir ownership')
@ -162,8 +198,12 @@ class NovaStatedirOwnershipManager(object):
pathinfo = PathManager(self.statedir)
LOG.info("Checking %s", pathinfo)
pathinfo.chown(self.target_uid, self.target_gid)
chcon = self.target_secontext is not None
self._walk(self.statedir)
if chcon:
pathinfo.chcon(self.target_secontext)
self._walk(self.statedir, chcon)
if self.upgrade:
LOG.info('Removing upgrade_marker %s',

152
container_config_scripts/tests/test_nova_statedir_ownership.py
View File

@ -17,12 +17,24 @@ import contextlib
import mock
import six
import stat
import sys
from oslotest import base
class FakeSelinux(object):
@staticmethod
def lgetfilecon(path):
pass
@staticmethod
def lsetfilecon(path, context):
pass
sys.modules["selinux"] = FakeSelinux
from container_config_scripts.nova_statedir_ownership import \
NovaStatedirOwnershipManager
from container_config_scripts.nova_statedir_ownership import PathManager
NovaStatedirOwnershipManager # noqa: E402
from container_config_scripts.nova_statedir_ownership import PathManager # noqa: E402
# Real chown would require root, so in order to test this we need to fake
# all of the methods that interact with the filesystem
@ -43,60 +55,83 @@ class FakeStatInfo(object):
def generate_testtree1(nova_uid, nova_gid):
return {
'/var/lib/nova':
FakeStatInfo(st_mode=stat.S_IFDIR,
'/var/lib/nova': {
'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid,
st_gid=nova_gid),
'nfs': False,
},
'/var/lib/_nova_secontext': {
'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid,
st_gid=nova_gid),
'nfs': False,
},
'/var/lib/nova/instances': {
'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid,
st_gid=nova_gid),
'/var/lib/nova/instances':
FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid,
st_gid=nova_gid),
'/var/lib/nova/instances/foo':
FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid,
st_gid=nova_gid),
'/var/lib/nova/instances/foo/bar':
FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=0,
st_gid=0),
'/var/lib/nova/instances/foo/baz':
FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=nova_uid,
st_gid=nova_gid),
'/var/lib/nova/instances/foo/abc':
FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=0,
st_gid=nova_gid),
'/var/lib/nova/instances/foo/def':
FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=nova_uid,
st_gid=0),
'nfs': False,
},
'/var/lib/nova/instances/foo': {
'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
st_uid=nova_uid,
st_gid=nova_gid),
'nfs': True,
},
'/var/lib/nova/instances/foo/bar': {
'stat': FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=0,
st_gid=0),
'nfs': True,
},
'/var/lib/nova/instances/foo/baz': {
'stat': FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=nova_uid,
st_gid=nova_gid),
'nfs': True,
},
'/var/lib/nova/instances/foo/abc': {
'stat': FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=0,
st_gid=nova_gid),
'nfs': True,
},
'/var/lib/nova/instances/foo/def': {
'stat': FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=nova_uid,
st_gid=0),
'nfs': True,
},
}
def generate_testtree2(marker_uid, marker_gid, *args, **kwargs):
tree = generate_testtree1(*args, **kwargs)
tree.update({
'/var/lib/nova/upgrade_marker':
FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=marker_uid,
st_gid=marker_gid)
'/var/lib/nova/upgrade_marker': {
'stat': FakeStatInfo(st_mode=stat.S_IFREG,
st_uid=marker_uid,
st_gid=marker_gid),
'nfs': False,
}
})
return tree
def generate_fake_stat(testtree):
def fake_stat(path):
return testtree.get(path)
return testtree.get(path, {}).get('stat')
return fake_stat
def generate_fake_chown(testtree):
def fake_chown(path, uid, gid):
if uid != -1:
testtree[path].st_uid = uid
testtree[path]['stat'].st_uid = uid
if gid != -1:
testtree[path].st_gid = gid
testtree[path]['stat'].st_gid = gid
return fake_chown
@ -123,6 +158,14 @@ def generate_fake_unlink(testtree):
return fake_unlink
def generate_fake_lsetfilecon(testtree):
def fake_lsetfilecon(path, context):
if testtree[path]['nfs']:
e = OSError('Operation not supported')
e.errno = 95
raise e
@contextlib.contextmanager
def fake_testtree(testtree):
fake_stat = generate_fake_stat(testtree)
@ -130,6 +173,7 @@ def fake_testtree(testtree):
fake_exists = generate_fake_exists(testtree)
fake_listdir = generate_fake_listdir(testtree)
fake_unlink = generate_fake_unlink(testtree)
fake_lsetfilecon = generate_fake_lsetfilecon(testtree)
with mock.patch('os.chown',
side_effect=fake_chown) as fake_chown:
with mock.patch('os.path.exists',
@ -144,17 +188,27 @@ def fake_testtree(testtree):
'os.unlink',
side_effect=fake_unlink
) as fake_unlink:
yield (fake_chown,
fake_exists,
fake_listdir,
fake_stat,
fake_unlink)
with mock.patch(
'selinux.lgetfilecon',
return_value=[10, 'newcontext'],
) as fake_lgetfilecon:
with mock.patch(
'selinux.lsetfilecon',
side_effect=fake_lsetfilecon,
) as fake_lsetfilecon:
yield (fake_chown,
fake_exists,
fake_listdir,
fake_stat,
fake_unlink,
fake_lgetfilecon,
fake_lsetfilecon)
def assert_ids(testtree, path, uid, gid):
statinfo = testtree[path]
statinfo = testtree[path]['stat']
assert (uid, gid) == (statinfo.st_uid, statinfo.st_gid), \
"{}: expected {}:{} actual {}:{}".format(
"{}: expected ownership {}:{} actual {}:{}".format(
path, uid, gid, statinfo.st_uid, statinfo.st_gid
)
@ -219,9 +273,13 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):
def test_no_upgrade_marker(self):
testtree = generate_testtree1(current_uid, current_gid)
with fake_testtree(testtree) as (fake_chown, _, _, _, _):
with fake_testtree(testtree) as (fake_chown, _, _, _, _, _, fake_lsetfilecon):
NovaStatedirOwnershipManager('/var/lib/nova').run()
fake_chown.assert_not_called()
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]
self.assertNotIn('/var/lib/nova/instances/foo/bar', chcon_paths)
def test_upgrade_marker_no_id_change(self):
testtree = generate_testtree2(current_uid,
@ -229,7 +287,7 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):
current_uid,
current_gid)
with fake_testtree(testtree) as (fake_chown, _, _, _, fake_unlink):
with fake_testtree(testtree) as (fake_chown, _, _, _, fake_unlink, _, _):
NovaStatedirOwnershipManager('/var/lib/nova').run()
fake_chown.assert_not_called()
fake_unlink.assert_called_with('/var/lib/nova/upgrade_marker')
@ -248,13 +306,17 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):
if k == '/var/lib/nova/upgrade_marker':
# Ignore the marker, it should be deleted
continue
if k == '/var/lib/_nova_secontext':
# Ignore, outside tree
continue
v = v['stat']
if v.st_uid == other_uid or v.st_gid == other_gid:
expected_changes[k] = (
current_uid if v.st_uid == other_uid else v.st_uid,
current_gid if v.st_gid == other_gid else v.st_gid
)
with fake_testtree(testtree) as (_, _, _, _, fake_unlink):
with fake_testtree(testtree) as (_, _, _, _, fake_unlink, _, _):
NovaStatedirOwnershipManager('/var/lib/nova').run()
for fn, expected in six.iteritems(expected_changes):
assert_ids(testtree, fn, expected[0], expected[1])

42
deployment/nova/nova-compute-container-puppet.yaml
View File

@ -745,16 +745,9 @@ outputs:
privileged: false
detach: false
volumes:
list_concat:
# podman fails to relable if nova_nfs_enabled where we have
# the nfs share mounted to /var/lib/nova/instances
-
if:
- nova_nfs_enabled
- - /var/lib/nova:/var/lib/nova:shared
- - /var/lib/nova:/var/lib/nova:shared,z
-
- /var/lib/container-config-scripts/:/container-config-scripts/:z
- /var/lib/nova:/var/lib/nova:shared
- /var/lib/_nova_secontext:/var/lib/_nova_secontext:shared,z
- /var/lib/container-config-scripts/:/container-config-scripts/:z
command: "/container-config-scripts/pyshim.sh /container-config-scripts/nova_statedir_ownership.py"
environment:
# NOTE: this should force this container to re-run on each
@ -797,13 +790,7 @@ outputs:
- /sys/class/net:/sys/class/net
- /sys/bus/pci:/sys/bus/pci
- /boot:/boot:ro
-
# podman fails to relable if nova_nfs_enabled where we have
# the nfs share mounted to /var/lib/nova/instances
if:
- nova_nfs_enabled
- - /var/lib/nova:/var/lib/nova:shared
- - /var/lib/nova:/var/lib/nova:shared,z
- /var/lib/nova:/var/lib/nova:shared
-
if:
- {equals: [{get_param: MultipathdEnable}, true]}
@ -842,7 +829,17 @@ outputs:
host_prep_tasks:
list_concat:
- {get_attr: [NovaLogging, host_prep_tasks]}
- - name: Mount Nova NFS Share
- - name: create persistent directories
file:
path: "{{ item.path }}"
state: directory
setype: "{{ item.setype }}"
with_items:
- { 'path': /var/lib/nova, 'setype': svirt_sandbox_file_t }
- { 'path': /var/lib/_nova_secontext, 'setype': svirt_sandbox_file_t }
- { 'path': /var/lib/nova/instances, 'setype': svirt_sandbox_file_t }
- { 'path': /var/lib/libvirt, 'setype': svirt_sandbox_file_t }
- name: Mount Nova NFS Share
vars:
nfs_backend_enable: {get_attr: [RoleParametersValue, value, nfs_backend_enable]}
nfs_share: {get_attr: [RoleParametersValue, value, nfs_share]}
@ -932,15 +929,6 @@ outputs:
name: tripleo_nova_libvirt_guests
enabled: yes
daemon_reload: yes
- name: create persistent directories
file:
path: "{{ item.path }}"
state: directory
setype: "{{ item.setype }}"
with_items:
- { 'path': /var/lib/nova, 'setype': svirt_sandbox_file_t }
- { 'path': /var/lib/nova/instances, 'setype': svirt_sandbox_file_t }
- { 'path': /var/lib/libvirt, 'setype': svirt_sandbox_file_t }
- name: ensure ceph configurations exist
file:
path: /etc/ceph

22
deployment/nova/nova-ironic-container-puppet.yaml
View File

@ -153,16 +153,9 @@ outputs:
privileged: false
detach: false
volumes:
list_concat:
# podman fails to relable if nova_nfs_enabled where we have
# the nfs share mounted to /var/lib/nova/instances
-
if:
- nova_nfs_enabled
- - /var/lib/nova:/var/lib/nova:shared
- - /var/lib/nova:/var/lib/nova:shared,z
-
- /var/lib/container-config-scripts/:/container-config-scripts/
- /var/lib/nova:/var/lib/nova:shared
- /var/lib/_nova_secontext:/var/lib/_nova_secontext:shared,z
- /var/lib/container-config-scripts/:/container-config-scripts/
command: "/container-config-scripts/pyshim.sh /container-config-scripts/nova_statedir_ownership.py"
step_5:
nova_compute:
@ -184,13 +177,7 @@ outputs:
- /dev:/dev
- /var/lib/iscsi:/var/lib/iscsi:z
- /var/log/containers/nova:/var/log/nova:z
-
# podman fails to relable if nova_nfs_enabled where we have
# the nfs share mounted to /var/lib/nova/instances
if:
- nova_nfs_enabled
- - /var/lib/nova:/var/lib/nova:shared
- - /var/lib/nova:/var/lib/nova:shared,z
- /var/lib/nova:/var/lib/nova:shared
-
if:
- {equals: [{get_param: MultipathdEnable}, true]}
@ -223,6 +210,7 @@ outputs:
with_items:
- { 'path': /var/log/containers/nova, 'setype': svirt_sandbox_file_t, 'mode': '0750' }
- { 'path': /var/lib/nova, 'setype': svirt_sandbox_file_t }
- { 'path': /var/lib/_nova_secontext, 'setype': svirt_sandbox_file_t}
- name: enable virt_sandbox_use_netlink for healthcheck
seboolean:
name: virt_sandbox_use_netlink

16
deployment/nova/nova-libvirt-container-puppet.yaml
View File

@ -670,13 +670,7 @@ outputs:
- /var/lib/libvirt:/var/lib/libvirt
- /etc/libvirt/qemu:/etc/libvirt/qemu:ro
- /var/log/libvirt/qemu:/var/log/libvirt/qemu
# podman fails to relable if nova_nfs_enabled where we have
# the nfs share mounted to /var/lib/nova/instances
-
if:
- nova_nfs_enabled
- - /var/lib/nova:/var/lib/nova:shared
- - /var/lib/nova:/var/lib/nova:shared,z
- /var/lib/nova:/var/lib/nova:shared
environment:
KOLLA_CONFIG_STRATEGY: COPY_ALWAYS
nova_libvirt:
@ -710,13 +704,7 @@ outputs:
- /var/lib/libvirt:/var/lib/libvirt:shared,z
- /var/log/libvirt/qemu:/var/log/libvirt/qemu:ro
- /var/lib/vhost_sockets:/var/lib/vhost_sockets:z
# podman fails to relable if nova_nfs_enabled where we have
# the nfs share mounted to /var/lib/nova/instances
-
if:
- nova_nfs_enabled
- - /var/lib/nova:/var/lib/nova:shared
- - /var/lib/nova:/var/lib/nova:shared,z
- /var/lib/nova:/var/lib/nova:shared
-
if:
- docker_enabled

8
deployment/nova/nova-migration-target-container-puppet.yaml
View File

@ -162,13 +162,7 @@ outputs:
- /var/lib/config-data/puppet-generated/nova_libvirt:/var/lib/kolla/config_files/src:ro
- /etc/ssh/:/host-ssh/:ro
- /run/libvirt:/run/libvirt
# podman fails to relable if nova_nfs_enabled where we have
# the nfs share mounted to /var/lib/nova/instances
-
if:
- nova_nfs_enabled
- - /var/lib/nova:/var/lib/nova:shared
- - /var/lib/nova:/var/lib/nova:shared,z
- /var/lib/nova:/var/lib/nova:shared
environment:
KOLLA_CONFIG_STRATEGY: COPY_ALWAYS
deploy_steps_tasks: