NFVBENCH-103 Add --hypervisor cli options and fix vm placement for multi-chain 03/63503/2 2.0.0
authorahothan <ahothan@cisco.com>
Sun, 14 Oct 2018 22:15:36 +0000 (15:15 -0700)
committerahothan <ahothan@cisco.com>
Sun, 14 Oct 2018 23:00:37 +0000 (16:00 -0700)
Change-Id: I80ab8b7c39221132ff43b95cd453dbfd4edd580c
Signed-off-by: ahothan <ahothan@cisco.com>
nfvbench/cfg.default.yaml
nfvbench/chain_runner.py
nfvbench/chaining.py
nfvbench/compute.py
nfvbench/nfvbench.py
nfvbench/traffic_gen/trex.py
pylint.rc
test/test_chains.py

index fdf40c6..bc40af4 100755 (executable)
@@ -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
index 0a2665d..c120501 100644 (file)
@@ -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
index e5a9f0a..a5ae680 100644 (file)
@@ -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' %
index 3f97166..d5a8119 100644 (file)
@@ -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
index 581206e..933d6fa 100644 (file)
@@ -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='<hypervisor name>',
+                        help='Where chains must run ("compute", "az:", "az:compute")')
+
     parser.add_argument('--l2-loopback', '--l2loopback', dest='l2_loopback',
                         action='store',
                         metavar='<vlan>',
@@ -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:
index 71b81c0..2f271aa 100644 (file)
@@ -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.
index a5be599..8f58824 100644 (file)
--- 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}.
index 519748b..e952eb8 100644 (file)
@@ -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]}},