Merge ~smoser/cloud-init:bug/1715241-openstack-maybe-on-non-intel into cloud-init:master

[Scott Moser]

ds-identify: OpenStack on arch other than intel, identify 'maybe'

OpenStack Nova identifies itself only to Intel guests.
Make ds-identify return 'MAYBE' for OpenStack on non-intel arches.

An unnecessary change here is to rename the 'policy_nodmi' kwarg
to 'policy_no_dmi' in the related unit tests.

LP: #1715241

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:7e33dc9d39cc7de7be7cd5306db12b6c78ed684b
https://jenkins.ubuntu.com/server/job/cloud-init-ci/256/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/256/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

On September 5, 2017 5:20:40 PM EDT, Ryan Harper <email address hidden> wrote:
>
>
>Diff comments:
>
>> diff --git a/tests/unittests/test_ds_identify.py
>b/tests/unittests/test_ds_identify.py
>> index 1a81a89..c389029 100644
>> --- a/tests/unittests/test_ds_identify.py
>> +++ b/tests/unittests/test_ds_identify.py
>> @@ -48,6 +54,7 @@ P_SEED_DIR = "var/lib/cloud/seed"
>> P_DSID_CFG = "etc/cloud/ds-identify.cfg"
>>
>> MOCK_VIRT_IS_KVM = {'name': 'detect_virt', 'RET': 'kvm', 'ret': 0}
>> +MOCK_UNAME_IS_PPC64 = {'name': 'uname', 'out': UNAME_PPC64EL, 'ret':
>0}
>
>why is there 'out' here vs. 'RET' in the other mock?
>

Detect_virt is a function that sets the _RET variable. It has no stdout or stderr. Uname is actually a command that has its stdout and stderr mocked.

[Ryan Harper (raharper)]

It may be more obvious for the MOCK_XXX values always have the same keys (even if they don't use them).

MOCK_VIRT_IS_KVM = {
  'name': 'detect_virt',
  'out': "",
  'err': "",
  'RET': "kvm",
  'ret': 0
}

[Scott Moser (smoser)]

It might be more obvious, but its more verbose.
Same as calling a function with default arguments.
The default values are filled in via 'write_mock' in call.

the reason that we have 'detect_virt' as a shell function is that the call binary it is wrapping is 'detect-virt', which is not a valid posix shell function name... so we can't just mock it by adding a function (which is what the 'write_mock' function does).

[Chad Smith (chad.smith)]

Ok so this adds a slight cost to non-x86 platforms init-network stage by placing a maybe on OpenStack datasource right? This maybe openstack still won't affect datasources which now run in init-local because they would have already been discovered. So ConfigDrive, Ec2,SmartOS, OpenNebula,DO etc would have already been discovered if their platforms were non-x86.

[Scott Moser (smoser)]

will address chad's comment.

c2637c7... by Scott Moser on 2017-09-07

address feedback: check that it returned maybe.

[Scott Moser (smoser)]

addressed chad's request.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:1722c4b5601da81197289c3ca87837172039922b
https://jenkins.ubuntu.com/server/job/cloud-init-ci/265/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/265/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:c2637c7cfa67049f69ba0e4260deb98a8b84d81d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/266/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/266/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

+1 on this changeset. Thanks!

review: Approve

diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
index 1a81a89..92454d7 100644
--- a/tests/unittests/test_ds_identify.py
+++ b/tests/unittests/test_ds_identify.py
@@ -11,6 +11,10 @@ from cloudinit.tests.helpers import (
 
 UNAME_MYSYS = ("Linux bart 4.4.0-62-generic #83-Ubuntu "
                "SMP Wed Jan 18 14:10:15 UTC 2017 x86_64 GNU/Linux")
+UNAME_PPC64EL = ("Linux diamond 4.4.0-83-generic #106-Ubuntu SMP "
+                 "Mon Jun 26 17:53:54 UTC 2017 "
+                 "ppc64le ppc64le ppc64le GNU/Linux")
+
 BLKID_EFI_ROOT = """
 DEVNAME=/dev/sda1
 UUID=8B36-5390
@@ -23,6 +27,8 @@ TYPE=ext4
 PARTUUID=30c65c77-e07d-4039-b2fb-88b1fb5fa1fc
 """
 
+POLICY_FOUND_ONLY = "search,found=all,maybe=none,notfound=disabled"
+POLICY_FOUND_OR_MAYBE = "search,found=all,maybe=all,notfound=disabled"
 DI_DEFAULT_POLICY = "search,found=all,maybe=all,notfound=enabled"
 DI_DEFAULT_POLICY_NO_DMI = "search,found=all,maybe=all,notfound=disabled"
 
@@ -48,6 +54,7 @@ P_SEED_DIR = "var/lib/cloud/seed"
 P_DSID_CFG = "etc/cloud/ds-identify.cfg"
 
 MOCK_VIRT_IS_KVM = {'name': 'detect_virt', 'RET': 'kvm', 'ret': 0}
+MOCK_UNAME_IS_PPC64 = {'name': 'uname', 'out': UNAME_PPC64EL, 'ret': 0}
 
 
 class TestDsIdentify(CiTestCase):
@@ -55,7 +62,7 @@ class TestDsIdentify(CiTestCase):
 
     def call(self, rootd=None, mocks=None, args=None, files=None,
              policy_dmi=DI_DEFAULT_POLICY,
-             policy_nodmi=DI_DEFAULT_POLICY_NO_DMI):
+             policy_no_dmi=DI_DEFAULT_POLICY_NO_DMI):
         if args is None:
             args = []
         if mocks is None:
@@ -81,7 +88,7 @@ class TestDsIdentify(CiTestCase):
             "PATH_ROOT='%s'" % rootd,
             ". " + self.dsid_path,
             'DI_DEFAULT_POLICY="%s"' % policy_dmi,
-            'DI_DEFAULT_POLICY_NO_DMI="%s"' % policy_nodmi,
+            'DI_DEFAULT_POLICY_NO_DMI="%s"' % policy_no_dmi,
             ""
         ]
 
