Saggi Mizrahi has uploaded a new change for review.
Change subject: Move fencing logic out of API.py
......................................................................
Move fencing logic out of API.py
API.py should not contain logic.
In this patch I also:
- Separate the `status` action to a different function because it does
something completely different from the other action and has different
logic.
- Refactor for better code reusability readability.
- Use real exception internally and have the API layer translate them to
actual exceptions.
- Don't support port strings in methods, just in the interface.
- Don't support string booleans in the methods, just in the interface.
- Added shutdownEvent to ClientIF so there is a public standard way of
inspecting vdsm shutdown.
- Make the fencing logic use betterPopen
Change-Id: I944c6548a42612f705a410fb4290215451bca035
Signed-off-by: Saggi Mizrahi <smizrahi(a)redhat.com>
---
M vdsm.spec.in
M vdsm/API.py
M vdsm/Makefile.am
M vdsm/clientIF.py
4 files changed, 25 insertions(+), 72 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/90/7190/1
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 23afd6b..f122cb5 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -558,6 +558,7 @@
%{_datadir}/%{vdsm_name}/caps.py*
%{_datadir}/%{vdsm_name}/clientIF.py*
%{_datadir}/%{vdsm_name}/API.py*
+%{_datadir}/%{vdsm_name}/fenceAgent.py*
%{_datadir}/%{vdsm_name}/hooking.py*
%{_datadir}/%{vdsm_name}/hooks.py*
%{_datadir}/%{vdsm_name}/libvirtev.py*
diff --git a/vdsm/API.py b/vdsm/API.py
index ae04e01..5826d81 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -19,12 +19,9 @@
#
import os
-import signal
import copy
-import subprocess
import pickle
import time
-import threading
import logging
from vdsm import utils
@@ -40,6 +37,7 @@
from vdsm.define import doneCode, errCode, Kbytes, Mbytes
import caps
from vdsm.config import config
+import fenceAgent
import supervdsm
@@ -981,82 +979,32 @@
agent is one of (rsa, ilo, drac5, ipmilan, etc)
action can be one of (status, on, off, reboot)."""
- def waitForPid(p, inp):
- """ Wait until p.pid exits. Kill it if vdsm exists before.
"""
- try:
- p.stdin.write(inp)
- p.stdin.close()
- while p.poll() is None:
- if not self._cif._enabled:
- self.log.debug('killing fence script pid %s', p.pid)
- os.kill(p.pid, signal.SIGTERM)
- time.sleep(1)
- try:
- # improbable race: p.pid may now belong to another
- # process
- os.kill(p.pid, signal.SIGKILL)
- except:
- pass
- return
- time.sleep(1)
- self.log.debug('rc %s inp %s out %s err %s', p.returncode,
- hidePasswd(inp),
- p.stdout.read(), p.stderr.read())
- except:
- self.log.error("Error killing fence script", exc_info=True)
-
- def hidePasswd(text):
- cleantext = ''
- for line in text.splitlines(True):
- if line.startswith('passwd='):
- line = 'passwd=XXXX\n'
- cleantext += line
- return cleantext
-
self.log.debug('fenceNode(addr=%s,port=%s,agent=%s,user=%s,' +
'passwd=%s,action=%s,secure=%s,options=%s)', addr, port, agent,
username, 'XXXX', action, secure, options)
- if action not in ('status', 'on', 'off',
'reboot'):
- raise ValueError('illegal action ' + action)
+ secure = utils.tobool(secure)
+ port = int(port)
- script = constants.EXT_FENCE_PREFIX + agent
+ if action == "status":
+ try:
+ power = fenceAgent.getFenceStatus(addr, port, agent, username,
+ password, secure, options)
+
+ return {'status': doneCode,
+ 'power': power}
+
+ except fenceAgent.FenceStatusCheckError as e:
+ return {'status': {'code': 1, 'message':
str(e)}}
try:
- p = subprocess.Popen([script], stdin=subprocess.PIPE,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- close_fds=True)
- except OSError, e:
- if e.errno == os.errno.ENOENT:
- return errCode['fenceAgent']
- raise
+ fenceAgent.fenceNode(addr, port, agent, username, password, secure,
+ options, self._cif.shutdownEvent)
- inp = ('agent=fence_%s\nipaddr=%s\nlogin=%s\noption=%s\n' +
- 'passwd=%s\n') % (agent, addr, username, action, password)
- if port != '':
- inp += 'port=%s\n' % (port,)
- if utils.tobool(secure):
- inp += 'secure=yes\n'
- inp += options
- if action == 'status':
- out, err = p.communicate(inp)
- self.log.debug('rc %s in %s out %s err %s', p.returncode,
- hidePasswd(inp), out, err)
- if not 0 <= p.returncode <= 2:
- return {'status': {'code': 1,
- 'message': out + err}}
- message = doneCode['message']
- if p.returncode == 0:
- power = 'on'
- elif p.returncode == 2:
- power = 'off'
- else:
- power = 'unknown'
- message = out + err
- return {'status': {'code': 0, 'message': message},
- 'power': power}
- threading.Thread(target=waitForPid, args=(p, inp)).start()
- return {'status': doneCode}
+ return {'status': doneCode}
+
+ except fenceAgent.UnsupportedFencingAgentError:
+ return errCode['fenceAgent']
def ping(self):
"Ping the server. Useful for tests"
diff --git a/vdsm/Makefile.am b/vdsm/Makefile.am
index 62ba982..4a7bc57 100644
--- a/vdsm/Makefile.am
+++ b/vdsm/Makefile.am
@@ -32,6 +32,7 @@
clientIF.py \
configNetwork.py \
debugPluginClient.py \
+ fenceAgent.py \
guestIF.py \
hooking.py \
hooks.py \
@@ -52,7 +53,8 @@
tc.py \
vdsmDebugPlugin.py \
vmChannels.py \
- vm.py
+ vm.py \
+ $(NULL)
dist_vdsmpylib_PYTHON = \
__init__.py \
diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index 8ba25a7..8760437 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -96,6 +96,7 @@
self.lastRemoteAccess = 0
self._memLock = threading.Lock()
self._enabled = True
+ self.shutdownEvent = threading.Event()
self._netConfigDirty = False
self._prepareMOM()
threading.Thread(target=self._recoverExistingVms,
@@ -219,6 +220,7 @@
for binding in self.bindings.values():
binding.prepareForShutdown()
self._enabled = False
+ self.shutdownEvent.set()
self.channelListener.stop()
self._hostStats.stop()
if self.mom:
--
To view, visit
http://gerrit.ovirt.org/7190
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I944c6548a42612f705a410fb4290215451bca035
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>