cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
> index 26c0240..5005d69 100644
> --- a/cloudinit/cmd/main.py
> +++ b/cloudinit/cmd/main.py
> @@ -140,23 +141,97 @@ def apply_reporting_cfg(cfg):
>          reporting.update_configuration(cfg.get('reporting'))
>  
>  
> +def parse_cmdline_url(cmdline, names=('cloud-config-url', 'url')):
> +    data = util.keyval_str_to_dict(cmdline)
> +    for key in names:
> +        if key in data:
> +            return data[key]
> +    return None
> +
> +
> +def attempt_cmdline_url(path, network=True, cmdline=None):
> +    """Write data from url referenced in command line to path.
> +
> +    path: a file to write content to if downloaded.
> +    network: should network access be assumed.
> +    cmdline: the cmdline to parse for cloud-config-url.
> +
> +    This is used in MAAS datasource, in "ephemeral" (read-only root)
> +    environment where the instance netboots to iscsi ro root.
> +    and the entity that controls the pxe config has to configure
> +    the maas datasource.
> +
> +    An attempt is made on network urls even in local datasource
> +    for case of network set up in initramfs.
> +
> +    Return value is a tuple of a logger function (logging.DEBUG)
> +    and a message indicating what happened.
> +    """
> +
> +    if cmdline is None:
> +        cmdline = util.get_cmdline()
> +
> +    url = parse_cmdline_url(cmdline, names=('cloud-config-url', 'url'))

names= is unneeded as it's default for parse_cmdline_url,  unless you want to expose callers of attempt_cmdline_url to pass in a list of names to match.

it's possible that we'd encode the keys that are searched by default in some cloud default config.  And ideally the docs for url parsing updated with these values.

> +    if not url:
> +        return (None, None)

This doesn't match the required signature of Logger.DEBUG and message tuple.

> +
> +    path_is_local = url.startswith("file://") or url.startswith("/")
> +
> +    if path_is_local and os.path.exists(path):
> +        if network:
> +            m = ("file '%s' existed, possibly from local stage download"
> +                 " of command line url '%s'. Not re-writing." % (path, url))
> +            level = logging.INFO
> +            if path_is_local:
> +                level = logging.DEBUG
> +        else:
> +            m = ("file '%s' existed, possibly from previous boot download"
> +                 " of command line url '%s'. Not re-writing." % (path, url))
> +            level = logging.WARN
> +
> +        return (level, m)
> +
> +    kwargs = {'url': url, 'timeout': 10, 'retries': 2}
> +    if network or path_is_local:
> +        level = logging.WARN
> +        kwargs['sec_between'] = 1
> +    else:
> +        level = logging.DEBUG
> +        kwargs['sec_between'] = .1
> +
> +    data = None
> +    header = b'#cloud-config'
> +    try:
> +        resp = util.read_file_or_url(**kwargs)
> +        if resp.ok():
> +            data = resp.contents
> +            if not resp.contents.startswith(header):
> +                return (
> +                    logging.INFO,

logging.ERROR ?

> +                    "contents of '%s' did not start with %s" % (url, header))
> +        else:
> +            return (level,
> +                    "url '%s' returned code %s. Ignoring." % (url, resp.code))
> +
> +    except url_helper.UrlError as e:
> +        return (level, "retrieving url '%s' failed: %s" % (url, e))
> +
> +    util.write_file(path, data, mode=0o600)
> +    return (logging.INFO,
> +            "wrote cloud-config data from '%s' to %s" % (url, path))
> +
> +
>  def main_init(name, args):
>      deps = [sources.DEP_FILESYSTEM, sources.DEP_NETWORK]
>      if args.local:
>          deps = [sources.DEP_FILESYSTEM]
>  
> -    if not args.local:
> -        # See doc/kernel-cmdline.txt
> -        #
> -        # This is used in maas datasource, in "ephemeral" (read-only root)
> -        # environment where the instance netboots to iscsi ro root.
> -        # and the entity that controls the pxe config has to configure
> -        # the maas datasource.
> -        #
> -        # Could be used elsewhere, only works on network based (not local).
> -        root_name = "%s.d" % (CLOUD_CONFIG)
> -        target_fn = os.path.join(root_name, "91_kernel_cmdline_url.cfg")
> -        util.read_write_cmdline_url(target_fn)
> +    early_logs = []
> +    _lvl, _msg = attempt_cmdline_url(
> +        path=os.path.join("%.d" % CLOUD_CONFIG, "91_kernel_cmdline_url.cfg"),
> +        network=not args.local)
> +    if _lvl:
> +        early_logs.extend(_lvl, _msg)
>  
>      # Cloud-init 'init' stage is broken up into the following sub-stages
>      # 1. Ensure that the init object fetches its config without errors


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/311549
Your team cloud init development team is requested to review the proposed merge of ~smoser/cloud-init:bug/noise-on-cmdline-url-fail into cloud-init:master.