Adam Litke has uploaded a new change for review.
Change subject: tests: Add a live merge functional test ......................................................................
tests: Add a live merge functional test
Test whether we can successfully merge the active layer. Uses lots of the functional test infrastructure! Only runs if vdsm says it can support live merge.
Change-Id: Idd5a2f7eedaef9e90981256de66fc3ed21658e89 Signed-off-by: Adam Litke alitke@redhat.com --- M tests/functional/utils.py M tests/functional/virtTests.py 2 files changed, 185 insertions(+), 5 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/29824/1
diff --git a/tests/functional/utils.py b/tests/functional/utils.py index 494be98..e3cdba6 100644 --- a/tests/functional/utils.py +++ b/tests/functional/utils.py @@ -228,3 +228,41 @@ def updateVmPolicy(self, vmId, vcpuLimit): result = self.vdscli.updateVmPolicy([vmId, vcpuLimit]) return result['status']['code'], result['status']['message'] + + def getTaskStatus(self, taskId): + result = self.vdscli.getTaskStatus(taskId) + return result['status']['code'], result['status']['message'],\ + result['taskStatus'] + + def getVolumeInfo(self, sdId, spId, imgId, volId): + result = self.vdscli.getVolumeInfo(sdId, spId, imgId, volId) + return result['status']['code'], result['status']['message'],\ + result['info'] + + def createVolume(self, sdId, spId, imgId, size, volFormat, preallocate, + diskType, volId, desc, baseImgId, baseVolId): + result = self.vdscli.createVolume(sdId, spId, imgId, size, volFormat, + preallocate, diskType, volId, desc, + baseImgId, baseVolId) + return result['status']['code'], result['status']['message'],\ + result['uuid'] + + def deleteVolume(self, sdId, spId, imgId, volIds, postZero=False, + force=False): + result = self.vdscli.deleteVolume(sdId, spId, imgId, volIds, postZero, + force) + return result['status']['code'], result['status']['message'],\ + result['uuid'] + + def snapshot(self, vmId, snapDrives, snapMemVolHandle=''): + result = self.vdscli.snapshot(vmId, snapDrives, snapMemVolHandle) + return result['status']['code'], result['status']['message'] + + def merge(self, vmId, drive, base, top, bandwidth, jobId): + result = self.vdscli.merge(vmId, drive, base, top, bandwidth, jobId) + return result['status']['code'], result['status']['message'] + + def list(self, fullStatus=False, vmList=()): + result = self.vdscli.list(fullStatus, vmList) + return result['status']['code'], result['status']['message'], \ + result['vmList'] diff --git a/tests/functional/virtTests.py b/tests/functional/virtTests.py index 94ce240..b811b92 100644 --- a/tests/functional/virtTests.py +++ b/tests/functional/virtTests.py @@ -22,6 +22,7 @@ import math import tempfile import logging +import uuid from stat import S_IROTH from functools import partial, wraps
@@ -32,7 +33,8 @@ from testrunner import temporaryPath
from vdsm.utils import CommandPath, RollbackContext -import storageTests as storage +import storageTests +import storage from storage.misc import execCmd
from utils import VdsProxy, SUCCESS @@ -109,6 +111,18 @@ return method(self, *args, **kwargs) else: raise SkipTest('KVM is not enabled') + return wrapped + + +def requireLiveMerge(method): + @wraps(method) + def wrapped(self, *args, **kwargs): + status, msg, result = self.vdsm.getVdsCapabilities() + self.assertEqual(status, SUCCESS, msg) + if result.get('liveMerge') == 'true': + return method(self, *args, **kwargs) + else: + raise SkipTest('Live Merge is not available') return wrapped
@@ -227,9 +241,9 @@ @requireKVM @permutations([['localfs'], ['iscsi'], ['nfs']]) def testVmWithStorage(self, backendType): - disk = storage.StorageTest() + disk = storageTests.StorageTest() disk.setUp() - conf = storage.storageLayouts[backendType] + conf = storageTests.storageLayouts[backendType] drives = disk.generateDriveConf(conf) customization = {'vmId': '88888888-eeee-ffff-aaaa-111111111111', 'vmName': 'testVmWithStorage' + backendType, @@ -247,8 +261,8 @@ def testVmWithDevice(self, *devices): customization = {'vmId': '77777777-ffff-3333-bbbb-222222222222', 'vmName': 'testVm', 'devices': [], 'display': 'vnc'} - storageLayout = storage.storageLayouts['localfs'] - diskSpecs = storage.StorageTest.generateDriveConf(storageLayout) + storageLayout = storageTests.storageLayouts['localfs'] + diskSpecs = storageTests.StorageTest.generateDriveConf(storageLayout) pciSpecs = {'bus': '0x00', 'domain': '0x0000', 'function': '0x0', 'type': 'pci'} ccidSpecs = {'slot': '0', 'controller': '0', 'type': 'ccid'} @@ -412,3 +426,131 @@ self.vdsm.updateVmPolicy(customization['vmId'], '50') self.assertEqual(status, SUCCESS, msg) + + +@expandPermutations +class LiveMergeTest(VirtTestBase): + def _waitTask(self, taskId): + def assertTaskOK(): + status, msg, result = self.vdsm.getTaskStatus(taskId) + self.assertEqual(status, SUCCESS, msg) + self.assertEquals(result['taskState'], 'finished') + + self.retryAssert(assertTaskOK, timeout=60) + + def _waitBlockJobs(self, vmId, jobIds): + def assertJobsGone(): + status, msg, result = self.vdsm.getVmStats(vmId) + self.assertEqual(status, SUCCESS, msg) + self.assertTrue('vmJobs' in result) + self.assertTrue(all([x not in result['vmJobs'].keys() + for x in jobIds])) + + self.retryAssert(assertJobsGone, timeout=60) + + def _snapshotVM(self, vmId, drives, rollback): + snapDrives = [] + for drive in drives: + sd = drive['domainID'] + sp = drive['poolID'] + img = drive['imageID'] + vol = drive['volumeID'] + newVol = str(uuid.uuid4()) + volFormat = storage.volume.COW_FORMAT + preallocate = storage.volume.SPARSE_VOL + desc = 'snapshot for %s' % vol + + # Create volume and wait + status, msg, result = self.vdsm.getVolumeInfo(sd, sp, img, vol) + self.assertEqual(status, SUCCESS, msg) + size = result['capacity'] + diskType = result['disktype'] + + status, msg, taskId = self.vdsm.createVolume(sd, sp, img, size, + volFormat, + preallocate, diskType, + newVol, desc, img, + vol) + self.assertEqual(status, SUCCESS, msg) + self._waitTask(taskId) + undo = lambda sd=sd, sp=sp, img=img, vol=newVol: \ + self._waitTask(self.vdsm.deleteVolume(sd, sp, img, vol)[2]) + rollback.prependDefer(undo) + + snapDrives.append({'domainID': sd, + 'imageID': img, + 'volumeID': newVol, + 'baseVolumeID': vol}) + + # Create snapshot + status, msg = self.vdsm.snapshot(vmId, snapDrives) + self.assertEqual(status, SUCCESS, msg) + return snapDrives + + def _orderChain(self, vmId, dev, chain): + parentMap = {} + for vol in chain: + status, msg, info = self.vdsm.getVolumeInfo(dev['domainID'], + dev['poolID'], + dev['imageID'], vol) + self.assertEqual(status, SUCCESS, msg) + parent = info['parent'] + parentMap[vol] = parent + + vol = dev['volumeID'] + chain = list() + while True: + chain.insert(0, vol) + vol = parentMap.get(vol, '00000000-0000-0000-0000-000000000000') + if vol == '00000000-0000-0000-0000-000000000000': + break + return chain + + def _getVolumeChains(self, vmId): + chains = {} + status, msg, result = self.vdsm.list(True, (vmId,)) + self.assertEqual(status, SUCCESS, msg) + vmDef = result[0] + for dev in vmDef['devices']: + if dev['device'] != 'disk': + continue + chains[dev['imageID']] = self._orderChain(vmId, dev, + [x['volumeID'] for x in + dev['volumeChain']]) + return chains + + @requireKVM + @requireLiveMerge + def testCapable(self): + pass + + @permutations([['localfs']]) + def testMergeActiveLayer(self, backendType): + disk = storageTests.StorageTest() + disk.setUp() + conf = storageTests.storageLayouts[backendType] + drives = disk.generateDriveConf(conf) + vmId = '12121212-abab-baba-abab-222222222222' + customization = {'vmId': vmId, + 'vmName': 'testMergeActive' + backendType, + 'drives': drives, + 'display': 'vnc'} + + with RollbackContext() as rollback: + disk.createVdsmStorageLayout(conf, 3, rollback) + with RunningVm(self.vdsm, customization) as vm: + self._waitForStartup(vm, VM_MINIMAL_UPTIME) + snapDrives = self._snapshotVM(vmId, drives, rollback) + chains = {} + jobIds = [] + for drive in snapDrives: + base = drive['baseVolumeID'] + top = drive['volumeID'] + jobId = str(uuid.uuid4()) + chains[drive['imageID']] = [base, top] + status, msg = self.vdsm.merge(vmId, drive, base, top, 0, + jobId) + jobIds.append(jobId) + self._waitBlockJobs(vmId, jobIds) + actual = self._getVolumeChains(vmId) + self.assertEquals(chains, actual)
oVirt Jenkins CI Server has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10117/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10902/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1208/ : There was an infra issue, please contact infra@ovirt.org
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11059/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/161... : There was an infra issue, please contact infra@ovirt.org
Yoav Kleinberger has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1: Code-Review-1
(3 comments)
http://gerrit.ovirt.org/#/c/29824/1/tests/functional/utils.py File tests/functional/utils.py:
Line 237: def getVolumeInfo(self, sdId, spId, imgId, volId): Line 238: result = self.vdscli.getVolumeInfo(sdId, spId, imgId, volId) Line 239: return result['status']['code'], result['status']['message'],\ Line 240: result['info'] Line 241: quite a bit of code duplication. Usually I don't like metaprogramming, but perhaps introduce something like this:
class VDSMMethod: _vdsm = VDSCli() def __init__(self, returnKey): self._returnKey = returnKey
def __call__(self, func): this = self def _decorated(self, *args, **kwargs): methodName = func.__name__ method = getattr(this._vdsm, methodName) result = method(*args, **kwargs) return result['status']['code'], result['status']['message'], result[this._returnKey]
return _decorated
Now use this in VDSProxy
class VDSProxy(object): @VDSMMethod(returnKey='uuid') def createVolume(self): pass
@VDSMMethod(returnKey='uuid') def deleteVolume(self): pass
@VDSMMethod(returnKey='vmList') def list(self): pass Line 242: def createVolume(self, sdId, spId, imgId, size, volFormat, preallocate, Line 243: diskType, volId, desc, baseImgId, baseVolId): Line 244: result = self.vdscli.createVolume(sdId, spId, imgId, size, volFormat, Line 245: preallocate, diskType, volId, desc,
http://gerrit.ovirt.org/#/c/29824/1/tests/functional/virtTests.py File tests/functional/virtTests.py:
Line 240: Line 241: @requireKVM Line 242: @permutations([['localfs'], ['iscsi'], ['nfs']]) Line 243: def testVmWithStorage(self, backendType): Line 244: disk = storageTests.StorageTest() why is a StorageTest instance called a "disk"?
looks to me quite convoluted, using a test-case from a different test as a library for another test. Line 245: disk.setUp() Line 246: conf = storageTests.storageLayouts[backendType] Line 247: drives = disk.generateDriveConf(conf) Line 248: customization = {'vmId': '88888888-eeee-ffff-aaaa-111111111111',
Line 552: jobId) Line 553: jobIds.append(jobId) Line 554: self._waitBlockJobs(vmId, jobIds) Line 555: actual = self._getVolumeChains(vmId) Line 556: self.assertEquals(chains, actual) Looks like you assert that VDSM reports the result you expect. If this is true, this test does not actually check that anything happened - only that VDSM says it did.
In principle, I could shutdown VDSM, and install a fake server that tells this test what it wants to hear - and the test will pass, with no VDSM even running. Am I wrong?
Nir Soffer has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1:
(1 comment)
Partial review
http://gerrit.ovirt.org/#/c/29824/1/tests/functional/utils.py File tests/functional/utils.py:
Line 237: def getVolumeInfo(self, sdId, spId, imgId, volId): Line 238: result = self.vdscli.getVolumeInfo(sdId, spId, imgId, volId) Line 239: return result['status']['code'], result['status']['message'],\ Line 240: result['info'] Line 241:
quite a bit of code duplication. Usually I don't like metaprogramming, but
There is indeed code duplication here, but Adam is trying to add a test, not fix the world.
The change you suggest or similar change should be fixed in a separate patch fixing all the utilities in this file, not only the few new calls added by this patch.
Please don't block important test just because it can be better. Line 242: def createVolume(self, sdId, spId, imgId, size, volFormat, preallocate, Line 243: diskType, volId, desc, baseImgId, baseVolId): Line 244: result = self.vdscli.createVolume(sdId, spId, imgId, size, volFormat, Line 245: preallocate, diskType, volId, desc,
Nir Soffer has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1:
(2 comments)
http://gerrit.ovirt.org/#/c/29824/1/tests/functional/virtTests.py File tests/functional/virtTests.py:
Line 240: Line 241: @requireKVM Line 242: @permutations([['localfs'], ['iscsi'], ['nfs']]) Line 243: def testVmWithStorage(self, backendType): Line 244: disk = storageTests.StorageTest()
why is a StorageTest instance called a "disk"?
Yea, this is a bit extreme. The common part should move to utils instead of reusing a test. Line 245: disk.setUp() Line 246: conf = storageTests.storageLayouts[backendType] Line 247: drives = disk.generateDriveConf(conf) Line 248: customization = {'vmId': '88888888-eeee-ffff-aaaa-111111111111',
Line 552: jobId) Line 553: jobIds.append(jobId) Line 554: self._waitBlockJobs(vmId, jobIds) Line 555: actual = self._getVolumeChains(vmId) Line 556: self.assertEquals(chains, actual)
Looks like you assert that VDSM reports the result you expect. If this is t
The chance that you can sneak into the machines running this test and install your fake vdsm is quite small :-)
Yoav Kleinberger has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1:
(2 comments)
http://gerrit.ovirt.org/#/c/29824/1/tests/functional/utils.py File tests/functional/utils.py:
Line 238: result = self.vdscli.getVolumeInfo(sdId, spId, imgId, volId) Line 239: return result['status']['code'], result['status']['message'],\ Line 240: result['info'] Line 241: Line 242: def createVolume(self, sdId, spId, imgId, size, volFormat, preallocate, OK Nir, I agree. Line 243: diskType, volId, desc, baseImgId, baseVolId): Line 244: result = self.vdscli.createVolume(sdId, spId, imgId, size, volFormat, Line 245: preallocate, diskType, volId, desc, Line 246: baseImgId, baseVolId)
http://gerrit.ovirt.org/#/c/29824/1/tests/functional/virtTests.py File tests/functional/virtTests.py:
Line 552: jobId) Line 553: jobIds.append(jobId) Line 554: self._waitBlockJobs(vmId, jobIds) Line 555: actual = self._getVolumeChains(vmId) Line 556: self.assertEquals(chains, actual)
The chance that you can sneak into the machines running this test and inst
No one is going to mock VDSM. My point is that this doesn't really check what it should. On the other hand, all the other functional tests are the same. Waiting to hear what Adam has to say.
Adam Litke has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1:
(3 comments)
http://gerrit.ovirt.org/#/c/29824/1/tests/functional/utils.py File tests/functional/utils.py:
Line 238: result = self.vdscli.getVolumeInfo(sdId, spId, imgId, volId) Line 239: return result['status']['code'], result['status']['message'],\ Line 240: result['info'] Line 241: Line 242: def createVolume(self, sdId, spId, imgId, size, volFormat, preallocate,
OK Nir, I agree.
Yeah, personally I do not like this proxy at all. I don't think it adds any value at all. But I am using it because someone did like it this way and apparently some reviewers did at the time too. I'd like to see it refactored and simplified once the cli switches to jsonRPC where we can also start verifying that the data returned actually conforms to the documented API schema.
Another battle for another day. Line 243: diskType, volId, desc, baseImgId, baseVolId): Line 244: result = self.vdscli.createVolume(sdId, spId, imgId, size, volFormat, Line 245: preallocate, diskType, volId, desc, Line 246: baseImgId, baseVolId)
http://gerrit.ovirt.org/#/c/29824/1/tests/functional/virtTests.py File tests/functional/virtTests.py:
Line 240: Line 241: @requireKVM Line 242: @permutations([['localfs'], ['iscsi'], ['nfs']]) Line 243: def testVmWithStorage(self, backendType): Line 244: disk = storageTests.StorageTest()
Yea, this is a bit extreme. The common part should move to utils instead of
Agreed. I just built on what other tests in this file are currently doing. In a future revision I will try to factor it out a bit better. Line 245: disk.setUp() Line 246: conf = storageTests.storageLayouts[backendType] Line 247: drives = disk.generateDriveConf(conf) Line 248: customization = {'vmId': '88888888-eeee-ffff-aaaa-111111111111',
Line 552: jobId) Line 553: jobIds.append(jobId) Line 554: self._waitBlockJobs(vmId, jobIds) Line 555: actual = self._getVolumeChains(vmId) Line 556: self.assertEquals(chains, actual)
No one is going to mock VDSM. My point is that this doesn't really check wh
I agree that we should actually confirm that the operations have done what we expect by using qemu-io. I just didn't get that far during the hackathon when I created this test.
Yoav Kleinberger has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1: -Code-Review
Jenkins CI RO has abandoned this change.
Change subject: tests: Add a live merge functional test ......................................................................
Abandoned
Abandoned due to no activity - please restore if still relevant
Jenkins CI RO has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1:
Abandoned due to no activity - please restore if still relevant
automation@ovirt.org has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found
Nir Soffer has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1:
Please restore, we need such tests
Jenkins CI RO has abandoned this change.
Change subject: tests: Add a live merge functional test ......................................................................
Abandoned
Abandoned due to no activity - please restore if still relevant
gerrit-hooks has posted comments on this change.
Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found
vdsm-patches@lists.fedorahosted.org