Change in vdsm[master]: gluster: Handle missing glusterfs-cli package

nsoffer at redhat.com nsoffer at redhat.com
Tue Dec 15 20:18:30 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: gluster: Handle missing glusterfs-cli package
......................................................................


Patch Set 9:

(9 comments)

https://gerrit.ovirt.org/#/c/50363/9/tests/storageServerTests.py
File tests/storageServerTests.py:

Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: 
Line 21: from monkeypatch import MonkeyPatch
Line 22: from gluster import cli as gluster_cli
Use:

    import gluster.cli
Line 23: from testlib import permutations, expandPermutations
Line 24: from testlib import VdsmTestCase
Line 25: from storage.storageServer import GlusterFSConnection
Line 26: from storage.storageServer import IscsiConnection


Line 128:     def fake_gluster_exists():
Line 129:         return True
Line 130: 
Line 131:     def fake_gluster_not_exists():
Line 132:         return False
Not needed, see bellow.
Line 133: 
Line 134:     def test_mountpoint(self):
Line 135:         mount_con = GlusterFSConnection("server:/volume", mountClass=FakeMount)
Line 136:         self.assertEquals(mount_con._mount.fs_spec,


Line 137:                           "server:/volume")
Line 138:         self.assertEquals(mount_con._mount.fs_file,
Line 139:                           "/tmp/glusterSD/server:_volume")
Line 140: 
Line 141:     @MonkeyPatch(gluster_cli, 'gluster_exists', fake_gluster_exists)
Use:

    @MonkeyPatch(gluster_cli, 'gluster_exists', lambda: True)

Same fro all tests when gluster should "exists"
Line 142:     @MonkeyPatch(storageServer, 'supervdsm', FakeSupervdsm())
Line 143:     def test_gluster_replica3_mount_options(self):
Line 144:         def glusterVolumeInfo(volname=None, volfileServer=None):
Line 145:             self.assertEqual(volname, "music")


Line 249:         gluster = GlusterFSConnection(spec="192.168.122.1:/music")
Line 250:         gluster.validate()
Line 251: 
Line 252:     @MonkeyPatch(gluster_cli, 'gluster_exists',
Line 253:                  fake_gluster_not_exists)
Use lambda: False
Line 254:     def test_glusterfs_cli_not_exists(self):
Line 255:         gluster = GlusterFSConnection(spec="192.168.122.1:/music")
Line 256:         self.assertEquals(gluster.volinfo, None)


https://gerrit.ovirt.org/#/c/50363/9/vdsm/gluster/cli.py
File vdsm/gluster/cli.py:

Line 1581:     except ge.GlusterCmdFailedException as e:
Line 1582:         raise ge.GlusterGeoRepSessionDeleteFailedException(rc=e.rc, err=e.err)
Line 1583: 
Line 1584: 
Line 1585: def gluster_exists():
Since this is the gluster.cli module, I wonder if we need to repeat the "gluster" in the name.  How about:

    def exists():
        ...

User code looks like:

    if  gluster.cli.exists():
        # get backup servers...
Line 1586:     try:
Line 1587:         return os.path.exists(_glusterCommandPath.cmd)
Line 1588:     except OSError as e:
Line 1589:         if e.errno != os.errno.ENOENT:


https://gerrit.ovirt.org/#/c/50363/9/vdsm/storage/storageServer.py
File vdsm/storage/storageServer.py:

Line 313
Line 314
Line 315
Line 316
Line 317
Check if gluster.cli.exists here, save the state, and log a warning.


Line 321
Line 322
Line 323
Line 324
Line 325
Skip if not self._have_gluster_cli


Line 42: import fileSD
Line 43: import iscsi
Line 44: from sync import asyncmethod, AsyncCallStub
Line 45: from mount import MountError
Line 46: from gluster import cli as gluster_cli
Use import gluster.cli
Line 47: import storage_exception as se
Line 48: 
Line 49: 
Line 50: class AliasAlreadyRegisteredError(RuntimeError):


Line 336:             self._volinfo = self._get_gluster_volinfo()
Line 337:         return self._volinfo
Line 338: 
Line 339:     def validate(self):
Line 340:         if not gluster_cli.gluster_exists():
Skip of not self._have_gluster_cli
Line 341:             return
Line 342: 
Line 343:         replicaCount = self.volinfo['replicaCount']
Line 344:         if replicaCount not in self.ALLOWED_REPLICA_COUNTS:


-- 
To view, visit https://gerrit.ovirt.org/50363
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9c33aa33e4ffe6a382d40e1bc63f6735efcfcd1f
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sahina Bose <sabose at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list