cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:bug/1674861-ds-identify-gce into cloud-init:master.

Commit message:
GCE: Search GCE in ds-identify, consider serial number in check.

While documentation indicates that the smbios product name should
contain 'Google Compute Engine', experimentation and bug reports
indicate that is not always the case.  The change here is to change
the check for GCE to also consider a serial number that starts with
'GoogleCompute-'.

Also, ds-identify was not currently searching for GCE if no config of
datasource_list was found.  Most images have a datasource_list defined.
So update the list to include GCE.

LP: #1674861

Requested reviews:
  cloud init development team (cloud-init-dev)
Related bugs:
  Bug #1674861 in cloud-init: "Google Compute Engine (GCE) datasource setting to none after update"
  https://bugs.launchpad.net/cloud-init/+bug/1674861

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/321094
-- 
Your team cloud init development team is requested to review the proposed merge of ~smoser/cloud-init:bug/1674861-ds-identify-gce into cloud-init:master.

diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py
index b1a1c8f..637c950 100644
--- a/cloudinit/sources/DataSourceGCE.py
+++ b/cloudinit/sources/DataSourceGCE.py
@@ -62,6 +62,9 @@ class DataSourceGCE(sources.DataSource):
             return public_key
 
     def get_data(self):
+        if not platform_reports_gce():
+            return False
+
         # url_map: (our-key, path, required, is_text)
         url_map = [
             ('instance-id', ('instance/id',), True, True),
@@ -144,6 +147,21 @@ class DataSourceGCE(sources.DataSource):
         return self.availability_zone.rsplit('-', 1)[0]
 
 
+def platform_reports_gce():
+    pname = util.read_dmi_data('system-product-name') or "N/A"
+    if pname == "Google Compute Engine":
+        return True
+
+    # system-product-name is not always guaranteed (LP: #1674861)
+    serial = util.read_dmi_data('system-serial-number') or "N/A"
+    if serial.startswith("GoogleCloud-"):
+        return True
+
+    LOG.debug("Not running on google cloud. product-name=%s serial=%s",
+              pname, serial)
+    return False
+
+
 # Used to match classes to dependencies
 datasources = [
     (DataSourceGCE, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)),
diff --git a/tests/unittests/test_datasource/test_gce.py b/tests/unittests/test_datasource/test_gce.py
index 4f83454..3eaa58e 100644
--- a/tests/unittests/test_datasource/test_gce.py
+++ b/tests/unittests/test_datasource/test_gce.py
@@ -5,6 +5,7 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
 import httpretty
+import mock
 import re
 
 from base64 import b64encode, b64decode
@@ -71,6 +72,11 @@ class TestDataSourceGCE(test_helpers.HttprettyTestCase):
         self.ds = DataSourceGCE.DataSourceGCE(
             settings.CFG_BUILTIN, None,
             helpers.Paths({}))
+        self.m_platform_reports_gce = mock.patch(
+            'cloudinit.sources.DataSourceGCE.platform_reports_gce',
+            return_value=True)
+        self.m_platform_reports_gce.start()
+        self.addCleanup(self.m_platform_reports_gce.stop)
         super(TestDataSourceGCE, self).setUp()
 
     def test_connection(self):
@@ -153,7 +159,13 @@ class TestDataSourceGCE(test_helpers.HttprettyTestCase):
 
     def test_only_last_part_of_zone_used_for_availability_zone(self):
         _set_mock_metadata()
-        self.ds.get_data()
+        r = self.ds.get_data()
+        self.assertEqual(True, r)
         self.assertEqual('bar', self.ds.availability_zone)
 
+    def test_get_data_returns_false_if_not_on_gce(self):
+        self.m_platform_reports_gce.return_value = False
+        self.assertEqual(False, self.ds.get_data())
+
+
 # vi: ts=4 expandtab
diff --git a/tools/ds-identify b/tools/ds-identify
index bf09a3a..8e9ab8b 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -108,7 +108,7 @@ DI_DSNAME=""
 # this has to match the builtin list in cloud-init, it is what will
 # be searched if there is no setting found in config.
 DI_DSLIST_DEFAULT="MAAS ConfigDrive NoCloud AltCloud Azure Bigstep \
-CloudSigma CloudStack DigitalOcean Ec2 OpenNebula OpenStack OVF SmartOS"
+CloudSigma CloudStack DigitalOcean Ec2 GCE OpenNebula OpenStack OVF SmartOS"
 DI_DSLIST=""
 DI_MODE=""
 DI_ON_FOUND=""
@@ -383,6 +383,14 @@ dmi_product_name_matches() {
     return 1
 }
 
+dmi_product_serial_matches() {
+    is_container && return 1
+    case "${DI_DMI_PRODUCT_SERIAL}" in
+        $1) return 0;;
+    esac
+    return 1
+}
+
 dmi_product_name_is() {
     is_container && return 1
     [ "${DI_DMI_PRODUCT_NAME}" = "$1" ]
@@ -767,7 +775,11 @@ dscheck_Ec2() {
 }
 
 dscheck_GCE() {
-    if dmi_product_name_is "Google Compute Engine"; then
+    dmi_product_name_is "Google Compute Engine"; then
+        return ${DS_FOUND}
+    fi
+    # product name is not guaranteed (LP: #1674861)
+    if dmi_product_serial_matches "GoogleCloud-*"; then
         return ${DS_FOUND}
     fi
     return ${DS_NOT_FOUND}