vmbuilder team mailing list archive

[Mathieu Trudel-Lapierre <mathieu.tl@xxxxxxxxx>]

Comments inline.

Diff comments:

> === modified file 'templates/img-extra-nets.tmpl'
> --- templates/img-extra-nets.tmpl	2017-08-22 19:29:36 +0000
> +++ templates/img-extra-nets.tmpl	2018-05-31 14:58:36 +0000
> @@ -5,6 +5,7 @@
>  # vi: ts=4 noexpandtab syntax=sh
>  
>  int_d="/run/network/interfaces.ephemeral.d"
> +netp_d="/etc/netplan"

I agree with the use of /etc/netplan here. You don't necessarily want to re-write configs in /run/netplan at every boot when devices changed, because you device changes are not necessarily driven by metadata changes -- it's possibly true for clouds, but definitely not true for bare metal. Furthermore, using /run/netplan means it will get very hard for a user to override what cloud-init writes in /run/netplan every boot if they have a different view of how the configuration of the device should be done.

>  
>  function config_upstart {
>      # Give an upstart job for defining a secondary interface
> @@ -52,6 +53,29 @@
>  
>  INTERFACE=\$1
>  net_int_d="${int_d}"
> +netplan_d="${netp_d}"
> +
> +
> +function configure_nic_netplan {
> +    # Write separate netplan yaml for each hot-plugged interface
> +    if [ -e \${netplan_d}/90-hotplug-\${INTERFACE}.yaml ]; then

+1 on "90-hotplug-*"; let's make sure this is as generic as possible so it still makes sense on other clouds if we want to test similar code. Marking it clearly as "hotplug" or "autodetected" or some other similar label makes it clear to users that it is automatically generated config.

> +        return
> +    fi
> +    cat << EOM > \${netplan_d}/90-hotplug-\${INTERFACE}.yaml
> +# Added by \$0 due to udev nic hotplug event
> +network:
> +    version: 2
> +    ethernets:
> +        \${INTERFACE}:
> +            dhcp4: true
> +            match:
> +              driver: hv_netvsc
> +              name: \${INTERFACE}
> +            optional: true

optional: true is likely to change to a different name pretty soon, just fyi. It's also one of the reasons why writing to /run makes any user-defined config written in /etc/ afterwards hard to predict the behavior of.

> +EOM
> +    netplan apply
> +}
> +
>  
>  function configure_nic {
>      # Ensure \${net_int_d} exists on the ephemeral mount
> @@ -97,8 +124,9 @@
>          ;;
>  esac
>  
> -# Add the network ephemeral mount...
> -cat << EOF >> ${mp}/etc/network/interfaces
> +# Add the network ephemeral mount... only on non-netplan images
> +if [ -e ${mp}/usr/sbin/netplan ]; then
> +    cat << EOF >> ${mp}/etc/network/interfaces

On systems with netplan installed, there should already be an /etc/network/interfaces file with appropriate comments to point back to /etc/netplan. Please don't undo this by clobbering the file if it exists.

>  
>  # Read the dynamically created configurations from tmpfs mount. If you want a static
>  # configuration, disable the line below. However, you will have to manually configure


-- 
https://code.launchpad.net/~chad.smith/vmbuilder/jenkins_kvm_azure_netplan_hotplug/+merge/347212
Your team VMBuilder is requested to review the proposed merge of lp:~chad.smith/vmbuilder/jenkins_kvm_azure_netplan_hotplug into lp:~ubuntu-on-ec2/vmbuilder/jenkins_kvm.