Adam Litke has uploaded a new change for review.
Change subject: tests: Add initial FakeLVM tests and fix errors
......................................................................
tests: Add initial FakeLVM tests and fix errors
The FakeLVM class inside storagefakelib.py is becoming increasingly
complex. In order to ensure that FakeLVM is providing correct LVM
emulation we need to test its output against what a real LVM environment
would present.
This first patch introduces the new test module and adds some tests for
createVG and getVG. We test that the VG properties and attributes
returned match a real LVM installation. After writing these tests I
found and fixed several errors in the existing FakeLVM implementation.
Change-Id: I1650b98f25df1a22ba2a0920b093e78fc2fdee88
Signed-off-by: Adam Litke <alitke(a)redhat.com>
---
M tests/Makefile.am
M tests/storagefakelib.py
A tests/storagefakelibTests.py
3 files changed, 161 insertions(+), 16 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/57/47857/1
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1168862..e8b9d6e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -102,6 +102,7 @@
stompAdapterTests.py \
stompAsyncClientTests.py \
stompTests.py \
+ storagefakelibTests.py \
storageMailboxTests.py \
storageMonitorTests.py \
storageServerTests.py \
diff --git a/tests/storagefakelib.py b/tests/storagefakelib.py
index 11954d5..a60aaf5 100644
--- a/tests/storagefakelib.py
+++ b/tests/storagefakelib.py
@@ -26,19 +26,29 @@
class FakeLVM(object):
- _PV_SIZE = 10 << 30 # We pretend all PVs contribute 10G of space
-
+ _PV_SIZE = 10 << 30 # We pretend all PVs are 10G in size
+ _PV_PE_SIZE = 128 << 20 # Found via inspection of real environment
+ _PV_MDA_COUNT = 2 # The number of PEs used for metadata areas
+ _PV_UNUSABLE = (_PV_PE_SIZE * # 2 PE for metadata + 1 PE to hold a header
+ (1 + _PV_MDA_COUNT))
def __init__(self, root):
self.root = root
os.mkdir(os.path.join(self.root, 'dev'))
+ self.pvmd = {}
self.vgmd = {}
self.lvmd = {}
def createVG(self, vgName, devices, initialTag, metadataSize,
- extentsize=134217728, force=False):
- devices = [_fqpvname(dev) for dev in devices]
- size = self._PV_SIZE * len(devices)
- extent_count = size / extentsize
+ extentsize=128, force=False):
+ # Convert params from MB to bytes to match other fields
+ metadataSize <<= 20
+ extentsize <<= 20
+
+ for dev in devices:
+ self._create_pv(dev, vgName, self._PV_SIZE)
+ pv_name = tuple([_fqpvname(pdev) for pdev in _normalizeargs(devices)])
+ extent_count = self._calc_vg_pe_count(vgName)
+ size = extent_count * self._PV_PE_SIZE
vg_attr = dict(permission='w',
resizeable='z',
@@ -49,17 +59,17 @@
vg_md = dict(uuid=fake_lvm_uuid(),
name=vgName,
attr=vg_attr,
- size=size,
- free=size,
- extent_size=extentsize,
- extent_count=extent_count,
- free_count=extent_count,
- tags=[initialTag],
- vg_mda_size=metadataSize,
- vg_mda_free=metadataSize,
+ size=str(size),
+ free=str(size),
+ extent_size=str(extentsize),
+ extent_count=str(extent_count),
+ free_count=str(extent_count),
+ tags=(initialTag,),
+ vg_mda_size=str(metadataSize),
+ vg_mda_free=None,
lv_count='0',
- pv_count=len(devices),
- pv_name=tuple(devices),
+ pv_count=str(len(devices)),
+ pv_name=pv_name,
writeable=True,
partial='OK')
self.vgmd[vgName] = vg_md
@@ -123,7 +133,31 @@
with open(volpath, "w") as f:
f.truncate(int(self.lvmd[vg_name][lv_name]['size']))
+ def _create_pv(self, pv_name, vg_name, size):
+ # pe_start is difficult to calculate correctly but since it's not
+ # currently needed by users of FakeLVM, set it to None.
+ pe_start = None
+ pe_count = (size - self._PV_UNUSABLE) / self._PV_PE_SIZE
+ pv_md = dict(uuid=fake_lvm_uuid(),
+ name='/dev/mapper/%s' % pv_name,
+ guid=pv_name,
+ size=str(pe_count * self._PV_PE_SIZE),
+ vg_name=vg_name,
+ vg_uuid=None, # This is calculated dynamically
+ pe_start=pe_start,
+ pe_count=str(pe_count),
+ pe_alloc_count='0',
+ mda_count=str(self._PV_MDA_COUNT),
+ dev_size=str(self._PV_SIZE))
+ self.pvmd[pv_name] = pv_md
+
+ def _calc_vg_pe_count(self, vg_name):
+ return sum(int(pv["pe_count"]) for pv in self.pvmd.values()
+ if pv["vg_name"] == vg_name)
+
+
_fqpvname = real_lvm._fqpvname
+_normalizeargs = real_lvm._normalizeargs
def fake_lvm_uuid():
diff --git a/tests/storagefakelibTests.py b/tests/storagefakelibTests.py
new file mode 100644
index 0000000..8c73a54
--- /dev/null
+++ b/tests/storagefakelibTests.py
@@ -0,0 +1,110 @@
+# Copyright 2015 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+from contextlib import contextmanager
+
+from testlib import VdsmTestCase, namedTemporaryDir
+from testlib import permutations, expandPermutations
+from storagefakelib import FakeLVM
+
+from storage import blockSD
+
+
+@expandPermutations
+class FakeLVMSimpleVGTests(VdsmTestCase):
+ VG_NAME = '1ffead52-7363-4968-a8c7-3bc34504d452'
+ DEVICES = ['360014054d75cb132d474c0eae9825766']
+
+ @contextmanager
+ def base_config(self):
+ """
+ Create a simple volume group from a single 10G LUN.
+
+ lvm.createVG('1ffead52-7363-4968-a8c7-3bc34504d452',
+ ['360014054d75cb132d474c0eae9825766'],
+ blockSD.STORAGE_UNREADY_DOMAIN_TAG,
+ blockSD.VG_METADATASIZE)
+
+ print lvm.getVG('1ffead52-7363-4968-a8c7-3bc34504d452')
+ VG(uuid='15hlPF-V3eG-F9Cp-SGtu-4Mq0-28Do-HC806y',
+ name='1ffead52-7363-4968-a8c7-3bc34504d452',
+ attr=VG_ATTR(permission='w', resizeable='z', exported='-',
+ partial='-', allocation='n', clustered='-'),
+ size='10334765056', free='10334765056', extent_size='134217728',
+ extent_count='77', free_count='77',
+ tags=('RHAT_storage_domain_UNREADY',), vg_mda_size='134217728',
+ vg_mda_free='67107328', lv_count='0', pv_count='1',
+ pv_name=('/dev/mapper/360014054d75cb132d474c0eae9825766',),
+ writeable=True, partial='OK')
+
+ print lvm.getPV('360014054d75cb132d474c0eae9825766')
+ PV(uuid='fIRjbD-usOA-tYgW-b2Uz-oUly-AJ49-bMMjYe',
+ name='/dev/mapper/360014054d75cb132d474c0eae9825766',
+ size='10334765056', vg_name='1ffead52-7363-4968-a8c7-3bc34504d452',
+ vg_uuid='15hlPF-V3eG-F9Cp-SGtu-4Mq0-28Do-HC806y',
+ pe_start='138412032', pe_count='77', pe_alloc_count='0',
+ mda_count='2', dev_size='10737418240',
+ guid='360014054d75cb132d474c0eae9825766')
+ """
+ with namedTemporaryDir() as tmpdir:
+ lvm = FakeLVM(tmpdir)
+ lvm.createVG(self.VG_NAME, self.DEVICES,
+ blockSD.STORAGE_UNREADY_DOMAIN_TAG,
+ blockSD.VG_METADATASIZE)
+ yield lvm
+
+ @permutations([
+ ['name', VG_NAME],
+ ['size', '10334765056'],
+ ['free', '10334765056'],
+ ['extent_size', '134217728'],
+ ['extent_count', '77'],
+ ['free_count', '77'],
+ ['tags', (blockSD.STORAGE_UNREADY_DOMAIN_TAG,)],
+ ['vg_mda_size', '134217728'],
+ ['lv_count', '0'],
+ ['pv_count', '1'],
+ ['writeable', True],
+ ['partial', 'OK'],
+ ['pv_name', tuple(['/dev/mapper/%s' % d for d in DEVICES])],
+ ])
+ def test_vg_properties(self, var, val):
+ with self.base_config() as lvm:
+ vg = lvm.getVG(self.VG_NAME)
+ self.assertEqual(val, getattr(vg, var))
+
+ @permutations([
+ ['permission', 'w'],
+ ['resizeable', 'z'],
+ ['exported', '-'],
+ ['partial', '-'],
+ ['allocation', 'n'],
+ ['clustered', '-'],
+ ])
+ def test_vg_attributes(self, var, val):
+ with self.base_config() as lvm:
+ vg = lvm.getVG(self.VG_NAME)
+ self.assertEqual(val, getattr(vg.attr, var))
+
+ def test_vg_mda_free(self):
+ # It is too complex to emulate vg_mda_free and at this point we do not
+ # rely on this value. For this reason, FakeLVM sets it to None.
+ with self.base_config() as lvm:
+ vg = lvm.getVG(self.VG_NAME)
+ self.assertEqual(None, vg.vg_mda_free)
--
To view, visit https://gerrit.ovirt.org/47857
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1650b98f25df1a22ba2a0920b093e78fc2fdee88
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Francesco Romani has uploaded a new change for review.
Change subject: periodic: extract required_on staticmethod
......................................................................
periodic: extract required_on staticmethod
It possible, that certains periodic operations should not be done
on some VMs, perhaps temporarily.
To check this, Operations exported a "required" attribute.
Problem is, this attribute was instance attribute, forcing the
periodic code to do a clumsy dance like:
- assume every operation is required on each vm
- create per-vm Operation instances
- check if a give operation should indeed run on a given vm
- if the operation should not run, discard it.
This patch eliminates this clumsiness translating "required"
attribute into a "required_on" staticmethod.
Now the periodic code can filter out unneeded Operation instances
in a much simpler way.
Change-Id: I2ef0750948c905a87750b99adf702c4c08fc925d
Bug-Url: https://bugzilla.redhat.com/1250839
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M vdsm/virt/periodic.py
1 file changed, 29 insertions(+), 35 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/33/46833/1
diff --git a/vdsm/virt/periodic.py b/vdsm/virt/periodic.py
index 68f8888..050c789 100644
--- a/vdsm/virt/periodic.py
+++ b/vdsm/virt/periodic.py
@@ -64,7 +64,8 @@
def per_vm_operation(func, period):
disp = VmDispatcher(
- cif.getVMs, _executor, func, _timeout_from(period))
+ cif.getVMs, _executor, func, _timeout_from(period),
+ func.required_on)
return Operation(disp, period, scheduler)
_operations = [
@@ -204,7 +205,8 @@
_log = logging.getLogger("virt.periodic.VmDispatcher")
- def __init__(self, get_vms, executor, create, timeout):
+ def __init__(self, get_vms, executor, create, timeout,
+ required_on=lambda vm_obj: True):
"""
get_vms: callable which will return a dict which maps
vm_ids to vm_instances
@@ -216,12 +218,15 @@
self._executor = executor
self._create = create
self._timeout = timeout
+ self._required_on = required_on
self._tracker = VmTracker()
def __call__(self):
# python 3 future compatibility: we want to materialize the sequence
vm_objs = set(self._get_vms().values())
- candidates = set(vm.id for vm in vm_objs)
+ candidates = set(vm_obj.id
+ for vm_obj in vm_objs
+ if self._required_on(vm_obj))
available = set(self._tracker.reserve_available(candidates))
skipped = set(candidates - available)
@@ -233,27 +238,11 @@
# still OK if occasional misdetection occours, but we
# definitely want to avoid known-bad situation and to
# needlessly overload libvirt.
- if vm_obj.id not in available:
- skipped.append(vm_obj.id)
- continue
-
+ op = self._create(self._tracker, vm_obj)
try:
- op = self._create(self._tracker, vm_obj)
-
- if not op.required:
- self._tracker.release(vm_obj.id)
- continue
-
- except Exception:
- self._tracker.release(vm_obj.id)
- # we want to make sure to have VM UUID logged
- self._log.exception("while dispatching %s to VM '%s'",
- self._create, vm_obj.id)
- else:
- try:
- self._executor.dispatch(op, self._timeout)
- except executor.TooManyTasks:
- skipped.add(vm_obj.id)
+ self._executor.dispatch(op, self._timeout)
+ except executor.TooManyTasks:
+ skipped.add(vm_obj.id)
if skipped:
self._log.warning('could not run %s on %s',
@@ -265,9 +254,14 @@
class _RunnableOnVm(object):
+
def __init__(self, tracker, vm):
self._tracker = tracker
self._vm = vm
+
+ @staticmethod
+ def required_on(vm):
+ return True
def __call__(self):
try:
@@ -286,10 +280,10 @@
class UpdateVolumes(_RunnableOnVm):
- @property
- def required(self):
+ @staticmathod
+ def required_on(vm):
# Avoid queries from storage during recovery process
- return self._vm.isDisksStatsCollectionEnabled()
+ return vm.isDisksStatsCollectionEnabled()
def _execute(self):
for drive in self._vm.getDiskDevices():
@@ -301,9 +295,9 @@
class NumaInfoMonitor(_RunnableOnVm):
- @property
- def required(self):
- return self._vm.hasGuestNumaNode
+ @staticmethod
+ def required_on(vm):
+ return vm.hasGuestNumaNode
def _execute(self):
self._vm.updateNumaInfo()
@@ -311,14 +305,14 @@
class BlockjobMonitor(_RunnableOnVm):
- @property
- def required(self):
+ @staticmethod
+ def required_on(vm):
# For performance reasons, we must avoid as much
# as possible to create per-vm executor tasks, even
# though they will do nothing but a few check and exit
# early, as they do if a VM doesn't have Block Jobs to
# monitor (most often true).
- return self._vm.hasVmJobs
+ return vm.hasVmJobs
def _execute(self):
self._vm.updateVmJobs()
@@ -326,10 +320,10 @@
class DriveWatermarkMonitor(_RunnableOnVm):
- @property
- def required(self):
+ @staticmethod
+ def required_on(vm):
# Avoid queries from storage during recovery process
- return self._vm.isDisksStatsCollectionEnabled()
+ return vm.isDisksStatsCollectionEnabled()
def _execute(self):
self._vm.extendDrivesIfNeeded()
--
To view, visit https://gerrit.ovirt.org/46833
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2ef0750948c905a87750b99adf702c4c08fc925d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Francesco Romani has uploaded a new change for review.
Change subject: virt: cosmetic fixes
......................................................................
virt: cosmetic fixes
Harmless stupid fixes to make the code look prettier.
Change-Id: If57f99257ae1fc36541d7d563e71e26ac9db4c54
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M vdsm/virt/periodic.py
M vdsm/virt/sampling.py
2 files changed, 9 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/44797/1
diff --git a/vdsm/virt/periodic.py b/vdsm/virt/periodic.py
index b9c30c3..9781afe 100644
--- a/vdsm/virt/periodic.py
+++ b/vdsm/virt/periodic.py
@@ -84,14 +84,14 @@
# Job monitoring need QEMU monitor access.
per_vm_operation(
- BlockjobMonitor,
+ BlockJobMonitor,
config.getint('vars', 'vm_sample_jobs_interval')),
# libvirt sampling using bulk stats can block, but unresponsive
- # domains are handled inside VMBulkSampler for performance reasons;
+ # domains are handled inside BulkStatsMonitor for performance reasons;
# thus, does not need dispatching.
Operation(
- sampling.VMBulkSampler(
+ sampling.BulkStatsMonitor(
libvirtconnection.get(cif),
cif.getVMs,
sampling.stats_cache),
@@ -299,7 +299,7 @@
self._vm.updateNumaInfo()
-class BlockjobMonitor(_RunnableVmOperation):
+class BlockJobMonitor(_RunnableVmOperation):
@property
def required(self):
diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py
index f17a5b3..15d8b71 100644
--- a/vdsm/virt/sampling.py
+++ b/vdsm/virt/sampling.py
@@ -497,7 +497,7 @@
_TIMEOUT = 40.0
-class VMBulkSampler(object):
+class BulkStatsMonitor(object):
def __init__(self, conn, get_vms, stats_cache,
stats_flags=0, timeout=_TIMEOUT):
self._conn = conn
@@ -506,14 +506,16 @@
self._stats_flags = stats_flags
self._skip_doms = ExpiringCache(timeout)
self._sampling = threading.Semaphore() # used as glorified counter
- self._log = logging.getLogger("virt.sampling.VMBulkSampler")
+ self._log = logging.getLogger("virt.sampling.BulkStatsMonitor")
def __call__(self):
timestamp = self._stats_cache.clock()
# we are deep in the hot path. bool(ExpiringCache)
# *is* costly so we should avoid it if we can.
fast_path = (
- self._sampling.acquire(blocking=False) and not self._skip_doms)
+ self._sampling.acquire(blocking=False) and
+ not self._skip_doms
+ )
try:
if fast_path:
# This is expected to be the common case.
--
To view, visit https://gerrit.ovirt.org/44797
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If57f99257ae1fc36541d7d563e71e26ac9db4c54
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Francesco Romani has uploaded a new change for review.
Change subject: tests: add tests for sampling.SampleVMs
......................................................................
tests: add tests for sampling.SampleVMs
The SampleVMs class is responisble for handling bulk stats sampling,
and includes all the logic to deal with blocked domains.
However, this logic has the time among its variables, and it is
built on quite some assumptions, like running on an executor and so on.
All those factors make it difficult to test the bare logic without
some (minor) cheating and without some significant faking.
However, the nasty spots should be the ones more tested, not less
tested, so this patch adds the initial batch of unit tests for the
sampling logic.
Change-Id: Id66dbd420ca29d08ae4063dc83b858be34b8940f
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M tests/Makefile.am
A tests/bulkSamplingTests.py
2 files changed, 213 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/53/40053/1
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7fa1c09..4f984dd 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -26,6 +26,7 @@
alignmentScanTests.py \
blocksdTests.py \
bridgeTests.py \
+ bulkSamplingTests.py \
cPopenTests.py \
capsTests.py \
clientifTests.py \
diff --git a/tests/bulkSamplingTests.py b/tests/bulkSamplingTests.py
new file mode 100644
index 0000000..5c536ff
--- /dev/null
+++ b/tests/bulkSamplingTests.py
@@ -0,0 +1,212 @@
+#
+# Copyright 2015 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+import virt.sampling as sampling
+
+from testlib import VdsmTestCase as TestCaseBase
+
+
+# these tests are complex and fragile. In order to maximize
+# readability and robustness, intentionally includes tailor-made
+# minimal Fakes to support those tests.
+
+
+class FakeStatsCache(object):
+ def __init__(self):
+ self.data = []
+ self._clock = 0 # private only for API clash
+
+ def clock(self):
+ return self._clock
+
+ def set_clock(self, value):
+ self._clock = value
+
+ def put(self, bulk_stats, timestamp):
+ self.data.append((bulk_stats, timestamp))
+
+
+class FakeDomain(object):
+ def __init__(self, name):
+ self._name = name
+ self._dom = self # yep, this is an ugly hack
+
+ def UUIDString(self):
+ # yes this is cheating
+ return self._name
+
+
+class FakeVM(object):
+ def __init__(self, vmid):
+ self.id = vmid
+ self._dom = FakeDomain(vmid)
+ self.ready_for_commands = True
+
+ def isDomainReadyForCommands(self):
+ return self.ready_for_commands
+
+
+class CollectMode(object):
+ NONE = 0
+ SLOW = 1
+ FAST = 2
+
+
+class FakeConnection(object):
+ def __init__(self, vms):
+ self.vms = vms
+ # testable fields
+ self.executions = []
+
+ def getVMs(self):
+ return self.vms
+
+ def getAllDomainStats(self, flags=0):
+ self.executions.append(CollectMode.FAST)
+ return [(vm._dom, {'vmid': vm._dom.UUIDString()})
+ for vm in self.vms.values()]
+
+ def domainListGetStats(self, doms, flags=0):
+ self.executions.append(CollectMode.SLOW)
+ return [(dom, {'vmid': dom.UUIDString()})
+ for dom in doms
+ if dom.UUIDString() in self.vms]
+
+
+class SampleVMsTests(TestCaseBase):
+
+ def test_collect_fast_path_as_default(self):
+ cache = FakeStatsCache()
+ vms = {
+ '1': FakeVM('1'),
+ '2': FakeVM('2')
+ }
+ conn = FakeConnection(vms)
+
+ sampler = sampling.SampleVMs(conn, conn.getVMs, cache)
+ sampler()
+
+ self.assertEqual(conn.executions[0], CollectMode.FAST)
+ self.assertVmsInBulkStats(vms.keys(), cache.data[0][0])
+
+ def test_collect_slow_path_after_blocked(self):
+ cache = FakeStatsCache()
+ vms = {
+ '1': FakeVM('1'),
+ '2': FakeVM('2')
+ }
+ conn = FakeConnection(vms)
+
+ sampler = sampling.SampleVMs(conn, conn.getVMs, cache)
+ # this is cheating, but setting up proper blocking is costly
+ # and fragile.
+ sampler._sampling = True
+ sampler()
+
+ self.assertEqual(conn.executions[0], CollectMode.SLOW)
+ # SampleVMs doesn't care in the slow path if all registed
+ # VMs are actually responsive or not. Quite the opposite, it is
+ # good sign if all VMs are now responsive, it means we can
+ # restart using getAllDomainStats on the next cycle.
+ self.assertVmsInBulkStats(vms.keys(), cache.data[0][0])
+
+ def test_collect_unresponsive_vm(self):
+ cache = FakeStatsCache()
+ vms = {
+ '1': FakeVM('1'),
+ '2': FakeVM('2'),
+ '3': FakeVM('3')
+ }
+ vms['2'].ready_for_commands = False
+ conn = FakeConnection(vms)
+
+ sampler = sampling.SampleVMs(conn, conn.getVMs, cache)
+ # this is cheating, but setting up proper blocking is costly
+ # and fragile.
+ sampler._sampling = True
+ sampler()
+
+ self.assertEqual(conn.executions[0], CollectMode.SLOW)
+ self.assertVmsInBulkStats(('1', '3'), cache.data[0][0])
+ self.assertVmStuck(sampler, '2')
+
+ def test_slow_collect_while_unresponsive_vm(self):
+ cache = FakeStatsCache()
+ vms = {
+ '1': FakeVM('1'),
+ '2': FakeVM('2'),
+ '3': FakeVM('3')
+ }
+ vms['2'].ready_for_commands = False
+ conn = FakeConnection(vms)
+
+ sampler = sampling.SampleVMs(conn, conn.getVMs, cache)
+ sampler._sampling = True # cheat to bootstrap slow collection
+ sampler()
+ sampler()
+ sampler()
+
+ self.assertSamplingExecutions(
+ conn.executions,
+ (CollectMode.SLOW, CollectMode.SLOW, CollectMode.SLOW))
+ for datum in cache.data:
+ self.assertVmsInBulkStats(('1', '3'), datum[0])
+ self.assertVmStuck(sampler, '2')
+
+ def test_fast_collect_once_vm_responsive(self):
+ cache = FakeStatsCache()
+ vms = {
+ '1': FakeVM('1'),
+ '2': FakeVM('2'),
+ '3': FakeVM('3')
+ }
+ vms['2'].ready_for_commands = False
+ conn = FakeConnection(vms)
+
+ sampler = sampling.SampleVMs(conn, conn.getVMs, cache)
+ sampler._sampling = True # cheat to bootstrap slow collection
+ sampler()
+ sampler()
+ vms['2'].ready_for_commands = True
+ sampler._skip_doms.clear() # cheating, again
+ sampler()
+
+ self.assertSamplingExecutions(
+ conn.executions,
+ (CollectMode.SLOW, CollectMode.SLOW, CollectMode.FAST))
+ for datum in cache.data[:-1]:
+ self.assertVmsInBulkStats(('1', '3'), datum[0])
+ self.assertVmsInBulkStats(vms.keys(), cache.data[-1][0])
+
+ def assertSamplingExecutions(self, executions, expected):
+ for done, exp in zip(executions[-len(expected):], expected):
+ self.assertEqual(done, exp)
+
+ def assertVmsInBulkStats(self, vm_ids, bulk_stats):
+ """
+ checks only for the presence of stats. Stats data is fake,
+ so ignore it as meaningless.
+ """
+ self.assertEqual(len(vm_ids), len(bulk_stats))
+ for vm_id in vm_ids:
+ self.assertIn(vm_id, bulk_stats)
+
+ def assertVmStuck(self, sampler, vm_id):
+ self.assertTrue(sampler._skip_doms.get(vm_id))
--
To view, visit https://gerrit.ovirt.org/40053
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id66dbd420ca29d08ae4063dc83b858be34b8940f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
automation(a)ovirt.org has posted comments on this change.
Change subject: WIP: DON'T MERGE
......................................................................
Patch Set 4:
* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
--
To view, visit https://gerrit.ovirt.org/48151
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic14ae80c9752574d5895aae39b4809923ec8188f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No