[ovs-dev] rhel: user/group openvswitch does not exist
diff mbox series

Message ID 20180410134936.26428-1-aconole@redhat.com
State Changes Requested
Headers show
Series
  • [ovs-dev] rhel: user/group openvswitch does not exist
Related show

Commit Message

Aaron Conole April 10, 2018, 1:49 p.m. UTC
From: Alan Pevec <alan.pevec@redhat.com>

Default ownership[1] for config files is failing on an empty system:
  Running scriptlet: openvswitch-2.9.0-3.fc28.x86_64
warning: user openvswitch does not exist - using root
warning: group openvswitch does not exist - using root
...

Required user/group need to be created in %pre as documented in
Fedora guideline[2]

[1] https://github.com/openvswitch/ovs/commit/951d79e638ecdb3b1dcd19df1adb2ff91fe61af8

[2] https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Dynamic_allocation

Submitted-at: https://github.com/openvswitch/ovs/pull/223
Signed-off-by: Alan Pevec <alan.pevec@redhat.com>
Co-authored-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
NOTE: This differs from the pull request upstream as I've also moved the dpdk
      section to %pre, after talking with Alan.

 rhel/openvswitch-fedora.spec.in | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Comments

Markos Chandras April 10, 2018, 1:56 p.m. UTC | #1
On 10/04/18 14:49, Aaron Conole wrote:
> From: Alan Pevec <alan.pevec@redhat.com>
> 
> Default ownership[1] for config files is failing on an empty system:
>   Running scriptlet: openvswitch-2.9.0-3.fc28.x86_64
> warning: user openvswitch does not exist - using root
> warning: group openvswitch does not exist - using root
> ...
> 
> Required user/group need to be created in %pre as documented in
> Fedora guideline[2]
> 
> [1] https://github.com/openvswitch/ovs/commit/951d79e638ecdb3b1dcd19df1adb2ff91fe61af8
> 
> [2] https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Dynamic_allocation
> 
> Submitted-at: https://github.com/openvswitch/ovs/pull/223
> Signed-off-by: Alan Pevec <alan.pevec@redhat.com>
> Co-authored-by: Aaron Conole <aconole@redhat.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---

Looks good to me.

Reviewed-by: Markos Chandras <mchandras@suse.de>
Markos Chandras April 10, 2018, 2:01 p.m. UTC | #2
On 10/04/18 14:49, Aaron Conole wrote:
> +%pre
> +getent group openvswitch >/dev/null || groupadd -r openvswitch
> +getent passwd openvswitch >/dev/null || \
> +    useradd -r -g openvswitch -d / -s /sbin/nologin \
> +    -c "Open vSwitch Daemons" openvswitch
> +
> +%if %{with dpdk}
> +    getent group hugetlbfs >/dev/null || groupadd hugetlbfs
> +    usermod -a -G hugetlbfs openvswitch
> +%endif
> +exit 0

Maybe this 'exit 0' is not actually needed here?
Timothy Redaelli April 10, 2018, 2:03 p.m. UTC | #3
On Tue, 10 Apr 2018 09:49:36 -0400
Aaron Conole <aconole@redhat.com> wrote:

> From: Alan Pevec <alan.pevec@redhat.com>
> 
> Default ownership[1] for config files is failing on an empty system:
>   Running scriptlet: openvswitch-2.9.0-3.fc28.x86_64
> warning: user openvswitch does not exist - using root
> warning: group openvswitch does not exist - using root
> ...
> 
> Required user/group need to be created in %pre as documented in
> Fedora guideline[2]
> 
> [1]
> https://github.com/openvswitch/ovs/commit/951d79e638ecdb3b1dcd19df1adb2ff91fe61af8
> 
> [2]
> https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Dynamic_allocation
> 
> Submitted-at: https://github.com/openvswitch/ovs/pull/223
> Signed-off-by: Alan Pevec <alan.pevec@redhat.com>
> Co-authored-by: Aaron Conole <aconole@redhat.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> NOTE: This differs from the pull request upstream as I've also moved
> the dpdk section to %pre, after talking with Alan.
> 
>  rhel/openvswitch-fedora.spec.in | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/rhel/openvswitch-fedora.spec.in
> b/rhel/openvswitch-fedora.spec.in index 4f2398d97..4b9831674 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -92,6 +92,7 @@ Requires: openssl hostname iproute module-init-tools
>  #Upstream kernel commit 4f647e0a3c37b8d5086214128614a136064110c3
>  #Requires: kernel >= 3.15.0-0
>  
> +Requires(pre): shadow-utils
>  Requires(post): /usr/bin/getent
>  Requires(post): /usr/sbin/useradd
>  Requires(post): /usr/bin/sed

