Re: [Cloud-init-dev] [Merge] ~rjschwei/cloud-init:breakLink into cloud-init:master
One inline question, probably for my own edification. Thanks! Diff comments: > diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py > index e381596..44a2f11 100644 > --- a/cloudinit/net/sysconfig.py > +++ b/cloudinit/net/sysconfig.py > @@ -708,6 +708,12 @@ class Renderer(renderer.Renderer): > resolv_content = self._render_dns(network_state, >existing_dns_path=dns_path) > if resolv_content: > +# netconfig checks if the resolv file is a link and if that Forgive my ignorance about the stack here, Robert, but is there anything we can do to configure DNS in netconfig itself so that even if it does assume control it won't break the configuration we want? > +# is true it assumes it is in controil of the file and > +# will clobber our changes, break the link to ensure the user > +# configuration sticks > +if os.path.islink(dns_path): > +os.unlink(dns_path) > util.write_file(dns_path, resolv_content, file_mode) > if self.networkmanager_conf_path: > nm_conf_path = util.target_path(target, -- https://code.launchpad.net/~rjschwei/cloud-init/+git/cloud-init/+merge/374517 Your team cloud-init Commiters is requested to review the proposed merge of ~rjschwei/cloud-init:breakLink into cloud-init:master. ___ 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
I'm happy with the way it is here... we got rid of all 'import yaml' from cloudinit.util at least. and down to a signle place that 'yaml.load' is called. -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master. ___ 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:e69df5dce264f9f329dd0630aad3954b32999153 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1234/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1234//rebuild -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master. ___ 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:e69df5dce264f9f329dd0630aad3954b32999153 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1233/ Executed test runs: FAILED: Checkout Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1233//rebuild -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master. ___ 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
OK. I just wanted to make sure that I wasn't missing something on the dump path. For relocating; there are quite a few yaml users inside util.py so we'd still need to import safeyaml into util; so module load wise, we're still pulling it in on any import of util. additionally, moving the load_yaml into safeyaml would mean inter-module deps; load_yaml uses decode_binary(), so we'd need to duplicate that. I'm fine with this as it is, but would also review moving all yaml operations into safeyaml. -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master. ___ 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:e7c84bfdc42bc533a8f737ecd6b9557b466e1ebd https://jenkins.ubuntu.com/server/job/cloud-init-ci/1232/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1232//rebuild -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master. ___ 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
not a real advantage other than getting closer to getting all yaml usage into a single place. On Thu, Oct 24, 2019 at 12:56 PM Ryan Harper wrote: > > Is there an advantage to moving dumps into safeyaml? I didn't think dump had > any concerns. Could we just have replaced the yaml.load() calls with > util.load_yaml() ? > -- > https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 > You are the owner of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage. -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master. ___ 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
just in better use of modules... putting yaml things into their own module rather than having them all in util. I'm willing to move load_yaml also if you'd like. which would mean util would have no yaml code in it. -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master. ___ 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
Is there an advantage to moving dumps into safeyaml? I didn't think dump had any concerns. Could we just have replaced the yaml.load() calls with util.load_yaml() ? -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master. ___ 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
[Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
Scott Moser has proposed merging ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master. Commit message: Fix usages of yaml, and move yaml_dump to safeyaml.dumps. Here we replace uses of the pyyaml module directly with functions provided by cloudinit.safeyaml. Also, change/move cloudinit.util.yaml_dumps to cloudinit.safeyaml.dumps LP: #1849640 Requested reviews: cloud-init Commiters (cloud-init-dev) For more details, see: https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 see commit message -- Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master. diff --git a/cloudinit/cmd/devel/net_convert.py b/cloudinit/cmd/devel/net_convert.py index 1ad7e0b..9b76830 100755 --- a/cloudinit/cmd/devel/net_convert.py +++ b/cloudinit/cmd/devel/net_convert.py @@ -5,13 +5,12 @@ import argparse import json import os import sys -import yaml from cloudinit.sources.helpers import openstack from cloudinit.sources import DataSourceAzure as azure from cloudinit.sources import DataSourceOVF as ovf -from cloudinit import distros +from cloudinit import distros, safeyaml from cloudinit.net import eni, netplan, network_state, sysconfig from cloudinit import log @@ -78,13 +77,12 @@ def handle_args(name, args): if args.kind == "eni": pre_ns = eni.convert_eni_data(net_data) elif args.kind == "yaml": -pre_ns = yaml.load(net_data) +pre_ns = safeyaml.load(net_data) if 'network' in pre_ns: pre_ns = pre_ns.get('network') if args.debug: sys.stderr.write('\n'.join( -["Input YAML", - yaml.dump(pre_ns, default_flow_style=False, indent=4), ""])) +["Input YAML", safeyaml.dumps(pre_ns), ""])) elif args.kind == 'network_data.json': pre_ns = openstack.convert_net_json( json.loads(net_data), known_macs=known_macs) @@ -100,9 +98,8 @@ def handle_args(name, args): "input data") if args.debug: -sys.stderr.write('\n'.join([ -"", "Internal State", -yaml.dump(ns, default_flow_style=False, indent=4), ""])) +sys.stderr.write('\n'.join( +["", "Internal State", safeyaml.dumps(ns), ""])) distro_cls = distros.fetch(args.distro) distro = distro_cls(args.distro, {}, None) config = {} diff --git a/cloudinit/cmd/tests/test_main.py b/cloudinit/cmd/tests/test_main.py index a1e534f..57b8fdf 100644 --- a/cloudinit/cmd/tests/test_main.py +++ b/cloudinit/cmd/tests/test_main.py @@ -6,8 +6,9 @@ import os from six import StringIO from cloudinit.cmd import main +from cloudinit import safeyaml from cloudinit.util import ( -ensure_dir, load_file, write_file, yaml_dumps) +ensure_dir, load_file, write_file) from cloudinit.tests.helpers import ( FilesystemMockingTestCase, wrap_and_call) @@ -39,7 +40,7 @@ class TestMain(FilesystemMockingTestCase): ], 'cloud_init_modules': ['write-files', 'runcmd'], } -cloud_cfg = yaml_dumps(self.cfg) +cloud_cfg = safeyaml.dumps(self.cfg) ensure_dir(os.path.join(self.new_root, 'etc', 'cloud')) self.cloud_cfg_file = os.path.join( self.new_root, 'etc', 'cloud', 'cloud.cfg') @@ -113,7 +114,7 @@ class TestMain(FilesystemMockingTestCase): """When local-hostname metadata is present, call cc_set_hostname.""" self.cfg['datasource'] = { 'None': {'metadata': {'local-hostname': 'md-hostname'}}} -cloud_cfg = yaml_dumps(self.cfg) +cloud_cfg = safeyaml.dumps(self.cfg) write_file(self.cloud_cfg_file, cloud_cfg) cmdargs = myargs( debug=False, files=None, force=False, local=False, reporter=None, diff --git a/cloudinit/config/cc_debug.py b/cloudinit/config/cc_debug.py index 0a039eb..610dbc8 100644 --- a/cloudinit/config/cc_debug.py +++ b/cloudinit/config/cc_debug.py @@ -33,6 +33,7 @@ from six import StringIO from cloudinit import type_utils from cloudinit import util +from cloudinit import safeyaml SKIP_KEYS = frozenset(['log_cfgs']) @@ -49,7 +50,7 @@ def _make_header(text): def _dumps(obj): -text = util.yaml_dumps(obj, explicit_start=False, explicit_end=False) +text = safeyaml.dumps(obj, explicit_start=False, explicit_end=False) return text.rstrip() diff --git a/cloudinit/config/cc_salt_minion.py b/cloudinit/config/cc_salt_minion.py index d6a21d7..cd9cb0b 100644 --- a/cloudinit/config/cc_salt_minion.py +++ b/cloudinit/config/cc_salt_minion.py @@ -45,7 +45,7 @@ specify them with ``pkg_name``, ``service_name`` and ``config_dir``. import os -from cloudinit import util +from cloudinit import safeyaml, util # Note: see https://docs.saltstack.com/en/latest/topics/installation/ # Note: see https://docs.saltstack.com/en/latest/ref/configuration/ @@
Re: [Cloud-init-dev] [Merge] ~tribaal/cloud-init:fix/exoscale-datasource-wait-timeout into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:46310972f0ce5513b93d1c3c813cf6ef0b62f2f4 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1229/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1229//rebuild -- https://code.launchpad.net/~tribaal/cloud-init/+git/cloud-init/+merge/374643 Your team cloud-init Commiters is requested to review the proposed merge of ~tribaal/cloud-init:fix/exoscale-datasource-wait-timeout into cloud-init:master. ___ 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
Re: [Cloud-init-dev] [Merge] ~tribaal/cloud-init:fix/exoscale-datasource-wait-timeout into cloud-init:master
Note: retrying 12 times on average might be overkill here - but we rather be pessimistic and add a lot of buffer. -- https://code.launchpad.net/~tribaal/cloud-init/+git/cloud-init/+merge/374643 Your team cloud-init Commiters is requested to review the proposed merge of ~tribaal/cloud-init:fix/exoscale-datasource-wait-timeout into cloud-init:master. ___ 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
[Cloud-init-dev] [Merge] ~tribaal/cloud-init:fix/exoscale-datasource-wait-timeout into cloud-init:master
Chris Glass has proposed merging ~tribaal/cloud-init:fix/exoscale-datasource-wait-timeout into cloud-init:master. Requested reviews: cloud-init Commiters (cloud-init-dev) For more details, see: https://code.launchpad.net/~tribaal/cloud-init/+git/cloud-init/+merge/374643 This change sets the url_max_wait for the exoscale datasource to 120 (similar to what is done in the EC2/Cloudstack datasources). In some rare cases the route to the datasource IP address is not available under 10 seconds (the datasource's default timeout), and not setting url_max_wait results in only a single attempt being made (the default value being -1). -- Your team cloud-init Commiters is requested to review the proposed merge of ~tribaal/cloud-init:fix/exoscale-datasource-wait-timeout into cloud-init:master. diff --git a/cloudinit/sources/DataSourceExoscale.py b/cloudinit/sources/DataSourceExoscale.py index fdfb4ed..4616daa 100644 --- a/cloudinit/sources/DataSourceExoscale.py +++ b/cloudinit/sources/DataSourceExoscale.py @@ -26,6 +26,8 @@ class DataSourceExoscale(sources.DataSource): dsname = 'Exoscale' +url_max_wait = 120 + def __init__(self, sys_cfg, distro, paths): super(DataSourceExoscale, self).__init__(sys_cfg, distro, paths) LOG.debug("Initializing the Exoscale datasource") ___ 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