| Message ID | 20180716103148.24120-1-mchandras@suse.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] utilities: Run ovsdb-server pre-startup DB steps as root | expand |
Markos Chandras <mchandras@suse.de> writes: > When ovsdb-server is starting, it performs some DB steps such as > creating and upgrading the OvS DB. When we are running as > 'non-root' user, the 'runuser' tool is used to manage the privileges. > However, when this happens during systemd boot, we observe the following > errors in journald: > > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to scope's control group: No such process > Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user openvswitch. > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state. > > According to the analysis performed on openSUSE bugzilla[1], it seems > that ovsdb-server.service creates (via the call to runuser) a user > session and therefore call pam_systemd which in its turn tries to start > a systemd user instance: "user@474.service". However "user@474.service" > is supposed to be started after systemd-user-sessions.service which is > supposed to be started after network.target. Additionally, > ovsdb-server.service uses Before=network.target hence the deadlock. > > We can workaround this by switching to 'root' user when we are > performing this pre-startup steps and fixup the DB permissions before > we start the actual ovsdb-server daemon. > > [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630 > Cc: Aaron Conole <aconole@redhat.com> > Signed-off-by: Markos Chandras <mchandras@suse.de> > --- > Probably not the cleanest option so I am open to suggestions :) I think there's actually a race condition here. Most likely, ovsdb-server doesn't need to be started before network.service. Looking at the bug, I think we can unroll some of the dependencies that each unit file has and get a cleaner systemd dependency chain, and preserve the existing user downgrade. I will install an OpenSUSE VM and work on it. Strange that we don't see this on the RHEL side. I'll also try to reproduce that. Thanks for pointing the issue out (and thanks to David and Franck on your side). -Aaron
Hello Aaron, On 18/07/18 16:24, Aaron Conole wrote: > > I think there's actually a race condition here. Most likely, > ovsdb-server doesn't need to be started before network.service. > > Looking at the bug, I think we can unroll some of the dependencies that > each unit file has and get a cleaner systemd dependency chain, and > preserve the existing user downgrade. > > I will install an OpenSUSE VM and work on it. Strange that we don't see > this on the RHEL side. I'll also try to reproduce that. > > Thanks for pointing the issue out (and thanks to David and Franck on > your side). > > -Aaron > Great thank you. If you are using vagrant you can use the following steps for a reproducer vagrant init opensuse/openSUSE-15.0-x86_64 vagrant up vagrant ssh (inside vagrant) sudo -s zypper -n in openvswitch systemctl enable openvswitch systemctl reboot checking the journald logs after the reboot should reveal the issue. Let me know if you need any help.
On Wed, Jul 18, 2018 at 11:24:43AM -0400, Aaron Conole wrote: > Markos Chandras <mchandras@suse.de> writes: > > > When ovsdb-server is starting, it performs some DB steps such as > > creating and upgrading the OvS DB. When we are running as > > 'non-root' user, the 'runuser' tool is used to manage the privileges. > > However, when this happens during systemd boot, we observe the following > > errors in journald: > > > > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to scope's control group: No such process > > Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user openvswitch. > > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state. > > > > According to the analysis performed on openSUSE bugzilla[1], it seems > > that ovsdb-server.service creates (via the call to runuser) a user > > session and therefore call pam_systemd which in its turn tries to start > > a systemd user instance: "user@474.service". However "user@474.service" > > is supposed to be started after systemd-user-sessions.service which is > > supposed to be started after network.target. Additionally, > > ovsdb-server.service uses Before=network.target hence the deadlock. > > > > We can workaround this by switching to 'root' user when we are > > performing this pre-startup steps and fixup the DB permissions before > > we start the actual ovsdb-server daemon. > > > > [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630 > > Cc: Aaron Conole <aconole@redhat.com> > > Signed-off-by: Markos Chandras <mchandras@suse.de> > > --- > > Probably not the cleanest option so I am open to suggestions :) > > I think there's actually a race condition here. Most likely, > ovsdb-server doesn't need to be started before network.service. Unfortunately it does because network.service will ifup OVS bridges and ports and then we need ovsdb already running. fbl > Looking at the bug, I think we can unroll some of the dependencies that > each unit file has and get a cleaner systemd dependency chain, and > preserve the existing user downgrade. > > I will install an OpenSUSE VM and work on it. Strange that we don't see > this on the RHEL side. I'll also try to reproduce that. > > Thanks for pointing the issue out (and thanks to David and Franck on > your side). > > -Aaron
Flavio Leitner <fbl@redhat.com> writes: > On Wed, Jul 18, 2018 at 11:24:43AM -0400, Aaron Conole wrote: >> Markos Chandras <mchandras@suse.de> writes: >> >> > When ovsdb-server is starting, it performs some DB steps such as >> > creating and upgrading the OvS DB. When we are running as >> > 'non-root' user, the 'runuser' tool is used to manage the privileges. >> > However, when this happens during systemd boot, we observe the following >> > errors in journald: >> > >> > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add >> > PIDs to scope's control group: No such process >> > Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user openvswitch. >> > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state. >> > >> > According to the analysis performed on openSUSE bugzilla[1], it seems >> > that ovsdb-server.service creates (via the call to runuser) a user >> > session and therefore call pam_systemd which in its turn tries to start >> > a systemd user instance: "user@474.service". However "user@474.service" >> > is supposed to be started after systemd-user-sessions.service which is >> > supposed to be started after network.target. Additionally, >> > ovsdb-server.service uses Before=network.target hence the deadlock. >> > >> > We can workaround this by switching to 'root' user when we are >> > performing this pre-startup steps and fixup the DB permissions before >> > we start the actual ovsdb-server daemon. >> > >> > [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630 >> > Cc: Aaron Conole <aconole@redhat.com> >> > Signed-off-by: Markos Chandras <mchandras@suse.de> >> > --- >> > Probably not the cleanest option so I am open to suggestions :) >> >> I think there's actually a race condition here. Most likely, >> ovsdb-server doesn't need to be started before network.service. > > Unfortunately it does because network.service will ifup OVS bridges > and ports and then we need ovsdb already running. Good point. Someday, I'll remember that the first time. > fbl > >> Looking at the bug, I think we can unroll some of the dependencies that >> each unit file has and get a cleaner systemd dependency chain, and >> preserve the existing user downgrade. >> >> I will install an OpenSUSE VM and work on it. Strange that we don't see >> this on the RHEL side. I'll also try to reproduce that. >> >> Thanks for pointing the issue out (and thanks to David and Franck on >> your side). >> >> -Aaron
Markos Chandras <mchandras@suse.de> writes: > Hello Aaron, > > On 18/07/18 16:24, Aaron Conole wrote: >> >> I think there's actually a race condition here. Most likely, >> ovsdb-server doesn't need to be started before network.service. >> >> Looking at the bug, I think we can unroll some of the dependencies that >> each unit file has and get a cleaner systemd dependency chain, and >> preserve the existing user downgrade. >> >> I will install an OpenSUSE VM and work on it. Strange that we don't see >> this on the RHEL side. I'll also try to reproduce that. >> >> Thanks for pointing the issue out (and thanks to David and Franck on >> your side). >> >> -Aaron >> > > Great thank you. If you are using vagrant you can use the following > steps for a reproducer > > vagrant init opensuse/openSUSE-15.0-x86_64 > vagrant up > vagrant ssh > (inside vagrant) > sudo -s > zypper -n in openvswitch > systemctl enable openvswitch > systemctl reboot > > checking the journald logs after the reboot should reveal the issue. > > Let me know if you need any help. Is it possible that the provided diff can fix most of the issue (rather than needing the corrective block in ovs-ctl)? If so, I'd advocate for the smaller change instead. I need to double check it on RHEL/Fedora. Flavio? Timothy? diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in index 92f98ad92..8db887ef6 100644 --- a/utilities/ovs-lib.in +++ b/utilities/ovs-lib.in @@ -389,7 +389,7 @@ move_ip_routes () { ovsdb_tool () { if [ "$OVS_USER" != "" ]; then - runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@" + setpriv --reuid "${OVS_USER%:*}" ovsdb-tool -vconsole:off "$@" else ovsdb-tool -vconsole:off "$@" fi --
I don't understand the conclusion in this thread. Does anyone want me to apply some patch? Or should I stay tuned for further discussion? Thanks, Ben.
Ben Pfaff <blp@ovn.org> writes: > I don't understand the conclusion in this thread. Does anyone want me > to apply some patch? Or should I stay tuned for further discussion? Stay tuned for the exciting conclusion, please :) > Thanks, > > Ben.
On Fri, Jul 27, 2018 at 8:16 PM, Aaron Conole <aconole@redhat.com> wrote: > Markos Chandras <mchandras@suse.de> writes: [...] > > Is it possible that the provided diff can fix most of the issue (rather > than needing the corrective block in ovs-ctl)? If so, I'd advocate for > the smaller change instead. I need to double check it on RHEL/Fedora. > > Flavio? Timothy? > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > index 92f98ad92..8db887ef6 100644 > --- a/utilities/ovs-lib.in > +++ b/utilities/ovs-lib.in > @@ -389,7 +389,7 @@ move_ip_routes () { > > ovsdb_tool () { > if [ "$OVS_USER" != "" ]; then > - runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@" > + setpriv --reuid "${OVS_USER%:*}" ovsdb-tool -vconsole:off "$@" > else > ovsdb-tool -vconsole:off "$@" > fi Hi, I tested your solution with SUSE (Vagrant), RHEL7 and Fedora 28. Unfortunately, as-is, it doesn't work on RHEL7 since the old setpriv version we use on RHEL7 doesn't support an username, but it wants the userid (the numeric one). Moreover if you don't specify --regid setpriv maintains 0 (root) as group id and this can be bad. I created a variant of this implementation that works on SUSE, RHEL7 and Fedora 28 and that fixes the problem, by keeping the same uid/gid/groups used by runuser. Is it ok? diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in index c3b76ec94..33776aac7 100644 --- a/utilities/ovs-lib.in +++ b/utilities/ovs-lib.in @@ -389,7 +389,10 @@ move_ip_routes () { ovsdb_tool () { if [ "$OVS_USER" != "" ]; then - runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@" + local uid=$(id -u "${OVS_USER%:*}") + local gid=$(id -g "${OVS_USER%:*}") + local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',') + setpriv --reuid "$uid" --regid "$gid" --groups "$groups" ovsdb-tool -vconsole:off "$@" else ovsdb-tool -vconsole:off "$@" fi
On Thu, Aug 2, 2018 at 4:58 PM, Timothy Redaelli <tredaelli@redhat.com> wrote: [...] > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > index c3b76ec94..33776aac7 100644 > --- a/utilities/ovs-lib.in > +++ b/utilities/ovs-lib.in > @@ -389,7 +389,10 @@ move_ip_routes () { > > ovsdb_tool () { > if [ "$OVS_USER" != "" ]; then > - runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@" > + local uid=$(id -u "${OVS_USER%:*}") > + local gid=$(id -g "${OVS_USER%:*}") > + local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',') > + setpriv --reuid "$uid" --regid "$gid" --groups "$groups" > ovsdb-tool -vconsole:off "$@" ^ I'm sorry, I had this long line wrapped. > else > ovsdb-tool -vconsole:off "$@" > fi This is, hopefully, the correct git-diff: diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in index c3b76ec94..33776aac7 100644 --- a/utilities/ovs-lib.in +++ b/utilities/ovs-lib.in @@ -389,7 +389,10 @@ move_ip_routes () { ovsdb_tool () { if [ "$OVS_USER" != "" ]; then - runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@" + local uid=$(id -u "${OVS_USER%:*}") + local gid=$(id -g "${OVS_USER%:*}") + local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',') + setpriv --reuid "$uid" --regid "$gid" --groups "$groups" ovsdb-tool -vconsole:off "$@" else ovsdb-tool -vconsole:off "$@" fi
Hello, On 08/02/2018 08:06 PM, Timothy Redaelli wrote: > > This is, hopefully, the correct git-diff: > > diff --git a/utilities/ovs-lib.in <http://ovs-lib.in> > b/utilities/ovs-lib.in <http://ovs-lib.in> > index c3b76ec94..33776aac7 100644 > --- a/utilities/ovs-lib.in <http://ovs-lib.in> > +++ b/utilities/ovs-lib.in <http://ovs-lib.in> > @@ -389,7 +389,10 @@ move_ip_routes () { > > ovsdb_tool () { > if [ "$OVS_USER" != "" ]; then > - runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@" > + local uid=$(id -u "${OVS_USER%:*}") > + local gid=$(id -g "${OVS_USER%:*}") > + local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',') > + setpriv --reuid "$uid" --regid "$gid" --groups "$groups" > ovsdb-tool -vconsole:off "$@" > else > ovsdb-tool -vconsole:off "$@" > fi I can also confirm that this seems to work on openSUSE as well. I will add my Reviewed-by tag once this is properly submitted. Thank you.
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in index 43c8f32b7..588f546fe 100755 --- a/utilities/ovs-ctl.in +++ b/utilities/ovs-ctl.in @@ -109,9 +109,15 @@ do_start_ovsdb () { if daemon_is_running ovsdb-server; then log_success_msg "ovsdb-server is already running" else - # Create initial database or upgrade database schema. - upgrade_db $DB_FILE $DB_SCHEMA || return 1 - + # Create initial database or upgrade database schema. The runuser calls + # in ovsdb_tool function will fail on system startup so we need to run + # as root and fix permissions later on. + [ "$OVS_USER" != "" ] && OVS_USER_OVSDB=${OVS_USER} + OVS_USER="" upgrade_db $DB_FILE $DB_SCHEMA || return 1 + if [ ! -z "${OVS_USER_OVSDB+x}" ]; then + OVS_USER=${OVS_USER_OVSDB} + chown -R "$OVS_USER" $etcdir $dbdir + fi # Start ovsdb-server. set ovsdb-server "$DB_FILE" for db in $EXTRA_DBS; do
When ovsdb-server is starting, it performs some DB steps such as creating and upgrading the OvS DB. When we are running as 'non-root' user, the 'runuser' tool is used to manage the privileges. However, when this happens during systemd boot, we observe the following errors in journald: Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to scope's control group: No such process Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user openvswitch. Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state. According to the analysis performed on openSUSE bugzilla[1], it seems that ovsdb-server.service creates (via the call to runuser) a user session and therefore call pam_systemd which in its turn tries to start a systemd user instance: "user@474.service". However "user@474.service" is supposed to be started after systemd-user-sessions.service which is supposed to be started after network.target. Additionally, ovsdb-server.service uses Before=network.target hence the deadlock. We can workaround this by switching to 'root' user when we are performing this pre-startup steps and fixup the DB permissions before we start the actual ovsdb-server daemon. [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630 Cc: Aaron Conole <aconole@redhat.com> Signed-off-by: Markos Chandras <mchandras@suse.de> --- Probably not the cleanest option so I am open to suggestions :) --- utilities/ovs-ctl.in | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)