Merge lp:~majduk/charms/trusty/neutron-contrail/trunk into lp:~sdn-charmers/charms/trusty/neutron-contrail/trunk

[Michał Ajduk]

Changelog:
  Allow changing dpdk_coremask, dpdk_mempool_size, dpdk_txd_size, dpdk_rxd_size using config-changed. Always restart supervisor-vrouter service as per Juniper recommen
  dation.
  Set kernel CPU frequency scaling_governor to performance. This is required on DPDK enabled nodes.
      Add custom restart function for vrouter. DPDK vRouter does require i40e driver to be reloaded as it is not correctly reassigned to the interfaces after vRouter stop.
  Render vrouter agent configuration (and in effect restart vrouter) only when configuration is complete. Incomplete configuration causes vrouter to deconfigure interf
  aces and fail to start.
      Store vhost0 interface IP, prefix length and gateway in config. This prevents issue where IP or Gateway is lost when config-changed is ran during vrouter restart or before vrouter was first started after the install
  Update vhost-phy, vhost-phy-ip and vhost-phy-gw on charm upgrade
      Add parameters for vrouter: dpdk-mempool-size, dpdk-txd-size, dpdk-rxd-size.
      Allow reconfiguration of dpdk-coremask in config-changed
  Use Contrail provided version of nova-*, libvirt-* and qemu-*
  Present vrouter status in juju to simplify debugging and quicker service status overview
  Setup jumboframe support

[Ante Karamatić (ivoks)]

See comments in line.

review: Needs Fixing

[Michał Ajduk (majduk)]

Replies inline.

[Ante Karamatić (ivoks)]

LGTM

review: Approve

=== modified file 'config.yaml'
--- config.yaml	2017-09-20 19:59:00 +0000
+++ config.yaml	2018-07-05 12:51:18 +0000
@@ -83,6 +83,16 @@
       Value can be specified as either a hexidecimal bitmask e.g. 0xFFFFFFFF or
       as a numbered list separated by commas e.g. 0,1 (ranges are also supported
       using '-' e.g. 0-2).
+  dpdk-mempool-size:
+    type: int
+    default: 65536
+    description: vrouter packet pool size
+  dpdk-txd-size:
+    type: int
+    description: vrouter TX packet descriptor pool size
+  dpdk-rxd-size:
+    type: int
+    description: vrouter RX packet descriptor pool size
   virtual-gateways:
     type: string
     description: |
 
=== modified file 'files/40contrail'
--- files/40contrail	2017-07-17 05:03:53 +0000
+++ files/40contrail	2018-07-05 12:51:18 +0000
@@ -1,3 +1,13 @@
+Explanation: Use contrail versions of libvirt packages
+Package: libvirt*
+Pin: version /contrail/
+Pin-Priority: 1001
+
+Explanation: Use contrail versions of qemu packages
+Package: qemu-*
+Pin: version /contrail/
+Pin-Priority: 1001
+
 Explanation: Use contrail versions of nova packages
 Package: nova-*
 Pin: version /contrail/
 
=== modified file 'hooks/neutron_contrail_hooks.py'
--- hooks/neutron_contrail_hooks.py	2018-03-21 19:34:17 +0000
+++ hooks/neutron_contrail_hooks.py	2018-07-05 12:51:18 +0000
@@ -40,6 +40,7 @@
     leader_get,
     leader_set,
     log,
+    status_set,
     relation_get,
     relation_ids,
     relation_set
@@ -49,7 +50,8 @@
     get_total_ram,
     restart_on_change,
     service_restart,
-    service_start
+    service_start,
+    service_stop
 )
 
 import charmhelpers.core.sysctl as sysctl
@@ -84,6 +86,9 @@
     provision_vrouter,
     reboot,
     set_dpdk_coremask,
+    set_dpdk_mempool_size,
+    set_dpdk_txd_size,
+    set_dpdk_rxd_size,
     set_huge_pages_parameters,
     units,
     unprovision_local_metadata,
