Merge ~rjschwei/cloud-init:noLnxDistro into cloud-init:master

[Robert Schweikert]

util: add get_linux_distro function to replace platform.dist

Allow the user to set the distribution with --distro argument to setup.py.
Fall back is to read /etc/os-release. Final backup is to use
platform.dist() Python function. The platform.dist() function is
deprecated and will be removed in Python 3.7

LP: #1745235

util: add get_linux_distro function to replace platform.dist

Allow the user to set the distribution with --distro argument to setup.py. Fall back is to read /etc/os-release. Final backup is to use platform.dist() Python function. The platform.dist() function is deprecated and will be removed in Python 3.7

LP: #1745235

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

PASSED: Continuous integration, rev:3c1a518f968a0f4caa30dedd4272a60067768ddf
https://jenkins.ubuntu.com/server/job/cloud-init-ci/745/
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/745/rebuild

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:cffee0de4985ea15e038a439c7f670409614db72
https://jenkins.ubuntu.com/server/job/cloud-init-ci/746/
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/746/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

I'm not I don't think we can store any non-dict values in the directory /etc/cloud/cloud.cfg.d/ because all files there are ultimately merged together in a giant system_config dictionary against which all of cloud-init runs.

If we have one offs that you want to store somewhere else, maybe just in /etc/cloud/<filename>, otherwise we risk tracebacks like this in /var/log/cloud-init-output.log

2018-05-14 22:14:46,663 - util.py[WARNING]: Failed loading yaml blob

I'm not really certain why we want to provide a facility for an image to override the distribution detection anyway with a config file on the image. Why not just make sure cloud-init detects properly the system on which it is running? Also, we do have a system_info: distro: <distro-name> config option which affects what underlying distro is selected by cloud-init and it feels like providing this additional mechanism is a bit confusing.

review: Needs Fixing

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

FAILED: Continuous integration, rev:84bcc3cab3222889604ff13367e16ce8144f7feb
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1121/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:923d318ed7622f53f17718e37916db1b9de230bb
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1122/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:9edca71ca9f3bb0bb63c646801ba2c09d18962c4
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1123/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

[Robert Schweikert (rjschwei)]

> I'm not I don't think we can store any non-dict values in the directory
> /etc/cloud/cloud.cfg.d/ because all files there are ultimately merged together
> in a giant system_config dictionary against which all of cloud-init runs.
>
> If we have one offs that you want to store somewhere else, maybe just in
> /etc/cloud/<filename>, otherwise we risk tracebacks like this in /var/log
> /cloud-init-output.log
>
> 2018-05-14 22:14:46,663 - util.py[WARNING]: Failed loading yaml blob
>
> I'm not really certain why we want to provide a facility for an image to
> override the distribution detection anyway with a config file on the image.
> Why not just make sure cloud-init detects properly the system on which it is
> running? Also, we do have a system_info: distro: <distro-name> config option
> which affects what underlying distro is selected by cloud-init and it feels
> like providing this additional mechanism is a bit confusing.

Scott requested that a method be provided for the user to override the "default" behavior. Thus if we allow the user to override the behavior at build time we have to know if the user did this in case any issues are reported. Therefore the value has to be persisted. If we do not persist the value and we get a question bug report and the config option "system_info: distro: <distro-name>" and the templates do not match the answer can only be "we do not why this happened". But if the file exists then we can say "someone overrode the distro detection mechanism during build"

I moved the file to /etc/cloud

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

PASSED: Continuous integration, rev:36c6158eb4151d37fecd53ac94b07bde721ee339
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1124/
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/1124/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

Robert, thanks for the fixes. I understand wanting to override at distro at build time and ensuring that config templates are properly generated for the target environment. What I'm not to certain about is the overriding dynamic get_linux_distro().

We had a meeting to discuss how we want to approach overriding dyanamic distro detection in your new get_linux_distro function.

I think we want to drop modifying runtime behavior from this branch until we sort a better way to override default behavior of get_linux_distro and correlate that potentially with which cloudinit.distro class if chosen as well.

As you mentioned in IRC, you can reserve the "I told you so" card for when this approach doesn't fly :)

Ultimately, we want to move the ability override distro-detection up to a higher level which has access to the consolidated system_cfg:distro as defined in /etc/cloud/cloud-cfg.d. We've also discussed the possibility of allowing a system_cfg: distro: 'auto' which would enable distro-class auto-detection using your get_linux_distro() versus a static config override from /etc/cloud/cloud.cfg

These approaches are something that will involve a bit more back and forth as far as design, so let's keep this branch simple so we can land it without the inevitable bikeshed when designing the right overridden behavior.

[Robert Schweikert (rjschwei)]

Should be all set now.

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

PASSED: Continuous integration, rev:a0c044737f0557bde3627869dc0436c8ab47d6e6
https://jenkins.ubuntu.com/server/job/cloud-init-ci/11/
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/11/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

Thank you for this rework Robert. LGTM!

review: Approve

[Chad Smith (chad.smith)]

Thank you for your merge proposal.

Your branch has been set to 'Work in progress'.
Please set the branch back to 'Needs Review' after resolving the issues below.

Thanks again,
Your friendly neighborhood cloud-init robot.

Please fix the following issues:
------------------------------
Commit message lints:
 - Line #2 has 171 too many characters. Line starts with: "Allow the user to set"...
------------------------------

For more information, see commit message guidelines at
https://cloudinit.readthedocs.io/en/latest/topics/hacking.html#do-these-things-for-each-feature-or-bug

review: Needs Fixing

[Chad Smith (chad.smith)]

oops sorry robert, automation snafu. fixing and landing

[Chad Smith (chad.smith)]

review: Approve

[Chad Smith (chad.smith)]

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=bbcc5e82

diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py
index 3c05a43..17853fc 100644
--- a/cloudinit/tests/test_util.py
+++ b/cloudinit/tests/test_util.py
@@ -3,11 +3,12 @@
 """Tests for cloudinit.util"""
 
 import logging
-from textwrap import dedent
+import platform
 
 import cloudinit.util as util
 
 from cloudinit.tests.helpers import CiTestCase, mock
+from textwrap import dedent
 
 LOG = logging.getLogger(__name__)
 
@@ -16,6 +17,29 @@ MOUNT_INFO = [
     '153 68 254:0 / /home rw,relatime shared:101 - xfs /dev/sda2 rw,attr2'
 ]
 
+OS_RELEASE_SLES = dedent("""\
+    NAME="SLES"\n
+    VERSION="12-SP3"\n
+    VERSION_ID="12.3"\n
+    PRETTY_NAME="SUSE Linux Enterprise Server 12 SP3"\n
+    ID="sles"\nANSI_COLOR="0;32"\n
+    CPE_NAME="cpe:/o:suse:sles:12:sp3"\n
+""")
+
+OS_RELEASE_UBUNTU = dedent("""\
+    NAME="Ubuntu"\n
+    VERSION="16.04.3 LTS (Xenial Xerus)"\n
+    ID=ubuntu\n
+    ID_LIKE=debian\n
+    PRETTY_NAME="Ubuntu 16.04.3 LTS"\n
+    VERSION_ID="16.04"\n
+    HOME_URL="http://www.ubuntu.com/"\n
+    SUPPORT_URL="http://help.ubuntu.com/"\n
+    BUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/"\n
+    VERSION_CODENAME=xenial\n
+    UBUNTU_CODENAME=xenial\n
+""")
+
 
 class FakeCloud(object):
 
@@ -261,4 +285,56 @@ class TestUdevadmSettle(CiTestCase):
         self.assertRaises(util.ProcessExecutionError, util.udevadm_settle)
 
 
[e-mail]('os.path.exists')
+class TestGetLinuxDistro(CiTestCase):
+
+    @classmethod
+    def os_release_exists(self, path):
+        """Side effect function"""
+        if path == '/etc/os-release':
+            return 1
+
+    @mock.patch('cloudinit.util.load_file')
+    def test_get_linux_distro_quoted_name(self, m_os_release, m_path_exists):
+        """Verify we get the correct name if the os-release file has
+        the distro name in quotes"""
+        m_os_release.return_value = OS_RELEASE_SLES
+        m_path_exists.side_effect = TestGetLinuxDistro.os_release_exists
+        dist = util.get_linux_distro()
+        self.assertEqual(('sles', '12.3', platform.machine()), dist)
+
+    @mock.patch('cloudinit.util.load_file')
+    def test_get_linux_distro_bare_name(self, m_os_release, m_path_exists):
+        """Verify we get the correct name if the os-release file does not
+        have the distro name in quotes"""
+        m_os_release.return_value = OS_RELEASE_UBUNTU
+        m_path_exists.side_effect = TestGetLinuxDistro.os_release_exists
+        dist = util.get_linux_distro()
+        self.assertEqual(('ubuntu', '16.04', platform.machine()), dist)
+
+    @mock.patch('platform.dist')
+    def test_get_linux_distro_no_data(self, m_platform_dist, m_path_exists):
+        """Verify we get no information if os-release does not exist"""
+        m_platform_dist.return_value = ('', '', '')
+        m_path_exists.return_value = 0
+        dist = util.get_linux_distro()
+        self.assertEqual(('', '', ''), dist)
+
+    @mock.patch('platform.dist')
+    def test_get_linux_distro_no_impl(self, m_platform_dist, m_path_exists):
+        """Verify we get an empty tuple when no information exists and
+        Exceptions are not propagated"""
+        m_platform_dist.side_effect = Exception()
+        m_path_exists.return_value = 0
+        dist = util.get_linux_distro()
+        self.assertEqual(('', '', ''), dist)
+
+    @mock.patch('platform.dist')
+    def test_get_linux_distro_plat_data(self, m_platform_dist, m_path_exists):
+        """Verify we get the correct platform information"""
+        m_platform_dist.return_value = ('foo', '1.1', 'aarch64')
+        m_path_exists.return_value = 0
+        dist = util.get_linux_distro()
+        self.assertEqual(('foo', '1.1', 'aarch64'), dist)
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/util.py b/cloudinit/util.py
index edfedc7..e7b5bc7 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -576,6 +576,39 @@ def get_cfg_option_int(yobj, key, default=0):
     return int(get_cfg_option_str(yobj, key, default=default))
 
 
