← Back to team overview

cloud-init-dev team mailing list archive

[Merge] ~gpiccoli/cloud-init:lp1802073 into cloud-init:master

 

Guilherme G. Piccoli has proposed merging ~gpiccoli/cloud-init:lp1802073 into cloud-init:master.

Commit message:
EC2: Rewrite network config on AWS Classic instances every boot
    
AWS EC2 instances' network come in 2 basic flavors: Classic and VPC
(Virtual Private Cloud). The former has an interesting behavior of having
its MAC address changed whenever the instance is stopped/restarted. This
behavior is not observed in VPC instances.

In Ubuntu 18.04 (Bionic) the network "management" changed from ENI-style
(etc/network/interfaces) to netplan, and when using netplan we observe
the following block present in /etc/netplan/50-cloud-init.yaml:

match:
  macaddress: aa:bb:cc:dd:ee:ff

Jani Ollikainen noticed in Launchpad bug #1802073 that the EC2 Classic
instances were booting without network access in Bionic after a stop/restart
procedure, due to their MAC address change behavior. It was narrowed down to
the netplan MAC match block, that kept the old MAC address after stopping
and restarting an instance, since the network configuration writing happens
by default only once in EC2 instances, in the first boot.

This patch changes the network configuration write to every boot in EC2
Classic instances, by checking against the "vpc-id" metadata information
provided only in the VPC instances - if we don't have this metadata value,
cloud-init will rewrite the network configuration file in every boot.

A validation against the configuration parameter "apply_network_config"
was also added in this patch, for cases when users don't want to rewrite
the configuration file in every boot of a Classic instance. The default
is True, but if an user's wish is to prevent this rewrite, it is enough
to write "apply_network_config: False" under the Datasource/Ec2 yaml block
in "/etc/cloud/cloud.cfg".

This was tested in an EC2 Classic instance and proved to fix the issue;
we also validated that "apply_network_config: False" prevents the rewrite
of the network config file in every boot.

LP: #1802073

Reported-by: Jani Ollikainen <jani.ollikainen@xxxxx>
Suggested-by: Chad Smith <chad.smith@xxxxxxxxxxxxx>
Suggested-by: Ryan Harper <ryan.harper@xxxxxxxxxxxxx>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@xxxxxxxxxxxxx>

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1802073 in cloud-init (Ubuntu): "No network in AWS (EC-Classic) after stopping and starting instance"
  https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/1802073

For more details, see:
https://code.launchpad.net/~gpiccoli/cloud-init/+git/cloud-init/+merge/362749
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~gpiccoli/cloud-init:lp1802073 into cloud-init:master.
diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
index eb6f27b..66d6e71 100644
--- a/cloudinit/sources/DataSourceEc2.py
+++ b/cloudinit/sources/DataSourceEc2.py
@@ -19,6 +19,7 @@ from cloudinit import sources
 from cloudinit import url_helper as uhelp
 from cloudinit import util
 from cloudinit import warnings
+from cloudinit.event import EventType
 
 LOG = logging.getLogger(__name__)
 
@@ -320,6 +321,14 @@ class DataSourceEc2(sources.DataSource):
         if isinstance(net_md, dict):
             result = convert_ec2_metadata_network_config(
                 net_md, macs_to_nics=macs_to_nics, fallback_nic=iface)
+            # Non-VPC (aka Classic) instances need to rewrite config files
+            # every boot due to MAC addr change, except if the user wants to
+            # prevent this rewrite by using "apply_network_config: false".
+            if not util.is_false(self.ds_cfg.get('apply_network_config')):
+                for item in result['config']:
+                    if item['vpc'] is False:
+                        self.update_events['network'].add(EventType.BOOT)
+                        break
         else:
             LOG.warning("Metadata 'network' key not valid: %s.", net_md)
         self._network_config = result