"Requires(post): /usr/bin/getent" and
"Requires(post): /usr/sbin/useradd" should be removed since they are
deprecated by "Requires(pre): shadow-utils" and not used anymore in
%post. Beside that it looks good to me.

> @@ -384,17 +385,23 @@ rm -rf $RPM_BUILD_ROOT
>      fi
>  %endif
>  
> +%pre
> +getent group openvswitch >/dev/null || groupadd -r openvswitch
> +getent passwd openvswitch >/dev/null || \
> +    useradd -r -g openvswitch -d / -s /sbin/nologin \
> +    -c "Open vSwitch Daemons" openvswitch
> +
> +%if %{with dpdk}
> +    getent group hugetlbfs >/dev/null || groupadd hugetlbfs
> +    usermod -a -G hugetlbfs openvswitch
> +%endif
> +exit 0
>  %post
>  if [ $1 -eq 1 ]; then
> -    getent passwd openvswitch >/dev/null || \
> -        useradd -r -d / -s /sbin/nologin -c "Open vSwitch Daemons"
> openvswitch -
>      sed -i
> 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch 
>  %if %{with dpdk}
> -    getent group hugetlbfs >/dev/null || \
> -        groupadd hugetlbfs
> -    usermod -a -G hugetlbfs openvswitch
>      sed -i \
>          's@OVS_USER_ID="openvswitch:openvswitch"@OVS_USER_ID="openvswitch:hugetlbfs"@'\
>          /etc/sysconfig/openvswitch

Patch
diff mbox series

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 4f2398d97..4b9831674 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -92,6 +92,7 @@  Requires: openssl hostname iproute module-init-tools
 #Upstream kernel commit 4f647e0a3c37b8d5086214128614a136064110c3
 #Requires: kernel >= 3.15.0-0
 
+Requires(pre): shadow-utils
 Requires(post): /usr/bin/getent
 Requires(post): /usr/sbin/useradd
 Requires(post): /usr/bin/sed
@@ -384,17 +385,23 @@  rm -rf $RPM_BUILD_ROOT
     fi
 %endif
 
+%pre
+getent group openvswitch >/dev/null || groupadd -r openvswitch
+getent passwd openvswitch >/dev/null || \
+    useradd -r -g openvswitch -d / -s /sbin/nologin \
+    -c "Open vSwitch Daemons" openvswitch
+
+%if %{with dpdk}
+    getent group hugetlbfs >/dev/null || groupadd hugetlbfs
+    usermod -a -G hugetlbfs openvswitch
+%endif
+exit 0
+
 %post
 if [ $1 -eq 1 ]; then
-    getent passwd openvswitch >/dev/null || \
-        useradd -r -d / -s /sbin/nologin -c "Open vSwitch Daemons" openvswitch
-
     sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
 
 %if %{with dpdk}
-    getent group hugetlbfs >/dev/null || \
-        groupadd hugetlbfs
-    usermod -a -G hugetlbfs openvswitch
     sed -i \
         's@OVS_USER_ID="openvswitch:openvswitch"@OVS_USER_ID="openvswitch:hugetlbfs"@'\
         /etc/sysconfig/openvswitch