cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/sources/DataSourceAliYun.py b/cloudinit/sources/DataSourceAliYun.py
> index 858e082..16abd09 100644
> --- a/cloudinit/sources/DataSourceAliYun.py
> +++ b/cloudinit/sources/DataSourceAliYun.py
> @@ -18,25 +18,17 @@ class DataSourceAliYun(EC2.DataSourceEc2):
>      min_metadata_version = '2016-01-01'
>      extended_metadata_versions = []
>  
> -    def __init__(self, sys_cfg, distro, paths):
> -        super(DataSourceAliYun, self).__init__(sys_cfg, distro, paths)
> -        self.seed_dir = os.path.join(paths.seed_dir, "AliYun")
> -
>      def get_hostname(self, fqdn=False, resolve_ip=False, metadata_only=False):
>          return self.metadata.get('hostname', 'localhost.localdomain')
>  
>      def get_public_ssh_keys(self):
>          return parse_public_keys(self.metadata.get('public-keys', {}))
>  
> -    @property
> -    def cloud_platform(self):
> -        if self._cloud_platform is None:
> -            if _is_aliyun():
> -                self._cloud_platform = EC2.Platforms.ALIYUN
> -            else:
> -                self._cloud_platform = EC2.Platforms.NO_EC2_METADATA
> -
> -        return self._cloud_platform
> +    def _get_cloud_name(self):
> +        if _is_aliyun():

This and EC2 are special caases as _get_cloud_name is only called if the cached _cloud_name is not set. So, caching is done up in parent class here before even calling this method. Since I dropped cloud_platform from EC2/Aliyun, we don't need any of this additional caching here. We do handle caching on DS.platform and subplatform.

> +            return EC2.CloudNames.ALIYUN
> +        else:
> +            return EC2.CloudNames.NO_EC2_METADATA
>  
>  
>  def _is_aliyun():
> diff --git a/cloudinit/sources/DataSourceAltCloud.py b/cloudinit/sources/DataSourceAltCloud.py
> index 8cd312d..5270fda 100644
> --- a/cloudinit/sources/DataSourceAltCloud.py
> +++ b/cloudinit/sources/DataSourceAltCloud.py
> @@ -99,7 +101,14 @@ class DataSourceAltCloud(sources.DataSource):
>              'RHEV', 'VSPHERE' or 'UNKNOWN'
>  
>          '''
> -
> +        if os.path.exists(CLOUD_INFO_FILE):

Maybe it's safest to drop the general seed-dir functionality from multiple datasources and CLOUD_INFO_FILE in a separate MP so this branch doesn't regress potential users out there. I'm all for dropping all these extra paths so we can simplify these datasources.

Per, can't test this anywhere, do you mean we can't integration test it? We can unit test with some mocks.

> +            try:
> +                cloud_type = util.load_file(CLOUD_INFO_FILE).strip().upper()
> +            except IOError:
> +                util.logexc(LOG, 'Unable to access cloud info file at %s.',
> +                            CLOUD_INFO_FILE)
> +                return 'UNKNOWN'
> +            return cloud_type
>          system_name = util.read_dmi_data("system-product-name")
>          if not system_name:
>              return 'UNKNOWN'
> diff --git a/cloudinit/sources/DataSourceOpenNebula.py b/cloudinit/sources/DataSourceOpenNebula.py
> index 77ccd12..f8df36b 100644
> --- a/cloudinit/sources/DataSourceOpenNebula.py
> +++ b/cloudinit/sources/DataSourceOpenNebula.py
> @@ -95,6 +95,14 @@ class DataSourceOpenNebula(sources.DataSource):
>          self.userdata_raw = results.get('userdata')
>          return True
>  
> +    def _get_subplatform(self):
> +        """Return the subplatform metadata source details."""
> +        if self.seed_dir in self.seed:
> +            subplatform_type =  'seed-dir'

agreed, will do as I don't want this branch to regress anyone that may have used the seed option that we baked into various datasources.

> +        else:
> +            subplatform_type =  'config-disk'
> +        return '%s (%s)' % (subplatform_type, self.seed)
> +
>      @property
>      def network_config(self):
>          if self.network is not None:
> diff --git a/cloudinit/util.py b/cloudinit/util.py
> index 5068096..948cfe5 100644
> --- a/cloudinit/util.py
> +++ b/cloudinit/util.py
> @@ -2171,6 +2171,23 @@ def is_container():
>      return False
>  
>  
> +def is_lxd():
> +    """Check to see if we are running in a lxd container."""
> +    if os.path.exists('/dev/lxd/sock'):
> +        return True
> +    for container_file in ['/run/systemd/container', '/run/container_type']:
> +        if os.path.exists(container_file):

simplified is_lxd. Ahh good catch, yes I was commiting a 'think-o' mistakenly linking any lxc container as being lxd, I got those paths looking at centos6, landscape-client vm detection and  as well as strace of systemd-detect-virt.

> +            if 'lxc' == load_file(container_file, decode=False):
> +                return True
> +    try:
> +        (virt_type, _err) = subp(['systemd-detect-virt'])
> +    except (IOError, OSError):
> +        return False
> +    if virt_type == 'lxc':
> +        return True
> +    return False
> +
> +
>  def get_proc_env(pid, encoding='utf-8', errors='replace'):
>      """
>      Return the environment in a dict that a given process id was started with.
> diff --git a/doc/rtd/topics/instancedata.rst b/doc/rtd/topics/instancedata.rst
> index 634e180..c1eed2e 100644
> --- a/doc/rtd/topics/instancedata.rst
> +++ b/doc/rtd/topics/instancedata.rst
> @@ -102,6 +102,12 @@ The standardized keys present:
>  | v1.local_hostname    | The internal or local hostname of the system  | ip-10-41-41-70,           |
>  |                      |                                               | <user-provided-hostname>  |
>  +----------------------+-----------------------------------------------+---------------------------+
> +| v1.platform_type     | The cloud platform or metadata api type       | ec2, openstack, seed-dir  |

done added missing _beta_keys and subplatform. I'll add more documentation related to the context you provided.

> +|                      |                                               |                           |
> ++----------------------+-----------------------------------------------+---------------------------+
> +| v1.public_ssh_keys   | A list of  ssh keys provided to the instance  | ['ssh-rsa AA...', ...]    |
> +|                      | by the datasource metadata.                   |                           |
> ++----------------------+-----------------------------------------------+---------------------------+
>  | v1.region            | The physical region/datacenter in which the   | us-east-2                 |
>  |                      | instance is deployed                          |                           |
>  +----------------------+-----------------------------------------------+---------------------------+


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/355999
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:cleanup/metadata-cloud-platform into cloud-init:master.