Merge ~chad.smith/cloud-init:ubuntu/artful into cloud-init:ubuntu/artful

[Chad Smith]

Sync tip of master including SRU fix for release into artful.

diff --git a/cloudinit/cmd/status.py b/cloudinit/cmd/status.py
index d7aaee9..ea79a85 100644
--- a/cloudinit/cmd/status.py
+++ b/cloudinit/cmd/status.py
@@ -105,12 +105,12 @@ def _get_status_details(paths):
 
     Values are obtained from parsing paths.run_dir/status.json.
     """
-
     status = STATUS_ENABLED_NOT_RUN
     status_detail = ''
     status_v1 = {}
 
     status_file = os.path.join(paths.run_dir, 'status.json')
+    result_file = os.path.join(paths.run_dir, 'result.json')
 
     (is_disabled, reason) = _is_cloudinit_disabled(
         CLOUDINIT_DISABLED_FILE, paths)
@@ -118,12 +118,15 @@ def _get_status_details(paths):
         status = STATUS_DISABLED
         status_detail = reason
     if os.path.exists(status_file):
+        if not os.path.exists(result_file):
+            status = STATUS_RUNNING
         status_v1 = load_json(load_file(status_file)).get('v1', {})
     errors = []
     latest_event = 0
     for key, value in sorted(status_v1.items()):
         if key == 'stage':
             if value:
+                status = STATUS_RUNNING
                 status_detail = 'Running in stage: {0}'.format(value)
         elif key == 'datasource':
             status_detail = value
diff --git a/cloudinit/cmd/tests/test_status.py b/cloudinit/cmd/tests/test_status.py
index a7c0a91..4a5a8c0 100644
--- a/cloudinit/cmd/tests/test_status.py
+++ b/cloudinit/cmd/tests/test_status.py
@@ -7,7 +7,7 @@ from textwrap import dedent
 
 from cloudinit.atomic_helper import write_json
 from cloudinit.cmd import status
-from cloudinit.util import write_file
+from cloudinit.util import ensure_file
 from cloudinit.tests.helpers import CiTestCase, wrap_and_call, mock
 
 mypaths = namedtuple('MyPaths', 'run_dir')
@@ -36,7 +36,7 @@ class TestStatus(CiTestCase):
 
     def test__is_cloudinit_disabled_false_on_sysvinit(self):
         '''When not in an environment using systemd, return False.'''
-        write_file(self.disable_file, '')  # Create the ignored disable file
+        ensure_file(self.disable_file)  # Create the ignored disable file
         (is_disabled, reason) = wrap_and_call(
             'cloudinit.cmd.status',
             {'uses_systemd': False},
@@ -47,7 +47,7 @@ class TestStatus(CiTestCase):
 
     def test__is_cloudinit_disabled_true_on_disable_file(self):
         '''When using systemd and disable_file is present return disabled.'''
-        write_file(self.disable_file, '')  # Create observed disable file
+        ensure_file(self.disable_file)  # Create observed disable file
         (is_disabled, reason) = wrap_and_call(
             'cloudinit.cmd.status',
             {'uses_systemd': True},
@@ -58,7 +58,7 @@ class TestStatus(CiTestCase):
 
     def test__is_cloudinit_disabled_false_on_kernel_cmdline_enable(self):
         '''Not disabled when using systemd and enabled via commandline.'''
-        write_file(self.disable_file, '')  # Create ignored disable file
+        ensure_file(self.disable_file)  # Create ignored disable file
         (is_disabled, reason) = wrap_and_call(
             'cloudinit.cmd.status',
             {'uses_systemd': True,
@@ -96,7 +96,7 @@ class TestStatus(CiTestCase):
     def test__is_cloudinit_disabled_false_when_enabled_in_systemd(self):
         '''Report enabled when systemd generator creates the enabled file.'''
         enabled_file = os.path.join(self.paths.run_dir, 'enabled')
-        write_file(enabled_file, '')
+        ensure_file(enabled_file)
         (is_disabled, reason) = wrap_and_call(
             'cloudinit.cmd.status',
             {'uses_systemd': True,
@@ -149,8 +149,25 @@ class TestStatus(CiTestCase):
         ''')
         self.assertEqual(expected, m_stdout.getvalue())
 
+    def test_status_returns_running_on_no_results_json(self):
+        '''Report running when status.json exists but result.json does not.'''
+        result_file = self.tmp_path('result.json', self.new_root)
+        write_json(self.status_file, {})
+        self.assertFalse(
+            os.path.exists(result_file), 'Unexpected result.json found')
+        cmdargs = myargs(long=False, wait=False)
+        with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
+            retcode = wrap_and_call(
+                'cloudinit.cmd.status',
+                {'_is_cloudinit_disabled': (False, ''),
+                 'Init': {'side_effect': self.init_class}},
+                status.handle_status_args, 'ignored', cmdargs)
+        self.assertEqual(0, retcode)
+        self.assertEqual('status: running\n', m_stdout.getvalue())
+
     def test_status_returns_running(self):
         '''Report running when status exists with an unfinished stage.'''
+        ensure_file(self.tmp_path('result.json', self.new_root))
         write_json(self.status_file,
                    {'v1': {'init': {'start': 1, 'finished': None}}})
         cmdargs = myargs(long=False, wait=False)
@@ -164,10 +181,11 @@ class TestStatus(CiTestCase):
         self.assertEqual('status: running\n', m_stdout.getvalue())
 
     def test_status_returns_done(self):
-        '''Reports done when stage is None and all stages are finished.'''
+        '''Report done results.json exists no stages are unfinished.'''
+        ensure_file(self.tmp_path('result.json', self.new_root))
         write_json(
             self.status_file,
-            {'v1': {'stage': None,
+            {'v1': {'stage': None,  # No current stage running
                     'datasource': (
                         'DataSourceNoCloud [seed=/var/.../seed/nocloud-net]'
                         '[dsmode=net]'),
@@ -187,6 +205,7 @@ class TestStatus(CiTestCase):
 
     def test_status_returns_done_long(self):
         '''Long format of done status includes datasource info.'''
+        ensure_file(self.tmp_path('result.json', self.new_root))
         write_json(
             self.status_file,
             {'v1': {'stage': None,
@@ -303,6 +322,8 @@ class TestStatus(CiTestCase):
                 write_json(self.status_file, running_json)
             elif self.sleep_calls == 3:
                 write_json(self.status_file, done_json)
+                result_file = self.tmp_path('result.json', self.new_root)
+                ensure_file(result_file)
 
         cmdargs = myargs(long=False, wait=True)
         with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index c015e79..f69c0ef 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -274,23 +274,52 @@ def apply_network_config_names(netcfg, strict_present=True, strict_busy=True):
     renames are only attempted for interfaces of type 'physical'.  It is
     expected that the network system will create other devices with the
     correct name in place."""