+def get_linux_distro():
+    distro_name = ''
+    distro_version = ''
+    if os.path.exists('/etc/os-release'):
+        os_release = load_file('/etc/os-release')
+        for line in os_release.splitlines():
+            if line.strip().startswith('ID='):
+                distro_name = line.split('=')[-1]
+                distro_name = distro_name.replace('"', '')
+            if line.strip().startswith('VERSION_ID='):
+                # Lets hope for the best that distros stay consistent ;)
+                distro_version = line.split('=')[-1]
+                distro_version = distro_version.replace('"', '')
+    else:
+        dist = ('', '', '')
+        try:
+            # Will be removed in 3.7
+            dist = platform.dist()  # pylint: disable=W1505
+        except Exception:
+            pass
+        finally:
+            found = None
+            for entry in dist:
+                if entry:
+                    found = 1
+            if not found:
+                LOG.warning('Unable to determine distribution, template '
+                            'expansion may have unexpected results')
+        return dist
+
+    return (distro_name, distro_version, platform.machine())
+
+
 def system_info():
     info = {
         'platform': platform.platform(),
@@ -583,19 +616,19 @@ def system_info():
         'release': platform.release(),
         'python': platform.python_version(),
         'uname': platform.uname(),
-        'dist': platform.dist(),  # pylint: disable=W1505
+        'dist': get_linux_distro()
     }
     system = info['system'].lower()
     var = 'unknown'
     if system == "linux":
         linux_dist = info['dist'][0].lower()
-        if linux_dist in ('centos', 'fedora', 'debian'):
+        if linux_dist in ('centos', 'debian', 'fedora', 'rhel', 'suse'):
             var = linux_dist
         elif linux_dist in ('ubuntu', 'linuxmint', 'mint'):
             var = 'ubuntu'
         elif linux_dist == 'redhat':
             var = 'rhel'
-        elif linux_dist == 'suse':
+        elif linux_dist in ('opensuse', 'sles'):
             var = 'suse'
         else:
             var = 'linux'
diff --git a/setup.py b/setup.py
index 85b2337..5ed8eae 100755
--- a/setup.py
+++ b/setup.py
@@ -25,7 +25,7 @@ from distutils.errors import DistutilsArgError
 import subprocess
 
 RENDERED_TMPD_PREFIX = "RENDERED_TEMPD"
-
+VARIANT = None
 
 def is_f(p):
     return os.path.isfile(p)
@@ -114,10 +114,20 @@ def render_tmpl(template):
     atexit.register(shutil.rmtree, tmpd)
     bname = os.path.basename(template).rstrip(tmpl_ext)
     fpath = os.path.join(tmpd, bname)
-    tiny_p([sys.executable, './tools/render-cloudcfg', template, fpath])
+    if VARIANT:
+        tiny_p([sys.executable, './tools/render-cloudcfg', '--variant',
+            VARIANT, template, fpath])
+    else:
+        tiny_p([sys.executable, './tools/render-cloudcfg', template, fpath])
     # return path relative to setup.py
     return os.path.join(os.path.basename(tmpd), bname)
 
+# User can set the variant for template rendering
+if '--distro' in sys.argv:
+    idx = sys.argv.index('--distro')
+    VARIANT = sys.argv[idx+1]
+    del sys.argv[idx+1]
+    sys.argv.remove('--distro')
 
 INITSYS_FILES = {
     'sysvinit': [f for f in glob('sysvinit/redhat/*') if is_f(f)],
@@ -260,7 +270,7 @@ requirements = read_requires()
 setuptools.setup(
     name='cloud-init',
     version=get_version(),
-    description='EC2 initialisation magic',
+    description='Cloud instance initialisation magic',
     author='Scott Moser',
     author_email='[e-mail]',
     url='http://launchpad.net/cloud-init/',
@@ -277,4 +287,5 @@ setuptools.setup(
     }
 )
 
+
 # vi: ts=4 expandtab