@@ -137,7 +144,7 @@ class TestDsIdentify(CiTestCase):
     def _call_via_dict(self, data, rootd=None, **kwargs):
         # return output of self.call with a dict input like VALID_CFG[item]
         xwargs = {'rootd': rootd}
-        for k in ('mocks', 'args', 'policy_dmi', 'policy_nodmi', 'files'):
+        for k in ('mocks', 'args', 'policy_dmi', 'policy_no_dmi', 'files'):
             if k in data:
                 xwargs[k] = data[k]
             if k in kwargs:
@@ -261,6 +268,31 @@ class TestDsIdentify(CiTestCase):
         self._check_via_dict(mydata, rc=RC_FOUND, dslist=['AliYun', DS_NONE],
                              policy_dmi=policy)
 
+    def test_default_openstack_intel_is_found(self):
+        """On Intel, openstack must be identified."""
+        self._test_ds_found('OpenStack')
+
+    def test_openstack_on_non_intel_is_maybe(self):
+        """On non-Intel, openstack without dmi info is maybe.
+
+        nova does not identify itself on platforms other than intel.
+           https://bugs.launchpad.net/cloud-init/+bugs?field.tag=dsid-nova"""
+
+        data = VALID_CFG['OpenStack'].copy()
+        del data['files'][P_PRODUCT_NAME]
+        data.update({'policy_dmi': POLICY_FOUND_OR_MAYBE,
+                     'policy_no_dmi': POLICY_FOUND_OR_MAYBE})
+
+        # this should show not found as default uname in tests is intel.
+        # and intel openstack requires positive identification.
+        self._check_via_dict(data, RC_NOT_FOUND, dslist=None)
+
+        # updating the uname to ppc64 though should get a maybe.
+        data.update({'mocks': [MOCK_VIRT_IS_KVM, MOCK_UNAME_IS_PPC64]})
+        (_, _, err, _, _) = self._check_via_dict(
+            data, RC_FOUND, dslist=['OpenStack', 'None'])
+        self.assertIn("check for 'OpenStack' returned maybe", err)
+
 
 def blkid_out(disks=None):
     """Convert a list of disk dictionaries into blkid content."""
@@ -341,6 +373,13 @@ VALID_CFG = {
         'files': {P_PRODUCT_SERIAL: 'GoogleCloud-8f2e88f\n'},
         'mocks': [MOCK_VIRT_IS_KVM],
     },
+    'OpenStack': {
+        'ds': 'OpenStack',
+        'files': {P_PRODUCT_NAME: 'OpenStack Nova\n'},
+        'mocks': [MOCK_VIRT_IS_KVM],
+        'policy_dmi': POLICY_FOUND_ONLY,
+        'policy_no_dmi': POLICY_FOUND_ONLY,
+    },
     'ConfigDrive': {
         'ds': 'ConfigDrive',
         'mocks': [
diff --git a/tools/ds-identify b/tools/ds-identify
index 33bd299..ee5e05a 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -833,6 +833,12 @@ dscheck_OpenStack() {
         return ${DS_FOUND}
     fi
 
+    # LP: #1715241 : arch other than intel are not identified properly.
+    case "$DI_UNAME_MACHINE" in
+        i?86|x86_64) :;;
+        *) return ${DS_MAYBE};;
+    esac
+
     return ${DS_NOT_FOUND}
 }