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 dougsland@redhat.com --- 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 has posted comments on this change.
Change subject: configurator: move usermod to spec ......................................................................
Patch Set 1:
need to be tested with ovirt-node
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 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 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 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 has posted comments on this change.
Change subject: configurator: move usermod to spec ......................................................................
Patch Set 2:
I mean, tested in the rhev-h
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 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
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 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.
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 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
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
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
Yaniv Bronhaim has posted comments on this change.
Change subject: Adding removeConf to sanlock module ......................................................................
Patch Set 6: Verified+1
rebased. please review
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
mooli tayer has posted comments on this change.
Change subject: Adding removeConf to sanlock module ......................................................................
Patch Set 6: Code-Review+1
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):
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):
Itamar Heim has posted comments on this change.
Change subject: Adding removeConf to sanlock module ......................................................................
Patch Set 6:
ping
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:
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:
Douglas Schilling Landgraf has abandoned this change.
Change subject: Adding removeConf to sanlock module ......................................................................
Abandoned
vdsm-patches@lists.fedorahosted.org