@@ -93,7 +98,11 @@
     write_nodemgr_config,
     write_vnc_api_config,
     write_vrouter_config,
-    write_vrouter_vgw_interfaces
+    write_vrouter_vgw_interfaces,
+    is_vrouter_running,
+    vhost_gateway,
+    vhost_ip,
+    apply_configuration
 )
 
 PACKAGES = [ "contrail-utils", "python-jinja2", "python-netifaces",
@@ -103,8 +112,6 @@
                   "contrail-vrouter-dpdk-init", "contrail-vrouter-agent",
                   "contrail-vrouter-common" ]
 
-PACKAGES_DPDK_DEPS = [ "dpdk-depends-packages" ]
-
 PACKAGES_LBAAS = [ "python-barbicanclient", "haproxy" ]
 
 PACKAGES_VROUTER = [ "contrail-vrouter-dkms", "contrail-vrouter-agent",
@@ -114,6 +121,13 @@
                          "contrail-nova-vif", "contrail-vrouter-init",
                          "contrail-vrouter-common" ]
 
+PACKAGES_LIBVIRT = [ "libvirt-bin", "libvirt0" ]
+
+PACKAGES_QEMU = [ "qemu-block-extra","qemu-system","qemu-system-arm",
+                  "qemu-system-common","qemu-system-mips","qemu-system-misc",
+                  "qemu-system-ppc","qemu-system-sparc","qemu-system-x86",
+                  "qemu-utils" ]
+
 hooks = Hooks()
 config = config()
 
@@ -150,6 +164,13 @@
         unprovision_vrouter()
         config["vrouter-provisioned"] = False
 
[e-mail]('update-status')
+def update_status():
+    if is_vrouter_running():
+        status_set('active',"vrouter is running")
+    else:
+        status_set('blocked',"vrouter is not running")
+
 @hooks.hook("config-changed")
 def config_changed():
     configure_huge_pages()
@@ -282,8 +303,11 @@
         check_local_metadata()
     contrail_discovery_relation()
 
-@restart_on_change({"/etc/contrail/contrail-vrouter-agent.conf": ["contrail-vrouter-agent"],
-                    "/etc/contrail/contrail-vrouter-nodemgr.conf": ["contrail-vrouter-nodemgr"]})
+@restart_on_change({"/etc/contrail/contrail-vrouter-agent.conf": ["supervisor-vrouter"],
+                    "/etc/contrail/contrail-vrouter-nodemgr.conf": ["supervisor-vrouter"]},
+                   False,
+                   {"supervisor-vrouter": apply_configuration}
+                   )
 def contrail_discovery_relation():
     write_vrouter_config()
     write_nodemgr_config()
@@ -306,7 +330,11 @@
     check_vrouter()
     check_local_metadata()
 
-@restart_on_change({"/etc/contrail/contrail-vrouter-agent.conf": ["contrail-vrouter-agent"]})
+@restart_on_change({"/etc/contrail/contrail-vrouter-agent.conf": ["supervisor-vrouter"],
+                    "/etc/contrail/contrail-vrouter-nodemgr.conf": ["supervisor-vrouter"]},
+                   False,
+                   {"supervisor-vrouter": apply_configuration}
+                   )
 def control_node_relation():
     write_vrouter_config()
 
@@ -346,7 +374,7 @@
 @hooks.hook()
 def install():
     configure_sources(True, "install-sources", "install-keys")
-    apt_upgrade(fatal=True, dist=True)
+    apt_upgrade(options=["--yes", "--force-yes"],  fatal=True, dist=True)
     fix_vrouter_scripts() # bug in 2.0+20141015.1 packages
     apt_install(PACKAGES, fatal=True)
     utils.CONTRAIL_VERSION = dpkg_version("contrail-utils")
@@ -361,19 +389,24 @@
         # install lbaas packages
         apt_install(PACKAGES_LBAAS, fatal=True)
 
+    apply_configuration("supervisor-vrouter")
     service_restart("nova-compute")
 
 def install_dpdk_vrouter():
