Change in vdsm[master]: storage: Add basic BlockVolumeArtifacts
nsoffer at redhat.com
nsoffer at redhat.com
Fri Apr 29 23:23:27 UTC 2016
Nir Soffer has posted comments on this change.
Change subject: storage: Add basic BlockVolumeArtifacts
......................................................................
Patch Set 18: Code-Review-1
(8 comments)
https://gerrit.ovirt.org/#/c/55987/18/tests/storage_volume_artifacts_test.py
File tests/storage_volume_artifacts_test.py:
Line 331: # If we fail to create the LV then storage is clean and we can retry
Line 332: with self.fake_env() as env:
Line 333: artifacts = env.sd_manifest.get_volume_artifacts(
Line 334: self.img_id, self.vol_id)
Line 335: artifacts._create_lv_artifact = self.failure
We don't want to know about private methods in the tests. We should mock our fake lvm.createLV instead.
Line 336: self.assertRaises(ExpectedFailure, artifacts.create,
Line 337: *BASE_RAW_PARAMS)
Line 338: self.validate_invisibility(env, artifacts, is_garbage=False)
Line 339:
Line 341: artifacts = env.sd_manifest.get_volume_artifacts(
Line 342: self.img_id, self.vol_id)
Line 343: artifacts.create(*BASE_RAW_PARAMS)
Line 344:
Line 345: def test_create_fail_acquiring_meta_slot(self):
We need to mock the domain method for acquiring metadata slot.
Line 346: # If we fail to acquire the meta_slot we have just a garbage LV
Line 347: with self.fake_env() as env:
Line 348: artifacts = env.sd_manifest.get_volume_artifacts(
Line 349: self.img_id, self.vol_id)
Line 352: *BASE_RAW_PARAMS)
Line 353: self.validate_invisibility(env, artifacts, is_garbage=True)
Line 354: self.validate_domain_has_garbage(env.sd_manifest)
Line 355:
Line 356: def test_create_fail_setting_meta_slot(self):
Name too similar to previous test - maybe test_create_fail_to_set_mdtag
Line 357: # If we fail to set the meta_slot in the LV tags that slot remains
Line 358: # available for allocation (even without garbage collection)
Line 359:
Line 360: with self.fake_env() as env:
Line 354: self.validate_domain_has_garbage(env.sd_manifest)
Line 355:
Line 356: def test_create_fail_setting_meta_slot(self):
Line 357: # If we fail to set the meta_slot in the LV tags that slot remains
Line 358: # available for allocation (even without garbage collection)
Nice!
Line 359:
Line 360: with self.fake_env() as env:
Line 361: def patch_and_fail(f):
Line 362: @wraps(f)
Line 364: with MonkeyPatchScope([
Line 365: [env.lvm, 'changeLVTags', self.failure]
Line 366: ]):
Line 367: f(*args, **kwargs)
Line 368: return wrapper
Too complicated.
Line 369:
Line 370: slot_before = self.get_next_free_slot(env)
Line 371: artifacts = env.sd_manifest.get_volume_artifacts(
Line 372: self.img_id, self.vol_id)
Line 370: slot_before = self.get_next_free_slot(env)
Line 371: artifacts = env.sd_manifest.get_volume_artifacts(
Line 372: self.img_id, self.vol_id)
Line 373: artifacts._acquire_metadata_slot = \
Line 374: patch_and_fail(artifacts._acquire_metadata_slot)
We can do:
with MonkeyPatchScope([[env.lvm, 'changeLVTags', self.failure]]):
try to create...
assert...
try to create again
assert
Line 375: self.assertRaises(ExpectedFailure, artifacts.create,
Line 376: *BASE_RAW_PARAMS)
Line 377: self.assertEqual(slot_before, self.get_next_free_slot(env))
Line 378: self.validate_invisibility(env, artifacts, is_garbage=True)
Line 384: with self.fake_env() as env:
Line 385: slot_before = self.get_next_free_slot(env)
Line 386: artifacts = env.sd_manifest.get_volume_artifacts(
Line 387: self.img_id, self.vol_id)
Line 388: artifacts._create_metadata = self.failure
same
Line 389: self.assertRaises(ExpectedFailure, artifacts.create,
Line 390: *BASE_RAW_PARAMS)
Line 391: self.validate_invisibility(env, artifacts, is_garbage=True)
Line 392: self.validate_domain_has_garbage(env.sd_manifest)
Line 396: # We leave behind a garbage LV and metadata area
Line 397: with self.fake_env() as env:
Line 398: artifacts = env.sd_manifest.get_volume_artifacts(
Line 399: self.img_id, self.vol_id)
Line 400: artifacts._create_lease = self.failure
Same - you should mock the domain clusterlock object.
Line 401: self.assertRaises(ExpectedFailure, artifacts.create,
Line 402: *BASE_RAW_PARAMS)
Line 403: self.validate_invisibility(env, artifacts, is_garbage=True)
Line 404: self.validate_domain_has_garbage(env.sd_manifest)
--
To view, visit https://gerrit.ovirt.org/55987
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4120c557fc89d82cc6bc854a9fdc8935e53bc93d
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list