cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Quick review things:
 * make a better commit message (the 'Commit message` above will be be used when this is ultimately squashed and merged)
 * add 'LP: #1841181' to your commit message.  See git log for examples. Thats how MPs get associated with bugs.
 * Tests needed. 
    * tests/unittests/test_ds_identify.py should be fairly to understand and update for Zstack
    * Probably add a tests/unittests/test_datasource/test_zstack.py 
 * Add doc/rtd/topics/datasources/zstack.rst and update doc/rtd/topics/datasources.rst  to reference it.  See Exoscale for a recently added datasource example.


  * Did you check to see if simply adding Zstack in the same way as BrightBox is added would work?   That could be even less code.

Diff comments:

> diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
> index 5c017bf..c75bfc8 100644
> --- a/cloudinit/sources/DataSourceEc2.py
> +++ b/cloudinit/sources/DataSourceEc2.py
> @@ -33,6 +33,7 @@ class CloudNames(object):
>      ALIYUN = "aliyun"
>      AWS = "aws"
>      BRIGHTBOX = "brightbox"
> +    ZStack = "zstack"

seems like this should be all caps.

>      # UNKNOWN indicates no positive id.  If strict_id is 'warn' or 'false',
>      # then an attempt at the Ec2 Metadata service will be made.
>      UNKNOWN = "unknown"
> diff --git a/cloudinit/sources/DataSourceZStack.py b/cloudinit/sources/DataSourceZStack.py
> new file mode 100644
> index 0000000..e3b50c7
> --- /dev/null
> +++ b/cloudinit/sources/DataSourceZStack.py
> @@ -0,0 +1,56 @@
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +from cloudinit import sources
> +from cloudinit.sources import DataSourceEc2 as EC2
> +from cloudinit import util
> +
> +CHASSIS_ASSET_TAG = "zstack.io"
> +
> +class DataSourceZStack(EC2.DataSourceEc2):
> +
> +    dsname = 'ZStack'
> +
> +    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', {}))
> +
> +    def _get_cloud_name(self):
> +        if _is_zstack():
> +            return EC2.CloudNames.ZStack
> +        else:
> +            return EC2.CloudNames.NO_EC2_METADATA
> +
> +def _is_zstack():
> +    asset_tag = util.read_dmi_data('chassis-asset-tag')
> +    return asset_tag == CHASSIS_ASSET_TAG
> +
> +
> +def parse_public_keys(public_keys):

Is this necessary?  I suspect it is copied from Aliyun, which has small differences in their public key meta-data compared to Ec2.  So 2 possibilities:
 a.) Zstack provides data just like Aliyun.
 b.) Zstack provides data just like amazon.  

In either option, lets avoid the copy paste.

> +    keys = []
> +    for _key_id, key_body in public_keys.items():
> +        if isinstance(key_body, str):
> +            keys.append(key_body.strip())
> +        elif isinstance(key_body, list):
> +            keys.extend(key_body)
> +        elif isinstance(key_body, dict):
> +            key = key_body.get('openssh-key', [])
> +            if isinstance(key, str):
> +                keys.append(key.strip())
> +            elif isinstance(key, list):
> +                keys.extend(key)
> +    return keys
> +
> +
> +# Used to match classes to dependencies
> +datasources = [
> +    (DataSourceZStack, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)),
> +]
> +
> +
> +# Return a list of data sources that match this set of dependencies
> +def get_datasource_list(depends):
> +    return sources.list_from_depends(depends, datasources)

Does Zstack provide any network configuration information?  EC2 provides some and cloud-init is moving to have "local" datasources that hit the metadata service to get information about how the network should be configured.  So if this info is available, it'd be great for the datasource to start its life as a Local datasource and use that.  Even if not... then we can still provide default networking from the datasource and run at local time frame...

> +
> +# vi: ts=4 expandtab


-- 
https://code.launchpad.net/~ruansx/cloud-init/+git/cloud-init/+merge/372445
Your team cloud-init Commiters is requested to review the proposed merge of ~ruansx/cloud-init:add-zstack-datasource into cloud-init:master.