-    apt_install(PACKAGES_DPDK_DEPS, fatal=True)
-    check_call(["/opt/contrail/contrail_packages_dpdk/setup.sh"])
     with open("/etc/init/supervisor-vrouter.override", "w") as conf:
         conf.write("manual\n")
-    apt_install(PACKAGES_DPDK, options=["--allow-unauthenticated"], fatal=True)
+
 
     if not os.path.exists("/etc/apt/preferences.d/40contrail"):
+        # contrail libvirt and qemu packages
+        shutil.copy("files/40contrail", "/etc/apt/preferences.d")
+        apt_install(PACKAGES_LIBVIRT, options=["--reinstall", "--force-yes"],
+                fatal=True)
+        apt_install(PACKAGES_QEMU, options=["--reinstall", "--force-yes"],
+                fatal=True)
+        apt_install(PACKAGES_DPDK, options=["--allow-unauthenticated"], fatal=True)
         # contrail nova packages contain vrouter vhostuser vif
-        shutil.copy("files/40contrail", "/etc/apt/preferences.d")
-        apt_install("nova-compute", options=["--reinstall", "--force-yes"],
+        apt_install("nova-compute", options=["--reinstall", "--force-yes", "-o Dpkg::Options::=\"--force-confold\"", "-o Dpkg::Options::=\"--force-confdef\""],
                     fatal=True)
 
     fix_kexec_tools()
@@ -385,16 +418,14 @@
     service_restart("libvirt-bin")
     configure_vrouter()
 
-    set_dpdk_coremask(config.get("dpdk-coremask"))
+    write_dpdk_config()
+
     write_agent_param_config()
     iface = config["vhost-phy"]
     config["dpdk-pci"] = pci_address(iface)
     config["dpdk-mac"] = netifaces.ifaddresses(iface)[netifaces.AF_LINK][0]["addr"]
     write_vrouter_config()
     os.remove("/etc/init/supervisor-vrouter.override")
-    service_start("supervisor-vrouter")
-    write_vrouter_config()
-    service_restart("contrail-vrouter-agent")
 
     fix_libvirt()
 
@@ -424,7 +455,11 @@
 
 @hooks.hook("neutron-metadata-relation-departed")
 @hooks.hook("neutron-metadata-relation-broken")
-@restart_on_change({"/etc/contrail/contrail-vrouter-agent.conf": ["contrail-vrouter-agent"]})
+@restart_on_change({"/etc/contrail/contrail-vrouter-agent.conf": ["supervisor-vrouter"],
+                    "/etc/contrail/contrail-vrouter-nodemgr.conf": ["supervisor-vrouter"]},
+                   False,
+                   {"supervisor-vrouter": apply_configuration}
+                   )
 def neutron_metadata_relation():
     write_vrouter_config()
 
@@ -462,22 +497,46 @@
     except UnregisteredHookError as e:
         log("Unknown hook {} - skipping.".format(e))
 
+def fill_vhost_phy_opts():
+    iface = config.get("vhost-interface")
+    #save physical iface setting
+    if iface:
+        config["vhost-phy"] = iface
+        config["vhost-phy-gw"] = vhost_gateway()
+        config["vhost-phy-ip"] = vhost_ip(iface)
+        if not config["vhost-phy-ip"]:
+            config["vhost-phy-ip"] = vhost_ip("vhost0")
+
 @hooks.hook("upgrade-charm")
 def upgrade_charm():
     if "vhost-phy" not in config:
-        config["vhost-phy"] = \
-          check_output(["awk", "/^physical_interface =/ { print $3 }", "/etc/contrail/contrail-vrouter-agent.conf"]).rstrip()
+        fill_vhost_phy_opts()
     write_vrouter_config()
     write_vnc_api_config()
     write_nodemgr_config()
-    service_restart("supervisor-vrouter")
-
-@restart_on_change({"/etc/contrail/contrail-vrouter-agent.conf": ["contrail-vrouter-agent"],
-                    "/etc/contrail/contrail-vrouter-nodemgr.conf": ["contrail-vrouter-nodemgr"]})
+    if config["dpdk"]:
+        write_dpdk_config()
+    apply_configuration("supervisor-vrouter")
+
+def write_dpdk_config():
+    log("Writing file /etc/contrail/supervisord_vrouter_files/contrail-vrouter-dpdk.ini")
+    set_dpdk_coremask(config.get("dpdk-coremask"))
+    set_dpdk_mempool_size(config.get("dpdk-mempool-size"))
+    set_dpdk_txd_size(config.get("dpdk-txd-size"))
+    set_dpdk_rxd_size(config.get("dpdk-rxd-size"))
+
+@restart_on_change({"/etc/contrail/contrail-vrouter-agent.conf": ["supervisor-vrouter"],
+                    "/etc/contrail/contrail-vrouter-nodemgr.conf": ["supervisor-vrouter"],
+                    "/etc/contrail/supervisord_vrouter_files/contrail-vrouter-dpdk.ini": ["supervisor-vrouter"]},
+                   False,
+                   {"supervisor-vrouter": apply_configuration }
+                   )
 def write_config():
     write_vrouter_config()
     write_vnc_api_config()
     write_nodemgr_config()
+    if config["dpdk"]:
+        write_dpdk_config()
 
 if __name__ == "__main__":
     main()
 
=== modified file 'hooks/neutron_contrail_utils.py'
--- hooks/neutron_contrail_utils.py	2018-03-21 19:34:17 +0000
+++ hooks/neutron_contrail_utils.py	2018-07-05 12:51:18 +0000
@@ -51,7 +51,8 @@
     fstab_mount,
     mkdir,
     service_restart,
-    service_start
+    service_start,
+    service_stop
 )
 
 from charmhelpers.core.templating import render
