← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~papodaca/cloud-init:chef_omnibus_version into cloud-init:master

 

Thanks for the patch.  I've a couple comments below.


Diff comments:

> diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py
> index 02c70b1..94500b6 100644
> --- a/cloudinit/config/cc_chef.py
> +++ b/cloudinit/config/cc_chef.py
> @@ -302,12 +302,16 @@ def install_chef(cloud, chef_cfg, log):
>          retries = max(0, util.get_cfg_option_int(chef_cfg,
>                                                   "omnibus_url_retries",
>                                                   default=OMNIBUS_URL_RETRIES))
> +        version = util.get_cfg_option_str(chef_cfg, "omnibus_version")

in install_chef function, there are other version strings, let's name this variable omnibus_version.

>          content = url_helper.readurl(url=url, retries=retries).contents
>          with util.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)
> +            params = [tmpf]

Let's replace params name, with something more descriptive,

chef_install_cmd = [tmpf]

> +            if version:
> +                params += ["-v", version]
> +            util.subp(params, capture=False)
>      else:
>          log.warn("Unknown chef install type '%s'", install_type)
>          run = False
> diff --git a/doc/examples/cloud-config-chef.txt b/doc/examples/cloud-config-chef.txt
> index 9d23581..ce76b5d 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', pinned version can passed
> + # to the instal script

# 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


-- 
https://code.launchpad.net/~papodaca/cloud-init/+git/cloud-init/+merge/328943
Your team cloud-init commiters is requested to review the proposed merge of ~papodaca/cloud-init:chef_omnibus_version into cloud-init:master.


References