cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

Thanks for the review, your comments seem sane.
I really do want to get this in, as currently this is a common path for failure in maas (node can't reach region controller) and there isn't much path as to what is going on there.


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'))

yeah, i guess we can drop the names= here.

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

true, we could return debug on that.

> +
> +    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,

I guess this could be made to be an error. One reason for 'info' only would be that url=http://.... is not 100% obviously for cloud-init.  So this is not really an error if something else would consume that.  Perhaps this is an error if cloud-config-url= but only info if url=

> +                    "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.