Change in vdsm[master]: configurator: move usermod to spec

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has uploaded a new change for review.

Change subject: configurator: move usermod to spec
......................................................................

configurator: move usermod to spec

vdsm-tool configure --force adds to /etc/group (qemu/kvm) sanlock
but doesn't persist the file in ovirt node distro which will
affect vdsm start on next reboot. This patch moves the usermod to
spec file.

Change-Id: I668552fa037414e9a6aee5b049d61749268f85d0
Signed-off-by: Douglas Schilling Landgraf 
---
M lib/vdsm/tool/configurator.py
M vdsm.spec.in
2 files changed, 5 insertions(+), 25 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/55/26055/1

diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index d1c876c..516a7bc 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -24,7 +24,7 @@

from .. import utils
 from . import service, expose
-from ..constants import P_VDSM_EXEC, QEMU_PROCESS_GROUP, VDSM_GROUP
+from ..constants import P_VDSM_EXEC

class _ModuleConfigure(object):
@@ -108,8 +108,6 @@

class SanlockModuleConfigure(_ModuleConfigure):

-    SANLOCK_GROUPS = (QEMU_PROCESS_GROUP, VDSM_GROUP)
-
     def __init__(self):
         super(SanlockModuleConfigure, self).__init__()

@@ -118,28 +116,6 @@

def getServices(self):
         return ['sanlock']
-
-    def configure(self):
-        """
-        Configure sanlock process groups
-        """
-        if os.getuid() != 0:
-            raise UserWarning("Must run as root")
-
-        rc, out, err = utils.execCmd(
-            (
-                '/usr/sbin/usermod',
-                '-a',
-                '-G',
-                ','.join(self.SANLOCK_GROUPS),
-                'sanlock'
-            ),
-            raw=True,
-        )
-        sys.stdout.write(out)
-        sys.stderr.write(err)
-        if rc != 0:
-            raise RuntimeError("Failed to perform sanlock config.")

def isconfigured(self):
         """
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 361a9c1..b149568 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -690,6 +690,7 @@
     /usr/sbin/useradd -r -u 36 -g %{vdsm_group} -d /var/lib/vdsm \
         -s /sbin/nologin -c "Node Virtualization Manager" %{vdsm_user}
 /usr/sbin/usermod -a -G %{qemu_group},%{snlk_group} %{vdsm_user}
+/usr/sbin/usermod -a -G %{qemu_group},%{vdsm_group} %{snlk_user}

%post
 %{_bindir}/vdsm-tool sebool-config || :
@@ -734,6 +735,9 @@
 %endif

%preun
+/usr/bin/gpasswd -d %{snlk_user} %{qemu_group}
+/usr/bin/gpasswd -d %{snlk_user} %{vdsm_group}
+
 if [ "$1" -eq 0 ]; then
         start_conf_section="## beginning of configuration section by vdsm"
         end_conf_section="## end of configuration section by vdsm"

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 1:

need to be tested with ovirt-node

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 1:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/399/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6816/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7606/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7716/ : SUCCESS

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 1:

jenkins failure not related to the patch.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 1: Code-Review-1

(2 comments)

http://gerrit.ovirt.org/#/c/26055/1/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 114:     def getName(self):
Line 115:         return 'sanlock'
Line 116: 
Line 117:     def getServices(self):
Line 118:         return ['sanlock']
we cannot just drop "configure". Non-rpm installation may depend on it; if it's gone, there's no much point in keeping SanlockModuleConfigure.
Line 119: 
Line 120:     def isconfigured(self):
Line 121:         """
Line 122:         True if sanlock service is configured, False if sanlock service

http://gerrit.ovirt.org/#/c/26055/1/vdsm.spec.in
File vdsm.spec.in:

Line 733: /bin/systemctl daemon-reload >/dev/null 2>&1 || :
Line 734: exit 0
Line 735: %endif
Line 736: 
Line 737: %preun
These should happen on %posun, after all files with this ownership are removed, and only there are no more copies of vdsm installed (if [ "$1" -eq 0 ]).
Line 738: /usr/bin/gpasswd -d %{snlk_user} %{qemu_group}
Line 739: /usr/bin/gpasswd -d %{snlk_user} %{vdsm_group}
Line 740: 
Line 741: if [ "$1" -eq 0 ]; then

Yaniv Bronhaim has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 1:

(2 comments)

http://gerrit.ovirt.org/#/c/26055/1/vdsm.spec.in
File vdsm.spec.in:

Line 689: /usr/bin/getent passwd %{vdsm_user} >/dev/null || \
Line 690:     /usr/sbin/useradd -r -u 36 -g %{vdsm_group} -d /var/lib/vdsm \
Line 691:         -s /sbin/nologin -c "Node Virtualization Manager" %{vdsm_user}
Line 692: /usr/sbin/usermod -a -G %{qemu_group},%{snlk_group} %{vdsm_user}
Line 693: /usr/sbin/usermod -a -G %{qemu_group},%{vdsm_group} %{snlk_user}
why not to call vdsm-tool configure --module sanlock ?
Line 694: 
Line 695: %post
Line 696: %{_bindir}/vdsm-tool sebool-config || :
Line 697: # set the vdsm "secret" password for libvirt

Line 733: /bin/systemctl daemon-reload >/dev/null 2>&1 || :
Line 734: exit 0
Line 735: %endif
Line 736: 
Line 737: %preun
> These should happen on %posun, after all files with this ownership are remo

again, do remove-config verb in vdsm-tool for sanlock module. much nicer imo
Line 738: /usr/bin/gpasswd -d %{snlk_user} %{qemu_group}
Line 739: /usr/bin/gpasswd -d %{snlk_user} %{vdsm_group}
Line 740: 
Line 741: if [ "$1" -eq 0 ]; then

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 2:

need to be tested in case looks good to reviewers.

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 2:

I mean, tested in the rhev-h

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 2:

Build Failed

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6837/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7627/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7737/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/406/ : FAILURE

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 2: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/26055/2/vdsm.spec.in
File vdsm.spec.in:

Line 828:     if [ "${vdsmd_start_required}" = 'yes' ]; then
Line 829:         %{_bindir}/vdsm-tool service-start vdsmd >/dev/null 2>&1 || :
Line 830:     fi
Line 831: 
Line 832:     %{_bindir}/vdsm-tool unconfigure --module sanlock
repeat: this must be run only when $1 equals 0.
Line 833: fi
Line 834: exit 0
Line 835: 
Line 836: %if 0%{?rhel} == 6

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 3:

Build Failed

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6842/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7632/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7742/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/408/ : FAILURE

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 4:

Requires: http://gerrit.ovirt.org/#/c/26080/ Jenkins said nothing to do with 26080 and I couldn't update this one.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 4:

Build Failed

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6843/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7633/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7743/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/409/ : FAILURE

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 4: Code-Review-1

Need to be resync, since we are splitting into http://gerrit.ovirt.org/26089

Yaniv Bronhaim has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 4: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/26055/4/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 144: unconfigure
i think its better that mtayer will take over and merge it with http://gerrit.ovirt.org/#/c/21772/

Itamar Heim has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 4:

ping

Yaniv Bronhaim has posted comments on this change.

Change subject: configurator: move usermod to spec
......................................................................

Patch Set 4:

will take care of this patch asap. it is useful

Change in vdsm[master]: Adding removeConf to sanlock module

Yaniv Bronhaim has posted comments on this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Patch Set 5:

(1 comment)

http://gerrit.ovirt.org/#/c/26055/5//COMMIT_MSG
Commit Message:

Line 7: Adding removeConf to sanlock module
Line 8: 
Line 9: Adding removal of sanlock group for admin users. This will be used in
Line 10: vdsm spec file to remove vdsm configuration after uninstall.
Line 11: 
currently we don't do that ,but i guess we should
Line 12: Change-Id: I668552fa037414e9a6aee5b049d61749268f85d0

