On Wed, Aug 14, 2019 at 6:08 PM Jaime Caamaño Ruiz <jcaam...@suse.de> wrote:
> Hello > > Some comments, that probably are due to me being confused checking the > new repo: > > Hi Jaime, This is patch 4 of the series. Patch 1 takes care of adding ovn specific run dirs (run, log, db dirs). Can you please take a look at the series ? Patch 0 - https://mail.openvswitch.org/pipermail/ovs-dev/2019-August/361628.html https://patchwork.ozlabs.org/project/openvswitch/list/?submitter=67480 > 1) > > > sed -i 's:\(.*su\).*:\1 ovn ovn:' %{_sysconfdir}/logrotate.d/ovn > > I dont see the file %{_sysconfdir}/logrotate.d/ovn included anywhere. > Anyway, since OVN_USER_ID defaults to openvswitch:openvswitch, > shouldn't this be > It's added in patch 3 of the series - https://patchwork.ozlabs.org/patch/1146492/ > sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' > %{_sysconfdir}/logrotate.d/ovn > > What about when the group is hugetlbfs? > > hugetlbfs is used when ovs is configured with dpdk. ovs-vswitchd/ovsdb-server will be running as user:group - "openvswitch:hugetlbfs". If ovn-controller runs as user:group - "openvswitch:openvswitch" it can still access the file - /var/run/openvswitch/db.sock and other br-*.mgmt socket files right ? That's what I thought. Please correct me If I am wrong. > 2) > > #OVN_USER_ID="openvswitch:openvswitch" > > Again, what about when the group should be hugetlbfs? > > 2) > > > chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_dbdir > > chown -R $INSTALL_USER:$INSTALL_GROUP $OVN_RUNDIR > > chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir > > Since OVN and OVS still share log and run directories (right?, anyway I > dont see where ovn_dbdir and ovn_logdir are set), changing OVN_USER_ID > to a different value than OVS_USER_ID without changing a bunch of other > stuff will break things. Doesn't it make sense to use OVS_USER_ID > instead of introducing OVN_USER_ID until there is a more meaningful > split? > > Please see patch 1 of the series which sets ovn_logdir, ovn_dbdir etc. The idea is to totally separate out OVN from OVS including user ids. But since its a bit tricky as ovn-controller needs to access /var/run/openvswitch/db.sock, I thought of using openvswitch user for now. But added the OVN_USER_ID so that it would be easier later to switch to new 'ovn' user. Is there a way to solve this issue ? If we run ovn services as "ovn:openvswitch" (or ovn:hugetlbs in the case of dpdk), can ovn-controller access /var/run/openvswitch/db.sock ? I tested it by running as "ovn:openvswitch" and it was not working for me. May be I am doing something wrong ? Thanks for the comments. Numan > BR > Jaime. > > -----Original Message----- > From: nusid...@redhat.com > To: d...@openvswitch.org > Cc: Jaime Caamano <jcaam...@suse.com> > Subject: [PATCH ovn 4/4] rhel: Run ovn services with the 'openvswitch' > user > Date: Tue, 13 Aug 2019 21:58:22 +0530 > > From: Numan Siddique <nusid...@redhat.com> > > This patch could have created a new user 'ovn' for ovn services instead > of using 'openvswitch' user. But this would require some amount of work > and > proper testing since the new user 'ovn' should be part of 'openvswitch' > group (to access /var/run/openvswitch/db.sock.). If ovs is compiled > with dpdk, > then it may get tricky (as ovs-vswitchd is run as user - > openvswitch:hugetlbfs). > We can support a new user for 'ovn' services in the future. > > Recently the commit [1] in ovs repo added support to run ovn services > with the > 'openvswitch' user, but this commit was not applied to ovn repo as we > had > already created a new OVN repo. During the OVS/OVN formal split, we > missed > out on applying the patch [1]. This patch takes some code from [1]. > > [1] - 94e1e8be3187 ("rhel: run ovn with the same user as ovs"). > > CC: Jaime Caamaño Ruiz <jcaam...@suse.com> > Signed-off-by: Numan Siddique <nusid...@redhat.com> > --- > rhel/automake.mk | 3 ++- > rhel/ovn-fedora.spec.in | 13 +++++++++++++ > ...r_lib_systemd_system_ovn-controller-vtep.service | 2 ++ > rhel/usr_lib_systemd_system_ovn-controller.service | 2 ++ > rhel/usr_lib_systemd_system_ovn-northd.service | 5 ++++- > ...usr_share_ovn_scripts_systemd_sysconfig.template | 13 +++++++++++++ > utilities/ovn-ctl | 12 ++++++++++++ > 7 files changed, 48 insertions(+), 2 deletions(-) > create mode 100644 > rhel/usr_share_ovn_scripts_systemd_sysconfig.template > > diff --git a/rhel/automake.mk b/rhel/automake.mk > index 39e216b01..a46e6579b 100644 > --- a/rhel/automake.mk > +++ b/rhel/automake.mk > @@ -15,7 +15,8 @@ EXTRA_DIST += \ > rhel/usr_lib_systemd_system_ovn-controller-vtep.service \ > rhel/usr_lib_systemd_system_ovn-northd.service \ > rhel/usr_lib_firewalld_services_ovn-central-firewall- > service.xml \ > - rhel/usr_lib_firewalld_services_ovn-host-firewall-service.xml > + rhel/usr_lib_firewalld_services_ovn-host-firewall-service.xml > \ > + rhel/usr_share_ovn_scripts_systemd_sysconfig.template > > update_rhel_spec = \ > $(AM_V_GEN)($(ro_shell) && sed -e 's,[@]VERSION[@],$(VERSION),g') \ > diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in > index cbca87511..14035de9a 100644 > --- a/rhel/ovn-fedora.spec.in > +++ b/rhel/ovn-fedora.spec.in > @@ -186,6 +186,10 @@ make %{?_smp_mflags} > rm -rf $RPM_BUILD_ROOT > make install DESTDIR=$RPM_BUILD_ROOT > > +install -p -D -m 0644 \ > + rhel/usr_share_ovn_scripts_systemd_sysconfig.template \ > + $RPM_BUILD_ROOT/%{_sysconfdir}/sysconfig/ovn > + > for service in ovn-controller ovn-controller-vtep ovn-northd; do > install -p -D -m 0644 \ > rhel/usr_lib_systemd_system_${service}.service > \ > @@ -319,6 +323,14 @@ fi > fi > %endif > > +%post > +%if %{with libcapng} > +if [ $1 -eq 1 ]; then > + sed -i 's:^#OVN_USER_ID=:OVN_USER_ID=:' > %{_sysconfdir}/sysconfig/ovn > + sed -i 's:\(.*su\).*:\1 ovn ovn:' %{_sysconfdir}/logrotate.d/ovn > +fi > +%endif > + > %post central > %if 0%{?systemd_post:1} > %systemd_post ovn-northd.service > @@ -413,6 +425,7 @@ if [ $1 -eq 1 ]; then > fi > > %files > +%config(noreplace) %{_sysconfdir}/sysconfig/ovn > %{_bindir}/ovn-nbctl > %{_bindir}/ovn-sbctl > %{_bindir}/ovn-trace > diff --git a/rhel/usr_lib_systemd_system_ovn-controller-vtep.service > b/rhel/usr_lib_systemd_system_ovn-controller-vtep.service > index 832849488..09ad0612c 100644 > --- a/rhel/usr_lib_systemd_system_ovn-controller-vtep.service > +++ b/rhel/usr_lib_systemd_system_ovn-controller-vtep.service > @@ -38,10 +38,12 @@ Restart=on-failure > Environment=OVS_RUNDIR=%t/openvswitch > Environment=OVN_RUNDIR=%t/ovn > Environment=OVN_DB=unix:%t/ovn/ovnsb_db.sock > +EnvironmentFile=-/etc/sysconfig/ovn > Environment=VTEP_DB=unix:%t/openvswitch/db.sock > EnvironmentFile=-/etc/sysconfig/ovn-controller-vtep > ExecStart=/usr/bin/ovn-controller-vtep -vconsole:emer -vsyslog:err > -vfile:info \ > --log-file=/var/log/ovn/ovn-controller-vtep.log \ > + --ovn-user=${OVN_USER_ID} \ > --no-chdir --pidfile=${OVN_RUNDIR}/ovn-controller-vtep.pid \ > --ovnsb-db=${OVN_DB} --vtep-db=${VTEP_DB} > > diff --git a/rhel/usr_lib_systemd_system_ovn-controller.service > b/rhel/usr_lib_systemd_system_ovn-controller.service > index 6c8f33a27..15d0ac853 100644 > --- a/rhel/usr_lib_systemd_system_ovn-controller.service > +++ b/rhel/usr_lib_systemd_system_ovn-controller.service > @@ -24,8 +24,10 @@ Type=forking > PIDFile=/var/run/ovn/ovn-controller.pid > Restart=on-failure > Environment=OVN_RUNDIR=%t/ovn OVS_RUNDIR=%t/openvswitch > +EnvironmentFile=-/etc/sysconfig/ovn > EnvironmentFile=-/etc/sysconfig/ovn-controller > ExecStart=/usr/share/ovn/scripts/ovn-ctl --no-monitor \ > + --ovn-user=${OVN_USER_ID} \ > start_controller $OVN_CONTROLLER_OPTS > ExecStop=/usr/share/ovn/scripts/ovn-ctl stop_controller > > diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service > b/rhel/usr_lib_systemd_system_ovn-northd.service > index 82c23cee4..d281f861c 100644 > --- a/rhel/usr_lib_systemd_system_ovn-northd.service > +++ b/rhel/usr_lib_systemd_system_ovn-northd.service > @@ -21,8 +21,11 @@ After=syslog.target > Type=oneshot > RemainAfterExit=yes > Environment=OVN_RUNDIR=%t/ovn OVN_DBDIR=/var/lib/ovn > +EnvironmentFile=-/etc/sysconfig/ovn > EnvironmentFile=-/etc/sysconfig/ovn-northd > -ExecStart=/usr/share/ovn/scripts/ovn-ctl start_northd $OVN_NORTHD_OPTS > +ExecStartPre=-/usr/bin/chown -R ${OVN_USER_ID} ${OVN_DBDIR} > +ExecStart=/usr/share/ovn/scripts/ovn-ctl \ > + --ovn-user=${OVN_USER_ID} start_northd $OVN_NORTHD_OPTS > ExecStop=/usr/share/ovn/scripts/ovn-ctl stop_northd > > [Install] > diff --git a/rhel/usr_share_ovn_scripts_systemd_sysconfig.template > b/rhel/usr_share_ovn_scripts_systemd_sysconfig.template > new file mode 100644 > index 000000000..4543d1bc9 > --- /dev/null > +++ b/rhel/usr_share_ovn_scripts_systemd_sysconfig.template > @@ -0,0 +1,13 @@ > +### Configuration options for OVN > +# > +# Set "nice" priority at which to run ovn-northd: > +# --ovn-northd-priority=-10 > +# > +# Set "nice" priority at which to run ovn-controller: > +# --ovn-controller-priority=-10 > +# > +# > +OPTIONS="" > + > +# Uncomment and set the OVN User/Group value > +#OVN_USER_ID="openvswitch:openvswitch" > diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl > index 39e03b189..f4ed8f5a8 100755 > --- a/utilities/ovn-ctl > +++ b/utilities/ovn-ctl > @@ -183,6 +183,18 @@ $cluster_remote_port > upgrade_db "$file" "$schema" > fi > > + # Set the owner of the ovn_dbdir (with -R option) to OVN_USER if > set. > + # This is required because the ovndbs are created with root > permission > + # if not present when create_cluster/upgrade_db is called. > + INSTALL_USER="root" > + INSTALL_GROUP="root" > + [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" > + [ "${OVN_USER##*:}" != "" ] && INSTALL_GROUP="${OVN_USER##*:}" > + > + chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_dbdir > + chown -R $INSTALL_USER:$INSTALL_GROUP $OVN_RUNDIR > + chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir > + > set ovsdb-server > set "$@" $log --log-file=$logfile > set "$@" --remote=punix:$sock --pidfile=$db_pid_file > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev