Merge ~morphis/snappy-hwe-snaps/+git/network-manager:netplan-support into ~snappy-hwe-team/snappy-hwe-snaps/+git/network-manager:master

[Simon Fels]

Add support for netplan.

This adds support to take configuration files into account which are generated by netplan. There are a few things we have to change for that:

* Don't copy default configuration to $SNAP_DATA but rather check if one is available there and use that one conditionally
* Manage ethernet devices again but only if netplan renders NetworkManager connection files (checks if /etc/netplan/00-default-nm-renderer.yaml is present)
* Don't auto configure ethernet ports when netplan is used to avoid mismatches between console-conf UI and actual configuration

[Tony Espy (awe)]

Looks good to me.

Please add a comment if/when you've tested on a real system, and I'll top-approve.

review: Approve

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:3798516ad2ff97eb260191ed862b5d16d89f1935
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/88/
Executed test runs:

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/88/rebuild

review: Approve (continuous-integration)

[Simon Fels (morphis)]

Things are looking good from first test results. Lets wait until the necessary changes for snapd/netplan have landed in a new edge OS snap so we can properly test everything.

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:d69a87ded3b7963d197b5e6b0f13b10d0d796c9c
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/123/
Executed test runs:
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/24/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/123/rebuild

review: Approve (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:876f84effb41fb269ac56bce64958bbc8d6c2c07
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/124/
Executed test runs:
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/25/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/124/rebuild

review: Approve (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:876f84effb41fb269ac56bce64958bbc8d6c2c07
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/125/
Executed test runs:
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/26/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/125/rebuild

review: Approve (continuous-integration)

[Scott Sweeny (ssweeny)]

I looked up the NetworkManager.conf manpage[1] and if I'm reading that right you should be able to specify a globbed set of interface names using the interface-name: syntax.

The section on no-auto-default directs you to the "Device List Format" section, which specifies that globbing is supported when you use interface-name.

[1] http://manpages.ubuntu.com/manpages/xenial/man5/NetworkManager.conf.5.html

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:ed3418fded6be655c4f052d50feeb26f97e0d044
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/144/
Executed test runs:
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/45/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/144/rebuild

review: Approve (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:ed3418fded6be655c4f052d50feeb26f97e0d044
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/145/
Executed test runs:
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/46/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/145/rebuild

review: Approve (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:d6dd3245cf832ea622e77744935c08e02fc65370
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/148/
Executed test runs:
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/49/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/148/rebuild

review: Approve (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:d6dd3245cf832ea622e77744935c08e02fc65370
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/149/
Executed test runs:
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/50/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/149/rebuild

review: Approve (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

FAILED: Continuous integration, rev:54f1db04843bdea952a2d3f97d60106cc8ef5095
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/154/
Executed test runs:
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/55/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/154/rebuild

review: Needs Fixing (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:560ba5bb2ead1b64aea252fcf338193af7fc48f0
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/156/
Executed test runs:
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/57/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/156/rebuild

review: Approve (continuous-integration)

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:560ba5bb2ead1b64aea252fcf338193af7fc48f0
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/157/
Executed test runs:
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/58/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/157/rebuild

review: Approve (continuous-integration)

[Tony Espy (awe)]

Too questions based on updates to this merge proposal:

1. What does the functionality does the 'network-setup-observe' interface cover? Ideally this would be explained in the commit message, but as it's a one-liner, it's not clear.

2. Is commit 6730e78 still required? I thought with the addition of Scott's changes to write DHCP lease files to /run/NetworkManager/dhcp, and the associated change to probert, we no longer have to disable ethernet mgmt by NM.

review: Needs Information

[Simon Fels (morphis)]

1) The network-setup-observe interface allows read-only access to /etc/netplan and /etc/network. We need it actually to check if the file /etc/netplan/00-default-nm-renderer.yaml exists to switch into the correct mode we're running the snap in.

2) 6730e78 isn't about disabling ethernet configuration. It sets the no-auto-default configuration item to prevent automatic network configuration of ethernet ports as this now done via the netplan configuration which is created automatically on first boot and maintained further inside console-conf.

[Tony Espy (awe)]

Got it, thanks for the note re: the observer interface. That sounds correct, but again in the future we should try and add more context/detail to our commit messages.

Re: auto-config of ethernet devices, that makes sense, this was yet another missing detail in our plan ( at least missing in my mind ). I wasn't sure if netplan actually had support for generating NM configuration files, and thus thought we were relying on NM's autoconfig for them.

[Tony Espy (awe)]

review: Approve

diff --git a/bin/networkmanager b/bin/networkmanager
index fb800cf..b7821e3 100755
--- a/bin/networkmanager
+++ b/bin/networkmanager
@@ -9,24 +9,43 @@ mkdir -p $SNAP_DATA/run
 # Create DHCP lease directory
 mkdir -p /run/NetworkManager/dhcp
 
-# Copy config into place
-#
-# TODO: if/when snappy grows support for provisioning
-# files into $SNAP_DATA, this code should be removed
-# in favor of the new mechanism.
-if [ ! -e $SNAP_DATA/NetworkManager.conf ]; then
-    cp $SNAP/etc/NetworkManager/NetworkManager.conf $SNAP_DATA
+# Select which config we're going to use. We offer our users
+# to provide their own configuration file in $SNAP_DATA but
+# will fallback if no one exists to the default one we ship
+# in $SNAP
+NM_CONF="$SNAP/etc/NetworkManager/NetworkManager.conf"
+if [ -e $SNAP_DATA/NetworkManager.conf ]; then
+    NM_CONF="$SNAP_DATA/NetworkManager.conf"
 fi
 
 # A directory where users can place any additional configuration
 # files for NetworkManager
 mkdir -p $SNAP_DATA/conf.d
 
+# If netplan is not configured to render by default to NetworkManager
+# configuration files we disable management of any ethernet device
+# as this will clash with any configuration netplan puts in place
+# for networkd.
+if [ ! -e "/etc/netplan/00-default-nm-renderer.yaml" ] ; then
+    if [ ! -e "$SNAP_DATA/conf.d/disable-ethernet.conf" ] ; then
+        echo "[keyfile]" > $SNAP_DATA/conf.d/disable-ethernet.conf
+        echo "unmanaged-devices+=interface-name:eth*,interface-name:enx*" >> $SNAP_DATA/conf.d/disable-ethernet.conf
+    fi
+else
+    if [ ! -e "$SNAP_DATA/conf.d/no-auto-default-ethernet.conf" ] ; then
+        # If we're running as the only network management service
+        # and are configured via netplan on first boot then we should
+        # not try to auto configure ethernet ports as this is up to
+        # netplan and will be the same for networkd.
+        echo "[main]" > $SNAP_DATA/conf.d/no-auto-default-ethernet.conf
+        echo "no-auto-default=interface-name:eth*,interface-name:enx*" >> $SNAP_DATA/conf.d/no-auto-default-ethernet.conf
+    fi
+fi
+
 export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$SNAP/usr/lib/NetworkManager"
 
-# TODO drop DEBUG
 $SNAP/usr/sbin/NetworkManager \
     --config-dir=$SNAP_DATA/conf.d/ \
-    --config=$SNAP_DATA/NetworkManager.conf \
+    --config=$NM_CONF \
     --log-level=INFO \
     --no-daemon
diff --git a/conf/NetworkManager.conf b/conf/NetworkManager.conf
index 43db0f9..1950212 100644
--- a/conf/NetworkManager.conf
+++ b/conf/NetworkManager.conf
@@ -15,9 +15,3 @@ dhcp=internal
 
 [ifupdown]
 managed=false
-
-[keyfile]
-# Ignore all ethernet devices for now until the core
-# image has a config option to disable network support
-# in cloud-init
-unmanaged-devices+=interface-name:eth*,interface-name:enx*
diff --git a/snapcraft.yaml b/snapcraft.yaml
index 01b0652..2915ccd 100644
--- a/snapcraft.yaml
+++ b/snapcraft.yaml
@@ -9,6 +9,7 @@ description: |
   VPN serivces.
   Please find the source code at https://code.launchpad.net/~snappy-hwe-team/snappy-hwe-snaps/+git/network-manager
 confinement: strict
+grade: stable
 
 slots:
   service: network-manager
@@ -24,7 +25,7 @@ apps:
     command: bin/networkmanager
     daemon: simple
     slots: [service]
-    plugs: [modem-manager, ppp]
+    plugs: [modem-manager, ppp, network-setup-observe]
 
 parts:
   networkmanager-common: