os-brick needs to provide it's own rootwrap filters file

Bug #1479842 reported by Matt Riedemann
20
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Cinder
Won't Fix
High
Sean McGinnis
OpenStack Compute (nova)
Won't Fix
High
Matt Riedemann
os-brick
Fix Released
High
Walt Boring
oslo.rootwrap
Won't Fix
Wishlist
Unassigned

Bug Description

This came up in the review for the scaleio libvirt volume driver in nova:

https://review.openstack.org/#/c/194454/16/etc/nova/rootwrap.d/compute.filters

Basically, having to define rootwrap filters in nova and cinder for things that are run in os-brick is kind of terrible since every time os-brick needs to add a new command to run as root, it has to be added to nova/cinder, and we have to deal with version compat issues (will the version of nova/cinder have the filters required for the version of os-brick that's running?).

This was already introduced with multipathd to compute.filters in the os-brick integration change:

https://review.openstack.org/#/c/175569/32/etc/nova/rootwrap.d/compute.filters

Rather than revert the os-brick integration change to nova, we should work this as a bug so that os-brick can carry it's own os-brick.filters file and then that can be dropped into /etc/nova/rootwrap.d and /etc/cinder/rootwrap.d.

So we'll need os-brick changes, nova, cinder and devstack changes to land this.

Also considered a release blocker for liberty for the nova team.

Matt Riedemann (mriedem)
Changed in cinder:
status: New → Confirmed
Changed in nova:
status: New → Confirmed
Changed in devstack:
status: New → Confirmed
Changed in os-brick:
status: New → Confirmed
Changed in cinder:
importance: Undecided → High
Changed in nova:
importance: Undecided → High
Changed in os-brick:
assignee: nobody → Walt Boring (walter-boring)
importance: Undecided → High
Revision history for this message
Matt Riedemann (mriedem) wrote :

We may want to see where this ends up, we should talk to dims and gus per IRC conversation:

https://blueprints.launchpad.net/oslo-incubator/+spec/privsep

However, I'm not sure how long we want to wait on that since the spec isn't even written yet.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to os-brick (master)

Fix proposed to branch: master
Review: https://review.openstack.org/207553

Changed in os-brick:
status: Confirmed → In Progress
Xing Yang (xing-yang)
Changed in devstack:
assignee: nobody → Xing Yang (xing-yang)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to devstack (master)

Fix proposed to branch: master
Review: https://review.openstack.org/207677

Changed in devstack:
status: Confirmed → In Progress
Changed in devstack:
assignee: Xing Yang (xing-yang) → Walt Boring (walter-boring)
Changed in devstack:
assignee: Walt Boring (walter-boring) → Xing Yang (xing-yang)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to os-brick (master)

Reviewed: https://review.openstack.org/207553
Committed: https://git.openstack.org/cgit/openstack/os-brick/commit/?id=c16abad3d8b771ffa80bf0984cf180506fd86eaf
Submitter: Jenkins
Branch: master

commit c16abad3d8b771ffa80bf0984cf180506fd86eaf
Author: Walter A. Boring IV <email address hidden>
Date: Thu Jul 30 10:19:04 2015 -0700

    Add rootwrap filters

    This patch adds os-bricks list of rootwrap filters for commands
    that are needed to execute. The filters are a self contained entire
    list of expected filters that os-brick needs to run. It's expected that
    this filter file is added to any rootwrap enabled service that needs to use
    os-brick.

    Devstack associated patch: https://review.openstack.org/#/c/207677/

    Partial-Bug: #1479842
    UpgradeImpact: Need to place the os-brick.filters file in service's
                    rootwrap.d directory to enable filters.

    Change-Id: I2b1e657b87c7b27548200a20b991f34c3413c24b

Xing Yang (xing-yang)
Changed in os-brick:
status: In Progress → Fix Committed
Changed in os-brick:
milestone: none → 0.4.0
status: Fix Committed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

Fix proposed to branch: master
Review: https://review.openstack.org/220745

Changed in cinder:
assignee: nobody → Sean McGinnis (sean-mcginnis)
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

Fix proposed to branch: master
Review: https://review.openstack.org/220750

Changed in nova:
assignee: nobody → Sean McGinnis (sean-mcginnis)
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.openstack.org/220819

Changed in nova:
assignee: Sean McGinnis (sean-mcginnis) → Matt Riedemann (mriedem)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on nova (master)

Change abandoned by Sean McGinnis (<email address hidden>) on branch: master
Review: https://review.openstack.org/220750
Reason: I'm not sure if 220819 is the right answer for this, but at least this patch is out here for Cinder third party CI's to pull in to get FC tests to pass for now.

I think we need a better answer for how to consume library rootwrap filters.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

Reviewed: https://review.openstack.org/220745
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=6996b5b449377aa1d653e66759f7761e2de802ba
Submitter: Jenkins
Branch: master

commit 6996b5b449377aa1d653e66759f7761e2de802ba
Author: Sean McGinnis <email address hidden>
Date: Sat Sep 5 13:04:05 2015 -0500

    Add os-brick's scsi_id command to Cinder rootwrap

    FC drivers are failing on calls to scsi_id. This command
    is in the os-brick rootwrap filters, but that file is not
    being pulled in to Cinder yet.

    As a temporary fix until that work can be completed, this
    patch adds the missing command directly to volume.filters
    to allow this command to run. Follow up work will be needed
    to remove this and other legacy brick commands from the
    volume.filters file and instead pull in os-brick's
    os-brick.filters values.

    Change-Id: Iba8e5c82f8d203759cda0e21de5fdf2404b18e69
    Partial-bug: #1479842

Revision history for this message
Sam Wan (sam-wan) wrote :

Need to add os-brick's scsi_id command to Nova rootwrap as well, otherwise nova attach fails
==================
2015-09-07 22:56:45.674 21904 ERROR nova.virt.block_device [instance: a182abab-74a9-4601-8534-e0ac3fa86814] Stderr: u'/usr/local/bin/nova-rootwrap: Unauthorized command: scsi_id --page 0x83 --whitelisted /dev/disk/by-path/pci-0000:06:00.0-fc-0x514f0c5000772c00-lun-3 (no filter matched)\n'
=====================

Revision history for this message
Xing Yang (xing-yang) wrote :

Hi Sam,

If the following devstack patch gets merged, os-brick's root wrap filters will be copied to Nova's directory as well:

https://review.openstack.org/#/c/207677/

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on devstack (master)

Change abandoned by xing-yang (<email address hidden>) on branch: master
Review: https://review.openstack.org/207677

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on os-brick (master)

Change abandoned by Walter A. Boring IV (hemna) (<email address hidden>) on branch: master
Review: https://review.openstack.org/221513
Reason: Not needed for the L release. We are manually patching Cinder and Nova for L. We'll come up with a solution for the M release. Look into the package_data to contain the data files.

Revision history for this message
Matt Riedemann (mriedem) wrote :

Dropping devstack from this. As a tactical fix for liberty we're going to just make sure the os-brick.filters contents are in volume.filters in cinder and compute.filters in nova. This is not ideal long-term because of the tight-coupling it creates and lockstep upgrades required for nova and cinder (and os-brick).

We're going to add oslo.rootwrap here since what we'd like to see discussed at the mitaka summit is the capability in oslo.rootwrap to load the package_data automatically from os-brick.

So the thinking is something like this, nova's rootwrap.conf would have some kind of key=value of library to filter to load, e.g.:

filters_from_libraries: os-brick=os-brick.filters(,os-brick-2.filters,etc),foo-lib=foo.filters

Then oslo.rootwrap could parse that and use:

http://peak.telecommunity.com/DevCenter/PythonEggs#accessing-package-resources

To load the data, e.g. resource_filename('os-brick', 'os-brick.filters').

no longer affects: devstack
Changed in oslo.rootwrap:
status: New → Confirmed
importance: Undecided → Wishlist
Changed in nova:
milestone: none → next
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on nova (master)

Change abandoned by Matt Riedemann (<email address hidden>) on branch: master
Review: https://review.openstack.org/220819
Reason: This isn't going to be the solution for liberty, we have another tactical fix for liberty and then we're going to pursue new feature function in oslo.rootwrap in mitaka to dynamically load the os-brick filters, details are in bug 1479842.

Matt Riedemann (mriedem)
Changed in nova:
status: In Progress → Confirmed
Changed in cinder:
status: In Progress → Confirmed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/220750
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=80b3b9ffb07b0c1d782fcbe60391ddbf0bc0e60c
Submitter: Jenkins
Branch: master

commit 80b3b9ffb07b0c1d782fcbe60391ddbf0bc0e60c
Author: Sean McGinnis <email address hidden>
Date: Sat Sep 5 14:22:06 2015 -0500

    Add os-brick's scsi_id command to rootwrap

    Fibre channel drivers are failing on calls from the
    os-brick library to scsi_id.

    This is a temporary fix until os-brick's rootwrap
    filters can be pulled in. It is not intended or
    desired that nova track all os-brick rootwrap filter
    values. But until that mechanism is finalized, we
    need necessary commands added here.

    Change-Id: Ie5f1fbe0a2da171d80faefe3245df91aef8ce1ab
    Partial-bug: #1479842

Revision history for this message
Matt Riedemann (mriedem) wrote :

I dropped the liberty-rc-potential from this bug since we have the tactical workaround fix for scsi_id in liberty, then we'll discuss the future changes with rootwrap at the mitaka summit.

tags: removed: liberty-rc-potential
Revision history for this message
Davanum Srinivas (DIMS) (dims-v) wrote :

The plan is to use oslo.privsep instead of oslo.rootwrap in os-brick.

Changed in oslo.rootwrap:
status: Confirmed → Won't Fix
Revision history for this message
ChangBo Guo(gcb) (glongwave) wrote :

os-brick review : https://review.openstack.org/#/c/277224, so os-brick doesn't need to provide it's own rootwrap filters file for Nova.

Changed in nova:
status: Confirmed → Won't Fix
Changed in cinder:
status: Confirmed → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.