@@ -122,8 +123,11 @@
     if config["dpdk"]:
         args.append("-d")
     iface = config.get("vhost-interface")
+    #save physical iface setting
     if iface:
         args.append(iface)
+        config["vhost-phy-ip"] = vhost_ip(iface)
+    config["vhost-phy-gw"] = vhost_gateway()
     config["vhost-phy"] = check_output(args, cwd="scripts").rstrip()
 
 def contrail_api_ctx():
@@ -343,7 +347,7 @@
         iface = config.get("control-interface")
         addr = netifaces.ifaddresses(iface)[netifaces.AF_INET][0]["addr"]
     except (ValueError, KeyError):
-        addr = None
+        addr = config["vhost-phy-ip"]["ip"]
     return { "control_network_ip": addr }
 
 def neutron_metadata_ctx():
@@ -389,7 +393,7 @@
 
 def provision_vrouter():
     hostname = gethostname()
-    ip = netifaces.ifaddresses("vhost0")[netifaces.AF_INET][0]["addr"]
+    ip = config["vhost-phy-ip"]["ip"]
     api_port = None
     api_ip = config.get("contrail-api-ip")
     if api_ip:
@@ -422,6 +426,34 @@
        "s!^command=.*/usr/bin/contrail-vrouter-dpdk!command=taskset {} /usr/bin/contrail-vrouter-dpdk!".format(args),
        "/etc/contrail/supervisord_vrouter_files/contrail-vrouter-dpdk.ini"])
 
+def set_dpdk_command_param(param,value):
+    command=check_output(["sed","-n", "-e",
+       "/^command/p",
+       "/etc/contrail/supervisord_vrouter_files/contrail-vrouter-dpdk.ini"]).split()
+    try:
+        idx = command.index(param)
+        if value and value > 0:
+            command[idx+1]=str(value)
+        else:
+            command[idx]=""
+            command[idx+1]=""
+    except ValueError:
+        if value and value > 0:
+            command.append(param)
+            command.append(str(value))
+    check_output(["sed","-i", "-e",
+       "s!^command.*$!{}!".format( " ".join( command)  ),
+       "/etc/contrail/supervisord_vrouter_files/contrail-vrouter-dpdk.ini"])
+
+def set_dpdk_mempool_size(size):
+    set_dpdk_command_param("--vr_mempool_sz",size)
+
+def set_dpdk_txd_size(size):
+    set_dpdk_command_param("--dpdk_txd_sz",size)
+
+def set_dpdk_rxd_size(size):
+    set_dpdk_command_param("--dpdk_rxd_sz",size)
+
 def set_huge_pages_parameters(size, pages):
     with open("/proc/cmdline", "r") as f:
         line = f.read()
