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

Reply via email to