cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
> index 2f9c7ed..818a639 100644
> --- a/cloudinit/sources/DataSourceEc2.py
> +++ b/cloudinit/sources/DataSourceEc2.py

There only seems to be one unit test which covers this module (tests/unittests/test_datasource/test_aliyun.py). Is that right?   Since we are adding some level of functionality it'd be nice to cover this change in functional behavior somewhere.

> @@ -64,7 +64,8 @@ class DataSourceEc2(sources.DataSource):
>          LOG.debug("strict_mode: %s, cloud_platform=%s",
>                    strict_mode, self.cloud_platform)
>          if strict_mode == "true" and self.cloud_platform == Platforms.UNKNOWN:
> -            return False
> +            if not _is_explicit_dslist(self.sys_cfg.get('datasource_list')):
> +                return False
>  
>          try:
>              if not self.wait_for_metadata_service():
> @@ -266,6 +267,12 @@ def parse_strict_mode(cfgval):
>      return mode, sleep
>  
>  
> +def _is_explicit_dslist(dslist):
> +    if 'Ec2' not in dslist:
> +        return False
> +    return (len(dslist) == 1 or (len(dslist) == 2 and 'None' in dslist))
> +
> +
>  def warn_if_necessary(cfgval, cfg):
>      try:
>          mode, sleep = parse_strict_mode(cfgval)
> @@ -276,6 +283,12 @@ def warn_if_necessary(cfgval, cfg):
>      if mode == "false":
>          return
>  
> +    dslist = cfg.get('datasource_list')
> +    if _is_explicit_dslist(cfg.get('datasource_list')):
> +        LOG.debug("mode=%s but explicit dslist (%s), not warning.",
> +                  mode, dslist)
> +        return
> +
>      warnings.show_warning('non_ec2_md', cfg, mode=True, sleep=sleep)
>  
>  
> diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
> index 9e14885..8559e1f 100644
> --- a/tests/unittests/test_ds_identify.py
> +++ b/tests/unittests/test_ds_identify.py
> @@ -210,6 +210,13 @@ class TestDsIdentify(CiTestCase):
>          mydata['files'][cfgpath] = 'datasource_list: ["NoCloud"]\n'
>          self._check_via_dict(mydata, rc=RC_FOUND, dslist=['NoCloud', DS_NONE])
>  
> +    def test_configured_list_with_none(self):
> +        """If user set a datasource_list, that should be used."""

This docstring should really be
"When a datasource_list already contains None, None is not added to the list.

The test just above it is already asserting datasource_list w/out None results in a list w/ None at the end.

> +        mydata = copy.deepcopy(VALID_CFG['GCE'])
> +        cfgpath = 'etc/cloud/cloud.cfg.d/myds.cfg'
> +        mydata['files'][cfgpath] = 'datasource_list: ["Ec2", "None"]\n'
> +        self._check_via_dict(mydata, rc=RC_FOUND, dslist=['Ec2', DS_NONE])
> +
>  
>  def blkid_out(disks=None):
>      """Convert a list of disk dictionaries into blkid content."""


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/324274
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:bug/1683038-ec2-no-warn-on-explicit into cloud-init:master.