@@ -523,9 +555,10 @@
     # determine vhost gateway
     gateway = config.get("vhost-gateway")
     if gateway == "auto":
+        iface = config.get("vhost-interface")
         for line in check_output(["route", "-n"]).splitlines()[2:]:
             l = line.split()
-            if "G" in l[3] and l[7] == "vhost0":
+            if "G" in l[3] and ( l[7] == "vhost0" or l[7] == iface ):
                 return l[1]
         gateway = None
     return gateway
@@ -536,15 +569,16 @@
         addr = netifaces.ifaddresses(iface)[netifaces.AF_INET][0]
         ip = addr["addr"]
         cidr = netaddr.IPNetwork(ip + "/" + addr["netmask"]).prefixlen
-        return ip + "/" + str(cidr)
+        return { "ip":ip, "prefixlen": str(cidr) }
     except (ValueError, KeyError):
         return None
 
 def vrouter_ctx():
-    return { "vhost_ip": vhost_ip("vhost0"),
-             "vhost_gateway": vhost_gateway(),
+    return { "vhost_ip": "%s/%s" % ( config["vhost-phy-ip"]["ip"], config["vhost-phy-ip"]["prefixlen"] ),
+             "vhost_gateway": config["vhost-phy-gw"],
              "vhost_physical": config["vhost-phy"] }
 
+
 def vrouter_vgw_ctx():
     ctx = {}
     vgws = config.get("virtual-gateways")
@@ -575,6 +609,45 @@
     ctx.update(identity_admin_ctx())
     render("vnc_api_lib.ini", "/etc/contrail/vnc_api_lib.ini", ctx)
 
+def is_ctx_complete(ctx,dpdk=False):
+    log("vrouter config ctx: %s" % str(ctx) )
+    try:
+        if ctx["control_nodes"] == "":
+            log("control_nodes missing")
+            return False
+        if ctx["discovery_server"] == "":
+            log("discovery_server missing")
+            return False
+        if ctx["metadata_secret"] == "":
+            log("metadata_secret missing")
+            return False
+        if ctx["control_network_ip"] == "":
+            log("control_network_ip missing")
+            return False
+        if ctx["vhost_ip"] is None or ctx["vhost_ip"] == "":
+            log("vhost_ip missing")
+            return False
+        if ctx["vhost_gateway"] is None or ctx["vhost_gateway"] == "":
+            log("vhost_gateway missing")
+            return False
+        if ctx["vhost_physical"] == "":
+            log("vhost_physical missing")
+            return False
+        if dpdk:
+            if ctx["phy_address"] == "":
+                log("dpdk phy_address missing")
+                return False
+            if ctx["phy_mac"] == "":
+                log("dpdk phy_mac missing")
+                return False
+            if ctx["phy_driver"] == "":
+                log("dpdk phy_driver missing")
+                return False
+        return True
+    except KeyError as e:
+        log("Error %s missing" % str(e) )
+        return False
+
 def write_vrouter_config():
     ctx = {}
     ctx.update(control_node_ctx())
@@ -586,9 +659,35 @@
     if config["dpdk"]:
         ctx["dpdk"] = True
         ctx.update(dpdk_ctx())
-    render("contrail-vrouter-agent.conf",
-           "/etc/contrail/contrail-vrouter-agent.conf", ctx, perms=0440)
+    if is_ctx_complete(ctx):
+        render("contrail-vrouter-agent.conf",
+             "/etc/contrail/contrail-vrouter-agent.conf", ctx, perms=0440)
+    else:
+        log("Configuration incomplete: contrail-vrouter-agent.conf")
+
 
 def write_vrouter_vgw_interfaces():
     ctx = vrouter_vgw_ctx()
     render("vrouter-vgw.cfg", "/etc/network/interfaces.d/vrouter-vgw.cfg", ctx)
+
+def is_vrouter_running():
+    try:
+        for line in check_output(["contrail-status","-d"]).splitlines():
+            state =  line.strip().split()
+            if len(state) > 0 and state[0] != '==':
+                if state[1] != 'active':
+                    return False
+        return True
+    except CalledProcessError:
+        return False
+
+def configure_scaling_governor():
+    check_call(["./set-scaling-governor.sh"], cwd="scripts")
+
+def apply_configuration(service):
+    #service is provided by @restart_on_change hook
+    log("Configuration changed for %s, restarting vrouter" % service)
+    service_stop("supervisor-vrouter")
+    if config["dpdk"]:
+        configure_scaling_governor()
+    service_start("supervisor-vrouter")
 
=== added symlink 'hooks/update-status'
=== target is u'neutron_contrail_hooks.py'
=== modified file 'scripts/create-vrouter.sh'
--- scripts/create-vrouter.sh	2018-03-21 19:34:17 +0000
+++ scripts/create-vrouter.sh	2018-07-05 12:51:18 +0000
@@ -29,6 +29,11 @@
 
 configVRouter()
 {
+	if [ $6 -gt 0 ];
+	        mtu_param="mtu $6"
+	else
+	        mtu_param=""	
+	fi
 	cat juju-header
 	if [ -s "$3" ]; then
 		printf "\n%s\n" "auto $2"
@@ -43,14 +48,15 @@
 		echo "iface vhost0 inet dhcp"
 	fi
 	if [ -n "$1" ]; then
-		cat <<-EOF
+		cat <<-EOF  
+		            $mtu_param
 			    post-up ip link set vhost0 address $5
 			    post-up [ -e /etc/network/routes ] \\
 			       && { service networking-routes stop; service networking-routes start; }
 			EOF
 	else
 		cat <<-EOF
-			    pre-up ip link add address \$(cat /sys/class/net/$2/address) type vhost
+			    pre-up ip link add address \$(cat /sys/class/net/$2/address)  type vhost
 			    pre-up vif --add $2 --mac \$(cat /sys/class/net/$2/address) --vrf 0 --vhost-phys --type physical
 			    pre-up vif --add vhost0 --mac \$(cat /sys/class/net/$2/address) --vrf 0 --type vhost --xconnect $2
 			    post-down vif --list | awk '/^vif.*OS: vhost0/ {split(\$1, arr, "\\/"); print arr[2];}' | xargs vif --delete
@@ -117,12 +123,13 @@
 		iface_cfg=/dev/null
 	fi
 	if [ -n "$1" ]; then
-		mac=$(cat /sys/class/net/$2/address)
+		mac=$(cat /sys/class/net/$2/address )
+		mtu=$(cat /sys/class/net/$2/mtu )
 	fi
 	ifacedown $iface_down vhost0 >&2; sleep 5
 	configureInterfacesDir
 	configureInterfaces $iface_delete
-	configVRouter "$1" $iface_up $iface_cfg $TMP/vrouter.cfg $mac \
+	configVRouter "$1" $iface_up $iface_cfg $TMP/vrouter.cfg ${mac:-0} ${mtu:-0} \
 	    > /etc/network/interfaces.d/vrouter.cfg
 	ifaceup $iface_up >&2
 	if [ -z "$1" ]; then
 
=== added file 'scripts/set-scaling-governor.sh'
--- scripts/set-scaling-governor.sh	1970-01-01 00:00:00 +0000
+++ scripts/set-scaling-governor.sh	2018-07-05 12:51:18 +0000
@@ -0,0 +1,8 @@
+#!/bin/sh -e
+#
+# Script used to set scaling_governor to performance
+
+for f in /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor ; do 
+  echo performance > $f ; 
+done
+