Change in vdsm[master]: Adding removeConf to sanlock module

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Patch Set 5:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9724/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8791/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9577/ : SUCCESS

Change in vdsm[master]: Adding removeConf to sanlock module

Yaniv Bronhaim has posted comments on this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Patch Set 6: Verified+1

rebased. please review

Change in vdsm[master]: Adding removeConf to sanlock module

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Patch Set 6:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9192/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9976/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10131/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5058/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3215/ : SUCCESS

Change in vdsm[master]: Adding removeConf to sanlock module

mooli tayer has posted comments on this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Patch Set 6: Code-Review+1

Change in vdsm[master]: Adding removeConf to sanlock module

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Patch Set 6: Code-Review-1

(2 comments)

Yaniv, have you tested in ovirtNode? Not sure if it will work in ovirt-node, please see my comments.

http://gerrit.ovirt.org/#/c/26055/6/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 614:                 ','.join(self.SANLOCK_GROUPS),
Line 615:                 SANLOCK_USER
Line 616:             ),
Line 617:             raw=True,
Line 618:         )
if isOvirt: persist /etc/group ?
Line 619:         sys.stdout.write(out)
Line 620:         sys.stderr.write(err)
Line 621:         if rc != 0:
Line 622:             raise RuntimeError("Failed to perform sanlock config.")

Line 639:                 raw=True,
Line 640:             )
Line 641:             sys.stdout.write(out)
Line 642:             sys.stderr.write(err)
Line 643: 
What about include:  if isOvirt: persist /etc/group
Line 644:             if rc != 0:
Line 645:                 raise InvalidRun("Failed to remove sanlock groups.")
Line 646: 
Line 647:     def isconfigured(self):

Change in vdsm[master]: Adding removeConf to sanlock module

Yaniv Bronhaim has posted comments on this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Patch Set 6:

(1 comment)

did you test configure sanlock module on ovirt node? where do we persist /etc/groups after the spec configure call that you added?

http://gerrit.ovirt.org/#/c/26055/6/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 639:                 raw=True,
Line 640:             )
Line 641:             sys.stdout.write(out)
Line 642:             sys.stderr.write(err)
Line 643:
> What about include:  if isOvirt: persist /etc/group

we need to add it both here and in configure as you said. please post a patch for that and rebase it on this one (I don't familiar with the node semantics enough)
Line 644:             if rc != 0:
Line 645:                 raise InvalidRun("Failed to remove sanlock groups.")
Line 646: 
Line 647:     def isconfigured(self):

Change in vdsm[master]: Adding removeConf to sanlock module

Itamar Heim has posted comments on this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Patch Set 6:

ping

Change in vdsm[master]: Adding removeConf to sanlock module

mooli tayer has posted comments on this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/26055/2/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 141:         sys.stderr.write(err)
Line 142:         if rc != 0:
Line 143:             raise RuntimeError("Failed to perform sanlock config.")
Line 144: 
Line 145:     def unconfigure(self):
I believe this is called removeConf now.
Line 146:         """
Line 147:         Unconfigure sanlock process groups
Line 148:         """
Line 149:         if os.getuid() != 0:

Change in vdsm[master]: Adding removeConf to sanlock module

mooli tayer has posted comments on this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/26055/2/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 141:         sys.stderr.write(err)
Line 142:         if rc != 0:
Line 143:             raise RuntimeError("Failed to perform sanlock config.")
Line 144: 
Line 145:     def unconfigure(self):
> I believe this is called removeConf now.

opps wrong patch set.

ignore
Line 146:         """
Line 147:         Unconfigure sanlock process groups
Line 148:         """
Line 149:         if os.getuid() != 0:

Change in vdsm[master]: Adding removeConf to sanlock module

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has abandoned this change.

Change subject: Adding removeConf to sanlock module
......................................................................

Abandoned