dpdk-bond-mapping tests for DPDKTest bundle#1342
Conversation
PanosKostopoulos
changed the title
| logging.info( | ||
| 'Verifying bond PCI addresses are whitelisted in dpdk-extra') | ||
| self._assert_dpdk_bond_pci_in_eal_args(bond_pci_addresses) | ||
|
|
There was a problem hiding this comment.
The PCI address is put in EAL args regardless of the use of dpdk-bond-mappings configuration option.
How is this a valid assertion?
| # self.disable_hugepages_vfio_on_hvs_in_vms() | ||
| # self._ovs_br_ex_port_is_system_interface() | ||
|
|
||
| def _test_dpdk_bond_mappings_pci_format(self): |
There was a problem hiding this comment.
Make this a separate def test_XXX method instead of this covert piggy-backing on existing tests.
The charm code will only actually populate the OVS database with bond configuration when there are more than one interface [0], so you likely need to enable the test harness to add multiple ports to the test VM as well as interrogate the environment and ensure there are enough ports available to run the test.
| """ | ||
| bond_pci_addresses = [] | ||
| for unit in zaza.model.get_units(self.application_name): | ||
| pci = self._get_br_ex_dpdk_pci_address(unit) |
There was a problem hiding this comment.
Basing input for your test assertion on data provided by the subject under test is not the safe thing to do, you should have the test code independently discover PCI addresses and/or MACs to use as input for the test so that you can accurately measure the output of the program.
| logging.warning( | ||
| 'Could not retrieve any PCI addresses for bond mapping test, ' | ||
| 'skipping') | ||
| return |
There was a problem hiding this comment.
This does not look safe, we'd likely want to know if this failed by having the test fail.
PanosKostopoulos
force-pushed
the
master
branch
17 times, most recently
from
4019259 to
68c32da
Compare
| logging.info('and is not in error.') | ||
| self._ovs_br_ex_interface_not_in_error() | ||
| instance_mac_map = self._add_second_interface() | ||
| self.bind_second_interface_to_dpdk(instance_mac_map) |
There was a problem hiding this comment.
Ensuring devices mentioned in the charm configuration are bound to a driver appropriate for use by DPDK is the responsibility of the charm.
The charm does this by updating /etc/dpdk/interfaces and restarting the dpdk service:
- https://github.com/openstack-charmers/charm-layer-ovn/blob/b0cee1d3b1bf7e7fc4236d9b2057419d3411771b/lib/charms/ovn_charm.py#L617-L627
- https://github.com/openstack-charmers/charm-layer-ovn/blob/master/templates/etc_dpdk_interfaces
If this is not happening for you, I'd consider if configuration change is happening in the correct order or whether you have found an actual charm bug.
PanosKostopoulos
force-pushed
the
master
branch
10 times, most recently
from
6ef7125 to
52af6db
Compare
PanosKostopoulos
force-pushed
the
master
branch
15 times, most recently
from
4824e39 to
a5869e1
Compare
PanosKostopoulos
force-pushed
the
master
branch
5 times, most recently
from
86c5c4f to
f40eafe
Compare
| if not DPDKTest._hugepages_configured: | ||
| self.enable_hugepages_vfio_on_hvs_in_vms(4) | ||
| DPDKTest._hugepages_configured = True |
There was a problem hiding this comment.
Use setUpClass / tearDownClass instead, which is designed for execution of once per class activities.
There was a problem hiding this comment.
I tried using setUpClass in the beginning since it was what we discussed. The issue is that enable_hugepages_vfio_on_hvs_in_vms is a self method, so in the setUpClass(cls) I do not have access to it. I could create a temp instance but I do not know if that is correct. Hence the use of SetUp(self). Am I missing something here.
There was a problem hiding this comment.
Looking at enable_hugepages_vfio_on_hvs_in_vms, the only variables prefixed with self is model_name and test_config.
Both of these are already defined as class level variables in the parent setUpClass so they should be easily accessible.
Would passing cls as instance argument to the instance methods work directly perhaps?
| def _add_second_interface(self): | ||
| """Add a second interface eth2 to networking charm units. | ||
|
|
||
| :returns: list of mac addresses(represented as strings). | ||
| """ | ||
|
|
||
| networking_data = openstack_utils.get_charm_networking_data() | ||
| ifname="eth2" | ||
| macs=[] | ||
| for instance in networking_data.unit_machine_ids: | ||
| openstack_utils.lxd_maybe_add_nic(instance, ifname, | ||
| "third") | ||
| macs.append(str(openstack_utils.lxd_get_nic_hwaddr(instance, ifname))) | ||
|
|
||
| if macs: | ||
| openstack_utils.configure_networking_charms( | ||
| networking_data, macs, use_juju_wait=False) | ||
| return macs |
There was a problem hiding this comment.
The act of adding an interface to test instances should be a deliberate action through configure / setup steps.
Either make zaza.openstack.charm_tests.neutron.setup.undercloud_and_charm_setup do what you want on the back of configuration influenced by environment variables, or add a new configure function for this purpose.
The use of the word second is also misleading as the instances likely already have two interfaces.
| if port_name == "dpdk-bond0": | ||
| if len(interfaces) == 2: | ||
| logging.info("DPDK bond was successfully created with 2 interfaces") | ||
| return | ||
| else: | ||
| logging.info("BOND FAILED") | ||
| raise Exception('Could not create DPDK bond with 2 interfaces') | ||
| return |
There was a problem hiding this comment.
Replace this is one of the unittest assert methods: https://docs.python.org/3/library/unittest.html#assert-methods
PanosKostopoulos
force-pushed
the
master
branch
7 times, most recently
from
2dd238c to
6c59083
Compare
Adding tests for both mac and pci format of dpdk-bond-mappings config option. Aslo adding a third interface to the chassis units, in order to be able to create a bond. Signed-off-by: Panos Kostopoulos Kyrimis <panos.kostopouloskyrimis@canonical.com>
2 participants
DPDK tests for dpdk-bond-mappings ovn-chassis config option. This first iteration includes only PCI addresses, but should include also MAC addresses (the difference is there should be an extra lookup to get the PCI addresses from the mac).
The tests have extended the test_enable_dpdk function, in this first iteration, to avoid a second setup for dpdk. This could be moved (and maybe should) in a seperate test function.