Merge ~jocha/cloud-init:jocha-reboot-bugfix into cloud-init:master

[Joshua Chan]

Add reported ready marker file.

This change is for Azure VM Preprovisioning. A bug was found when after azure VMs report ready the first time, during the time when VM is polling indefinitely for the new ovf-env.xml from Instance Metadata Service (IMDS), if a reboot happens, we send another report ready signal to the fabric, which deletes the reprovisioning data on the node.

This marker file is used to fix this issue so that we will only send a report ready signal to the fabric when no marker file is present. Then, create a marker file so that when a reboot does occur, we check if a marker file has been created and decide whether we would like to send the repot ready signal.

LP: #1765214

[Chad Smith (chad.smith)]

review: Needs Fixing

Unmerged commits

23002d0... by Joshua Chan on 2018-04-24

fix string formatting and split up unit tests

eb99235... by Mike Gerdts on 2018-04-23

DataSourceSmartOS: add locking of serial device.

cloud-init and mdata-get each have their own implementation of the
SmartOS metadata protocol. If cloud-init and other services that call
mdata-get are run concurrently, crosstalk on the serial port can cause
them both to become confused.

This change makes it so that cloud-init uses the same cooperative
locking scheme that's used by mdata-get, thus preventing cross-talk
between mdata-get and cloud-init.

For testing, a VM running on a SmartOS host and pyserial are required.
If the tests are run on a platform other than SmartOS, those that use a
real serial port are skipped. pyserial remains commented in
requirements.txt because most testers will not be running atop SmartOS.

LP: #1746605

3352633... by Mike Gerdts on 2018-04-21

DataSourceSmartOS: sdc:hostname is ignored

There are three potential sources of the hostname, one of which is
documented SmartOS's vmadm(1M) via the hostname property. That
property's value is retrieved via the sdc:hostname key. The other
two sources for the hostname are a hostname key in customer_metadata
and the VM's uuid (sdc:uuid). Of these three, the sdc:hostname value
is not used in a meaningful way by DataSourceSmartOS.

This fix changes the fallback mechanism when hostname is not
specified in customer_metadata. The order of precedence for setting
the hostname is now 1) hostname in customer_metadata,
2) sdc:hostname, then 3) sdc:uuid.

LP: #1765085

fe82866... by Mike Gerdts on 2018-04-21

DataSourceSmartOS: list() should always return a list

If customer_metadata has no keys, the KEYS request returns an empty
string. Callers of the list() method expect a list to be returned and
will give a stack trace if this expectation is not met.

LP: #1763480

22d5d58... by Scott Moser on 2018-04-21

schema: in validation, raise ImportError if strict but no jsonschema.

validate_cloudconfig_schema with strict=True would not actually validate
if there was no jsonschema available. That seems kind of strange.
The change here is to make it raise an exception if strict was passed in.
And then to fix the one test that needed a skipIfJsonSchema wrapper.

5114828... by Scott Moser on 2018-04-20

set_passwords: Add newline to end of sshd config, only restart if updated.

This admittedly does a fairly extensive re-factor to simply add a newline
to the end of sshd_config.

It makes the ssh_config updating portion of set_passwords more testable
and adds tests for that.

The new function is in 'update_ssh_config_lines' which allows you
to update a config with multiple changes even though only a single one
is currently used.

We also only restart the ssh daemon now if a change was made to the
config file. Before it was always restarted if the user specified
a value for ssh_pwauth other than 'unchanged'.

Thanks to Lorens Kockum for initial diagnosis and patch.

LP: #1677205

222cabe... by Scott Moser on 2018-04-20

pylint: pay attention to unused variable warnings.

This enables warnings produced by pylint for unused variables (W0612),
and fixes the existing errors.

f7ac3e6... by Junjie.Wang on 2018-04-19

doc: Add documentation for AliYun datasource.

Just add some documentation to readthedocs for AliYun.

5670b53... by Scott Moser on 2018-04-19

Schema: do not warn on duplicate items in commands.

