cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Some thoughts:
a.) Why?  Why do two writes for every write?  
b.) currently atomic write_file is not covered.


Diff comments:

> diff --git a/cloudinit/util.py b/cloudinit/util.py
> index aa23b3f..950921c 100644
> --- a/cloudinit/util.py
> +++ b/cloudinit/util.py
> @@ -1841,6 +1843,55 @@ def chmod(path, mode):
>              os.chmod(path, real_mode)
>  
>  
> +def write_file_log_append(filename, omode, target=None):
> +    """ Create an audit log of files that cloud-init has written.
> +
> +    The log is located at: /var/lib/cloud/instance/write_file.log
> +
> +    The format is JSON dict, one per line
> +      {'timestamp': time.time(), 'path': filename, 'mode': omode}
> +
> +    Example entries:
> +      {'filename': '/etc/apt/sources.list', 'mode': 'wb', 'timestamp': ts}
> +
> +    """
> +    global WRITE_FILE_LOG
> +    global _ENABLE_WRITE_FILE_LOG
> +
> +    if not _ENABLE_WRITE_FILE_LOG:
> +        return
> +
> +    log_file = target_path(target, path=WRITE_FILE_LOG)
> +    if not os.path.exists(os.path.dirname(log_file)):
> +        return
> +
> +    record = {'timestamp': time.time(), 'filename': filename, 'mode': omode}
> +    content = json.dumps(record, default=json_serialize_default)
> +    try:
> +        with open(log_file, "ab") as wfl:
> +            wfl.write((content + '\n').encode('utf-8'))

why open this in binary mode so you can convert text  to binary?

Just open in text mode and write the text?

> +            wfl.flush()

unnecessary flush().
the close will cover that.

> +    except IOError as e:
> +        LOG.debug('Failed to append to write file log (%s): %s', log_file, e)
> +
> +
> +def write_file_log_read(target=None):
> +    """ Read the WRITE_FILE_LOG and yield the contents.
> +
> +    :returns a list of record dicts
> +    """
> +    global WRITE_FILE_LOG
> +
> +    log_file = target_path(target, path=WRITE_FILE_LOG)
> +    if os.path.exists(log_file):
> +        with open(log_file, "rb") as wfl:
> +            contents = wfl.read()

seems like you're just doing more work than necessary here, and breaking the builtin iterator on a file()

with open(log_file, "r") as fp:
    for line in fp:
        record = load_json(line)
        if record:
            yield record

> +        for line in contents.splitlines():
> +            record = load_json(line)
> +            if record:
> +                yield record
> +
> +
>  def write_file(filename, content, mode=0o644, omode="wb", copy_mode=False):
>      """
>      Writes a file with the given content and sets the file mode as specified.


-- 
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/372946
Your team cloud-init Commiters is requested to review the proposed merge of ~raharper/cloud-init:feature/cloudinit-clean-from-write-log into cloud-init:master.