-    renames = []
-    for ent in netcfg.get('config', {}):
-        if ent.get('type') != 'physical':
-            continue
-        mac = ent.get('mac_address')
-        if not mac:
-            continue
-        name = ent.get('name')
-        driver = ent.get('params', {}).get('driver')
-        device_id = ent.get('params', {}).get('device_id')
-        if not driver:
-            driver = device_driver(name)
-        if not device_id:
-            device_id = device_devid(name)
-        renames.append([mac, name, driver, device_id])
-
-    return _rename_interfaces(renames)
+
+    def _version_1(netcfg):
+        renames = []
+        for ent in netcfg.get('config', {}):
+            if ent.get('type') != 'physical':
+                continue
+            mac = ent.get('mac_address')
+            if not mac:
+                continue
+            name = ent.get('name')
+            driver = ent.get('params', {}).get('driver')
+            device_id = ent.get('params', {}).get('device_id')
+            if not driver:
+                driver = device_driver(name)
+            if not device_id:
+                device_id = device_devid(name)
+            renames.append([mac, name, driver, device_id])
+        return renames
+
+    def _version_2(netcfg):
+        renames = []
+        for key, ent in netcfg.get('ethernets', {}).items():
+            # only rename if configured to do so
+            name = ent.get('set-name')
+            if not name:
+                continue
+            # cloud-init requires macaddress for renaming
+            mac = ent.get('match', {}).get('macaddress')
+            if not mac:
+                continue
+            driver = ent.get('match', {}).get('driver')
+            device_id = ent.get('match', {}).get('device_id')
+            if not driver:
+                driver = device_driver(name)
+            if not device_id:
+                device_id = device_devid(name)
+            renames.append([mac, name, driver, device_id])
+        return renames
+
+    if netcfg.get('version') == 1:
+        return _rename_interfaces(_version_1(netcfg))
+    elif netcfg.get('version') == 2:
+        return _rename_interfaces(_version_2(netcfg))
+
+    raise RuntimeError('Failed to apply network config names. Found bad'
+                       ' network config version: %s' % netcfg.get('version'))
 
 
 def interface_has_own_mac(ifname, strict=False):
diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py
index 8cb4114..276556e 100644
--- a/cloudinit/net/tests/test_init.py
+++ b/cloudinit/net/tests/test_init.py
@@ -4,6 +4,8 @@ import copy
 import errno
 import mock
 import os
+import textwrap
+import yaml
 
 import cloudinit.net as net
 from cloudinit.util import ensure_file, write_file, ProcessExecutionError
@@ -520,3 +522,92 @@ class TestEphemeralIPV4Network(CiTestCase):
         with net.EphemeralIPv4Network(**params):
             self.assertEqual(expected_setup_calls, m_subp.call_args_list)
         m_subp.assert_has_calls(expected_teardown_calls)
+
+
+class TestApplyNetworkCfgNames(CiTestCase):
+    V1_CONFIG = textwrap.dedent("""\
+        version: 1
+        config:
+            - type: physical
+              name: interface0
+              mac_address: "52:54:00:12:34:00"
+              subnets:
+                  - type: static
+                    address: 10.0.2.15
+                    netmask: 255.255.255.0
+                    gateway: 10.0.2.2
+    """)
+    V2_CONFIG = textwrap.dedent("""\
+      version: 2
+      ethernets:
+          interface0:
+            match:
+              macaddress: "52:54:00:12:34:00"
+            addresses:
+              - 10.0.2.15/24
+            gateway4: 10.0.2.2
+            set-name: interface0
+    """)
+
+    V2_CONFIG_NO_SETNAME = textwrap.dedent("""\
+      version: 2
+      ethernets:
+          interface0:
+            match:
+              macaddress: "52:54:00:12:34:00"
+            addresses:
+              - 10.0.2.15/24
+            gateway4: 10.0.2.2
+    """)
+
+    V2_CONFIG_NO_MAC = textwrap.dedent("""\
+      version: 2
+      ethernets:
+          interface0:
+            match:
+              driver: virtio-net
+            addresses:
+              - 10.0.2.15/24
+            gateway4: 10.0.2.2
+            set-name: interface0
+    """)
+
+    @mock.patch('cloudinit.net.device_devid')
+    @mock.patch('cloudinit.net.device_driver')
+    @mock.patch('cloudinit.net._rename_interfaces')
+    def test_apply_v1_renames(self, m_rename_interfaces, m_device_driver,
+                              m_device_devid):
+        m_device_driver.return_value = 'virtio_net'
+        m_device_devid.return_value = '0x15d8'
+
+        net.apply_network_config_names(yaml.load(self.V1_CONFIG))
+
+        call = ['52:54:00:12:34:00', 'interface0', 'virtio_net', '0x15d8']
+        m_rename_interfaces.assert_called_with([call])
+
+    @mock.patch('cloudinit.net.device_devid')
+    @mock.patch('cloudinit.net.device_driver')
+    @mock.patch('cloudinit.net._rename_interfaces')
+    def test_apply_v2_renames(self, m_rename_interfaces, m_device_driver,
+                              m_device_devid):
+        m_device_driver.return_value = 'virtio_net'
+        m_device_devid.return_value = '0x15d8'
+
+        net.apply_network_config_names(yaml.load(self.V2_CONFIG))
+
+        call = ['52:54:00:12:34:00', 'interface0', 'virtio_net', '0x15d8']
+        m_rename_interfaces.assert_called_with([call])
+
+    @mock.patch('cloudinit.net._rename_interfaces')
+    def test_apply_v2_renames_skips_without_setname(self, m_rename_interfaces):
+        net.apply_network_config_names(yaml.load(self.V2_CONFIG_NO_SETNAME))
+        m_rename_interfaces.assert_called_with([])
+
+    @mock.patch('cloudinit.net._rename_interfaces')
+    def test_apply_v2_renames_skips_without_mac(self, m_rename_interfaces):
+        net.apply_network_config_names(yaml.load(self.V2_CONFIG_NO_MAC))
+        m_rename_interfaces.assert_called_with([])
+
+    def test_apply_v2_renames_raises_runtime_error_on_unknown_version(self):
+        with self.assertRaises(RuntimeError):
+            net.apply_network_config_names(yaml.load("version: 3"))
diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
index e14553b..21e9ef8 100644
--- a/cloudinit/sources/DataSourceEc2.py
+++ b/cloudinit/sources/DataSourceEc2.py
@@ -147,6 +147,12 @@ class DataSourceEc2(sources.DataSource):
     def get_instance_id(self):
         if self.cloud_platform == Platforms.AWS:
             # Prefer the ID from the instance identity document, but fall back
