Change in vdsm[master]: testValidation: Add LeakedProcessesPlugin
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: testValidation: Add LeakedProcessesPlugin
......................................................................
Patch Set 1:
(3 comments)
https://gerrit.ovirt.org/#/c/69324/1/tests/testValidation.py
File tests/testValidation.py:
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: import errno
Line 21: import os
Line 22: import json
time to tidy up import order?
Line 23: from nose.plugins.skip import SkipTest
Line 24: from functools import wraps
Line 25: from nose.plugins import Plugin
Line 26: import subprocess
Line 113: raise AssertionError("Test leaked child processes:\n" +
Line 114: json.dumps(info, indent=4))
Line 115:
Line 116: def _child_processes(self):
Line 117: cmd = ["pgrep", "-P", "%s" % os.getpid()]
stating a subprocess for each tests feels a bit wasteful to me.
Consider doing
subporcesses = set()
for pid in utils.iteratePids():
try:
if utils.pidStat(pid).ppid == self.PID:
subporcesses.add(utils.getCmdArgs(pid))
except EnvironmentError as e:
if e.errno != errno.ENOENT:
raise
subporcesses.add('terminated')
Line 118: proc = subprocess.Popen(cmd,
Line 119: stdin=None,
Line 120: stdout=subprocess.PIPE,
Line 121: stderr=subprocess.PIPE)
Line 128: % (proc.returncode, err))
Line 129: return frozenset(out.splitlines())
Line 130:
Line 131: def _process_info(self, pid):
Line 132: path = "/proc/%s/cmdline" % pid
we have vdsm.utils.getCmdArgs() for that.
Line 133: try:
Line 134: with open(path) as f:
Line 135: line = f.readline()
Line 136: except EnvironmentError as e:
--
To view, visit https://gerrit.ovirt.org/69324
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I451490d56f70ea8a6a685569d984495798e8b297
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 3 months
Change in vdsm[master]: net: reformat dhcp response data and always report it
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: net: reformat dhcp response data and always report it
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/69159
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa531e4697b0154849ef02c06100dde26c46b7eb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 3 months
Change in vdsm[master]: automation: resolving test output being printed twice
by Code Review
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: automation: resolving test output being printed twice
......................................................................
Patch Set 3:
do we need to backport to ovirt-4.1 ?
--
To view, visit https://gerrit.ovirt.org/69278
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9e2bb064f3fe2131225ec81bc9c1f1b653ed168
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 3 months
Change in vdsm[master]: automation: change check-patch repos file
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: automation: change check-patch repos file
......................................................................
automation: change check-patch repos file
changed lago repo url to contain $distro variable instead
of fc24.
Change-Id: I016caf43bb49cc689c83d56497644079c597e636
Signed-off-by: Irit Goihman <igoihman(a)redhat.com>
---
M automation/check-patch.repos.fc24
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Yaniv Bronhaim: Looks good to me, but someone else must approve
Jenkins CI: Passed CI tests
Irit Goihman: Verified
Dan Kenigsberg: Looks good to me, approved
--
To view, visit https://gerrit.ovirt.org/69239
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I016caf43bb49cc689c83d56497644079c597e636
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
7 years, 3 months
Change in vdsm[master]: automation: change check-patch repos file
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: automation: change check-patch repos file
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/69239
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I016caf43bb49cc689c83d56497644079c597e636
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 3 months
Change in vdsm[master]: automation: resolving test output being printed twice
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: automation: resolving test output being printed twice
......................................................................
automation: resolving test output being printed twice
currently test output in check-merged is being duplicated due to both
printing results on the fly and then printing the same results
after they're saved to a file with a cat call.
removing the cat call.
Change-Id: Ib9e2bb064f3fe2131225ec81bc9c1f1b653ed168
Signed-off-by: Leon Goldberg <lgoldber(a)redhat.com>
---
M automation/check-merged.sh
1 file changed, 0 insertions(+), 2 deletions(-)
Approvals:
Jenkins CI: Passed CI tests
Irit Goihman: Looks good to me, but someone else must approve
Dan Kenigsberg: Looks good to me, approved
Leon Goldberg: Verified
--
To view, visit https://gerrit.ovirt.org/69278
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib9e2bb064f3fe2131225ec81bc9c1f1b653ed168
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
7 years, 4 months
Change in vdsm[master]: automation: resolving test output being printed twice
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: automation: resolving test output being printed twice
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/69278
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9e2bb064f3fe2131225ec81bc9c1f1b653ed168
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 4 months
Change in vdsm[master]: net test: Fix test_ipv4_default_route default route assertion
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: net test: Fix test_ipv4_default_route default route assertion
......................................................................
net test: Fix test_ipv4_default_route default route assertion
On some functional test runs, the test fails on the default route
assertion, which checks that there is no default route at the end of
the test.
The checks is wrong, as it should actually check that the configured
default route is not the one on the setup.
The setup itself may have an original default route already or none.
Note: It seems that we do not (always) preserve the default route that
was originally set on the host. This should be fixed in future patches.
Change-Id: Iacfce818f8faa35d533bdd8c0d6a784b4fc026d1
Signed-off-by: Edward Haas <edwardh(a)redhat.com>
---
M tests/functional/networkTests.py
1 file changed, 4 insertions(+), 1 deletion(-)
Approvals:
Jenkins CI: Passed CI tests
Dan Kenigsberg: Looks good to me, approved
Edward Haas: Verified
--
To view, visit https://gerrit.ovirt.org/69291
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iacfce818f8faa35d533bdd8c0d6a784b4fc026d1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
7 years, 4 months
Change in vdsm[master]: net test: Fix test_ipv4_default_route default route assertion
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: net test: Fix test_ipv4_default_route default route assertion
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/69291
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacfce818f8faa35d533bdd8c0d6a784b4fc026d1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 4 months
Change in vdsm[master]: net func test: Existing NM test should cleanup even if it fails
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: net func test: Existing NM test should cleanup even if it fails
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/69276/1/tests/network/func_net_basic_test.py
File tests/network/func_net_basic_test.py:
Line 105: iface = iface_name()
Line 106: NET = {NETWORK_NAME: {'bonding': iface, 'switch': self.switch}}
Line 107: with nm_connections(iface, IPv4_ADDRESS, con_count=3):
Line 108: try:
Line 109: with self.setupNetworks(NET, {}, NOCHK):
shouldn't you open the try-block only here, after we are sure that the bond has been acquired? If setupNetwork explodes before it yields, we should not attempt to clean up.
Line 110: self.assertNetwork(NETWORK_NAME, NET[NETWORK_NAME])
Line 111: finally:
Line 112: # The bond was acquired, therefore VDSM needs to clean it.
Line 113: BONDREMOVE = {iface: {'remove': True}}
--
To view, visit https://gerrit.ovirt.org/69276
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If66640134404be54792cf69a22056b64a2c94b85
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 4 months