Yeela Kaplan has uploaded a new change for review.
Change subject: [WIP]fencenode: split fenceNode into its own module ......................................................................
[WIP]fencenode: split fenceNode into its own module
Change-Id: I7bd5e7246cf6da21e355849014a8fc71d5edbde6 Signed-off-by: Yeela Kaplan ykaplan@redhat.com --- M debian/vdsm-python.install M lib/vdsm/Makefile.am A lib/vdsm/fencenode.py M vdsm.spec.in M vdsm/API.py 5 files changed, 120 insertions(+), 80 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/43597/1
diff --git a/debian/vdsm-python.install b/debian/vdsm-python.install index d9402c3..901edb6 100644 --- a/debian/vdsm-python.install +++ b/debian/vdsm-python.install @@ -10,6 +10,7 @@ ./usr/lib/python2.7/dist-packages/vdsm/exception.py ./usr/lib/python2.7/dist-packages/vdsm/executor.py ./usr/lib/python2.7/dist-packages/vdsm/ipwrapper.py +./usr/lib/python2.7/dist-packages/vdsm/fencenode.py ./usr/lib/python2.7/dist-packages/vdsm/jsonrpcvdscli.py ./usr/lib/python2.7/dist-packages/vdsm/libvirtconnection.py ./usr/lib/python2.7/dist-packages/vdsm/netconfpersistence.py diff --git a/lib/vdsm/Makefile.am b/lib/vdsm/Makefile.am index 95e236f..875507c 100644 --- a/lib/vdsm/Makefile.am +++ b/lib/vdsm/Makefile.am @@ -30,6 +30,7 @@ exception.py \ executor.py \ ipwrapper.py \ + fencenode.py \ jsonrpcvdscli.py \ libvirtconnection.py \ netconfpersistence.py \ diff --git a/lib/vdsm/fencenode.py b/lib/vdsm/fencenode.py new file mode 100644 index 0000000..3456b2f --- /dev/null +++ b/lib/vdsm/fencenode.py @@ -0,0 +1,100 @@ +# +# Copyright 2015 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# + +import logging +import os +import signal + +import utils +import constants +from define import doneCode, errCode + + +@utils.traceback(on=logging.name) +def fence(script, data): + # non-status actions are sent asyncronously. deathSignal is set to + # make sure that no stray fencing scripts are left behind if Vdsm + # crashes. + rc, out, err = utils.execCmd( + [script], deathSignal=signal.SIGTERM, data=data) + logging.debug('rc %s data %s out %s err %s', rc, + hidePasswd(data), out, err) + return rc, out, err + + +def hidePasswd(text): + cleantext = '' + for line in text.splitlines(True): + if line.startswith('passwd='): + line = 'passwd=XXXX\n' + cleantext += line + return cleantext + + +def fenceNode(addr, port, agent, username, password, action, + secure=False, options='', policy=None): + + logging.debug('fenceNode(addr=%s,port=%s,agent=%s,user=%s,passwd=%s,' + 'action=%s,secure=%s,options=%s,policy=%s)', + addr, port, agent, username, password, action, secure, + options, policy) + + if action not in ('status', 'on', 'off', 'reboot'): + raise ValueError('illegal action ' + action) + + script = constants.EXT_FENCE_PREFIX + agent + + data = ('agent=fence_%s\nipaddr=%s\nlogin=%s\naction=%s\n' + 'passwd=%s\n') % (agent, addr, username, action, password.value) + if port != '': + data += 'port=%s\n' % (port,) + if utils.tobool(secure): + data += 'secure=yes\n' + data += options + + try: + rc, out, err = fence(script, data) + except OSError as e: + if e.errno == os.errno.ENOENT: + return errCode['fenceAgent'] + raise + + logging.debug('rc %s data %s out %s err %s', + rc, hidePasswd(data), out, err) + + if not 0 <= rc <= 2: + return {'status': {'code': 1, + 'message': out + err}} + + message = doneCode['message'] + if action == 'status': + if rc == 0: + power = 'on' + elif rc == 2: + power = 'off' + else: + power = 'unknown' + message = out + err + return {'status': {'code': 0, 'message': message}, + 'power': power} + if rc != 0: + message = out + err + return {'status': {'code': rc, 'message': message}, + 'power': 'unknown', 'operationStatus': 'initiated'} diff --git a/vdsm.spec.in b/vdsm.spec.in index 966a974..9e69162 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -1071,6 +1071,7 @@ %{python_sitelib}/%{vdsm_name}/exception.py* %{python_sitelib}/%{vdsm_name}/executor.py* %{python_sitelib}/%{vdsm_name}/ipwrapper.py* +%{python_sitelib}/%{vdsm_name}/fencenode.py* %{python_sitelib}/%{vdsm_name}/jsonrpcvdscli.py* %{python_sitelib}/%{vdsm_name}/libvirtconnection.py* %{python_sitelib}/%{vdsm_name}/netinfo.py* diff --git a/vdsm/API.py b/vdsm/API.py index 0a93d2a..0719926 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -21,8 +21,6 @@ # pylint: disable=R0904
from contextlib import contextmanager -import os -import signal import six import sys import time @@ -38,6 +36,7 @@ from clientIF import clientIF from vdsm import netinfo from vdsm import constants +from vdsm import fencenode import storage.misc import storage.clusterlock import storage.volume @@ -1187,6 +1186,11 @@ APIBase.__init__(self)
# General Host functions + def ping(self): + "Ping the server. Useful for tests" + updateTimestamp() + return {'status': doneCode} + def fenceNode(self, addr, port, agent, username, password, action, secure=False, options='', policy=None): """Send a fencing command to a remote node. @@ -1194,44 +1198,24 @@ agent is one of (rsa, ilo, drac5, ipmilan, etc) action can be one of (status, on, off, reboot)."""
- @utils.traceback(on=self.log.name) - def fence(script, inp): - # non-status actions are sent asyncronously. deathSignal is set to - # make sure that no stray fencing scripts are left behind if Vdsm - # crashes. - rc, out, err = utils.execCmd( - [script], deathSignal=signal.SIGTERM, - data=inp) - self.log.debug('rc %s inp %s out %s err %s', rc, - hidePasswd(inp), out, err) - return rc, out, err - - def hidePasswd(text): - cleantext = '' - for line in text.splitlines(True): - if line.startswith('passwd='): - line = 'passwd=XXXX\n' - cleantext += line - return cleantext - def should_fence(policy): # skip fence execution if map of storage domains with host id is # entered and at least one storage domain connection from host is # alive if policy is None: - self.log.debug('No policy specified') + logging.debug('No policy specified') return True
hostIdMap = policy.get('storageDomainHostIdMap') if not hostIdMap: - self.log.warning('Invalid policy specified') + logging.warning('Invalid policy specified') return True
result = self._irs.getHostLeaseStatus(hostIdMap) - if result['status']['code'] != 0: - self.log.error( - "Error getting host lease status, error code '%s'", - result['status']['code']) + rc = result['status']['code'] + if rc != 0: + logging.error( + "Error getting host lease status, error code '%s'", rc) return True
# HOST_STATUS_LIVE means that host renewed its lease in last 80 @@ -1239,65 +1223,18 @@ # fencing, even when it's unreachable from engine for sd, status in result['domains'].iteritems(): if status == storage.clusterlock.HOST_STATUS_LIVE: - self.log.debug("Host has live lease on '%s'", sd) + logging.debug("Host has live lease on '%s'", sd) return False
- self.log.debug("Host doesn't have any live lease") + logging.debug("Host doesn't have any live lease") return True
- self.log.debug('fenceNode(addr=%s,port=%s,agent=%s,user=%s,passwd=%s,' - 'action=%s,secure=%s,options=%s,policy=%s)', - addr, port, agent, username, password, action, secure, - options, policy) - - if action not in ('status', 'on', 'off', 'reboot'): - raise ValueError('illegal action ' + action) - if action != 'status' and not should_fence(policy): - self.log.debug("Skipping execution of action '%s'", action) + logging.debug("Skipping execution of action '%s'", action) return {'status': doneCode, 'operationStatus': 'skipped'}
- script = constants.EXT_FENCE_PREFIX + agent - - inp = ('agent=fence_%s\nipaddr=%s\nlogin=%s\naction=%s\n' - 'passwd=%s\n') % (agent, addr, username, action, password.value) - if port != '': - inp += 'port=%s\n' % (port,) - if utils.tobool(secure): - inp += 'secure=yes\n' - inp += options - - try: - rc, out, err = fence(script, inp) - except OSError as e: - if e.errno == os.errno.ENOENT: - return errCode['fenceAgent'] - raise - self.log.debug('rc %s in %s out %s err %s', rc, - hidePasswd(inp), out, err) - if not 0 <= rc <= 2: - return {'status': {'code': 1, - 'message': out + err}} - message = doneCode['message'] - if action == 'status': - if rc == 0: - power = 'on' - elif rc == 2: - power = 'off' - else: - power = 'unknown' - message = out + err - return {'status': {'code': 0, 'message': message}, - 'power': power} - if rc != 0: - message = out + err - return {'status': {'code': rc, 'message': message}, - 'power': 'unknown', 'operationStatus': 'initiated'} - - def ping(self): - "Ping the server. Useful for tests" - updateTimestamp() - return {'status': doneCode} + return fencenode.fenceNode(addr, port, agent, username, password, + action, secure, options, policy)
def getCapabilities(self): """
automation@ovirt.org has posted comments on this change.
Change subject: [WIP]fencenode: split fenceNode into its own module ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Dan Kenigsberg has posted comments on this change.
Change subject: [WIP]fencenode: split fenceNode into its own module ......................................................................
Patch Set 1:
(5 comments)
https://gerrit.ovirt.org/#/c/43597/1/lib/vdsm/fencenode.py File lib/vdsm/fencenode.py:
Line 28: Line 29: Line 30: @utils.traceback(on=logging.name) Line 31: def fence(script, data): Line 32: # non-status actions are sent asyncronously. deathSignal is set to I think that this comment is out of date - all actions are run in sync. However, using deathSignal=SIGTERM is a good idea anyway. Line 33: # make sure that no stray fencing scripts are left behind if Vdsm Line 34: # crashes. Line 35: rc, out, err = utils.execCmd( Line 36: [script], deathSignal=signal.SIGTERM, data=data)
Line 38: hidePasswd(data), out, err) Line 39: return rc, out, err Line 40: Line 41: Line 42: def hidePasswd(text): please use pep8-client names in the new module.
also, please keep functions private unless needed outside the module. Line 43: cleantext = '' Line 44: for line in text.splitlines(True): Line 45: if line.startswith('passwd='): Line 46: line = 'passwd=XXXX\n'
Line 96: 'power': power} Line 97: if rc != 0: Line 98: message = out + err Line 99: return {'status': {'code': rc, 'message': message}, Line 100: 'power': 'unknown', 'operationStatus': 'initiated'} operationStatus should be added by the caller, depnding on whether we skipped fencing or not.
https://gerrit.ovirt.org/#/c/43597/1/vdsm/API.py File vdsm/API.py:
Line 1185: def __init__(self): Line 1186: APIBase.__init__(self) Line 1187: Line 1188: # General Host functions Line 1189: def ping(self): why's this move? Line 1190: "Ping the server. Useful for tests" Line 1191: updateTimestamp() Line 1192: return {'status': doneCode} Line 1193:
Line 1213: Line 1214: result = self._irs.getHostLeaseStatus(hostIdMap) Line 1215: rc = result['status']['code'] Line 1216: if rc != 0: Line 1217: logging.error( I don't think self.log -> logging relates to this patch. Line 1218: "Error getting host lease status, error code '%s'", rc) Line 1219: return True Line 1220: Line 1221: # HOST_STATUS_LIVE means that host renewed its lease in last 80
Oved Ourfali has posted comments on this change.
Change subject: [WIP]fencenode: split fenceNode into its own module ......................................................................
Patch Set 1:
We need to separate it
Dan Kenigsberg has abandoned this change.
Change subject: [WIP]fencenode: split fenceNode into its own module ......................................................................
Abandoned
On Sun, Sep 18, 2016 at 09:32:19AM +0300, Yaniv Bronheim wrote:
Hi Dan, please abandon this patch
gerrit-hooks has posted comments on this change.
Change subject: [WIP]fencenode: split fenceNode into its own module ......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found
vdsm-patches@lists.fedorahosted.org