+            if not getattr(self, 'identity', None):
+                # If re-using cached datasource, it's get_data run didn't
+                # setup self.identity. So we need to do that now.
+                api_version = self.get_metadata_api_version()
+                self.identity = ec2.get_instance_identity(
+                    api_version, self.metadata_address).get('document', {})
             return self.identity.get(
                 'instanceId', self.metadata['instance-id'])
         else:
diff --git a/cloudinit/ssh_util.py b/cloudinit/ssh_util.py
index b95b956..882517f 100644
--- a/cloudinit/ssh_util.py
+++ b/cloudinit/ssh_util.py
@@ -171,16 +171,13 @@ def parse_authorized_keys(fname):
 
 
 def update_authorized_keys(old_entries, keys):
-    to_add = list(keys)
-
+    to_add = list([k for k in keys if k.valid()])
     for i in range(0, len(old_entries)):
         ent = old_entries[i]
         if not ent.valid():
             continue
         # Replace those with the same base64
         for k in keys:
-            if not ent.valid():
-                continue
             if k.base64 == ent.base64:
                 # Replace it with our better one
                 ent = k
diff --git a/debian/changelog b/debian/changelog
index 35cabb1..fe1b23d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+cloud-init (17.2-35-gf576b2a2-0ubuntu1~17.10.1) artful-proposed; urgency=medium
+
+  * New upstream snapshot. (LP: #1747059)
+    - tests: add support for logs with lxd from snap and future lxd 3.
+    - EC2: Fix get_instance_id called against cached datasource pickle.
+    - cli: fix cloud-init status to report running when before result.json
+    - net: accept network-config in netplan format for renaming interfaces
+    - Fix ssh keys validation in ssh_util [Tatiana Kholkina]
+
+ -- Chad Smith <[e-mail]>  Mon, 12 Feb 2018 10:16:42 -0700
+
 cloud-init (17.2-30-gf7deaf15-0ubuntu1~17.10.1) artful-proposed; urgency=medium
 
   * New upstream snapshot. (LP: #1747059)
diff --git a/tests/cloud_tests/collect.py b/tests/cloud_tests/collect.py
index 5ea88e5..d4f9135 100644
--- a/tests/cloud_tests/collect.py
+++ b/tests/cloud_tests/collect.py
@@ -44,8 +44,9 @@ def collect_console(instance, base_dir):
     LOG.debug('getting console log for %s to %s', instance, logfile)
     try:
         data = instance.console_log()
-    except NotImplementedError:
-        data = b'instance.console_log: not implemented'
+    except NotImplementedError as e:
+        # args[0] is hacky, but thats all I see to get at the message.
+        data = b'NotImplementedError:' + e.args[0].encode()
     with open(logfile, "wb") as fp:
         fp.write(data)
 
diff --git a/tests/cloud_tests/platforms/lxd/instance.py b/tests/cloud_tests/platforms/lxd/instance.py
index d2d2a1f..0488da5 100644
--- a/tests/cloud_tests/platforms/lxd/instance.py
+++ b/tests/cloud_tests/platforms/lxd/instance.py
@@ -6,7 +6,9 @@ import os
 import shutil
 from tempfile import mkdtemp
 
-from cloudinit.util import subp, ProcessExecutionError
+from cloudinit.util import load_yaml, subp, ProcessExecutionError, which
+from tests.cloud_tests import LOG
+from tests.cloud_tests.util import PlatformError
 
 from ..instances import Instance
 
@@ -15,6 +17,8 @@ class LXDInstance(Instance):
     """LXD container backed instance."""
 
     platform_name = "lxd"
+    _console_log_method = None
+    _console_log_file = None
 
     def __init__(self, platform, name, properties, config, features,
                  pylxd_container):
@@ -30,8 +34,8 @@ class LXDInstance(Instance):
         super(LXDInstance, self).__init__(
             platform, name, properties, config, features)
         self.tmpd = mkdtemp(prefix="%s-%s" % (type(self).__name__, name))
-        self._setup_console_log()
         self.name = name
+        self._setup_console_log()
 
     @property
     def pylxd_container(self):
@@ -39,21 +43,6 @@ class LXDInstance(Instance):
         self._pylxd_container.sync()
         return self._pylxd_container
 
-    def _setup_console_log(self):
-        logf = os.path.join(self.tmpd, "console.log")
-
-        # doing this ensures we can read it. Otherwise it ends up root:root.
-        with open(logf, "w") as fp:
-            fp.write("# %s\n" % self.name)
-
-        cfg = "lxc.console.logfile=%s" % logf
-        orig = self._pylxd_container.config.get('raw.lxc', "")
-        if orig:
-            orig += "\n"
-        self._pylxd_container.config['raw.lxc'] = orig + cfg
-        self._pylxd_container.save()
-        self._console_log_file = logf
-
     def _execute(self, command, stdin=None, env=None):
         if env is None:
             env = {}
@@ -97,19 +86,80 @@ class LXDInstance(Instance):
         """
         self.pylxd_container.files.put(remote_path, data)
 
+    @property
+    def console_log_method(self):
+        if self._console_log_method is not None:
+            return self._console_log_method
+
+        client = which('lxc')
+        if not client:
+            raise PlatformError("No 'lxc' client.")
+
+        elif _has_proper_console_support():
+            self._console_log_method = 'show-log'
+        elif client.startswith("/snap"):
+            self._console_log_method = 'logfile-snap'
+        else:
+            self._console_log_method = 'logfile-tmp'
+
+        LOG.debug("Set console log method to %s", self._console_log_method)
+        return self._console_log_method
+
+    def _setup_console_log(self):
+        method = self.console_log_method
+        if not method.startswith("logfile-"):
+            return
+
+        if method == "logfile-snap":
+            log_dir = "/var/snap/lxd/common/consoles"
+            if not os.path.exists(log_dir):
+                raise PlatformError(
+                    "Unable to log with snap lxc.  Please run:\n"
+                    "  sudo mkdir --mode=1777 -p %s" % log_dir)
+        elif method == "logfile-tmp":
+            log_dir = "/tmp"
+        else:
+            raise PlatformError(
+                "Unexpected value for console method: %s" % method)
+
+        # doing this ensures we can read it. Otherwise it ends up root:root.
+        log_file = os.path.join(log_dir, self.name)
+        with open(log_file, "w") as fp:
+            fp.write("# %s\n" % self.name)
+
+        cfg = "lxc.console.logfile=%s" % log_file
+        orig = self._pylxd_container.config.get('raw.lxc', "")
+        if orig:
+            orig += "\n"
+        self._pylxd_container.config['raw.lxc'] = orig + cfg
+        self._pylxd_container.save()
+        self._console_log_file = log_file
+
     def console_log(self):
         """Console log.
 
-        @return_value: bytes of this instance’s console
+        @return_value: bytes of this instance's console
         """
-        if not os.path.exists(self._console_log_file):
-            raise NotImplementedError(
-                "Console log '%s' does not exist. If this is a remote "
-                "lxc, then this is really NotImplementedError.  If it is "
-                "A local lxc, then this is a RuntimeError."
-                "https://github.com/lxc/lxd/issues/1129")
-        with open(self._console_log_file, "rb") as fp:
-            return fp.read()
+
+        if self._console_log_file:
+            if not os.path.exists(self._console_log_file):
+                raise NotImplementedError(
+                    "Console log '%s' does not exist. If this is a remote "
+                    "lxc, then this is really NotImplementedError.  If it is "
+                    "A local lxc, then this is a RuntimeError."
+                    "https://github.com/lxc/lxd/issues/1129")
+            with open(self._console_log_file, "rb") as fp:
+                return fp.read()
+
+        try:
+            stdout, stderr = subp(
+                ['lxc', 'console', '--show-log', self.name], decode=False)
+            return stdout
+        except ProcessExecutionError as e:
+            raise PlatformError(
+                "console log",
+                "Console log failed [%d]: stdout=%s stderr=%s" % (
+                    e.exit_code, e.stdout, e.stderr))
 
     def reboot(self, wait=True):
         """Reboot instance."""
@@ -146,7 +196,37 @@ class LXDInstance(Instance):
         if self.platform.container_exists(self.name):
             raise OSError('container {} was not properly removed'
                           .format(self.name))
+        if self._console_log_file and os.path.exists(self._console_log_file):
+            os.unlink(self._console_log_file)
         shutil.rmtree(self.tmpd)
         super(LXDInstance, self).destroy()
 
+
+def _has_proper_console_support():
+    stdout, _ = subp(['lxc', 'info'])
+    info = load_yaml(stdout)
+    reason = None
+    if 'console' not in info.get('api_extensions', []):
+        reason = "LXD server does not support console api extension"
+    else:
+        dver = info.get('environment', {}).get('driver_version', "")
+        if dver.startswith("2.") or dver.startwith("1."):
+            reason = "LXD Driver version not 3.x+ (%s)" % dver
+        else:
+            try:
+                stdout, stderr = subp(['lxc', 'console', '--help'],
+                                      decode=False)
+                if not (b'console' in stdout and b'log' in stdout):
+                    reason = "no '--log' in lxc console --help"
+            except ProcessExecutionError as e:
+                reason = "no 'console' command in lxc client"
+
+    if reason:
+        LOG.debug("no console-support: %s", reason)
+        return False
+    else:
+        LOG.debug("console-support looks good")
+        return True
+
+
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
index 0f7267b..dff8b1e 100644
--- a/tests/unittests/test_datasource/test_ec2.py
+++ b/tests/unittests/test_datasource/test_ec2.py
@@ -2,6 +2,7 @@
 
 import copy
 import httpretty
+import json
 import mock
 
 from cloudinit import helpers
@@ -9,6 +10,29 @@ from cloudinit.sources import DataSourceEc2 as ec2
 from cloudinit.tests import helpers as test_helpers
 
 
+DYNAMIC_METADATA = {
+    "instance-identity": {
+        "document": json.dumps({
+            "devpayProductCodes": None,
+            "marketplaceProductCodes": ["1abc2defghijklm3nopqrs4tu"],
+            "availabilityZone": "us-west-2b",
+            "privateIp": "10.158.112.84",
+            "version": "2017-09-30",
+            "instanceId": "my-identity-id",
+            "billingProducts": None,
+            "instanceType": "t2.micro",
+            "accountId": "123456789012",
+            "imageId": "ami-5fb8c835",
+            "pendingTime": "2016-11-19T16:32:11Z",
+            "architecture": "x86_64",
+            "kernelId": None,
+            "ramdiskId": None,
+            "region": "us-west-2"
+        })
+    }
+}
+
+
 # collected from api version 2016-09-02/ with
 # python3 -c 'import json
 # from cloudinit.ec2_utils import get_instance_metadata as gm
@@ -85,7 +109,7 @@ DEFAULT_METADATA = {
     "public-keys": {"brickies": ["ssh-rsa AAAAB3Nz....w== brickies"]},
     "reservation-id": "r-01efbc9996bac1bd6",
     "security-groups": "my-wide-open",
-    "services": {"domain": "amazonaws.com", "partition": "aws"}
+    "services": {"domain": "amazonaws.com", "partition": "aws"},
 }
 
 
@@ -341,6 +365,39 @@ class TestEc2(test_helpers.HttprettyTestCase):
         self.assertEqual(expected, ds.network_config)
 
     @httpretty.activate
+    def test_ec2_get_instance_id_refreshes_identity_on_upgrade(self):
+        """get_instance-id gets DataSourceEc2Local.identity if not present.
+
+        This handles an upgrade case where the old pickled datasource didn't
+        set up self.identity, but 'systemctl cloud-init init' runs
+        get_instance_id which traces on missing self.identity. lp:1748354.
+        """
+        self.datasource = ec2.DataSourceEc2Local
+        ds = self._setup_ds(
+            platform_data=self.valid_platform_data,
+            sys_cfg={'datasource': {'Ec2': {'strict_id': False}}},
+            md=DEFAULT_METADATA)
+        # Mock 404s on all versions except latest
+        all_versions = (
+            [ds.min_metadata_version] + ds.extended_metadata_versions)
+        for ver in all_versions[:-1]:
+            register_mock_metaserver(
+                'http://169.254.169.254/{0}/meta-data/instance-id'.format(ver),
+                None)
+        ds.metadata_address = 'http://169.254.169.254'
+        register_mock_metaserver(
+            '{0}/{1}/meta-data/'.format(ds.metadata_address, all_versions[-1]),
+            DEFAULT_METADATA)
+        # Register dynamic/instance-identity document which we now read.
+        register_mock_metaserver(
+            '{0}/{1}/dynamic/'.format(ds.metadata_address, all_versions[-1]),
+            DYNAMIC_METADATA)
+        ds._cloud_platform = ec2.Platforms.AWS
+        # Setup cached metadata on the Datasource
+        ds.metadata = DEFAULT_METADATA
+        self.assertEqual('my-identity-id', ds.get_instance_id())
+
+    @httpretty.activate
     @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
     def test_valid_platform_with_strict_true(self, m_dhcp):
         """Valid platform data should return true with strict_id true."""
diff --git a/tests/unittests/test_sshutil.py b/tests/unittests/test_sshutil.py
index 2a8e6ab..4c62c8b 100644
--- a/tests/unittests/test_sshutil.py
+++ b/tests/unittests/test_sshutil.py
@@ -126,6 +126,48 @@ class TestAuthKeyLineParser(test_helpers.TestCase):
         self.assertFalse(key.valid())
 
 
+class TestUpdateAuthorizedKeys(test_helpers.TestCase):
+
+    def test_new_keys_replace(self):
+        """new entries with the same base64 should replace old."""
+        orig_entries = [
+            ' '.join(('rsa', VALID_CONTENT['rsa'], 'orig_comment1')),
+            ' '.join(('dsa', VALID_CONTENT['dsa'], 'orig_comment2'))]
+
+        new_entries = [
+            ' '.join(('rsa', VALID_CONTENT['rsa'], 'new_comment1')), ]
+
+        expected = '\n'.join([new_entries[0], orig_entries[1]]) + '\n'
+
+        parser = ssh_util.AuthKeyLineParser()
+        found = ssh_util.update_authorized_keys(
+            [parser.parse(p) for p in orig_entries],
+            [parser.parse(p) for p in new_entries])
+
+        self.assertEqual(expected, found)
+
+    def test_new_invalid_keys_are_ignored(self):
+        """new entries that are invalid should be skipped."""
+        orig_entries = [
+            ' '.join(('rsa', VALID_CONTENT['rsa'], 'orig_comment1')),
+            ' '.join(('dsa', VALID_CONTENT['dsa'], 'orig_comment2'))]
+
+        new_entries = [
+            ' '.join(('rsa', VALID_CONTENT['rsa'], 'new_comment1')),
+            'xxx-invalid-thing1',
+            'xxx-invalid-blob2'
+        ]
+
+        expected = '\n'.join([new_entries[0], orig_entries[1]]) + '\n'
+
+        parser = ssh_util.AuthKeyLineParser()
+        found = ssh_util.update_authorized_keys(
+            [parser.parse(p) for p in orig_entries],
+            [parser.parse(p) for p in new_entries])
+
+        self.assertEqual(expected, found)
+
+
 class TestParseSSHConfig(test_helpers.TestCase):
 
     def setUp(self):