Change in vdsm[master]: Advertise aggregator ID in bonding interfaces
sshnaidm at redhat.com
sshnaidm at redhat.com
Mon Feb 22 12:00:35 UTC 2016
Sagi Shnaidman has posted comments on this change.
Change subject: Advertise aggregator ID in bonding interfaces
......................................................................
Patch Set 14:
(6 comments)
https://gerrit.ovirt.org/#/c/53100/14/tests/functional/networkTests.py
File tests/functional/networkTests.py:
Line 2836: with veth_pair() as (n1, n2), veth_pair() as (n3, n4):
: nics = [n1, n2, n3, n4]
: bonds = {BONDING_NAME: (n1, n3), BONDING_NAME + "0": (n2, n4)}
: for bond, pair in six.iteritems(bonds):
: bonding = {'nics': pair}
: bonding.update({'options': bond_options})
: status, msg = self.setupNetworks(
: {},
: {bond: bonding},
: NOCHK)
: self.assertEqual(status, SUCCESS, msg)
: self.assertBondExists(bond, pair)
:
: status, msg, info = self.vdsm_net.getVdsCapabilities()
> this preparation can be placed in a helper function, to be called by two se
Done
Line 2850: bonds
> If we can make the test more explicit and avoid parsing based on input stru
Done
Line 2852: nics
> Same here
Done
Line 2850: bond_info = [info['bondings'][b] for b in bonds]
Line 2851: bond_ag_macs = [b.get('ad_partner_mac') for b in bond_info]
Line 2852: nics_info = [info['nics'][nic] for nic in nics]
Line 2853:
Line 2854: if bond_options == 'mode=4 lacp_rate=1 miimon=100':
> If we have such differences inside the test due to the permutations, it wil
Done
Line 2855: macs = list(reversed([b['hwaddr'] for b in bond_info]))
Line 2856: for bond in bond_info:
Line 2857: self.assertIn('ad_aggregator_id', bond)
Line 2858: self.assertIn('ad_partner_mac', bond)
Line 2859: self.assertEqual(macs, bond_ag_macs)
> I would prefer to see what is compared here exactly, not as a group.
Done
Line 2865: self.assertNotIn('ad_aggregator_id', bond)
Line 2866: self.assertNotIn('ad_partner_mac', bond)
Line 2867: for nic_info in nics_info:
Line 2868: self.assertNotIn('ad_aggregator_id', nic_info)
Line 2869: for bond in bonds:
> What about having a context manager for creating/removing the bonds? Seems
I did some, but I plan to do it more generally in different commit and then to apply to all bond tests.
Line 2870: status, msg = self.setupNetworks(
Line 2871: {}, {bond: {'remove': True}}, NOCHK)
Line 2872: self.assertEqual(status, SUCCESS, msg)
--
To view, visit https://gerrit.ovirt.org/53100
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I85267967c9cb1b0a626d91cb1953361ed4de727a
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sagi Shnaidman <sshnaidm at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: Sagi Shnaidman <sshnaidm at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list