runcmd, bootcmd, snap/commands, ubuntu-advantage/commands would
log warning (and fail if strict) on duplicate values in the commands.
But those should be allowed. Example, it is perfectly valid to do:
   runcmd: ['sleep 1', 'sleep 1']

LP: #1764264

3bd8fd3... by Chad Smith on 2018-04-18

net: Depend on iproute2's ip instead of net-tools ifconfig or route

The net-tools package is deprecated and will eventually be dropped. Use
"ip route", "link" or "address" instead of "ifconfig" or "route" calls.
Cloud-init can now run in an environment that no longer has net-tools.
This affects the network and route printing emitted to
cloud-config-output.log as well as the cc_disable_ec2_metadata module.

Additional changes:
 - separate readResource and resourceLocation into standalone test
   functions
 - Fix ipv4 address rows to report scopes represented by ip addr show
 - Formatted route/address ouput now handles multiple ipv4 and ipv6
   addresses on a single interface

Co-authored-by: James Hogarth <email address hidden>
Co-authored-by: Robert Schweikert <email address hidden>

diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index a71197a..b47a6af 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -48,6 +48,7 @@ DEFAULT_FS = 'ext4'
 # DMI chassis-asset-tag is set static for all azure instances
 AZURE_CHASSIS_ASSET_TAG = '7783-7084-3265-9085-8269-3286-77'
 REPROVISION_MARKER_FILE = "/var/lib/cloud/data/poll_imds"
+REPORTED_READY_MARKER_FILE = "/var/lib/cloud/data/reported_ready"
 IMDS_URL = "http://169.254.169.254/metadata/reprovisiondata"
 
 
@@ -450,13 +451,16 @@ class DataSourceAzure(sources.DataSource):
             # call DHCP and setup the ephemeral network to acquire the new IP.
             return False
 
-        need_report = report_ready
         while True:
             try:
                 with EphemeralDHCPv4() as lease:
-                    if need_report:
+                    if report_ready:
                         self._report_ready(lease=lease)
-                        need_report = False
+                        path = REPORTED_READY_MARKER_FILE
+                        LOG.info("Create flag file, reported ready: %s", path)
+                        util.write_file(path, "{pid}: {time}\n".format(
+                            pid=os.getpid(), time=time()))
+                        report_ready = False
                     return readurl(url, timeout=1, headers=headers,
                                    exception_cb=exc_cb, infinite=True).contents
             except UrlError:
@@ -490,14 +494,17 @@ class DataSourceAzure(sources.DataSource):
         if (cfg.get('PreprovisionedVm') is True or
                 os.path.isfile(path)):
             if not os.path.isfile(path):
-                LOG.info("Creating a marker file to poll imds")
-                util.write_file(path, "%s: %s\n" % (os.getpid(), time()))
+                LOG.info("Creating a marker file to poll imds: %s",
+                         path)
+                util.write_file(path, "{pid}: {time}\n".format(
+                    pid=os.getpid(), time=time()))
             return True
         return False
 
     def _reprovision(self):
         """Initiate the reprovisioning workflow."""
-        contents = self._poll_imds()
+        report_ready = bool(not os.path.isfile(REPORTED_READY_MARKER_FILE))
+        contents = self._poll_imds(report_ready)
         md, ud, cfg = read_azure_ovf(contents)
         return (md, ud, cfg, {'ovf-env.xml': contents})
 
@@ -526,6 +533,7 @@ class DataSourceAzure(sources.DataSource):
                 "Error communicating with Azure fabric; You may experience."
                 "connectivity issues.", exc_info=True)
             return False
+        util.del_file(REPORTED_READY_MARKER_FILE)
         util.del_file(REPROVISION_MARKER_FILE)
         return fabric_data
 
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 88fe76c..4ff2a66 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -1163,12 +1163,13 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
         cfg = ret[2]
         self.assertFalse(cfg['PreprovisionedVm'])
 
