From c0ef57f8ec086c07053d529510992c869c30c9d2 Mon Sep 17 00:00:00 2001 From: ahothan Date: Sun, 14 Oct 2018 15:15:36 -0700 Subject: NFVBENCH-103 Add --hypervisor cli options and fix vm placement for multi-chain Change-Id: I80ab8b7c39221132ff43b95cd453dbfd4edd580c Signed-off-by: ahothan --- nfvbench/cfg.default.yaml | 6 +- nfvbench/chain_runner.py | 6 +- nfvbench/chaining.py | 151 +++++++++++++++++++++++++++++++++++++++---- nfvbench/compute.py | 128 +++++------------------------------- nfvbench/nfvbench.py | 8 +++ nfvbench/traffic_gen/trex.py | 2 +- pylint.rc | 2 +- test/test_chains.py | 52 ++++++++++++--- 8 files changed, 214 insertions(+), 141 deletions(-) diff --git a/nfvbench/cfg.default.yaml b/nfvbench/cfg.default.yaml index fdf40c6..bc40af4 100755 --- a/nfvbench/cfg.default.yaml +++ b/nfvbench/cfg.default.yaml @@ -65,11 +65,11 @@ flavor: # Name of the availability zone to use for the test VMs # Must be one of the zones listed by 'nova availability-zone-list' -# If the selected zone contains only 1 compute node and PVVP inter-node flow is selected, -# application will use intra-node PVVP flow. -# List of compute nodes can be specified, must be in given availability zone if not empty # availability_zone: 'nova' availability_zone: +# To force placement on a given hypervisor, set the name here +# (if multiple names are provided, the first will be used) +# Leave empty to let openstack pick the hypervisor compute_nodes: # Type of service chain to run, possible options are PVP, PVVP and EXT diff --git a/nfvbench/chain_runner.py b/nfvbench/chain_runner.py index 0a2665d..c120501 100644 --- a/nfvbench/chain_runner.py +++ b/nfvbench/chain_runner.py @@ -147,9 +147,11 @@ class ChainRunner(object): return: the results of the benchmark as a dict """ - LOG.info('Starting %s chain...', self.chain_name) - results = {} + if self.config.no_traffic: + return results + + LOG.info('Starting %dx%s benchmark...', self.config.service_chain_count, self.chain_name) self.__setup_traffic() # now that the dest MAC for all VNFs is known in all cases, it is time to create # workers as they might be needed to extract stats prior to sending traffic diff --git a/nfvbench/chaining.py b/nfvbench/chaining.py index e5a9f0a..a5ae680 100644 --- a/nfvbench/chaining.py +++ b/nfvbench/chaining.py @@ -399,8 +399,7 @@ class ChainVnf(object): self._reuse_exception('Left network mismatch') if networks[RIGHT].name not in instance.networks: self._reuse_exception('Right network mismatch') - # Other checks not performed (yet) - # check if az and compute node match + self.reuse = True self.instance = instance LOG.info('Reusing existing instance %s on %s', @@ -413,17 +412,13 @@ class ChainVnf(object): # if no reuse, actual vm creation is deferred after all ports in the chain are created # since we need to know the next mac in a multi-vnf chain - def get_az(self): - """Get the AZ associated to this VNF.""" - return self.manager.az[0] - def create_vnf(self, remote_mac_pair): """Create the VNF instance if it does not already exist.""" if self.instance is None: port_ids = [{'port-id': vnf_port.port['id']} for vnf_port in self.ports] vm_config = self._get_vm_config(remote_mac_pair) - az = self.get_az() + az = self.manager.placer.get_required_az() server = self.manager.comp.create_server(self.name, self.manager.image_instance, self.manager.flavor.flavor, @@ -435,8 +430,38 @@ class ChainVnf(object): config_drive=True, files={NFVBENCH_CFG_VM_PATHNAME: vm_config}) if server: - LOG.info('Created instance %s on %s', self.name, az) self.instance = server + if self.manager.placer.is_resolved(): + LOG.info('Created instance %s on %s', self.name, az) + else: + # the location is undetermined at this point + # self.get_hypervisor_name() will return None + LOG.info('Created instance %s - waiting for placement resolution...', self.name) + # here we MUST wait until this instance is resolved otherwise subsequent + # VNF creation can be placed in other hypervisors! + config = self.manager.config + max_retries = (config.check_traffic_time_sec + + config.generic_poll_sec - 1) / config.generic_poll_sec + retry = 0 + for retry in range(max_retries): + status = self.get_status() + if status == 'ACTIVE': + hyp_name = self.get_hypervisor_name() + LOG.info('Instance %s is active and has been placed on %s', + self.name, hyp_name) + self.manager.placer.register_full_name(hyp_name) + break + if status == 'ERROR': + raise ChainException('Instance %s creation error: %s' % + (self.name, + self.instance.fault['message'])) + LOG.info('Waiting for instance %s to become active (retry %d/%d)...', + self.name, retry + 1, max_retries + 1) + time.sleep(config.generic_poll_sec) + else: + # timing out + LOG.error('Instance %s creation timed out', self.name) + raise ChainException('Instance %s creation timed out' % self.name) self.reuse = False else: raise ChainException('Unable to create instance: %s' % (self.name)) @@ -514,6 +539,9 @@ class Chain(object): self.instances.append(ChainVnf(self, chain_instance_index, self.networks)) + # at this point new VNFs are not created yet but + # verify that all discovered VNFs are on the same hypervisor + self._check_hypervisors() # now that all VNF ports are created we need to calculate the # left/right remote MAC for each VNF in the chain # before actually creating the VNF itself @@ -525,6 +553,25 @@ class Chain(object): self.delete() raise + def _check_hypervisors(self): + common_hypervisor = None + for instance in self.instances: + # get the full hypervizor name (az:compute) + hname = instance.get_hypervisor_name() + if hname: + if common_hypervisor: + if hname != common_hypervisor: + raise ChainException('Discovered instances on different hypervisors:' + ' %s and %s' % (hname, common_hypervisor)) + else: + common_hypervisor = hname + if common_hypervisor: + # check that the common hypervisor name matchs the requested hypervisor name + # and set the name to be used by all future instances (if any) + if not self.manager.placer.register_full_name(common_hypervisor): + raise ChainException('Discovered hypervisor placement %s is incompatible' % + common_hypervisor) + def get_length(self): """Get the number of VNF in the chain.""" return len(self.networks) - 1 @@ -629,6 +676,84 @@ class Chain(object): network.delete() +class InstancePlacer(object): + """A class to manage instance placement for all VNFs in all chains. + + A full az string is made of 2 parts AZ and hypervisor. + The placement is resolved when both parts az and hypervisor names are known. + """ + + def __init__(self, req_az, req_hyp): + """Create a new instance placer. + + req_az: requested AZ (can be None or empty if no preference) + req_hyp: requested hypervisor name (can be None of empty if no preference) + can be any of 'nova:', 'comp1', 'nova:comp1' + if it is a list, only the first item is used (backward compatibility in config) + + req_az is ignored if req_hyp has an az part + all other parts beyond the first 2 are ignored in req_hyp + """ + # if passed a list just pick the first item + if req_hyp and isinstance(req_hyp, list): + req_hyp = req_hyp[0] + # only pick first part of az + if req_az and ':' in req_az: + req_az = req_az.split(':')[0] + if req_hyp: + # check if requested hypervisor string has an AZ part + split_hyp = req_hyp.split(':') + if len(split_hyp) > 1: + # override the AZ part and hypervisor part + req_az = split_hyp[0] + req_hyp = split_hyp[1] + self.requested_az = req_az if req_az else '' + self.requested_hyp = req_hyp if req_hyp else '' + # Nova can accept AZ only (e.g. 'nova:', use any hypervisor in that AZ) + # or hypervisor only (e.g. ':comp1') + # or both (e.g. 'nova:comp1') + if req_az: + self.required_az = req_az + ':' + self.requested_hyp + else: + # no ":" needed + self.required_az = self.requested_hyp if req_hyp else '' + # placement is resolved when both AZ and hypervisor names are known and set + self.resolved = self.requested_az != '' and self.requested_hyp != '' + + def get_required_az(self): + """Return the required az (can be resolved or not).""" + return self.required_az + + def register_full_name(self, discovered_az): + """Verify compatibility and register a discovered hypervisor full name. + + discovered_az: a discovered AZ in az:hypervisor format + return: True if discovered_az is compatible and set + False if discovered_az is not compatible + """ + if self.resolved: + return discovered_az == self.required_az + + # must be in full az format + split_daz = discovered_az.split(':') + if len(split_daz) != 2: + return False + if self.requested_az and self.requested_az != split_daz[0]: + return False + if self.requested_hyp and self.requested_hyp != split_daz[1]: + return False + self.required_az = discovered_az + self.resolved = True + return True + + def is_resolved(self): + """Check if the full AZ is resolved. + + return: True if resolved + """ + return self.resolved + + class ChainManager(object): """A class for managing all chains for a given run. @@ -663,6 +788,7 @@ class ChainManager(object): config = self.config self.openstack = (chain_runner.cred is not None) and not config.l2_loopback self.chain_count = config.service_chain_count + self.az = None if self.openstack: # openstack only session = chain_runner.cred.get_session() @@ -672,14 +798,9 @@ class ChainManager(object): self.comp = compute.Compute(self.nova_client, self.glance_client, config) - self.az = None try: if config.service_chain != ChainType.EXT: - # we need to find 1 hypervisor - az_list = self.comp.get_enabled_az_host_list(1) - if not az_list: - raise ChainException('No matching hypervisor found') - self.az = az_list + self.placer = InstancePlacer(config.availability_zone, config.compute_nodes) self._setup_image() self.flavor = ChainFlavor(config.flavor_type, config.flavor, self.comp) # Get list of all existing instances to check if some instances can be reused @@ -785,6 +906,8 @@ class ChainManager(object): for instance in instances: status = instance.get_status() if status == 'ACTIVE': + LOG.info('Instance %s is ACTIVE on %s', + instance.name, instance.get_hypervisor_name()) continue if status == 'ERROR': raise ChainException('Instance %s creation error: %s' % diff --git a/nfvbench/compute.py b/nfvbench/compute.py index 3f97166..d5a8119 100644 --- a/nfvbench/compute.py +++ b/nfvbench/compute.py @@ -11,6 +11,8 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +"""Module to interface with nova and glance.""" + import time import traceback @@ -26,12 +28,16 @@ from log import LOG class Compute(object): + """Class to interface with nova and glance.""" + def __init__(self, nova_client, glance_client, config): + """Create a new compute instance to interact with nova and glance.""" self.novaclient = nova_client self.glance_client = glance_client self.config = config def find_image(self, image_name): + """Find an image by name.""" try: return next(self.glance_client.images.list(filters={'name': image_name}), None) except (novaclient.exceptions.NotFound, keystoneauth1.exceptions.http.NotFound, @@ -78,6 +84,7 @@ class Compute(object): return True def delete_image(self, img_name): + """Delete an image by name.""" try: LOG.log("Deleting image %s...", img_name) img = self.find_image(image_name=img_name) @@ -93,7 +100,7 @@ class Compute(object): def create_server(self, vmname, image, flavor, key_name, nic, sec_group, avail_zone=None, user_data=None, config_drive=None, files=None): - + """Create a new server.""" if sec_group: security_groups = [sec_group['id']] else: @@ -113,16 +120,20 @@ class Compute(object): return instance def poll_server(self, instance): + """Poll a server from its reference.""" return self.novaclient.servers.get(instance.id) def get_server_list(self): + """Get the list of all servers.""" servers_list = self.novaclient.servers.list() return servers_list def delete_server(self, server): + """Delete a server from its object reference.""" self.novaclient.servers.delete(server) def find_flavor(self, flavor_type): + """Find a flavor by name.""" try: flavor = self.novaclient.flavors.find(name=flavor_type) return flavor @@ -130,122 +141,15 @@ class Compute(object): return None def create_flavor(self, name, ram, vcpus, disk, ephemeral=0): + """Create a flavor.""" return self.novaclient.flavors.create(name=name, ram=ram, vcpus=vcpus, disk=disk, ephemeral=ephemeral) - def normalize_az_host(self, az, host): - if not az: - az = self.config.availability_zone - return az + ':' + host - - def auto_fill_az(self, host_list, host): - """Auto fill az:host. - - no az provided, if there is a host list we can auto-fill the az - else we use the configured az if available - else we return an error - """ - if host_list: - for hyp in host_list: - if hyp.host == host: - return self.normalize_az_host(hyp.zone, host) - # no match on host - LOG.error('Passed host name does not exist: %s', host) - return None - if self.config.availability_zone: - return self.normalize_az_host(None, host) - LOG.error('--hypervisor passed without an az and no az configured') - return None - - def sanitize_az_host(self, host_list, az_host): - """Sanitize the az:host string. + def get_hypervisor(self, hyper_name): + """Get the hypervisor from its name. - host_list: list of hosts as retrieved from openstack (can be empty) - az_host: either a host or a az:host string - if a host, will check host is in the list, find the corresponding az and - return az:host - if az:host is passed will check the host is in the list and az matches - if host_list is empty, will return the configured az if there is no - az passed - """ - if ':' in az_host: - # no host_list, return as is (no check) - if not host_list: - return az_host - # if there is a host_list, extract and verify the az and host - az_host_list = az_host.split(':') - zone = az_host_list[0] - host = az_host_list[1] - for hyp in host_list: - if hyp.host == host: - if hyp.zone == zone: - # matches - return az_host - # else continue - another zone with same host name? - # no match - LOG.error('No match for availability zone and host %s', az_host) - return None - else: - return self.auto_fill_az(host_list, az_host) - - # - # Return a list of 0, 1 or 2 az:host - # - # The list is computed as follows: - # The list of all hosts is retrieved first from openstack - # if this fails, checks and az auto-fill are disabled - # - # If the user provides a configured az name (config.availability_zone) - # up to the first 2 hosts from the list that match the az are returned - # - # If the user did not configure an az name - # up to the first 2 hosts from the list are returned - # Possible return values: - # [ az ] - # [ az:hyp ] - # [ az1:hyp1, az2:hyp2 ] - # [] if an error occurred (error message printed to console) - # - def get_enabled_az_host_list(self, required_count=1): - """Check which hypervisors are enabled and on which compute nodes they are running. - - Pick up to the required count of hosts (can be less or zero) - - :param required_count: count of compute-nodes to return - :return: list of enabled available compute nodes + Can raise novaclient.exceptions.NotFound """ - host_list = [] - hypervisor_list = [] - - try: - hypervisor_list = self.novaclient.hypervisors.list() - host_list = self.novaclient.services.list() - except novaclient.exceptions.Forbidden: - LOG.warning('Operation Forbidden: could not retrieve list of hypervisors' - ' (likely no permission)') - - hypervisor_list = [h for h in hypervisor_list if h.status == 'enabled' and h.state == 'up'] - if self.config.availability_zone: - host_list = [h for h in host_list if h.zone == self.config.availability_zone] - - if self.config.compute_nodes: - host_list = [h for h in host_list if h.host in self.config.compute_nodes] - - hosts = [h.hypervisor_hostname for h in hypervisor_list] - host_list = [h for h in host_list if h.host in hosts] - - avail_list = [] - for host in host_list: - candidate = self.normalize_az_host(host.zone, host.host) - if candidate: - avail_list.append(candidate) - if len(avail_list) == required_count: - return avail_list - - return avail_list - - def get_hypervisor(self, hyper_name): - # can raise novaclient.exceptions.NotFound # first get the id from name hyper = self.novaclient.hypervisors.search(hyper_name)[0] # get full hypervisor object diff --git a/nfvbench/nfvbench.py b/nfvbench/nfvbench.py index 581206e..933d6fa 100644 --- a/nfvbench/nfvbench.py +++ b/nfvbench/nfvbench.py @@ -383,6 +383,11 @@ def parse_opts_from_cli(): action='store', help='Custom label for performance records') + parser.add_argument('--hypervisor', dest='hypervisor', + action='store', + metavar='', + help='Where chains must run ("compute", "az:", "az:compute")') + parser.add_argument('--l2-loopback', '--l2loopback', dest='l2_loopback', action='store', metavar='', @@ -520,6 +525,9 @@ def main(): config.service_chain_count = opts.service_chain_count if opts.no_vswitch_access: config.no_vswitch_access = opts.no_vswitch_access + if opts.hypervisor: + # can be any of 'comp1', 'nova:', 'nova:comp1' + config.compute_nodes = opts.hypervisor # port to port loopback (direct or through switch) if opts.l2_loopback: diff --git a/nfvbench/traffic_gen/trex.py b/nfvbench/traffic_gen/trex.py index 71b81c0..2f271aa 100644 --- a/nfvbench/traffic_gen/trex.py +++ b/nfvbench/traffic_gen/trex.py @@ -72,7 +72,7 @@ class TRex(AbstractTrafficGenerator): def get_version(self): """Get the Trex version.""" - return self.client.get_server_version() + return self.client.get_server_version() if self.client else '' def get_pg_id(self, port, chain_id): """Calculate the packet group IDs to use for a given port/stream type/chain_id. diff --git a/pylint.rc b/pylint.rc index a5be599..8f58824 100644 --- a/pylint.rc +++ b/pylint.rc @@ -197,7 +197,7 @@ indent-string=' ' max-line-length=100 # Maximum number of lines in a module -max-module-lines=1000 +max-module-lines=1200 # List of optional constructs for which whitespace checking is disabled. `dict- # separator` is used to allow tabulation in dicts, etc.: {1 : 1,\n222: 2}. diff --git a/test/test_chains.py b/test/test_chains.py index 519748b..e952eb8 100644 --- a/test/test_chains.py +++ b/test/test_chains.py @@ -19,9 +19,11 @@ from mock_trex import no_op from mock import MagicMock from mock import patch +import pytest from nfvbench.chain_runner import ChainRunner from nfvbench.chaining import ChainVnfPort +from nfvbench.chaining import InstancePlacer from nfvbench.compute import Compute import nfvbench.credentials from nfvbench.factory import BasicFactory @@ -37,6 +39,7 @@ from nfvbench.traffic_client import TrafficClient from nfvbench.traffic_gen.traffic_base import Latency from nfvbench.traffic_gen.trex import TRex + # just to get rid of the unused function warning no_op() @@ -78,13 +81,9 @@ def test_chain_runner_ext_no_openstack(): runner = ChainRunner(config, None, specs, BasicFactory()) runner.close() -def _mock_get_enabled_az_host_list(self, required_count=1): - return ['nova:c1', 'nova:c2'] - def _mock_find_image(self, image_name): return True -@patch.object(Compute, 'get_enabled_az_host_list', _mock_get_enabled_az_host_list) @patch.object(Compute, 'find_image', _mock_find_image) @patch('nfvbench.chaining.Client') @patch('nfvbench.chaining.neutronclient') @@ -112,7 +111,6 @@ def test_pvp_chain_runner(): config = _get_chain_config(sc, scc, shared_net) _test_pvp_chain(config, cred) -@patch.object(Compute, 'get_enabled_az_host_list', _mock_get_enabled_az_host_list) @patch.object(Compute, 'find_image', _mock_find_image) @patch('nfvbench.chaining.Client') @patch('nfvbench.chaining.neutronclient') @@ -168,7 +166,6 @@ def _mock_get_mac(dummy): mac_seq += 1 return '01:00:00:00:00:%02x' % mac_seq -@patch.object(Compute, 'get_enabled_az_host_list', _mock_get_enabled_az_host_list) @patch.object(Compute, 'find_image', _mock_find_image) @patch.object(TrafficClient, 'skip_sleep', lambda x: True) @patch.object(ChainVnfPort, 'get_mac', _mock_get_mac) @@ -186,7 +183,6 @@ def test_nfvbench_run(mock_cred, mock_glance, mock_neutron, mock_client): mock_neutron.Client.return_value.list_networks.return_value = {'networks': None} _check_nfvbench_openstack() -@patch.object(Compute, 'get_enabled_az_host_list', _mock_get_enabled_az_host_list) @patch.object(Compute, 'find_image', _mock_find_image) @patch.object(TrafficClient, 'skip_sleep', lambda x: True) @patch('nfvbench.chaining.Client') @@ -202,7 +198,6 @@ def test_nfvbench_ext_arp(mock_cred, mock_glance, mock_neutron, mock_client): mock_neutron.Client.return_value.list_networks.return_value = {'networks': [netw]} _check_nfvbench_openstack(sc=ChainType.EXT) -@patch.object(Compute, 'get_enabled_az_host_list', _mock_get_enabled_az_host_list) @patch.object(Compute, 'find_image', _mock_find_image) @patch.object(TrafficClient, 'skip_sleep', lambda x: True) @patch('nfvbench.chaining.Client') @@ -282,6 +277,47 @@ def test_trex_streams_stats(): assert if_stats[1].tx == CH1_P1_TX + LCH1_P1_TX assert if_stats[1].rx == CH1_P1_RX + LCH1_P1_RX +def check_placer(az, hyp, req_az, resolved=False): + """Combine multiple combinatoons of placer tests.""" + placer = InstancePlacer(az, hyp) + assert placer.is_resolved() == resolved + assert placer.get_required_az() == req_az + assert placer.register_full_name('nova:comp1') + assert placer.is_resolved() + assert placer.get_required_az() == 'nova:comp1' + +def test_placer_no_user_pref(): + """Test placement when user does not provide any preference.""" + check_placer(None, None, '') + +def test_placer_user_az(): + """Test placement when user only provides an az.""" + check_placer('nova', None, 'nova:') + check_placer(None, 'nova:', 'nova:') + check_placer('nebula', 'nova:', 'nova:') + +def test_placer_user_hyp(): + """Test placement when user provides a hypervisor.""" + check_placer(None, 'comp1', 'comp1') + check_placer('nova', 'comp1', 'nova:comp1', resolved=True) + check_placer(None, 'nova:comp1', 'nova:comp1', resolved=True) + # hyp overrides az + check_placer('nebula', 'nova:comp1', 'nova:comp1', resolved=True) + # also check for cases of extra parts (more than 1 ':') + check_placer('nova:nebula', 'comp1', 'nova:comp1', resolved=True) + + +def test_placer_negative(): + """Run negative tests on placer.""" + # AZ mismatch + with pytest.raises(Exception): + placer = InstancePlacer('nova', None) + placer.register('nebula:comp1') + # comp mismatch + with pytest.raises(Exception): + placer = InstancePlacer(None, 'comp1') + placer.register('nebula:comp2') + # without total, with total and only 2 col CHAIN_STATS = [{0: {'packets': [2000054, 1999996, 1999996]}}, -- cgit 1.2.3-korg