Merge ~oddbloke/cloud-init/+git/cloud-init:networking into cloud-init:master

[Dan Watkins]

[Ryan Harper (raharper)]

If the interface already has an IP, it may not have anything else that we'd apply after parsing the lease output (routes, etc).

This sort of feels OK in that we skip tearing down when we don't do anything else, but it also seems wrong in that the whole context manager should exit if we don't bring up DHCP.

[Dan Watkins (oddbloke)]

On Wed, Sep 04, 2019 at 06:30:07PM -0000, Ryan Harper wrote:
> If the interface already has an IP, it may not have anything else that
> we'd apply after parsing the lease output (routes, etc).

If the interface already has an IP before we DHCP, it's not 100% clear
to me that we should consider routes etc. from the DHCP response
applicable.

> This sort of feels OK in that we skip tearing down when we don't do
> anything else, but it also seems wrong in that the whole context
> manager should exit if we don't bring up DHCP.

What do you mean by "should exit" here? The way we use the context
manager (in DataSourceOpenStack, at least), is essentially "ensure I
have network while executing the body", so not needing to configure
networking isn't really an error condition in that usecase.

Unmerged commits

c37ec07... by Dan Watkins on 2019-09-04

[WIP] net: only apply routes if we bring an ephemeral DHCP interface up

TODO:
* Tests

diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index ea707c0..f308d03 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -756,7 +756,11 @@ class EphemeralIPv4Network(object):
                     ' to %s', self.connectivity_url)
                 return
 
-        self._bringup_device()
+        device_brought_up = self._bringup_device()
+        if not device_brought_up:
+            # If we didn't bring the device up, no need to do anything else
+            # with it
+            return
 
         # rfc3442 requires us to ignore the router config *if* classless static
         # routes are provided.
@@ -787,7 +791,13 @@ class EphemeralIPv4Network(object):
             capture=True)
 
     def _bringup_device(self):
-        """Perform the ip comands to fully setup the device."""
+        """
+        Perform the ip comands to fully setup the device.
+
+        :return:
+            True if the device was brought up, False if it already had an
+            address.
+        """
         cidr = '{0}/{1}'.format(self.ip, self.prefix)
         LOG.debug(
             'Attempting setup of ephemeral network on %s with %s brd %s',
@@ -803,17 +813,19 @@ class EphemeralIPv4Network(object):
             LOG.debug(
                 'Skip ephemeral network setup, %s already has address %s',
                 self.interface, self.ip)
-        else:
-            # Address creation success, bring up device and queue cleanup
-            util.subp(
-                ['ip', '-family', 'inet', 'link', 'set', 'dev', self.interface,
-                 'up'], capture=True)
-            self.cleanup_cmds.append(
-                ['ip', '-family', 'inet', 'link', 'set', 'dev', self.interface,
-                 'down'])
-            self.cleanup_cmds.append(
-                ['ip', '-family', 'inet', 'addr', 'del', cidr, 'dev',
-                 self.interface])
+            return False
+
+        # Address creation success, bring up device and queue cleanup
+        util.subp(
+            ['ip', '-family', 'inet', 'link', 'set', 'dev', self.interface,
+             'up'], capture=True)
+        self.cleanup_cmds.append(
+            ['ip', '-family', 'inet', 'link', 'set', 'dev', self.interface,
+             'down'])
+        self.cleanup_cmds.append(
+            ['ip', '-family', 'inet', 'addr', 'del', cidr, 'dev',
+             self.interface])
+        return True
 
     def _bringup_static_routes(self):
         # static_routes = [("169.254.169.254/32", "130.56.248.255"),