cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Chad Smith has proposed merging ~chad.smith/cloud-init:chef_omnibus_version into cloud-init:master.

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1462693 in cloud-init: "Need way to pin Chef version for omnibus install"
  https://bugs.launchpad.net/cloud-init/+bug/1462693

For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/330712

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
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:chef_omnibus_version into cloud-init:master.

diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py
index c192dd3..fdcb3eb 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:
@@ -280,6 +283,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,6 +326,7 @@ 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':
+<<<<<<< cloudinit/config/cc_chef.py
         # 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,
@@ -309,6 +338,12 @@ def install_chef(cloud, chef_cfg, log):
             tmpf = "%s/chef-omnibus-install" % tmpd
             util.write_file(tmpf, content, mode=0o700)
             util.subp([tmpf], capture=False)
+=======
+        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"),
+            version=util.get_cfg_option_str(chef_cfg, "omnibus_version"))
+>>>>>>> cloudinit/config/cc_chef.py
     else:
         log.warn("Unknown chef install type '%s'", install_type)
         run = False
diff --git a/cloudinit/util.py b/cloudinit/util.py
index ae5cda8..fdb4936 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 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..d424262 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,76 @@ from cloudinit import helpers
 from cloudinit.sources import DataSourceNone
 from cloudinit import util
 
+<<<<<<< tests/unittests/test_handler/test_handler_chef.py
 from cloudinit.tests import helpers as t_help
+=======
+from cloudinit.tests.helpers import (
+    CiTestCase, FilesystemMockingTestCase, mock, skipIf)
+>>>>>>> tests/unittests/test_handler/test_handler_chef.py
 
 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))
+
+    @httpretty.activate
+    @mock.patch('cloudinit.config.cc_chef.util.subp_blob_in_tempfile')
+    def test_install_chef_from_omnibus_retries_url(self, m_subp_blob):
+        """install_chef_from_omnibus retries OMNIBUS_URL upon failure."""
+        response = '#!/bin/bash\necho "Hi Mom" > {0}/chef.out'.format(
+            self.new_root)
+        self.call_count = 0
+
+        def get_request_callback(method, uri, headers):
+            self.call_count += 1
+            if self.call_count < cc_chef.OMNIBUS_URL_RETRIES:
+                return (404, headers, '')
+            return (200, headers, response)
+
+        httpretty.register_uri(
+            httpretty.GET, cc_chef.OMNIBUS_URL, body=get_request_callback)
+        cc_chef.install_chef_from_omnibus()
+        self.assertEqual(
+            [mock.call(blob=response, args=[], basename='chef-omnibus-install',
+                       capture=False)],
+            m_subp_blob.call_args_list)
+        self.assertEqual(cc_chef.OMNIBUS_URL_RETRIES, self.call_count)
+
+    @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."""
+        response = '#!/bin/bash\necho "Hi Mom" > {0}/chef.out'.format(
+            self.new_root)
+        httpretty.register_uri(
+            httpretty.GET, cc_chef.OMNIBUS_URL, body=response)
+        cc_chef.install_chef_from_omnibus(omnibus_version='2.0')
+        self.assertEqual(
+            [mock.call(blob=response, args=['-v', '2.0'],
+                       basename='chef-omnibus-install', capture=False)],
+            m_subp_blob.call_args_list)
+
+
+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 +100,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 +179,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 +200,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')