@@ -521,13 +530,16 @@ def convert_ec2_metadata_network_config(network_md, macs_to_nics=None,
         nic_metadata = macs_metadata.get(mac)
         if not nic_metadata:
             continue  # Not a physical nic represented in metadata
-        nic_cfg = {'type': 'physical', 'name': nic_name, 'subnets': []}
+        nic_cfg = {'type': 'physical', 'name': nic_name, 'subnets': [],
+                   'vpc': False}
         nic_cfg['mac_address'] = mac
         if (nic_name == fallback_nic or nic_metadata.get('public-ipv4s') or
                 nic_metadata.get('local-ipv4s')):
             nic_cfg['subnets'].append({'type': 'dhcp4'})
         if nic_metadata.get('ipv6s'):
             nic_cfg['subnets'].append({'type': 'dhcp6'})
+        if nic_metadata.get('vpc-id'):
+            nic_cfg['vpc'] = True
         netcfg['config'].append(nic_cfg)
     return netcfg
 
diff --git a/doc/rtd/topics/datasources/ec2.rst b/doc/rtd/topics/datasources/ec2.rst
index 64c325d..473b979 100644
--- a/doc/rtd/topics/datasources/ec2.rst
+++ b/doc/rtd/topics/datasources/ec2.rst
@@ -79,6 +79,10 @@ The settings that may be configured are:
  * **timeout**: the timeout value provided to urlopen for each individual http
    request.  This is used both when selecting a metadata_url and when crawling
    the metadata service. (default: 50)
+ * **apply_network_config**: Boolean used to allow users choose if they want to
+   rewrite the network config file for EC2 Classic (aka non-VPC) instances at every
+   boot. The automatic rewrite in every boot prevents instances from booting without
+   network in classic instances due to MAC change. (default: True)
 
 An example configuration with the default values is provided below:
 
@@ -89,5 +93,6 @@ An example configuration with the default values is provided below:
     metadata_urls: ["http://169.254.169.254:80";, "http://instance-data:8773";]
     max_wait: 120
     timeout: 50
+    apply_network_config: True
 
 .. vi: textwidth=78
diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
index 1a5956d..6a7229d 100644
--- a/tests/unittests/test_datasource/test_ec2.py
+++ b/tests/unittests/test_datasource/test_ec2.py
@@ -280,7 +280,7 @@ class TestEc2(test_helpers.HttprettyTestCase):
         expected = {'version': 1, 'config': [
             {'mac_address': '06:17:04:d7:26:09', 'name': 'eth9',
              'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}],
-             'type': 'physical'}]}
+             'vpc': True, 'type': 'physical'}]}
         patch_path = (
             'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac')
         get_interface_mac_path = (
@@ -311,7 +311,7 @@ class TestEc2(test_helpers.HttprettyTestCase):
         mac1 = '06:17:04:d7:26:0A'  # IPv4 only in DEFAULT_METADATA
         expected = {'version': 1, 'config': [
             {'mac_address': '06:17:04:d7:26:0A', 'name': 'eth9',
-             'subnets': [{'type': 'dhcp4'}],
+             'subnets': [{'type': 'dhcp4'}], 'vpc': True,
              'type': 'physical'}]}
         patch_path = (
             'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac')
@@ -366,7 +366,7 @@ class TestEc2(test_helpers.HttprettyTestCase):
             {'mac_address': '06:17:04:d7:26:09',
              'name': 'eth9',
              'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}],
-             'type': 'physical'}]}
+             'vpc': True, 'type': 'physical'}]}
         self.assertEqual(expected, ds.network_config)
 
     def test_ec2_get_instance_id_refreshes_identity_on_upgrade(self):
@@ -534,7 +534,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
         # DE:AD:BE:EF:FF:FF represented by OS but not in metadata
         expected = {'version': 1, 'config': [
             {'mac_address': self.mac1, 'type': 'physical',
-             'name': 'eth9', 'subnets': [{'type': 'dhcp4'}]}]}
+             'name': 'eth9', 'subnets': [{'type': 'dhcp4'}], 'vpc': False}]}
         self.assertEqual(
             expected,
             ec2.convert_ec2_metadata_network_config(
@@ -550,7 +550,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
         nic1_metadata.pop('public-ipv4s')
         expected = {'version': 1, 'config': [
             {'mac_address': self.mac1, 'type': 'physical',
-             'name': 'eth9', 'subnets': [{'type': 'dhcp6'}]}]}
+             'name': 'eth9', 'subnets': [{'type': 'dhcp6'}], 'vpc': False}]}
         self.assertEqual(
             expected,
             ec2.convert_ec2_metadata_network_config(
@@ -566,7 +566,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
         nic1_metadata.pop('public-ipv4s')
         expected = {'version': 1, 'config': [
             {'mac_address': self.mac1, 'type': 'physical',
-             'name': 'eth9', 'subnets': [{'type': 'dhcp4'}]}]}
+             'name': 'eth9', 'subnets': [{'type': 'dhcp4'}], 'vpc': False}]}
         self.assertEqual(
             expected,
             ec2.convert_ec2_metadata_network_config(
@@ -583,7 +583,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
         # When no ipv4 or ipv6 content but fallback_nic set, set dhcp4 config.
         expected = {'version': 1, 'config': [
             {'mac_address': self.mac1, 'type': 'physical',
-             'name': 'eth9', 'subnets': [{'type': 'dhcp4'}]}]}
+             'name': 'eth9', 'subnets': [{'type': 'dhcp4'}], 'vpc': False}]}
         self.assertEqual(
             expected,
             ec2.convert_ec2_metadata_network_config(
@@ -601,7 +601,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
         expected = {'version': 1, 'config': [
             {'mac_address': self.mac1, 'type': 'physical',
              'name': 'eth9',
-             'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}]}]}
+             'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}], 'vpc': False}]}
         self.assertEqual(
             expected,
             ec2.convert_ec2_metadata_network_config(
@@ -617,7 +617,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
         expected = {'version': 1, 'config': [
             {'mac_address': self.mac1, 'type': 'physical',
              'name': 'eth9',
-             'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}]}]}
+             'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}], 'vpc': False}]}
         self.assertEqual(
             expected,
             ec2.convert_ec2_metadata_network_config(
@@ -628,7 +628,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
         expected = {'version': 1, 'config': [
             {'mac_address': self.mac1, 'type': 'physical',
              'name': 'eth9',
-             'subnets': [{'type': 'dhcp4'}]}]}
+             'subnets': [{'type': 'dhcp4'}], 'vpc': False}]}
         patch_path = (
             'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac')
         with mock.patch(patch_path) as m_get_interfaces_by_mac: