Merge ~chad.smith/cloud-init:chef_omnibus_version into cloud-init:master

[Chad Smith]

Add option to pin chef omnibus install version

Most users of chef will want to pin the version that is installed.
Typically new versions of chef have to be evaluated for breakage etc.

This change proposes a new optional `omnibus_version` field to the
chef configuration. This changes also adds an reference to the new
field to the chef example.

LP: #1462693

[Chad Smith (chad.smith)]

Pulled the original content from https://code.launchpad.net/~papodaca/cloud-init/+git/cloud-init/+merge/328943 and addresses review comments as well as added unit tests.

6cfd206... by Chad Smith on 2017-09-13

tempdir moved to temp_utils in master. fix per rebase

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

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

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

review: Needs Fixing (continuous-integration)

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

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

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

review: Needs Fixing (continuous-integration)

f74d44e... by Chad Smith on 2017-09-14

rework unit tests to handle py2 py3 string/bytes differences and mock readurl to avoid test_install_chef_from_omnibus_retries_url to avoid executing time-consuming retries inside readurl

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

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

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

review: Needs Fixing (continuous-integration)

accd83c... by Chad Smith on 2017-09-14

lint param name is now omnibus_version

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

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

review: Approve

diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py
index c192dd3..46abedd 100644
--- a/cloudinit/config/cc_chef.py
+++ b/cloudinit/config/cc_chef.py
@@ -58,6 +58,9 @@ file).
       log_level:
       log_location:
       node_name:
+      omnibus_url:
+      omnibus_url_retries:
+      omnibus_version:
       pid_file:
       server_url:
       show_time:
@@ -71,7 +74,6 @@ import itertools
 import json
 import os
 
-from cloudinit import temp_utils
 from cloudinit import templater
 from cloudinit import url_helper
 from cloudinit import util
@@ -280,6 +282,31 @@ def run_chef(chef_cfg, log):
     util.subp(cmd, capture=False)
 
 
+def install_chef_from_omnibus(url=None, retries=None, omnibus_version=None):
+    """Install an omnibus unified package from url.
+
+    @param url: URL where blob of chef content may be downloaded. Defaults to
+        OMNIBUS_URL.
+    @param retries: Number of retries to perform when attempting to read url.
+        Defaults to OMNIBUS_URL_RETRIES
+    @param omnibus_version: Optional version string to require for omnibus
+        install.
+    """
+    if url is None:
+        url = OMNIBUS_URL
+    if retries is None:
+        retries = OMNIBUS_URL_RETRIES
+
+    if omnibus_version is None:
+        args = []
+    else:
+        args = ['-v', omnibus_version]
+    content = url_helper.readurl(url=url, retries=retries).contents
+    return util.subp_blob_in_tempfile(
+        blob=content, args=args,
+        basename='chef-omnibus-install', capture=False)
+
+
 def install_chef(cloud, chef_cfg, log):
     # If chef is not installed, we install chef based on 'install_type'
     install_type = util.get_cfg_option_str(chef_cfg, 'install_type',
@@ -298,17 +325,11 @@ def install_chef(cloud, chef_cfg, log):
         # This will install and run the chef-client from packages
         cloud.distro.install_packages(('chef',))
     elif install_type == 'omnibus':
-        # This will install as a omnibus unified package
-        url = util.get_cfg_option_str(chef_cfg, "omnibus_url", OMNIBUS_URL)
-        retries = max(0, util.get_cfg_option_int(chef_cfg,
-                                                 "omnibus_url_retries",
-                                                 default=OMNIBUS_URL_RETRIES))
-        content = url_helper.readurl(url=url, retries=retries).contents
-        with temp_utils.tempdir() as tmpd:
-            # Use tmpdir over tmpfile to avoid 'text file busy' on execute
-            tmpf = "%s/chef-omnibus-install" % tmpd
-            util.write_file(tmpf, content, mode=0o700)
-            util.subp([tmpf], capture=False)
+        omnibus_version = util.get_cfg_option_str(chef_cfg, "omnibus_version")
+        install_chef_from_omnibus(
+            url=util.get_cfg_option_str(chef_cfg, "omnibus_url"),
+            retries=util.get_cfg_option_int(chef_cfg, "omnibus_url_retries"),
+            omnibus_version=omnibus_version)
     else:
         log.warn("Unknown chef install type '%s'", install_type)
         run = False
diff --git a/cloudinit/util.py b/cloudinit/util.py
index ae5cda8..7e9d94f 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1742,6 +1742,31 @@ def delete_dir_contents(dirname):
             del_file(node_fullpath)
 
 
+def subp_blob_in_tempfile(blob, *args, **kwargs):
+    """Write blob to a tempfile, and call subp with args, kwargs. Then cleanup.
+
+    'basename' as a kwarg allows providing the basename for the file.
+    The 'args' argument to subp will be updated with the full path to the
+    filename as the first argument.
+    """
+    basename = kwargs.pop('basename', "subp_blob")
+
+    if len(args) == 0 and 'args' not in kwargs:
+        args = [tuple()]
+
+    # Use tmpdir over tmpfile to avoid 'text file busy' on execute
+    with temp_utils.tempdir() as tmpd:
+        tmpf = os.path.join(tmpd, basename)
+        if 'args' in kwargs:
+            kwargs['args'] = [tmpf] + list(kwargs['args'])
+        else:
+            args = list(args)
+            args[0] = [tmpf] + args[0]
+
+        write_file(tmpf, blob, mode=0o700)
+        return subp(*args, **kwargs)
+
+
 def subp(args, data=None, rcs=None, env=None, capture=True, shell=False,
          logstring=False, decode="replace", target=None, update_env=None):
 
diff --git a/doc/examples/cloud-config-chef.txt b/doc/examples/cloud-config-chef.txt
index 9d23581..58d5fdc 100644
--- a/doc/examples/cloud-config-chef.txt
+++ b/doc/examples/cloud-config-chef.txt
@@ -94,6 +94,10 @@ chef:
  # if install_type is 'omnibus', change the url to download
  omnibus_url: "https://www.chef.io/chef/install.sh"
 
+ # if install_type is 'omnibus', pass pinned version string
+ # to the install script
+ omnibus_version: "12.3.0"
+
 
 # Capture all subprocess output into a logfile
 # Useful for troubleshooting cloud-init issues
diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py
index e5785cf..0136a93 100644
--- a/tests/unittests/test_handler/test_handler_chef.py
+++ b/tests/unittests/test_handler/test_handler_chef.py
@@ -1,11 +1,10 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
+import httpretty
 import json
 import logging
 import os
-import shutil
 import six
-import tempfile
 
 from cloudinit import cloud
 from cloudinit.config import cc_chef
@@ -14,18 +13,83 @@ from cloudinit import helpers
 from cloudinit.sources import DataSourceNone
 from cloudinit import util
 
-from cloudinit.tests import helpers as t_help
+from cloudinit.tests.helpers import (
+    CiTestCase, FilesystemMockingTestCase, mock, skipIf)
 
 LOG = logging.getLogger(__name__)
 
 CLIENT_TEMPL = os.path.sep.join(["templates", "chef_client.rb.tmpl"])
 
 
-class TestChef(t_help.FilesystemMockingTestCase):
+class TestInstallChefOmnibus(CiTestCase):
+
+    def setUp(self):
+        self.new_root = self.tmp_dir()
+
+    @httpretty.activate
+    def test_install_chef_from_omnibus_runs_chef_url_content(self):
+        """install_chef_from_omnibus runs downloaded OMNIBUS_URL as script."""
+        chef_outfile = self.tmp_path('chef.out', self.new_root)
+        response = '#!/bin/bash\necho "Hi Mom" > {0}'.format(chef_outfile)
+        httpretty.register_uri(
+            httpretty.GET, cc_chef.OMNIBUS_URL, body=response, status=200)
+        cc_chef.install_chef_from_omnibus()
+        self.assertEqual('Hi Mom\n', util.load_file(chef_outfile))
+
+    @mock.patch('cloudinit.config.cc_chef.url_helper.readurl')
+    @mock.patch('cloudinit.config.cc_chef.util.subp_blob_in_tempfile')
+    def test_install_chef_from_omnibus_retries_url(self, m_subp_blob, m_rdurl):
+        """install_chef_from_omnibus retries OMNIBUS_URL upon failure."""
+
+        class FakeURLResponse(object):
+            contents = '#!/bin/bash\necho "Hi Mom" > {0}/chef.out'.format(
+                self.new_root)
+
+        m_rdurl.return_value = FakeURLResponse()
+
+        cc_chef.install_chef_from_omnibus()
+        expected_kwargs = {'retries': cc_chef.OMNIBUS_URL_RETRIES,
+                           'url': cc_chef.OMNIBUS_URL}
+        self.assertItemsEqual(expected_kwargs, m_rdurl.call_args_list[0][1])
+        cc_chef.install_chef_from_omnibus(retries=10)
+        expected_kwargs = {'retries': 10,
+                           'url': cc_chef.OMNIBUS_URL}
+        self.assertItemsEqual(expected_kwargs, m_rdurl.call_args_list[1][1])
+        expected_subp_kwargs = {
+            'args': ['-v', '2.0'],
+            'basename': 'chef-omnibus-install',
+            'blob': m_rdurl.return_value.contents,
+            'capture': False
+        }
+        self.assertItemsEqual(
+            expected_subp_kwargs,
+            m_subp_blob.call_args_list[0][1])
+
+    @httpretty.activate
+    @mock.patch('cloudinit.config.cc_chef.util.subp_blob_in_tempfile')
+    def test_install_chef_from_omnibus_has_omnibus_version(self, m_subp_blob):
+        """install_chef_from_omnibus provides version arg to OMNIBUS_URL."""
+        chef_outfile = self.tmp_path('chef.out', self.new_root)
+        response = '#!/bin/bash\necho "Hi Mom" > {0}'.format(chef_outfile)
+        httpretty.register_uri(
+            httpretty.GET, cc_chef.OMNIBUS_URL, body=response)
+        cc_chef.install_chef_from_omnibus(omnibus_version='2.0')
+
+        called_kwargs = m_subp_blob.call_args_list[0][1]
+        expected_kwargs = {
+            'args': ['-v', '2.0'],
+            'basename': 'chef-omnibus-install',
+            'blob': response,
+            'capture': False
+        }
+        self.assertItemsEqual(expected_kwargs, called_kwargs)
+
+
+class TestChef(FilesystemMockingTestCase):
+
     def setUp(self):
         super(TestChef, self).setUp()
-        self.tmp = tempfile.mkdtemp()
-        self.addCleanup(shutil.rmtree, self.tmp)
+        self.tmp = self.tmp_dir()
 
     def fetch_cloud(self, distro_kind):
         cls = distros.fetch(distro_kind)
@@ -43,8 +107,8 @@ class TestChef(t_help.FilesystemMockingTestCase):
         for d in cc_chef.CHEF_DIRS:
             self.assertFalse(os.path.isdir(d))
 
-    @t_help.skipIf(not os.path.isfile(CLIENT_TEMPL),
-                   CLIENT_TEMPL + " is not available")
+    @skipIf(not os.path.isfile(CLIENT_TEMPL),
+            CLIENT_TEMPL + " is not available")
     def test_basic_config(self):
         """
         test basic config looks sane
@@ -122,8 +186,8 @@ class TestChef(t_help.FilesystemMockingTestCase):
                 'c': 'd',
             }, json.loads(c))
 
-    @t_help.skipIf(not os.path.isfile(CLIENT_TEMPL),
-                   CLIENT_TEMPL + " is not available")
+    @skipIf(not os.path.isfile(CLIENT_TEMPL),
+            CLIENT_TEMPL + " is not available")
     def test_template_deletes(self):
         tpl_file = util.load_file('templates/chef_client.rb.tmpl')
         self.patchUtils(self.tmp)
@@ -143,8 +207,8 @@ class TestChef(t_help.FilesystemMockingTestCase):
         self.assertNotIn('json_attribs', c)
         self.assertNotIn('Formatter.show_time', c)
 
-    @t_help.skipIf(not os.path.isfile(CLIENT_TEMPL),
-                   CLIENT_TEMPL + " is not available")
+    @skipIf(not os.path.isfile(CLIENT_TEMPL),
+            CLIENT_TEMPL + " is not available")
     def test_validation_cert_and_validation_key(self):
         # test validation_cert content is written to validation_key path
         tpl_file = util.load_file('templates/chef_client.rb.tmpl')