Hello Ayal Baron, Timothy Asir, Saggi Mizrahi, Federico Simoncelli, Dan
Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/7951
to review the following change.
Change subject: [WIP] used gluster cli xml output
......................................................................
[WIP] used gluster cli xml output
Below verbs are using gluster cli xml output
glusterVolumesList
glusterVolumeCreate
glusterVolumeStart
glusterVolumeStop
glusterVolumeDelete
glusterVolumeSet
glusterVolumeReset
glusterVolumeAddBrick
glusterVolumeRemoveBrickForce
glusterHostAdd
glusterHostRemove
Change-Id: I364930675dcc433dfc0432a343fc66e91721e801
Signed-off-by: Bala.FA <[email protected]>
---
M tests/gluster_cli_tests.py
M vdsm/gluster/api.py
M vdsm/gluster/cli.py
3 files changed, 195 insertions(+), 179 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/51/7951/1
diff --git a/tests/gluster_cli_tests.py b/tests/gluster_cli_tests.py
index bb88918..e5fe1ce 100644
--- a/tests/gluster_cli_tests.py
+++ b/tests/gluster_cli_tests.py
@@ -39,41 +39,98 @@
raise SkipTest("vdsm-gluster not found")
def _parseVolumeInfo_empty_test(self):
- out = ['No volumes present']
- self.assertFalse(gcli._parseVolumeInfo(out))
+ out = """<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<cliOutput>
+ <opRet>0</opRet>
+ <opErrno>0</opErrno>
+ <opErrstr/>
+ <volInfo/>
+</cliOutput>
+"""
+ tree = etree.fromstring(out)
+ self.assertFalse(gcli._parseVolumeInfo(tree))
def _parseVolumeInfo_test(self):
- out = [' ',
- 'Volume Name: music',
- 'Type: Distribute',
- 'Volume ID: 1e7d2638-fb77-4325-94fd-3242650a013c',
- 'Status: Stopped',
- 'Number of Bricks: 2',
- 'Transport-type: tcp',
- 'Bricks:',
- 'Brick1: 192.168.122.167:/tmp/music-b1',
- 'Brick2: 192.168.122.167:/tmp/music-b2',
- 'Options Reconfigured:',
- 'auth.allow: *']
- volumeInfo = gcli._parseVolumeInfo(out)
- for volumeName in volumeInfo:
- if volumeName == 'music':
- self.assertEquals(volumeInfo['music']['volumeName'],
- volumeName)
- self.assertEquals(volumeInfo['music']['uuid'],
- '1e7d2638-fb77-4325-94fd-3242650a013c')
- self.assertEquals(volumeInfo['music']['volumeType'],
- 'DISTRIBUTE')
- self.assertEquals(volumeInfo['music']['volumeStatus'],
- 'STOPPED')
- self.assertEquals(volumeInfo['music']['transportType'],
- ['TCP'])
- self.assertEquals(volumeInfo['music']['bricks'],
- ['192.168.122.167:/tmp/music-b1',
- '192.168.122.167:/tmp/music-b2'])
- self.assertEquals(volumeInfo['music']['brickCount'], '2')
- self.assertEquals(volumeInfo['music']['options'],
- {'auth.allow': '*'})
+ out = """<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<cliOutput>
+ <opRet>0</opRet>
+ <opErrno>0</opErrno>
+ <opErrstr/>
+ <volInfo>
+ <volumes>
+ <volume>
+ <name>music</name>
+ <id>b3114c71-741b-4c6f-a39e-80384c4ea3cf</id>
+ <type>2</type>
+ <status>1</status>
+ <brickCount>2</brickCount>
+ <distCount>2</distCount>
+ <stripeCount>1</stripeCount>
+ <replicaCount>2</replicaCount>
+ <transport>0</transport>
+ <bricks>
+ <brick>192.168.122.2:/tmp/music-b1</brick>
+ <brick>192.168.122.2:/tmp/music-b2</brick>
+ </bricks>
+ <optCount>1</optCount>
+ <options>
+ <option>
+ <name>auth.allow</name>
+ <value>*</value>
+ </option>
+ </options>
+ </volume>
+ <volume>
+ <name>test1</name>
+ <id>2e562614-00ad-4acb-a630-4429049a9818</id>
+ <type>0</type>
+ <status>2</status>
+ <brickCount>2</brickCount>
+ <distCount>1</distCount>
+ <stripeCount>1</stripeCount>
+ <replicaCount>1</replicaCount>
+ <transport>1</transport>
+ <bricks>
+ <brick>192.168.122.2:/tmp/test1-b1</brick>
+ <brick>192.168.122.2:/tmp/test1-b2</brick>
+ </bricks>
+ <optCount>0</optCount>
+ <options/>
+ </volume>
+ <count>2</count>
+ </volumes>
+ </volInfo>
+</cliOutput>
+"""
+ tree = etree.fromstring(out)
+ oVolumeInfo = {'music':
+ {'brickCount': '2',
+ 'bricks': ['192.168.122.2:/tmp/music-b1',
+ '192.168.122.2:/tmp/music-b2'],
+ 'distCount': '2',
+ 'options': {'auth.allow': '*'},
+ 'replicaCount': '2',
+ 'stripeCount': '1',
+ 'transportType': [gcli.TransportType.TCP],
+ 'uuid': 'b3114c71-741b-4c6f-a39e-80384c4ea3cf',
+ 'volumeName': 'music',
+ 'volumeStatus': gcli.VolumeStatus.ONLINE,
+ 'volumeType': '2'},
+ 'test1':
+ {'brickCount': '2',
+ 'bricks': ['192.168.122.2:/tmp/test1-b1',
+ '192.168.122.2:/tmp/test1-b2'],
+ 'distCount': '1',
+ 'options': {},
+ 'replicaCount': '1',
+ 'stripeCount': '1',
+ 'transportType': [gcli.TransportType.RDMA],
+ 'uuid': '2e562614-00ad-4acb-a630-4429049a9818',
+ 'volumeName': 'test1',
+ 'volumeStatus': gcli.VolumeStatus.OFFLINE,
+ 'volumeType': '0'}}
+ volumeInfo = gcli._parseVolumeInfo(tree)
+ self.assertEquals(volumeInfo, oVolumeInfo)
def test_parseVolumeInfo(self):
self._parseVolumeInfo_empty_test()
diff --git a/vdsm/gluster/api.py b/vdsm/gluster/api.py
index ba1f374..3c1bc58 100644
--- a/vdsm/gluster/api.py
+++ b/vdsm/gluster/api.py
@@ -42,11 +42,6 @@
return wrapper
-class VolumeStatus():
- ONLINE = 'ONLINE'
- OFFLINE = 'OFFLINE'
-
-
class GlusterApi(object):
"""
The gluster interface of vdsm.
@@ -120,35 +115,14 @@
@exportAsVerb
def volumesList(self, volumeName=None, options=None):
- """
- Returns:
- {'status' : {'code': CODE, 'message': MESSAGE},
- 'volumes': {VOLUMENAME: {'brickCount': BRICKCOUNT,
- 'bricks': [BRICK1, BRICK2, ...],
- 'options': {OPTION: VALUE, ...},
- 'transportType': [TCP, RDMA, ...],
- 'uuid': UUID,
- 'volumeName': NAME,
- 'volumeStatus': STATUS,
- 'volumeType': TYPE}, ...}}
- """
- volumeInfoDict = self.svdsmProxy.glusterVolumeInfo(volumeName)
- for name, info in volumeInfoDict.iteritems():
- info["volumeType"] = info["volumeType"].replace("-", "_")
- if info["volumeStatus"] == "STARTED":
- info["volumeStatus"] = VolumeStatus.ONLINE
- else:
- info["volumeStatus"] = VolumeStatus.OFFLINE
- return {'volumes': volumeInfoDict}
+ return {'volumes': self.svdsmProxy.glusterVolumeInfo(volumeName)}
@exportAsVerb
def volumeCreate(self, volumeName, brickList, replicaCount=0,
stripeCount=0, transportList=[], options=None):
- self.svdsmProxy.glusterVolumeCreate(volumeName, brickList,
- replicaCount, stripeCount,
- transportList)
- volumeList = self.svdsmProxy.glusterVolumeInfo()
- return {'uuid': volumeList[volumeName]['uuid']}
+ return self.svdsmProxy.glusterVolumeCreate(volumeName, brickList,
+ replicaCount, stripeCount,
+ transportList)
@exportAsVerb
def volumeStart(self, volumeName, force=False, options=None):
diff --git a/vdsm/gluster/cli.py b/vdsm/gluster/cli.py
index e31c3f4..2ead7c9 100644
--- a/vdsm/gluster/cli.py
+++ b/vdsm/gluster/cli.py
@@ -18,7 +18,6 @@
# Refer to the README and COPYING files for full details of the license
#
-import re
import xml.etree.cElementTree as etree
from functools import wraps
@@ -30,7 +29,6 @@
_glusterCommandPath = utils.CommandPath("gluster",
"/usr/sbin/gluster",
)
-_brickCountRegEx = re.compile('(\d+) x (\d+) = (\d+)')
def _getGlusterVolCmd():
@@ -74,6 +72,16 @@
RUNNING = 'RUNNING'
FAILED = 'FAILED'
COMPLETED = 'COMPLETED'
+
+
+class VolumeStatus:
+ ONLINE = 'ONLINE'
+ OFFLINE = 'OFFLINE'
+
+
+class TransportType:
+ TCP = 'TCP'
+ RDMA = 'RDMA'
def _execGluster(cmd):
@@ -354,76 +362,47 @@
raise ge.GlusterXmlErrorException(err=[etree.tostring(xmltree)])
-def _parseVolumeInfo(out):
- if not out[0].strip():
- del out[0]
- if out[-1].strip():
- out += [""]
-
- if out[0].strip().upper() == "NO VOLUMES PRESENT":
- return {}
-
- volumeInfoDict = {}
- volumeInfo = {}
- volumeName = None
- brickList = []
- volumeOptions = {}
- for line in out:
- line = line.strip()
- if not line:
- if volumeName and volumeInfo:
- volumeInfo["bricks"] = brickList
- volumeInfo["options"] = volumeOptions
- volumeInfoDict[volumeName] = volumeInfo
- volumeInfo = {}
- volumeName = None
- brickList = []
- volumeOptions = {}
- continue
-
- tokens = line.split(":", 1)
- key = tokens[0].strip().upper()
- if key == "BRICKS":
- continue
- elif key == "OPTIONS RECONFIGURED":
- continue
- elif key == "VOLUME NAME":
- volumeName = tokens[1].strip()
- volumeInfo["volumeName"] = volumeName
- elif key == "VOLUME ID":
- volumeInfo["uuid"] = tokens[1].strip()
- elif key == "TYPE":
- volumeInfo["volumeType"] = tokens[1].strip().upper()
- elif key == "STATUS":
- volumeInfo["volumeStatus"] = tokens[1].strip().upper()
- elif key == "TRANSPORT-TYPE":
- volumeInfo["transportType"] = tokens[1].strip().upper().split(',')
- elif key.startswith("BRICK"):
- brickList.append(tokens[1].strip())
- elif key == "NUMBER OF BRICKS":
- volumeInfo["brickCount"] = tokens[1].strip()
+def _parseVolumeInfo(tree):
+ """
+ {VOLUMENAME: {'brickCount': BRICKCOUNT,
+ 'bricks': [BRICK1, BRICK2, ...],
+ 'options': {OPTION: VALUE, ...},
+ 'transportType': [TCP,RDMA, ...],
+ 'uuid': UUID,
+ 'volumeName': NAME,
+ 'volumeStatus': STATUS,
+ 'volumeType': TYPE}, ...}
+ """
+ volumes = {}
+ for el in tree.findall('volInfo/volumes/volume'):
+ value = {}
+ value['volumeName'] = el.find('name').text
+ value['uuid'] = el.find('id').text
+ value['volumeType'] = el.find('type').text
+ status = el.find('status').text
+ if status == '1':
+ value["volumeStatus"] = VolumeStatus.ONLINE
else:
- volumeOptions[tokens[0].strip()] = tokens[1].strip()
-
- for volumeName, volumeInfo in volumeInfoDict.iteritems():
- if volumeInfo["volumeType"] == "REPLICATE":
- volumeInfo["replicaCount"] = volumeInfo["brickCount"]
- elif volumeInfo["volumeType"] == "STRIPE":
- volumeInfo["stripeCount"] = volumeInfo["brickCount"]
- elif volumeInfo["volumeType"] == "DISTRIBUTED-REPLICATE":
- m = _brickCountRegEx.match(volumeInfo["brickCount"])
- if m:
- volumeInfo["replicaCount"] = m.groups()[1]
- else:
- volumeInfo["replicaCount"] = ""
-
- elif volumeInfo["volumeType"] == "DISTRIBUTED-STRIPE":
- m = _brickCountRegEx.match(volumeInfo["brickCount"])
- if m:
- volumeInfo["stripeCount"] = m.groups()[1]
- else:
- volumeInfo["stripeCount"] = ""
- return volumeInfoDict
+ value["volumeStatus"] = VolumeStatus.OFFLINE
+ value['brickCount'] = el.find('brickCount').text
+ value['distCount'] = el.find('distCount').text
+ value['stripeCount'] = el.find('stripeCount').text
+ value['replicaCount'] = el.find('replicaCount').text
+ transportType = el.find('transport').text
+ if transportType == '0':
+ value['transportType'] = [TransportType.TCP]
+ elif transportType == '1':
+ value['transportType'] = [TransportType.RDMA]
+ else:
+ value['transportType'] = [TransportType.TCP, TransportType.RDMA]
+ value['bricks'] = []
+ value['options'] = {}
+ for b in el.findall('bricks/brick'):
+ value['bricks'].append(b.text)
+ for o in el.findall('options/option'):
+ value['options'][o.find('name').text] = o.find('value').text
+ volumes[value['volumeName']] = value
+ return volumes
@exportToSuperVdsm
@@ -442,11 +421,14 @@
command = _getGlusterVolCmd() + ["info"]
if volumeName:
command.append(volumeName)
- rc, out, err = _execGluster(command)
- if rc:
- raise ge.GlusterVolumesListFailedException(rc, out, err)
- else:
- return _parseVolumeInfo(out)
+ try:
+ xmltree = _execGlusterXml(command)
+ except ge.GlusterCmdFailedException, e:
+ raise ge.GlusterVolumesListFailedException(rc=e.rc, err=e.err)
+ try:
+ return _parseVolumeInfo(xmltree)
+ except (etree.ParseError, AttributeError, ValueError):
+ raise ge.GlusterXmlErrorException(err=[etree.tostring(xmltree)])
@exportToSuperVdsm
@@ -460,12 +442,14 @@
if transportList:
command += ["transport", ','.join(transportList)]
command += brickList
-
- rc, out, err = _execGluster(command)
- if rc:
- raise ge.GlusterVolumeCreateFailedException(rc, out, err)
- else:
- return True
+ try:
+ xmltree = _execGlusterXml(command)
+ except ge.GlusterCmdFailedException, e:
+ raise ge.GlusterVolumeCreateFailedException(rc=e.rc, err=e.err)
+ try:
+ return {'uuid': xmltree.find('id').text}
+ except (etree.ParseError, AttributeError, ValueError):
+ raise ge.GlusterXmlErrorException(err=[etree.tostring(xmltree)])
@exportToSuperVdsm
@@ -473,11 +457,11 @@
command = _getGlusterVolCmd() + ["start", volumeName]
if force:
command.append('force')
- rc, out, err = _execGluster(command)
- if rc:
- raise ge.GlusterVolumeStartFailedException(rc, out, err)
- else:
+ try:
+ _execGlusterXml(command)
return True
+ except ge.GlusterCmdFailedException, e:
+ raise ge.GlusterVolumeStartFailedException(rc=e.rc, err=e.err)
@exportToSuperVdsm
@@ -485,30 +469,31 @@
command = _getGlusterVolCmd() + ["stop", volumeName]
if force:
command.append('force')
- rc, out, err = _execGluster(command)
- if rc:
- raise ge.GlusterVolumeStopFailedException(rc, out, err)
- else:
+ try:
+ _execGlusterXml(command)
return True
+ except ge.GlusterCmdFailedException, e:
+ raise ge.GlusterVolumeStopFailedException(rc=e.rc, err=e.err)
@exportToSuperVdsm
def volumeDelete(volumeName):
- rc, out, err = _execGluster(_getGlusterVolCmd() + ["delete", volumeName])
- if rc:
- raise ge.GlusterVolumeDeleteFailedException(rc, out, err)
- else:
+ command = _getGlusterVolCmd() + ["delete", volumeName]
+ try:
+ _execGlusterXml(command)
return True
+ except ge.GlusterCmdFailedException, e:
+ raise ge.GlusterVolumeDeleteFailedException(rc=e.rc, err=e.err)
@exportToSuperVdsm
def volumeSet(volumeName, option, value):
- rc, out, err = _execGluster(_getGlusterVolCmd() + ["set", volumeName,
- option, value])
- if rc:
- raise ge.GlusterVolumeSetFailedException(rc, out, err)
- else:
+ command = _getGlusterVolCmd() + ["set", volumeName, option, value]
+ try:
+ _execGlusterXml(command)
return True
+ except ge.GlusterCmdFailedException, e:
+ raise ge.GlusterVolumeSetFailedException(rc=e.rc, err=e.err)
def _parseVolumeSetHelpXml(out):
@@ -538,11 +523,11 @@
command.append(option)
if force:
command.append('force')
- rc, out, err = _execGluster(command)
- if rc:
- raise ge.GlusterVolumeResetFailedException(rc, out, err)
- else:
+ try:
+ _execGlusterXml(command)
return True
+ except ge.GlusterCmdFailedException, e:
+ raise ge.GlusterVolumeResetFailedException(rc=e.rc, err=e.err)
@exportToSuperVdsm
@@ -554,12 +539,11 @@
if replicaCount:
command += ["replica", "%s" % replicaCount]
command += brickList
-
- rc, out, err = _execGluster(command)
- if rc:
- raise ge.GlusterVolumeBrickAddFailedException(rc, out, err)
- else:
+ try:
+ _execGlusterXml(command)
return True
+ except ge.GlusterCmdFailedException, e:
+ raise ge.GlusterVolumeBrickAddFailedException(rc=e.rc, err=e.err)
@exportToSuperVdsm
@@ -820,11 +804,12 @@
@exportToSuperVdsm
def peerProbe(hostName):
- rc, out, err = _execGluster(_getGlusterPeerCmd() + ["probe", hostName])
- if rc:
- raise ge.GlusterHostAddFailedException(rc, out, err)
- else:
+ command = _getGlusterPeerCmd() + ["probe", hostName]
+ try:
+ _execGlusterXml(command)
return True
+ except ge.GlusterCmdFailedException, e:
+ raise ge.GlusterHostAddFailedException(rc=e.rc, err=e.err)
@exportToSuperVdsm
@@ -832,11 +817,11 @@
command = _getGlusterPeerCmd() + ["detach", hostName]
if force:
command.append('force')
- rc, out, err = _execGluster(command)
- if rc:
- raise ge.GlusterHostRemoveFailedException(rc, out, err)
- else:
+ try:
+ _execGlusterXml(command)
return True
+ except ge.GlusterCmdFailedException, e:
+ raise ge.GlusterHostRemoveFailedException(rc=e.rc, err=e.err)
def _parsePeerStatus(tree, gHostName, gUuid, gStatus):
--
To view, visit http://gerrit.ovirt.org/7951
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I364930675dcc433dfc0432a343fc66e91721e801
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Timothy Asir <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
