cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
> index b802b03..10a8b6f 100644
> --- a/cloudinit/sources/DataSourceAzure.py
> +++ b/cloudinit/sources/DataSourceAzure.py
> @@ -266,97 +258,92 @@ class DataSourceAzureNet(sources.DataSource):
>          # quickly (local check only) if self.instance_id is still valid
>          return sources.instance_id_matches_system_uuid(self.get_instance_id())
>  
> -
> -def count_files(mp):
> -    return len(fnmatch.filter(os.listdir(mp), '*[!cdrom]*'))
> +    def activate(self, cfg, is_new_instance):
> +        address_ephemeral_resize(is_new_instance=is_new_instance)
> +        return
>  
>  
> -def find_fabric_formatted_ephemeral_part():
> -    """
> -    Locate the first fabric formatted ephemeral device.
> -    """
> -    potential_locations = ['/dev/disk/cloud/azure_resource-part1',
> -                           '/dev/disk/azure/resource-part1']
> -    device_location = None
> -    for potential_location in potential_locations:
> -        if os.path.exists(potential_location):
> -            device_location = potential_location
> +def can_dev_be_reformatted(devpath):
> +    # determine if the ephemeral block device path devpath
> +    # is newly formatted after a resize.
> +    if not os.path.isfile(devpath):
> +        return False, 'device %s is not a file' % devpath
> +
> +    # devpath of /dev/sd[a-z] or /dev/disk/cloud/azure_resource
> +    # where partitions are "<devpath>1" or "<devpath>-part1" or "<devpath>p1"
> +    partpath = None
> +    for suff in ("-part", "p", ""):
> +        cand = devpath + suff + "1"
> +        if os.path.isfile(cand):
> +            if os.path.isfile(devpath + suff + "2"):
> +                msg = ('device %s had more than 1 partition: %s, %s' %
> +                       devpath, cand, devpath + suff + "2")
> +                return False, msg
> +            partpath = cand
>              break
> -    if device_location is None:
> -        LOG.debug("no azure resource disk partition path found")
> -        return None
> +
> +    if partpath is None:
> +        return False, 'device %s was not partitioned' % devpath
> +
> +    real_partpath = os.path.realpath(partpath)
>      ntfs_devices = util.find_devs_with("TYPE=ntfs")
> -    real_device = os.path.realpath(device_location)
> -    if real_device in ntfs_devices:
> -        return device_location
> -    LOG.debug("'%s' existed (%s) but was not ntfs formated",
> -              device_location, real_device)
> -    return None
> +    if real_partpath not in ntfs_devices:
> +        msg = ('partition 1 (%s -> %s) on device %s was not ntfs formatted' %
> +               partpath, real_partpath, devpath)
> +        return False, msg
>  
> +    def count_files(mp):
> +        ignored = {'dataloss_warning_readme.txt'}
> +        return len([f for f in os.listdir(mp) if f.lower() not in ignored])
>  
> -def find_fabric_formatted_ephemeral_disk():
> -    """
> -    Get the ephemeral disk.
> -    """
> -    part_dev = find_fabric_formatted_ephemeral_part()
> -    if part_dev:
> -        return part_dev.split('-')[0]
> -    return None
> +    bmsg = ('partition 1 (%s -> %s) on device %s was ntfs formatted' %
> +            partpath, real_partpath, devpath)
> +    try:
> +        file_count = util.mount_cb(devpath, count_files)
> +    except util.MountFailedError as e:
> +        return False, bmsg + ' but mount failed: %s' % e
>  
> +    if file_count != 0:
> +        return False, bmsg + ' but had %d files on it.' % file_count
>  
> -def support_new_ephemeral(cfg):
> -    """
> -    Windows Azure makes ephemeral devices ephemeral to boot; a ephemeral device
> -    may be presented as a fresh device, or not.
> +    return True, bmsg + ' and had important files.'

This is confusing ... if it can be reformatted but it had "important files" shouldn't we NOT format it?

>  
> -    Since the knowledge of when a disk is supposed to be plowed under is
> -    specific to Windows Azure, the logic resides here in the datasource. When a
> -    new ephemeral device is detected, cloud-init overrides the default
> -    frequency for both disk-setup and mounts for the current boot only.
> -    """
> -    device = find_fabric_formatted_ephemeral_part()
> -    if not device:
> -        LOG.debug("no default fabric formated ephemeral0.1 found")
> -        return None
> -    LOG.debug("fabric formated ephemeral0.1 device at %s", device)
>  
> -    file_count = 0
> -    try:
> -        file_count = util.mount_cb(device, count_files)
> -    except Exception:
> -        return None
> -    LOG.debug("fabric prepared ephmeral0.1 has %s files on it", file_count)
> -
> -    if file_count >= 1:
> -        LOG.debug("fabric prepared ephemeral0.1 will be preserved")
> -        return None
> +def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, maxwait=120,
> +                             is_new_instance=False):
> +    # wait for ephemeral disk to come up
> +    naplen = .2
> +    missing = wait_for_files([devpath], maxwait=maxwait, naplen=naplen)

wait_for_files should announce via LOG.info that it is waiting and for how long; otherwise users will just see the hang (for up to 120 seconds).

or use the util.log_time method elsewhere which announces the wait.

> +
> +    if missing:
> +        LOG.warn("ephemeral device '%s' did not appear after %d seconds.",
> +                 devpath, maxwait)
> +        return
> +
> +    result = False
> +    msg = None
> +    if is_new_instance:
> +        result, msg = (True, "First instance boot.")
>      else:
> -        # if device was already mounted, then we need to unmount it
> -        # race conditions could allow for a check-then-unmount
> -        # to have a false positive. so just unmount and then check.
> -        try:
> -            util.subp(['umount', device])
> -        except util.ProcessExecutionError as e:
> -            if device in util.mounts():
> -                LOG.warn("Failed to unmount %s, will not reformat.", device)
> -                LOG.debug("Failed umount: %s", e)
> -                return None
> -
> -    LOG.debug("cloud-init will format ephemeral0.1 this boot.")
> -    LOG.debug("setting disk_setup and mounts modules 'always' for this boot")
> -
> -    cc_modules = cfg.get('cloud_init_modules')
> -    if not cc_modules:
> -        return None
> -
> -    mod_list = []
> -    for mod in cc_modules:
> -        if mod in ("disk_setup", "mounts"):
> -            mod_list.append([mod, PER_ALWAYS])
> -            LOG.debug("set module '%s' to 'always' for this boot", mod)
> +        result, msg = can_dev_be_reformatted(devpath)
> +
> +    LOG.debug("reformattable=%s: %s" % (result, msg))
> +    if not result:
> +        return
> +
> +    for mod in ['disk_config', 'config_mounts']:
> +        sempath = '/var/lib/cloud/instance/sem/config_' + mod
> +        bmsg = 'Marker "%s" for module "%s"' % (sempath, mod)
> +        if os.path.exists(sempath):
> +            try:
> +                os.unlink(sempath)
> +                LOG.debug(bmsg + " removed.")
> +            except Exception as e:
> +                # python3 throws FileNotFoundError, python2 throws OSError
> +                LOG.warn(bmsg + ": remove failed! (%s)" % e)
>          else:
> -            mod_list.append(mod)
> -    return mod_list
> +            LOG.debug(bmsg + " did not exist.")
> +    return
>  
>  
>  def perform_hostname_bounce(hostname, cfg, prev_hostname):


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/311205
Your team cloud init development team is requested to review the proposed merge of ~smoser/cloud-init:feature/ds-init into cloud-init:master.