hi,

I put some comments inline.  I'd like to get this in.

We have moved development to git now. Information is available 
https://git.launchpad.net/cloud-init/tree/HACKING.rst

If you want to continue on here, we can, but ultimately we'll have to get it 
into the git branch.


Diff comments:

> 
> === added file 'cloudinit/sources/DataSourceAliYun.py'
> --- cloudinit/sources/DataSourceAliYun.py     1970-01-01 00:00:00 +0000
> +++ cloudinit/sources/DataSourceAliYun.py     2016-07-25 04:31:42 +0000
> @@ -0,0 +1,112 @@
> +# vi: ts=4 expandtab
> +#
> +#    Copyright (C) 2009-2010 Canonical Ltd.
> +#    Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> +#    Copyright (C) 2012 Yahoo! Inc.
> +#
> +#    Author: Scott Moser <scott.mo...@canonical.com>
> +#    Author: Juerg Hafliger <juerg.haefli...@hp.com>
> +#    Author: Joshua Harlow <harlo...@yahoo-inc.com>
> +#
> +#    This program is free software: you can redistribute it and/or modify
> +#    it under the terms of the GNU General Public License version 3, as
> +#    published by the Free Software Foundation.
> +#
> +#    This program is distributed in the hope that it will be useful,
> +#    but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +#    GNU General Public License for more details.
> +#
> +#    You should have received a copy of the GNU General Public License
> +#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +import os
> +import time
> +from copy import deepcopy
> +
> +from cloudinit import ec2_utils as ec2
> +from cloudinit import log as logging
> +from cloudinit import sources
> +from cloudinit import url_helper as uhelp
> +from cloudinit import util
> +from cloudinit.sources import DataSourceEc2 as EC2
> +
> +LOG = logging.getLogger(__name__)
> +
> +DEF_MD_URL = "http://100.100.100.200";
> +
> +DEF_MD_VERSION = 'latest'
> +
> +# Default metadata urls that will be used if none are provided
> +# They will be checked for 'resolveability' and some of the
> +# following may be discarded if they do not resolve
> +DEF_MD_URLS = [DEF_MD_URL,]
> +
> +
> +class DataSourceAliYun(EC2.DataSourceEc2):
> +    EC2.DEF_MD_URL = DEF_MD_URL
> +    EC2.DEF_MD_URLS = DEF_MD_URLS
> +

we should probably adjust the ec2 datasource to make this easier for you.
seems silly / wrong for you to change EC2.<values>.  when i suggested you 
inherit, i didn't realize the Ec2 datasource used so much state outside the 
class.

> +    def __init__(self, sys_cfg, distro, paths):
> +        super(DataSourceAliYun, self).__init__(sys_cfg, distro, paths)
> +        self.metadata_address = DEF_MD_URL
> +        self.seed_dir = os.path.join(paths.seed_dir, "AliYun")
> +        self.api_ver = DEF_MD_VERSION
> +
> +    def get_instance_id(self):
> +        if not self.metadata:
> +            url = uhelp.combine_url(self.metadata_address, self.api_ver, 
> 'meta-data/', 'instance-id')
> +            try:
> +                out = util.read_file_or_url(url=url, ssl_details=None, 
> timeout=5, retries=5)
> +            except Exception as e:
> +                out = ''
> +            finally:

shouldnt we raise an exception if we cannot get to the instance-id?
Are you trying to avoid failing badly if its not there?

> +                return str(out).strip()
> +        else:
> +            return self.metadata.get('instance-id', '')
> +
> +    def get_hostname(self, fqdn=False, _resolve_ip=False):
> +        return self.metadata.get('hostname', 'localhost.localdomain')
> +
> +    def get_public_ssh_keys(self):
> +        if not self.metadata:
> +            return []
> +        public_keys = self.metadata.get('public-keys', {})
> +
> +        keys = []

this hunk of code can easily be moved out of the class or into a class function 
and then tests written for it for all the expected inputs.

> +        for key_id, key_body in public_keys.items():
> +            if isinstance(key_body, (str, basestring)):
> +                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
> +
> +    def get_ntp_conf(self):
> +        if not self.metadata:
> +            return {}
> +        return self.metadata.get('ntp-conf')
> +
> +    def get_source_address(self):
> +        if not self.metadata or not self.metadata.get('source-address'):
> +            return []
> +        source_address = self.metadata.get('source-address')
> +        if isinstance(source_address, str):
> +            source_address = [source_address]
> +        return source_address
> +
> +
> +# Used to match classes to dependencies
> +datasources = [
> +  (DataSourceAliYun, (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)


-- 
https://code.launchpad.net/~kaihuan-pkh/cloud-init/cloud-init/+merge/284627
Your team cloud init development team is requested to review the proposed merge 
of lp:~kaihuan-pkh/cloud-init/cloud-init into lp:cloud-init.

_______________________________________________
Mailing list: https://launchpad.net/~cloud-init-dev
Post to     : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to