vdsm-patches September 2015

vdsm-patches@lists.fedorahosted.org

46 participants
667 discussions

Change in vdsm[ovirt-3.5]: net: configurators: persist custom bond option
by phoracek＠redhat.com 15 Oct '15

15 Oct '15

Petr Horáček has uploaded a new change for review. Change subject: net: configurators: persist custom bond option ...................................................................... net: configurators: persist custom bond option Until now, custom bond property was exposed only to before_network_setup hook and then dropped (so it was not passed to configurators and persisted). But this is not how custom *network* property behaves. We want these two custom properties to be symmetric - they have to be persisted (if unified persistence is used). Unlike custom network properties (which are known as 'special' by Engine), custom bond properties have to be reported in netinfo. Last but not least, we need this custom property to be able to tag bonding as 'OVS Bond' so it will be handled by OVS hook, not by default configurator. This patch moves removal of custom property down to configurators. Thanks to it, custom property is persisted, but not set as a bond option. Custom property is not dropped only in configurators, but in models.py:Bond:areOptionsApplied as well - we don't want configurator to reconfigure bond when there is a custom property defined while its value is not used by configurator. Change-Id: Ic4f1564e19904cc1d53bc6d1cf732ca35375332e Signed-off-by: Petr Horáček <phoracek(a)redhat.com> --- M debian/vdsm.install M lib/vdsm/netinfo.py M tests/functional/networkTests.py M vdsm.spec.in M vdsm/network/Makefile.am M vdsm/network/configurators/ifcfg.py M vdsm/network/configurators/iproute2.py M vdsm/network/models.py A vdsm/network/utils.py 9 files changed, 85 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/44022/1 diff --git a/debian/vdsm.install b/debian/vdsm.install index 819f165..b69c7c1 100644 --- a/debian/vdsm.install +++ b/debian/vdsm.install @@ -73,6 +73,7 @@ ./usr/share/vdsm/network/configurators/ifcfg.py ./usr/share/vdsm/network/configurators/iproute2.py ./usr/share/vdsm/network/configurators/libvirt.py +./usr/share/vdsm/network/utils.py ./usr/share/vdsm/numaUtils.py ./usr/share/vdsm/parted_utils.py ./usr/share/vdsm/ppc64HardwareInfo.py diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index 01f25c7..4dbe308 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -73,7 +73,7 @@ _BONDING_FAILOVER_MODES = frozenset(('1', '3')) _BONDING_LOADBALANCE_MODES = frozenset(('0', '2', '4', '5', '6')) _EXCLUDED_BONDING_ENTRIES = frozenset(( - 'slaves', 'active_slave', 'mii_status', 'queue_id', 'ad_aggregator', + 'slaves', 'active_slave', 'mii_status', 'queue_id', 'ad_aggregator', 'ad_num_ports', 'ad_actor_key', 'ad_partner_key', 'ad_partner_mac' )) _IFCFG_ZERO_SUFFIXED = frozenset( @@ -610,6 +610,15 @@ return info +def _bondCustomOpts(dev, devinfo, running_config): + """Add custom bonding options read from running_config.""" + if dev.name in running_config.bonds: + for option in running_config.bonds[dev.name]['options'].split(): + if option.startswith('custom='): + devinfo['opts']['custom'] = option.split('=', 1)[1] + break + + def _devinfo(link, ipaddrs): ipv4addr, ipv4netmask, ipv4addrs, ipv6addrs = getIpInfo(link.name, ipaddrs) return {'addr': ipv4addr, @@ -740,6 +749,7 @@ ipv6routes = getIPv6Routes() paddr = permAddr() ipaddrs = _getIpAddrs() + running_config = RunningConfig() if vdsmnets is None: nets = networks() @@ -755,6 +765,7 @@ d['nics'][dev.name] = _nicinfo(dev, paddr, ipaddrs) elif dev.isBOND(): d['bondings'][dev.name] = _bondinfo(dev, ipaddrs) + _bondCustomOpts(dev, d, running_config) elif dev.isVLAN(): d['vlans'][dev.name] = _vlaninfo(dev, ipaddrs) diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index c5da1f2..5bc11d8 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -1505,7 +1505,7 @@ self.assertEquals(status, SUCCESS, msg) self.vdsm_net.save_config() - + @cleanupNet @RequireDummyMod @ValidateRunningAsRoot @@ -1526,7 +1526,7 @@ status, msg = self.setupNetworks( {NETWORK_NAME: {'remove': True}}, {}, NOCHK) self.assertEquals(status, SUCCESS, msg) - + @cleanupNet @RequireDummyMod @ValidateRunningAsRoot @@ -1689,7 +1689,7 @@ def _verify_running_config_intact(): self.assertEqual(set([NET_MGMT, NET_CHANGED, NET_UNCHANGED, - NET_ADDITIONAL]), + NET_ADDITIONAL]), set(self.vdsm_net.config.networks.keys())) self.assertEqual(set([BOND_CHANGED, BOND_UNCHANGED]), set(self.vdsm_net.config.bonds.keys())) @@ -1721,7 +1721,7 @@ _assert_all_nets_exist() _verify_running_config_intact() self.assertEqual(set(['ifcfg-%s' % NET_ADDITIONAL, - 'ifcfg-%s' % nic_d]), + 'ifcfg-%s' % nic_d]), set(os.listdir(NET_CONF_BACK_DIR))) # another 'boot' should restore nothing @@ -2505,3 +2505,28 @@ self.assertNetworkDoesntExist(NETWORK_NAME) self.assertBondDoesntExist(BONDING_NAME, [nic]) self.vdsm_net.save_config() + + @cleanupNet + @ValidateRunningAsRoot + def test_setupNetworks_bond_with_custom_option(self): + with dummyIf(2) as nics: + status, msg = self.setupNetworks( + {}, + {BONDING_NAME: {'nics': nics, + 'options': 'custom=foo=bar mode=4'}}, + NOCHK) + self.assertEqual(status, SUCCESS, msg) + self.assertBondExists(BONDING_NAME, nics) + if vdsm.config.config.get('vars', 'net_persistence') == 'unified': + # custom property has to be shown in netinfo and persisted (if + # unified persistence is used). + self._assert_exact_bond_opts(BONDING_NAME, + ['mode=4', 'custom=foo=bar']) + bond = self.vdsm_net.config.bonds.get(BONDING_NAME) + self.assertSetEqual(set(['mode=4', 'custom=foo=bar']), + set(bond.get('options').split())) + + status, msg = self.setupNetworks( + {}, {BONDING_NAME: {'remove': True}}, NOCHK) + self.assertEqual(status, SUCCESS, msg) + self.assertBondDoesntExist(BONDING_NAME, nics) diff --git a/vdsm.spec.in b/vdsm.spec.in index 8a54ed8..7ba765a 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -1087,6 +1087,7 @@ %{_datadir}/%{vdsm_name}/network/configurators/ifcfg.py* %{_datadir}/%{vdsm_name}/network/configurators/libvirt.py* %{_datadir}/%{vdsm_name}/network/configurators/iproute2.py* +%{_datadir}/%{vdsm_name}/network/utils.py* %{_datadir}/%{vdsm_name}/storage/__init__.py* %{_datadir}/%{vdsm_name}/storage/blockSD.py* %{_datadir}/%{vdsm_name}/storage/blockVolume.py* diff --git a/vdsm/network/Makefile.am b/vdsm/network/Makefile.am index 9aa3f23..071cf53 100644 --- a/vdsm/network/Makefile.am +++ b/vdsm/network/Makefile.am @@ -31,4 +31,5 @@ sourceroute.py \ sourceroutethread.py \ tc.py \ + utils.py \ $(NULL) diff --git a/vdsm/network/configurators/ifcfg.py b/vdsm/network/configurators/ifcfg.py index c3f9893..d78c5db 100644 --- a/vdsm/network/configurators/ifcfg.py +++ b/vdsm/network/configurators/ifcfg.py @@ -45,6 +45,7 @@ from ..errors import ConfigNetworkError, ERR_FAILED_IFUP from ..models import Nic, Bridge, IpConfig from ..sourceroute import StaticSourceRoute, DynamicSourceRoute +from ..utils import remove_custom_bond_option import dsaversion # TODO: Make parent package import when vdsm is a package @@ -585,7 +586,9 @@ def addBonding(self, bond, **opts): """ Create ifcfg-* file with proper fields for bond """ - conf = 'BONDING_OPTS=%s\n' % pipes.quote(bond.options or '') + # 'custom' is not a real bond option, it just piggybacks custom values + options = remove_custom_bond_option(bond.options) + conf = 'BONDING_OPTS=%s\n' % pipes.quote(options) opts['hotplug'] = 'no' # So that udev doesn't trigger an ifup if bond.bridge: conf += 'BRIDGE=%s\n' % pipes.quote(bond.bridge.name) diff --git a/vdsm/network/configurators/iproute2.py b/vdsm/network/configurators/iproute2.py index b06e0ad..cf31a38 100644 --- a/vdsm/network/configurators/iproute2.py +++ b/vdsm/network/configurators/iproute2.py @@ -35,6 +35,7 @@ from ..errors import ConfigNetworkError, ERR_FAILED_IFUP, ERR_FAILED_IFDOWN from ..models import Nic from ..sourceroute import DynamicSourceRoute +from ..utils import remove_custom_bond_option _BRIDGING_OPT_PATH = '/sys/class/net/%s/bridge/%s' _ETHTOOL_BINARY = CommandPath( @@ -321,7 +322,9 @@ def addBondOptions(self, bond): logging.debug('Add bond options %s', bond.options) - for option in bond.options.split(): + # 'custom' is not a real bond option, it just piggybacks custom values + options = remove_custom_bond_option(bond.options) + for option in options: key, value = option.split('=') with open(netinfo.BONDING_OPT % (bond.name, key), 'w') as f: f.write(value) diff --git a/vdsm/network/models.py b/vdsm/network/models.py index 8e33950..4ae7508 100644 --- a/vdsm/network/models.py +++ b/vdsm/network/models.py @@ -28,6 +28,7 @@ from .errors import ConfigNetworkError from . import errors as ne +from .utils import remove_custom_bond_option class NetDevice(object): @@ -208,9 +209,11 @@ # TODO: this method returns True iff self.options are a subset of the # TODO: current bonding options. VDSM should probably compute if the # TODO: non-default settings are equal to the non-default state. - if not self.options: + # 'custom' is not a real bond option, it just piggybacks custom values + options = remove_custom_bond_option(self.options) + if options == '': return True - confOpts = [option.split('=', 1) for option in self.options.split(' ')] + confOpts = [option.split('=', 1) for option in options.split(' ')] activeOpts = netinfo.bondOpts(self.name, (name for name, value in confOpts)) return all(value in activeOpts[name] for name, value in confOpts) diff --git a/vdsm/network/utils.py b/vdsm/network/utils.py new file mode 100644 index 0000000..370d82e --- /dev/null +++ b/vdsm/network/utils.py @@ -0,0 +1,28 @@ +# 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 +# + + +def remove_custom_bond_option(options): + """ Removes 'custom' option from bond options string. + + >>> remove_custom_bond_option('custom=foo=bar mode=1') + 'mode=1' + """ + return ' '.join((option for option in options.split() + if not option.startswith('custom='))) -- To view, visit https://gerrit.ovirt.org/44022 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic4f1564e19904cc1d53bc6d1cf732ca35375332e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>

2 7

Change in vdsm[ovirt-3.5]: hooks: introduce before/after_get_stats hook points
by phoracek＠redhat.com 15 Oct '15

15 Oct '15

Hello Ido Barkan, Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/43274 to review the following change. Change subject: hooks: introduce before/after_get_stats hook points ...................................................................... hooks: introduce before/after_get_stats hook points This patches adds a hook point that allows to modify the host statistics which Vdsm reports on its getVdsStats API. This hook can become useful if a hook writer would like to provide fake usage statistics to a fake network that is reported by an after_get_caps hook script. Change-Id: Id1ff93837abb73f88ececc7571115645b95486cf Signed-off-by: Dan Kenigsberg <danken(a)redhat.com> Reviewed-on: https://gerrit.ovirt.org/40403 Reviewed-by: Ido Barkan <ibarkan(a)redhat.com> Reviewed-by: Petr Horáček <phoracek(a)redhat.com> Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com> Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1234867 --- M debian/vdsm.dirs M vdsm.spec.in M vdsm/API.py M vdsm/hooks.py M vdsm/vdsmd.8.in M vdsm_hooks/Makefile.am 6 files changed, 27 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/43274/1 diff --git a/debian/vdsm.dirs b/debian/vdsm.dirs index 7b5db1b..743550d 100644 --- a/debian/vdsm.dirs +++ b/debian/vdsm.dirs @@ -17,6 +17,7 @@ usr/libexec/vdsm/hooks/after_disk_hotunplug usr/libexec/vdsm/hooks/after_get_all_vm_stats usr/libexec/vdsm/hooks/after_get_caps +usr/libexec/vdsm/hooks/after_get_stats usr/libexec/vdsm/hooks/after_get_vm_stats usr/libexec/vdsm/hooks/after_network_setup usr/libexec/vdsm/hooks/after_nic_hotplug @@ -44,6 +45,7 @@ usr/libexec/vdsm/hooks/before_disk_hotunplug usr/libexec/vdsm/hooks/before_get_all_vm_stats usr/libexec/vdsm/hooks/before_get_caps +usr/libexec/vdsm/hooks/before_get_stats usr/libexec/vdsm/hooks/before_get_vm_stats usr/libexec/vdsm/hooks/before_network_setup usr/libexec/vdsm/hooks/before_nic_hotplug diff --git a/vdsm.spec.in b/vdsm.spec.in index 8a54ed8..0711055 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -1192,6 +1192,8 @@ %dir %{_libexecdir}/%{vdsm_name}/hooks/after_get_all_vm_stats %dir %{_libexecdir}/%{vdsm_name}/hooks/before_get_caps %dir %{_libexecdir}/%{vdsm_name}/hooks/after_get_caps +%dir %{_libexecdir}/%{vdsm_name}/hooks/before_get_stats +%dir %{_libexecdir}/%{vdsm_name}/hooks/after_get_stats %{_datadir}/%{vdsm_name}/addNetwork %{_datadir}/%{vdsm_name}/delNetwork %{_datadir}/%{vdsm_name}/dumpStorageTable.py* diff --git a/vdsm/API.py b/vdsm/API.py index 36fafe2..e892590 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -1303,6 +1303,7 @@ meminfo = utils.readMemInfo() return meminfo['SwapTotal'] / 1024, meminfo['SwapFree'] / 1024 + hooks.before_get_stats() stats = {} decStats = self._cif._hostStats.get() @@ -1342,6 +1343,7 @@ # For backwards compatibility, will be removed in the future stats['haScore'] = stats['haStats']['score'] + stats = hooks.after_get_stats(stats) return {'status': doneCode, 'info': stats} def setLogLevel(self, level): diff --git a/vdsm/hooks.py b/vdsm/hooks.py index 9dbfcba..1d6605f 100644 --- a/vdsm/hooks.py +++ b/vdsm/hooks.py @@ -369,6 +369,16 @@ hookType=_JSON_HOOK) +def before_get_stats(): + return _runHooksDir({}, 'before_get_stats', raiseError=True, + hookType=_JSON_HOOK) + + +def after_get_stats(caps): + return _runHooksDir(caps, 'after_get_stats', raiseError=False, + hookType=_JSON_HOOK) + + def _getScriptInfo(path): try: with file(path) as f: diff --git a/vdsm/vdsmd.8.in b/vdsm/vdsmd.8.in index cca7fac..61df3e8 100644 --- a/vdsm/vdsmd.8.in +++ b/vdsm/vdsmd.8.in @@ -61,7 +61,8 @@ before_set_num_of_cpus, after_set_num_of_cpus, before_get_vm_stats, after_get_vm_stats, before_get_all_vm_stats, after_get_all_vm_stats, - before_get_caps, after_get_caps. + before_get_caps, after_get_caps, + before_get_stats, after_get_stats. Each hook executes the scripts under .FN /usr/libexec/vdsm/hooks/<hook-name>/ @@ -71,7 +72,8 @@ Each hook script (except before_vdsm_start, after_vdsm_stop, before_network_setup and after_network_setup, before_get_vm_stats, after_get_vm_stats, before_get_all_vm_stats, after_get_all_vm_stats, -before_get_caps and after_get_caps) inherit +before_get_caps, after_get_caps, +before_get_stats, after_get_stats) inherit the environment of the VDSM process, with an additional variable .B _hook_domxml which holds the path of libvirt's @@ -162,6 +164,11 @@ a getVdsCapabilities API request. after_get_caps receives the complete capabilities dictionary within _hook_json. +before_get_stats and after_get_stats are called before (and after) +a getVdsStats API request. +after_get_stats receives the complete host statistics dictionary within +_hook_json. + .SS Hook execution before_vdsm_start and after_vdsm_stop scripts are executed as user .I root. diff --git a/vdsm_hooks/Makefile.am b/vdsm_hooks/Makefile.am index 028d25a..128d017 100644 --- a/vdsm_hooks/Makefile.am +++ b/vdsm_hooks/Makefile.am @@ -115,6 +115,8 @@ after_get_all_vm_stats \ before_get_caps \ after_get_caps \ + before_get_stats \ + after_get_stats \ $(NULL) all-local: \ -- To view, visit https://gerrit.ovirt.org/43274 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id1ff93837abb73f88ececc7571115645b95486cf Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com>

3 10

Change in vdsm[master]: ssl: ssl socket may throw sslerror during reading
by Piotr Kliczewski 15 Oct '15

15 Oct '15

Piotr Kliczewski has uploaded a new change for review. Change subject: ssl: ssl socket may throw sslerror during reading ...................................................................... ssl: ssl socket may throw sslerror during reading When client closes socket in not clean way sometimes we can get SSLError with unexpected eof. When this situation occurs we need to make sure to handle this situation properly. Bug-Url: https://bugzilla.redhat.com/1256446 Change-Id: I8de60d91f81b08e9cb78df07f09d2bcc903c1bad Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com> --- M lib/yajsonrpc/betterAsyncore.py 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/25/46625/1 diff --git a/lib/yajsonrpc/betterAsyncore.py b/lib/yajsonrpc/betterAsyncore.py index ef5738b..7896aaf 100644 --- a/lib/yajsonrpc/betterAsyncore.py +++ b/lib/yajsonrpc/betterAsyncore.py @@ -21,6 +21,7 @@ import socket from errno import EWOULDBLOCK +from vdsm.m2cutils import SSL from vdsm.infra.eventfd import EventFD from vdsm.utils import traceback @@ -122,6 +123,8 @@ return '' else: raise + except SSL.SSLError: + self.handle_close() def send(self, data): try: -- To view, visit https://gerrit.ovirt.org/46625 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8de60d91f81b08e9cb78df07f09d2bcc903c1bad Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>

8 23

Change in vdsm[ovirt-3.5]: safelease: Increase spmprotect timeouts
by Nir Soffer 15 Oct '15

15 Oct '15

Hello Adam Litke, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/46485 to review the following change. Change subject: safelease: Increase spmprotect timeouts ...................................................................... safelease: Increase spmprotect timeouts When spmprotect.sh fail to renew the lease, it start a fencing process: 1. Send SIGUSR1 signal to vdsm 2. Send SIGTERM signal to vdsm after 7 seconds 3. Send SIGKILL signal to vdsm after 9 seconds 4. Reboot the machine after 20 seconds When vdsm receives the SIGUSR1 signal, the signal handler invokes stopSpm, which releases the cluster lock. Releasing the cluster lock will terminate the waiting spmprotect.sh, preventing termination of vdsm and reboot. If vdsm fails to release the cluster lock within 7 seconds, spmprotect.sh will terminate it, and if did not terminate, spmprotect.sh will kill it. When systemd starts vdsm again, vdsm looks for spmprotect.sh processes and tries to release the lease. If the lease cannot be released after 10 seconds, it kills the pending spmprotect.sh processes, preventing reboot. Testing with both block and file storage show that this flow is broken when access to master domain is blocked: 1. In block storage, vdsm gets stuck trying to unmount the master mount, and spmprotect.sh kills it before it try to release the cluster lock. 2. In file storage, vdsm gets stuck trying to write spm status to the master domain, and spmprotect.sh kills it before it try to release the cluster lock. 3. When vdsm starts up, sometimes it manage to kill the waiting spmprotect.sh process, and sometimes spmprotect.sh reboot the machine before vdsm kills it. We cannot fix 1 and 2 easily. 3 can be fixed by giving vdsm more time for stopSpm flow, and more time to startup and kill pending spmprotect.sh process. This patch increases spmprotect timeouts to increase the chance of clean shutdown and decrease the chance of unneeded reboot. New spmprotect.sh flow is: 1. Send SIGUSR1 signal to vdsm 2. Send SIGTERM signal to vdsm after 10 seconds 3. Send SIGKILL signal to vdsm after 20 seconds 4. Reboot the machine after 60 seconds Change-Id: Ib71fa06c21602fd9d43516c5b4c997c481708697 Bug-Url: https://bugzilla.redhat.com/1222564 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> Reviewed-on: https://gerrit.ovirt.org/46057 Continuous-Integration: Jenkins CI Reviewed-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/protect/spmprotect.sh 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/85/46485/1 diff --git a/vdsm/storage/protect/spmprotect.sh b/vdsm/storage/protect/spmprotect.sh index fc0e390..b13b07c 100755 --- a/vdsm/storage/protect/spmprotect.sh +++ b/vdsm/storage/protect/spmprotect.sh @@ -71,12 +71,12 @@ trap "" INT log "Fencing sdUUID=$sdUUID id=$ID lease_path=$LEASE_FILE" - (sleep 20 && echodo $REBOOTCMD) & + (sleep 60 && echodo $REBOOTCMD) & disown - (sleep 7 + (sleep 10 log "Trying to stop vdsm for sdUUID=$sdUUID id=$ID lease_path=$LEASE_FILE" echodo $KILL "$VDSM_PID" - sleep 2 + sleep 10 echodo $KILL -9 "$VDSM_PID" )& disown -- To view, visit https://gerrit.ovirt.org/46485 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib71fa06c21602fd9d43516c5b4c997c481708697 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>

5 12

Change in vdsm[ovirt-3.5]: safelease: Unbreak safelease on systemd
by Nir Soffer 15 Oct '15

15 Oct '15

Hello Adam Litke, Allon Mureinik, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/46484 to review the following change. Change subject: safelease: Unbreak safelease on systemd ...................................................................... safelease: Unbreak safelease on systemd spmprotect.sh depends on Vdsm pid file for fencing Vdsm when the SPM lease cannot be renewed. However, on systemd, we do not create Vdsm pid file. When spmprotect tries to fence Vdsm it fails and reboots the host, killing all running vms. This issue effects only v1 storage domains, typically old systems using dc compatibility version 3.0. Now we pass vdsm pid to spmprotect.sh helper script via command line argument, restoring safelease operation. Adding another argument to a script with 9 arguments is ugly, but I don't want to make risky changes to this delicate and critical code. Change-Id: I230b6909781269531eab3d71b516b28ab22de856 Bug-Url: https://bugzilla.redhat.com/1222564 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> Reviewed-on: https://gerrit.ovirt.org/45963 Continuous-Integration: Jenkins CI Reviewed-by: Allon Mureinik <amureini(a)redhat.com> Reviewed-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/clusterlock.py M vdsm/storage/protect/spmprotect.sh 2 files changed, 11 insertions(+), 8 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/84/46484/1 diff --git a/vdsm/storage/clusterlock.py b/vdsm/storage/clusterlock.py index 24a5d81..c9dab35 100644 --- a/vdsm/storage/clusterlock.py +++ b/vdsm/storage/clusterlock.py @@ -132,7 +132,8 @@ acquireLockCommand = subprocess.list2cmdline([ lockUtil, "start", self._sdUUID, str(hostID), str(self._lockRenewalIntervalSec), str(self._leasesPath), - str(leaseTimeMs), str(ioOpTimeoutMs), str(self._leaseFailRetry) + str(leaseTimeMs), str(ioOpTimeoutMs), + str(self._leaseFailRetry), str(os.getpid()) ]) cmd = [constants.EXT_SU, misc.IOUSER, '-s', constants.EXT_SH, '-c', diff --git a/vdsm/storage/protect/spmprotect.sh b/vdsm/storage/protect/spmprotect.sh index f569a91..fc0e390 100755 --- a/vdsm/storage/protect/spmprotect.sh +++ b/vdsm/storage/protect/spmprotect.sh @@ -31,8 +31,6 @@ CHECKVDSM=${CHECKVDSM:-"/usr/bin/pgrep vdsm"} REBOOTCMD=${REBOOTCMD:-"sudo /sbin/reboot -f"} RENEWDIR="/var/run/vdsm/spmprotect/$$" -VDSM_PIDFILE="/var/run/vdsm/vdsmd.pid" -VDSM_PID=`/bin/cat "$VDSM_PIDFILE"` function usage() { if [ -n "$1" ]; then @@ -41,7 +39,7 @@ trap EXIT echo "usage: $0 COMMAND PARAMETERS" echo "Commands:" - echo " start { sdUUID hostId renewal_interval_sec lease_path[:offset] lease_time_ms io_op_timeout_ms fail_retries }" + echo " start { sdUUID hostId renewal_interval_sec lease_path[:offset] lease_time_ms io_op_timeout_ms fail_retries vdsm_pid [debug] }" echo "Parameters:" echo " sdUUID - domain uuid" echo " hostId - host id in pool" @@ -51,6 +49,8 @@ echo " lease_time_ms - time limit within which lease must be renewed (at least 2*renewal_interval_sec)" echo " io_op_timeout_ms - I/O operation timeout" echo " fail_retries - Maximal number of attempts to retry to renew the lease before fencing (<= lease_time_ms/renewal_interval_sec)" + echo " vdsm_pid - Vdsm process ID" + echo " debug - enable debug mode (optional)" exit 1 } @@ -119,12 +119,14 @@ LEASE_TIME_MS="$4" IO_OP_TIMEOUT_MS="$5" LAST_RENEWAL="$6" + VDSM_PID="$7" # Make sure params are integers [ "$RENEWAL_INTERVAL" -eq "$RENEWAL_INTERVAL" 2>/dev/null ] || usage "error - Renewal interval not an integer" [ "$LEASE_TIME_MS" -eq "$LEASE_TIME_MS" 2>/dev/null ] || usage "error - Lease time not an integer" [ "$LEASE_TIME_MS" -ge $((RENEWAL_INTERVAL*2)) ] || usage "error - Lease time too small" [ "$IO_OP_TIMEOUT_MS" -eq "$IO_OP_TIMEOUT_MS" 2>/dev/null ] || usage "error - IO op timeout not an integer" + [ "$VDSM_PID" -eq "$VDSM_PID" 2>/dev/null ] || usage "error - Vdsm PID is not a process ID" } function renew() { @@ -195,12 +197,12 @@ ####################################################### Main ################################################### -if [ "$#" -lt 8 ]; then +if [ "$#" -lt 9 ]; then usage "error - wrong number of arguments" fi -validate_args $3 $4 $5 $6 $7 $8 -DEBUG="$9" +validate_args $3 $4 $5 $6 $7 $8 $9 +DEBUG="${10}" dbg="" if [ "$DEBUG" -eq "$DEBUG" 2>/dev/null ]; then dbg="-d" @@ -221,7 +223,7 @@ trap release USR1 exec 0>&- && exec 1>&- && exec 2>&- # Close stdin, stdout and stderr - $SETSID $0 renew $sdUUID $ID $RENEWAL_INTERVAL $LEASE_FILE $LEASE_TIME_MS $IO_OP_TIMEOUT_MS $LAST_RENEWAL $DEBUG >> $LOGFILE 2>&1 & + $SETSID $0 renew $sdUUID $ID $RENEWAL_INTERVAL $LEASE_FILE $LEASE_TIME_MS $IO_OP_TIMEOUT_MS $LAST_RENEWAL $VDSM_PID $DEBUG >> $LOGFILE 2>&1 & trap EXIT exit 0 ;; -- To view, visit https://gerrit.ovirt.org/46484 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I230b6909781269531eab3d71b516b28ab22de856 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com> Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>

5 11

Change in vdsm[master]: stomp: allow to mock monotonic_time method
by Piotr Kliczewski 14 Oct '15

14 Oct '15

Piotr Kliczewski has uploaded a new change for review. Change subject: stomp: allow to mock monotonic_time method ...................................................................... stomp: allow to mock monotonic_time method Change-Id: I59b65b12ddbcc547dc2bf45bf4dc3742a7778dea Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com> --- M lib/yajsonrpc/stomp.py 1 file changed, 5 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/45/43745/1 diff --git a/lib/yajsonrpc/stomp.py b/lib/yajsonrpc/stomp.py index a02a06f..4a2364d 100644 --- a/lib/yajsonrpc/stomp.py +++ b/lib/yajsonrpc/stomp.py @@ -19,7 +19,7 @@ from uuid import uuid4 from collections import deque -from vdsm.utils import monotonic_time +from vdsm import utils import re # REQUIRED_FOR: engine-3.5 @@ -361,10 +361,11 @@ return self._parser.popFrame() def _update_outgoing_heartbeat(self): - self._lastOutgoingTimeStamp = monotonic_time() + self._lastOutgoingTimeStamp = utils.monotonic_time() def _outgoing_heartbeat_expiration_interval(self): - since_last_update = (monotonic_time() - self._lastOutgoingTimeStamp) + now = utils.monotonic_time() + since_last_update = (now - self._lastOutgoingTimeStamp) return (self._outgoing_heartbeat_in_milis / 1000.0) - since_last_update def next_check_interval(self): @@ -411,7 +412,7 @@ return True def _milis(self): - return int(round(monotonic_time() * 1000)) + return int(round(utils.monotonic_time() * 1000)) def handle_close(self, dispatcher): self.connection.close() -- To view, visit https://gerrit.ovirt.org/43745 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I59b65b12ddbcc547dc2bf45bf4dc3742a7778dea Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>

6 16

Change in vdsm[master]: tests: AsyncClient test suite
by Piotr Kliczewski 14 Oct '15

14 Oct '15

Piotr Kliczewski has uploaded a new change for review. Change subject: tests: AsyncClient test suite ...................................................................... tests: AsyncClient test suite Change-Id: Ie102948c2bb7ff964586701b9b81b2867f7ba840 Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com> --- M lib/yajsonrpc/stomp.py M tests/Makefile.am A tests/stompAsyncClientTests.py 3 files changed, 137 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/43393/1 diff --git a/lib/yajsonrpc/stomp.py b/lib/yajsonrpc/stomp.py index 7436984..a02a06f 100644 --- a/lib/yajsonrpc/stomp.py +++ b/lib/yajsonrpc/stomp.py @@ -525,8 +525,13 @@ return sub def unsubscribe(self, sub): - self.queue_frame(Frame(Command.UNSUBSCRIBE, - {"id": sub.id})) + try: + del self._subscriptions[sub.id] + except KeyError: + self.log.warning('No subscription with %s id', sub.id) + else: + self.queue_frame(Frame(Command.UNSUBSCRIBE, + {"id": sub.id})) class _Subscription(object): diff --git a/tests/Makefile.am b/tests/Makefile.am index eff79f5..4d42b55 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -95,6 +95,7 @@ sslhelper.py \ sslTests.py \ stompAdapterTests.py \ + stompAsyncClientTests.py \ storageMailboxTests.py \ storageMonitorTests.py \ storageServerTests.py \ diff --git a/tests/stompAsyncClientTests.py b/tests/stompAsyncClientTests.py new file mode 100644 index 0000000..4ffc327 --- /dev/null +++ b/tests/stompAsyncClientTests.py @@ -0,0 +1,129 @@ +# +# 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 uuid import uuid4 + +from testlib import VdsmTestCase as TestCaseBase +from yajsonrpc.stomp import AsyncClient, Command, Frame, Headers, StompError +from stompAdapterTests import TestSubscription + + +class AsyncClientTest(TestCaseBase): + + def test_connect(self): + client = AsyncClient() + client.handle_connect() + + req_frame = client.pop_message() + + self.assertEquals(req_frame.command, Command.CONNECT) + self.assertEquals(req_frame.headers[Headers.ACCEPT_VERSION], '1.2') + self.assertEquals(req_frame.headers[Headers.HEARTEBEAT], '0,5000') + + def test_subscribe(self): + client = AsyncClient() + + id = str(uuid4()) + client.subscribe(destination='jms.queue.events', ack='client', + sub_id=id) + + req_frame = client.pop_message() + self.assertTrue(len(client._subscriptions) == 1) + self.assertEquals(req_frame.command, Command.SUBSCRIBE) + self.assertEquals(req_frame.headers['destination'], 'jms.queue.events') + self.assertEquals(req_frame.headers['id'], id) + self.assertEquals(req_frame.headers['ack'], 'client') + + def test_manage_subscription(self): + client = AsyncClient() + + subscription = client.subscribe(destination='jms.queue.events', + ack='client', + sub_id=str(uuid4())) + client.unsubscribe(subscription) + self.assertTrue(len(client._subscriptions) == 0) + + def test_unsubscribe_with_different_id(self): + client = AsyncClient() + + client.subscribe(destination='jms.queue.events', + ack='client-individual', + sub_id=str(uuid4())) + # ignore subscribe frame + client.pop_message() + + client.unsubscribe(TestSubscription('jms.queue.events', + 'ad052acb-a934-4e10-8ec3')) + + self.assertTrue(len(client._subscriptions) == 1) + self.assertFalse(client.has_outgoing_messages) + + def test_send(self): + client = AsyncClient() + data = ('{"jsonrpc":"2.0","method":"Host.getAllVmStats","params":{},' + '"id":"e8a936a6-d886-4cfa-97b9-2d54209053ff"}') + headers = {Headers.REPLY_TO: 'jms.topic.vdsm_responses', + Headers.CONTENT_LENGTH: '103'} + client.send('jms.topic.vdsm_requests', data, headers) + + req_frame = client.pop_message() + self.assertEquals(req_frame.command, Command.SEND) + self.assertEquals(req_frame.headers['destination'], + 'jms.topic.vdsm_requests') + self.assertEquals(req_frame.headers[Headers.REPLY_TO], + 'jms.topic.vdsm_responses') + self.assertEquals(req_frame.body, data) + + def test_receive_connected(self): + client = AsyncClient() + frame = Frame(Command.CONNECTED, + {'version': '1.2', Headers.HEARTEBEAT: '8000,0'}) + + client.handle_frame(None, frame) + + self.assertTrue(client.connected) + + def test_receive_message(self): + client = AsyncClient() + id = 'ad052acb-a934-4e10-8ec3-00c7417ef8d1' + headers = {Headers.CONTENT_LENGTH: '78', + Headers.DESTINATION: 'jms.topic.vdsm_responses', + Headers.CONTENT_TYPE: 'application/json', + Headers.SUBSCRIPTION: id} + body = ('{"jsonrpc": "2.0", "id": "e8a936a6-d886-4cfa-97b9-2d54209053f' + 'f", "result": []}') + frame = Frame(command=Command.MESSAGE, headers=headers, body=body) + + def message_handler(sub, frame): + client.queue_frame(frame) + + client.subscribe('', 'auto', id, message_handler) + # ignore subscribe frame + client.pop_message() + + client.handle_frame(None, frame) + + self.assertEquals(frame, client.pop_message()) + + def test_receive_error(self): + client = AsyncClient() + frame = Frame(command=Command.ERROR, body='Test error') + + with self.assertRaises(StompError): + client.handle_frame(None, frame) -- To view, visit https://gerrit.ovirt.org/43393 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie102948c2bb7ff964586701b9b81b2867f7ba840 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>

5 16

Change in vdsm[master]: stomp: send unsubscribed frame only if there was subscription
by Piotr Kliczewski 14 Oct '15

14 Oct '15

Piotr Kliczewski has uploaded a new change for review. Change subject: stomp: send unsubscribed frame only if there was subscription ...................................................................... stomp: send unsubscribed frame only if there was subscription Change-Id: I8789b84d131e5551b7383e61e6b71c6da0a512fe Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com> --- M lib/yajsonrpc/stomp.py 1 file changed, 7 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/33/43733/1 diff --git a/lib/yajsonrpc/stomp.py b/lib/yajsonrpc/stomp.py index 7436984..a02a06f 100644 --- a/lib/yajsonrpc/stomp.py +++ b/lib/yajsonrpc/stomp.py @@ -525,8 +525,13 @@ return sub def unsubscribe(self, sub): - self.queue_frame(Frame(Command.UNSUBSCRIBE, - {"id": sub.id})) + try: + del self._subscriptions[sub.id] + except KeyError: + self.log.warning('No subscription with %s id', sub.id) + else: + self.queue_frame(Frame(Command.UNSUBSCRIBE, + {"id": sub.id})) class _Subscription(object): -- To view, visit https://gerrit.ovirt.org/43733 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8789b84d131e5551b7383e61e6b71c6da0a512fe Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>

5 9

Change in vdsm[master]: tests: StompAdapter test suite
by Piotr Kliczewski 14 Oct '15

14 Oct '15

Piotr Kliczewski has uploaded a new change for review. Change subject: tests: StompAdapter test suite ...................................................................... tests: StompAdapter test suite Change-Id: Ic22a8ec43413431ff57ecf66da62e065c534bac7 Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com> --- M lib/yajsonrpc/stompreactor.py M tests/Makefile.am A tests/stompAdapterTests.py 3 files changed, 343 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/43342/1 diff --git a/lib/yajsonrpc/stompreactor.py b/lib/yajsonrpc/stompreactor.py index e3b71bf..467a588 100644 --- a/lib/yajsonrpc/stompreactor.py +++ b/lib/yajsonrpc/stompreactor.py @@ -187,9 +187,8 @@ frame.body) return else: - try: - subs = self._sub_dests[destination] - except KeyError: + subs = self._sub_dests[destination] + if len(subs) == 0: self._send_error("Subscription not available", dispatcher.connection) return diff --git a/tests/Makefile.am b/tests/Makefile.am index 7ae8912..eff79f5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -94,6 +94,7 @@ sourceroutingTests.py \ sslhelper.py \ sslTests.py \ + stompAdapterTests.py \ storageMailboxTests.py \ storageMonitorTests.py \ storageServerTests.py \ diff --git a/tests/stompAdapterTests.py b/tests/stompAdapterTests.py new file mode 100644 index 0000000..a0be47f --- /dev/null +++ b/tests/stompAdapterTests.py @@ -0,0 +1,340 @@ +# +# 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 collections import defaultdict + +from testlib import VdsmTestCase as TestCaseBase +from yajsonrpc import JsonRpcRequest +from yajsonrpc.betterAsyncore import Reactor +from yajsonrpc.stomp import \ + Command, \ + Frame, \ + Headers, \ + LEGACY_SUBSCRIPTION_ID_REQUEST +from yajsonrpc.stompreactor import StompAdapterImpl + + +class TestConnection(object): + + def __init__(self, client): + self._client = client + + def send_raw(self, msg): + self._client.queue_frame(msg) + + def handleMessage(self, data): + self._client.queue_frame(data) + + +class TestDispatcher(object): + + def __init__(self, client): + self._client = client + + def setHeartBeat(self, outgoing, incoming=0): + pass + + @property + def connection(self): + return TestConnection(self._client) + + +class TestSubscription(object): + + def __init__(self, destination, id): + self._destination = destination + self._id = id + + def set_client(self, client): + self._client = TestConnection(client) + + @property + def destination(self): + return self._destination + + @property + def id(self): + return self._id + + @property + def client(self): + return self._client + + +class ConnectFrameTest(TestCaseBase): + + def test_connect(self): + frame = Frame(Command.CONNECT, + {Headers.ACCEPT_VERSION: '1.2', + Headers.HEARTEBEAT: '0,8000'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.CONNECTED) + self.assertEquals(resp_frame.headers['version'], '1.2') + self.assertEquals(resp_frame.headers[Headers.HEARTEBEAT], '8000,0') + + def test_min_heartbeat(self): + frame = Frame(Command.CONNECT, + {Headers.ACCEPT_VERSION: '1.2', + Headers.HEARTEBEAT: '0,500'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.CONNECTED) + self.assertEquals(resp_frame.headers['version'], '1.2') + self.assertEquals(resp_frame.headers[Headers.HEARTEBEAT], '1000,0') + + def test_unsuported_version(self): + frame = Frame(Command.CONNECT, + {Headers.ACCEPT_VERSION: '1.0'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, 'Version unsupported') + + def test_no_heartbeat(self): + frame = Frame(Command.CONNECT, + {Headers.ACCEPT_VERSION: '1.2'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.CONNECTED) + self.assertEquals(resp_frame.headers['version'], '1.2') + self.assertEquals(resp_frame.headers[Headers.HEARTEBEAT], '0,0') + + def test_no_headers(self): + frame = Frame(Command.CONNECT) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, 'Version unsupported') + + +class SubscriptionFrameTest(TestCaseBase): + + def test_subscribe(self): + frame = Frame(Command.SUBSCRIBE, + {Headers.DESTINATION: 'jms.queue.events', + 'ack': 'auto', + 'id': 'ad052acb-a934-4e10-8ec3-00c7417ef8d1'}) + + destinations = defaultdict(list) + + adapter = StompAdapterImpl(Reactor(), destinations, {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + subscription = destinations['jms.queue.events'][0] + self.assertEquals(subscription.id, + 'ad052acb-a934-4e10-8ec3-00c7417ef8d1') + self.assertEquals(subscription.destination, + 'jms.queue.events') + + def test_no_destination(self): + frame = Frame(Command.SUBSCRIBE, + {'ack': 'auto', + 'id': 'ad052acb-a934-4e10-8ec3-00c7417ef8d1'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, + 'Missing destination or subscription id header') + + def test_no_id(self): + frame = Frame(Command.SUBSCRIBE, + {'ack': 'auto', + Headers.DESTINATION: 'jms.queue.events'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, + 'Missing destination or subscription id header') + + +class UnsubscribeFrameTest(TestCaseBase): + + def test_unsubscribe(self): + frame = Frame(Command.UNSUBSCRIBE, + {'id': 'ad052acb-a934-4e10-8ec3-00c7417ef8d1'}) + + subscription = TestSubscription('jms.queue.events', + 'ad052acb-a934-4e10-8ec3-00c7417ef8d1') + + destinations = defaultdict(list) + destinations['jms.queue.events'].append(subscription) + + adapter = StompAdapterImpl(Reactor(), destinations, {}) + adapter._sub_ids['ad052acb-a934-4e10-8ec3-00c7417ef8d1'] = subscription + adapter.handle_frame(TestDispatcher(adapter), frame) + + self.assertTrue(len(adapter._sub_ids) == 0) + self.assertTrue(len(destinations) == 0) + + def test_multipe_subscriptions(self): + frame = Frame(Command.UNSUBSCRIBE, + {'id': 'ad052acb-a934-4e10-8ec3-00c7417ef8d1'}) + + subscription = TestSubscription('jms.queue.events', + 'ad052acb-a934-4e10-8ec3-00c7417ef8d1') + subscription2 = TestSubscription('jms.queue.events', + 'e8a93a6-d886-4cfa-97b9-2d54209053ff') + + destinations = defaultdict(list) + destinations['jms.queue.events'].append(subscription) + destinations['jms.queue.events'].append(subscription2) + + adapter = StompAdapterImpl(Reactor(), destinations, {}) + adapter._sub_ids['ad052acb-a934-4e10-8ec3-00c7417ef8d1'] = subscription + adapter.handle_frame(TestDispatcher(adapter), frame) + + self.assertTrue(len(adapter._sub_ids) == 0) + self.assertTrue(len(destinations) == 1) + self.assertEquals(destinations['jms.queue.events'], [subscription2]) + + def test_no_id(self): + frame = Frame(Command.UNSUBSCRIBE) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, 'Missing id header') + + +class SendFrameTest(TestCaseBase): + + def test_send(self): + frame = Frame(command=Command.SEND, + headers={Headers.DESTINATION: 'jms.topic.vdsm_requests', + Headers.REPLY_TO: 'jms.topic.vdsm_responses', + Headers.CONTENT_LENGTH: '103'}, + body=('{"jsonrpc":"2.0","method":"Host.getAllVmStats",' + '"params":{},"id":"e8a936a6-d886-4cfa-97b9-2d54209' + '053ff"}' + ) + ) + + ids = {} + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), ids) + adapter.handle_frame(TestDispatcher(adapter), frame) + + data = adapter.pop_message() + self.assertIsNot(data, None) + request = JsonRpcRequest.decode(data) + self.assertEquals(request.method, 'Host.getAllVmStats') + self.assertTrue(len(ids) == 1) + + def test_send_legacy(self): + dest = LEGACY_SUBSCRIPTION_ID_REQUEST + frame = Frame(command=Command.SEND, + headers={Headers.DESTINATION: dest, + Headers.REPLY_TO: 'jms.topic.vdsm_responses', + Headers.CONTENT_LENGTH: '103'}, + body=('{"jsonrpc":"2.0","method":"Host.getAllVmStats",' + '"params":{},"id":"e8a936a6-d886-4cfa-97b9-2d54209' + '053ff"}' + ) + ) + + ids = {} + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), ids) + adapter.handle_frame(TestDispatcher(adapter), frame) + + data = adapter.pop_message() + self.assertIsNot(data, None) + request = JsonRpcRequest.decode(data) + self.assertEquals(request.method, 'Host.getAllVmStats') + self.assertTrue(len(ids) == 1) + + def test_send_no_destination(self): + frame = Frame(Command.SEND, + {Headers.DESTINATION: 'jms.topic.unknown'}) + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), {}) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.ERROR) + self.assertEquals(resp_frame.body, 'Subscription not available') + + def test_send_batch(self): + body = ('[{"jsonrpc":"2.0","method":"Host.getAllVmStats","params":{},' + '"id":"e8a936a6-d886-4cfa-97b9-2d54209053ff"},' + '{"jsonrpc":"2.0","method":"Host.getAllVmStats","params":{},' + '"id":"1202b274-5a06-4671-8b13-1d2715429668"}]') + + frame = Frame(command=Command.SEND, + headers={Headers.DESTINATION: 'jms.topic.vdsm_requests', + Headers.REPLY_TO: 'jms.topic.vdsm_responses', + Headers.CONTENT_LENGTH: '209'}, + body=body + ) + + ids = {} + + adapter = StompAdapterImpl(Reactor(), defaultdict(list), ids) + adapter.handle_frame(TestDispatcher(adapter), frame) + + data = adapter.pop_message() + self.assertIsNot(data, None) + self.assertTrue(len(ids) == 2) + + def test_send_broker(self): + frame = Frame(command=Command.SEND, + headers={Headers.DESTINATION: 'jms.topic.destination', + Headers.CONTENT_LENGTH: '103'}, + body=('{"jsonrpc":"2.0","method":"Host.getAllVmStats",' + '"params":{},"id":"e8a936a6-d886-4cfa-97b9-2d54209' + '053ff"}' + ) + ) + + subscription = TestSubscription('jms.topic.destination', + 'e8a936a6-d886-4cfa-97b9-2d54209053ff') + + destinations = defaultdict(list) + destinations['jms.topic.destination'].append(subscription) + + adapter = StompAdapterImpl(Reactor(), destinations, {}) + subscription.set_client(adapter) + adapter.handle_frame(TestDispatcher(adapter), frame) + + resp_frame = adapter.pop_message() + self.assertEquals(resp_frame.command, Command.MESSAGE) -- To view, visit https://gerrit.ovirt.org/43342 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic22a8ec43413431ff57ecf66da62e065c534bac7 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>

5 22

Change in vdsm[master]: jsonrpc: ignore unknown message id
by Piotr Kliczewski 14 Oct '15

14 Oct '15

Piotr Kliczewski has uploaded a new change for review. Change subject: jsonrpc: ignore unknown message id ...................................................................... jsonrpc: ignore unknown message id Change-Id: Ide306f44c8c42d6661932698307efa4ce163e90e Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com> --- M lib/yajsonrpc/__init__.py 1 file changed, 5 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/43746/1 diff --git a/lib/yajsonrpc/__init__.py b/lib/yajsonrpc/__init__.py index 68af242..1dd1ae2 100644 --- a/lib/yajsonrpc/__init__.py +++ b/lib/yajsonrpc/__init__.py @@ -293,7 +293,11 @@ self._responses.append(response) def requestDone(self, response): - del self._requests[response.id] + try: + del self._requests[response.id] + except KeyError: + # ignore when request had no id + pass self.addResponse(response) self.sendReply() -- To view, visit https://gerrit.ovirt.org/43746 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ide306f44c8c42d6661932698307efa4ce163e90e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>

6 17

← Newer
1
...
20
21
22
23
24
25
26
...
67
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches September 2015