+    @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file')
     @mock.patch('cloudinit.sources.DataSourceAzure.util.is_FreeBSD')
     @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network')
     @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
     @mock.patch('requests.Session.request')
     def test_poll_imds_returns_ovf_env(self, fake_resp, m_dhcp, m_net,
-                                       m_is_bsd, *args):
+                                       m_is_bsd, write_f, *args):
         """The _poll_imds method should return the ovf_env.xml."""
         m_is_bsd.return_value = False
         m_dhcp.return_value = [{
@@ -1194,12 +1195,13 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
             prefix_or_mask='255.255.255.0', router='192.168.2.1')
         self.assertEqual(m_net.call_count, 1)
 
+    @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file')
     @mock.patch('cloudinit.sources.DataSourceAzure.util.is_FreeBSD')
     @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network')
     @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
     @mock.patch('requests.Session.request')
     def test__reprovision_calls__poll_imds(self, fake_resp, m_dhcp, m_net,
-                                           m_is_bsd, *args):
+                                           m_is_bsd, write_f, *args):
         """The _reprovision method should call poll IMDS."""
         m_is_bsd.return_value = False
         m_dhcp.return_value = [{
@@ -1231,6 +1233,74 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
             prefix_or_mask='255.255.255.0', router='192.168.2.1')
         self.assertEqual(m_net.call_count, 1)
 
+    @mock.patch('cloudinit.sources.DataSourceAzure.DataSourceAzure._poll_imds')
+    @mock.patch('os.path.isfile')
+    def test_reprovision_report_ready(self, isfile, _poll_imds,
+                                      *args):
+        """_reprovision should call _poll_IMDS(True) (with report ready)
+           if flag file is not present"""
+        isfile.return_value = False
+        hostname = "myhost"
+        username = "myuser"
+        odata = {'HostName': hostname, 'UserName': username}
+        _poll_imds.return_value = construct_valid_ovf_env(data=odata)
+        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        md, ud, cfg, d = dsa._reprovision()
+        self.assertEqual(_poll_imds.call_args[0], tuple([True]))
+
+    @mock.patch('cloudinit.sources.DataSourceAzure.DataSourceAzure._poll_imds')
+    @mock.patch('os.path.isfile')
+    def test_reprovision_no_report_ready(self, isfile, _poll_imds,
+                                         *args):
+        """_reprovision should call _poll_IMDS(False) (without report ready)
+           if flag file is present"""
+        isfile.return_value = True
+        hostname = "myhost"
+        username = "myuser"
+        odata = {'HostName': hostname, 'UserName': username}
+        _poll_imds.return_value = construct_valid_ovf_env(data=odata)
+        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        md, ud, cfg, d = dsa._reprovision()
+        self.assertEqual(_poll_imds.call_args[0], tuple([False]))
+
+    @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file')
+    @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network')
+    @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
+    @mock.patch('requests.Session.request')
+    @mock.patch(
+        'cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready')
+    def test_poll_imds_report_ready_true(self, report_ready_func,
+                                         fake_resp, m_dhcp, m_net,
+                                         write_f, *args):
+        """The poll_imds should call report_ready when flag is true"""
+        m_dhcp.return_value = [{
+            'interface': 'eth9', 'fixed-address': '192.168.2.9',
+            'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0',
+            'unknown-245': '624c3620'}]
+        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        report_ready_flag = True
+        dsa._poll_imds(report_ready_flag)
+        self.assertEqual(report_ready_func.call_count, 1)
+
+    @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network')
+    @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
+    @mock.patch('requests.Session.request')
+    @mock.patch(
+        'cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready')
+    def test_poll_imds_report_ready_false(self, report_ready_func,
+                                          fake_resp, m_dhcp, m_net,
+                                          *args):
+        """The poll_imds should not call reporting ready
+           when flag is false"""
+        m_dhcp.return_value = [{
+            'interface': 'eth9', 'fixed-address': '192.168.2.9',
+            'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0',
+            'unknown-245': '624c3620'}]
+        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        report_ready_flag = False
+        dsa._poll_imds(report_ready_flag)
+        self.assertEqual(report_ready_func.call_count, 0)
+
     @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file')
     @mock.patch('os.path.isfile')
     def test__should_reprovision_with_true_cfg(self, isfile, write_f, *args):