cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #04619
[Merge] ~smoser/cloud-init:fix/read_file_or_url-contents-should-be-text into cloud-init:master
Scott Moser has proposed merging ~smoser/cloud-init:fix/read_file_or_url-contents-should-be-text into cloud-init:master.
Commit message:
read_file_or_url: move to url_helper, fix bug in its FileResponse.
The result of a read_file_or_url on a file and on a url would differ
in behavior.
str(UrlResponse) would return UrlResponse.contents.decode('utf-8')
while
str(FileResponse) would return str(FileResponse.contents)
The difference being "b'foo'" versus "foo".
As part of the general goal of cleaning util, move read_file_or_url
into url_helper.
Requested reviews:
cloud-init commiters (cloud-init-dev)
For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/343127
--
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/read_file_or_url-contents-should-be-text into cloud-init:master.
diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
index 3f2dbb9..d6ba90f 100644
--- a/cloudinit/cmd/main.py
+++ b/cloudinit/cmd/main.py
@@ -187,7 +187,7 @@ def attempt_cmdline_url(path, network=True, cmdline=None):
data = None
header = b'#cloud-config'
try:
- resp = util.read_file_or_url(**kwargs)
+ resp = url_helper.read_file_or_url(**kwargs)
if resp.ok():
data = resp.contents
if not resp.contents.startswith(header):
diff --git a/cloudinit/config/cc_phone_home.py b/cloudinit/config/cc_phone_home.py
index 878069b..3be0d1c 100644
--- a/cloudinit/config/cc_phone_home.py
+++ b/cloudinit/config/cc_phone_home.py
@@ -41,6 +41,7 @@ keys to post. Available keys are:
"""
from cloudinit import templater
+from cloudinit import url_helper
from cloudinit import util
from cloudinit.settings import PER_INSTANCE
@@ -136,9 +137,9 @@ def handle(name, cfg, cloud, log, args):
}
url = templater.render_string(url, url_params)
try:
- util.read_file_or_url(url, data=real_submit_keys,
- retries=tries, sec_between=3,
- ssl_details=util.fetch_ssl_details(cloud.paths))
+ url_helper.read_file_or_url(
+ url, data=real_submit_keys, retries=tries, sec_between=3,
+ ssl_details=util.fetch_ssl_details(cloud.paths))
except Exception:
util.logexc(log, "Failed to post phone home data to %s in %s tries",
url, tries)
diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py
index ca7d0d5..ddc3bd2 100644
--- a/cloudinit/config/schema.py
+++ b/cloudinit/config/schema.py
@@ -4,7 +4,7 @@
from __future__ import print_function
from cloudinit import importer
-from cloudinit.util import find_modules, read_file_or_url
+from cloudinit.util import find_modules, load_file
import argparse
from collections import defaultdict
@@ -139,7 +139,7 @@ def validate_cloudconfig_file(config_path, schema, annotate=False):
"""
if not os.path.exists(config_path):
raise RuntimeError('Configfile {0} does not exist'.format(config_path))
- content = read_file_or_url('file://{0}'.format(config_path)).contents
+ content = load_file(config_path, decode=False)
if not content.startswith(CLOUD_CONFIG_HEADER):
errors = (
('header', 'File {0} needs to begin with "{1}"'.format(
diff --git a/cloudinit/ec2_utils.py b/cloudinit/ec2_utils.py
index dc3f0fc..3b7b17f 100644
--- a/cloudinit/ec2_utils.py
+++ b/cloudinit/ec2_utils.py
@@ -150,11 +150,9 @@ def get_instance_userdata(api_version='latest',
# NOT_FOUND occurs) and just in that case returning an empty string.
exception_cb = functools.partial(_skip_retry_on_codes,
SKIP_USERDATA_CODES)
- response = util.read_file_or_url(ud_url,
- ssl_details=ssl_details,
- timeout=timeout,
- retries=retries,
- exception_cb=exception_cb)
+ response = url_helper.read_file_or_url(
+ ud_url, ssl_details=ssl_details, timeout=timeout,
+ retries=retries, exception_cb=exception_cb)
user_data = response.contents
except url_helper.UrlError as e:
if e.code not in SKIP_USERDATA_CODES:
@@ -169,9 +167,9 @@ def _get_instance_metadata(tree, api_version='latest',
ssl_details=None, timeout=5, retries=5,
leaf_decoder=None):
md_url = url_helper.combine_url(metadata_address, api_version, tree)
- caller = functools.partial(util.read_file_or_url,
- ssl_details=ssl_details, timeout=timeout,
- retries=retries)
+ caller = functools.partial(
+ url_helper.read_file_or_url, ssl_details=ssl_details,
+ timeout=timeout, retries=retries)
def mcaller(url):
return caller(url).contents
diff --git a/cloudinit/sources/DataSourceMAAS.py b/cloudinit/sources/DataSourceMAAS.py
index 6ac8863..aeedb38 100644
--- a/cloudinit/sources/DataSourceMAAS.py
+++ b/cloudinit/sources/DataSourceMAAS.py
@@ -198,7 +198,7 @@ def read_maas_seed_url(seed_url, read_file_or_url=None, timeout=None,
If version is None, then <version>/ will not be used.
"""
if read_file_or_url is None:
- read_file_or_url = util.read_file_or_url
+ read_file_or_url = url_helper.read_file_or_url
if seed_url.endswith("/"):
seed_url = seed_url[:-1]
diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py
index 90c12df..e5696b1 100644
--- a/cloudinit/sources/helpers/azure.py
+++ b/cloudinit/sources/helpers/azure.py
@@ -14,6 +14,7 @@ from cloudinit import temp_utils
from contextlib import contextmanager
from xml.etree import ElementTree
+from cloudinit import url_helper
from cloudinit import util
LOG = logging.getLogger(__name__)
@@ -55,14 +56,14 @@ class AzureEndpointHttpClient(object):
if secure:
headers = self.headers.copy()
headers.update(self.extra_secure_headers)
- return util.read_file_or_url(url, headers=headers)
+ return url_helper.read_file_or_url(url, headers=headers)
def post(self, url, data=None, extra_headers=None):
headers = self.headers
if extra_headers is not None:
headers = self.headers.copy()
headers.update(extra_headers)
- return util.read_file_or_url(url, data=data, headers=headers)
+ return url_helper.read_file_or_url(url, data=data, headers=headers)
class GoalState(object):
diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py
index b778a3a..113249d 100644
--- a/cloudinit/tests/test_url_helper.py
+++ b/cloudinit/tests/test_url_helper.py
@@ -1,7 +1,10 @@
# This file is part of cloud-init. See LICENSE file for license information.
-from cloudinit.url_helper import oauth_headers
+from cloudinit.url_helper import oauth_headers, read_file_or_url
from cloudinit.tests.helpers import CiTestCase, mock, skipIf
+from cloudinit import util
+
+import httpretty
try:
@@ -38,3 +41,26 @@ class TestOAuthHeaders(CiTestCase):
'url', 'consumer_key', 'token_key', 'token_secret',
'consumer_secret')
self.assertEqual('url', return_value)
+
+
+class TestReadFileOrUrl(CiTestCase):
+ def test_read_file_or_url_str_from_file(self):
+ """Test that str(result.contents) on file is text version of contents.
+ It should not be "b'data'", but just "'data'" """
+ tmpf = self.tmp_path("myfile1")
+ data = b'This is my file content\n'
+ util.write_file(tmpf, data, omode="wb")
+ result = read_file_or_url("file://%s" % tmpf)
+ self.assertEqual(result.contents, data)
+ self.assertEqual(str(result), data.decode('utf-8'))
+
+ @httpretty.activate
+ def test_read_file_or_url_str_from_url(self):
+ """Test that str(result.contents) on url is text version of contents.
+ It should not be "b'data'", but just "'data'" """
+ url = 'http://hostname/path'
+ data = b'This is my url content\n'
+ httpretty.register_uri(httpretty.GET, url, data)
+ result = read_file_or_url(url)
+ self.assertEqual(result.contents, data)
+ self.assertEqual(str(result), data.decode('utf-8'))
diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py
index 03a573a..2437c0d 100644
--- a/cloudinit/url_helper.py
+++ b/cloudinit/url_helper.py
@@ -15,6 +15,7 @@ import six
import time
from email.utils import parsedate
+from errno import ENOENT
from functools import partial
from itertools import count
from requests import exceptions
@@ -80,6 +81,32 @@ def combine_url(base, *add_ons):
return url
+def read_file_or_url(url, timeout=5, retries=10,
+ headers=None, data=None, sec_between=1, ssl_details=None,
+ headers_cb=None, exception_cb=None):
+ url = url.lstrip()
+ if url.startswith("/"):
+ url = "file://%s" % url
+ if url.lower().startswith("file://"):
+ if data:
+ LOG.warning("Unable to post data to file resource %s", url)
+ file_path = url[len("file://"):]
+ try:
+ with open(file_path, "rb") as fp:
+ contents = fp.read()
+ except IOError as e:
+ code = e.errno
+ if e.errno == ENOENT:
+ code = NOT_FOUND
+ raise UrlError(cause=e, code=code, headers=None, url=url)
+ return FileResponse(file_path, contents=contents)
+ else:
+ return readurl(url, timeout=timeout, retries=retries, headers=headers,
+ headers_cb=headers_cb, data=data,
+ sec_between=sec_between, ssl_details=ssl_details,
+ exception_cb=exception_cb)
+
+
# Made to have same accessors as UrlResponse so that the
# read_file_or_url can return this or that object and the
# 'user' of those objects will not need to know the difference.
@@ -96,7 +123,7 @@ class StringResponse(object):
return True
def __str__(self):
- return self.contents
+ return self.contents.decode('utf-8')
class FileResponse(StringResponse):
diff --git a/cloudinit/user_data.py b/cloudinit/user_data.py
index cc55daf..8f6aba1 100644
--- a/cloudinit/user_data.py
+++ b/cloudinit/user_data.py
@@ -19,7 +19,7 @@ import six
from cloudinit import handlers
from cloudinit import log as logging
-from cloudinit.url_helper import UrlError
+from cloudinit.url_helper import read_file_or_url, UrlError
from cloudinit import util
LOG = logging.getLogger(__name__)
@@ -224,8 +224,8 @@ class UserDataProcessor(object):
content = util.load_file(include_once_fn)
else:
try:
- resp = util.read_file_or_url(include_url,
- ssl_details=self.ssl_details)
+ resp = read_file_or_url(include_url,
+ ssl_details=self.ssl_details)
if include_once_on and resp.ok():
util.write_file(include_once_fn, resp.contents,
mode=0o600)
diff --git a/cloudinit/util.py b/cloudinit/util.py
index acdc0d8..2d6ea59 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -857,37 +857,6 @@ def fetch_ssl_details(paths=None):
return ssl_details
-def read_file_or_url(url, timeout=5, retries=10,
- headers=None, data=None, sec_between=1, ssl_details=None,
- headers_cb=None, exception_cb=None):
- url = url.lstrip()
- if url.startswith("/"):
- url = "file://%s" % url
- if url.lower().startswith("file://"):
- if data:
- LOG.warning("Unable to post data to file resource %s", url)
- file_path = url[len("file://"):]
- try:
- contents = load_file(file_path, decode=False)
- except IOError as e:
- code = e.errno
- if e.errno == ENOENT:
- code = url_helper.NOT_FOUND
- raise url_helper.UrlError(cause=e, code=code, headers=None,
- url=url)
- return url_helper.FileResponse(file_path, contents=contents)
- else:
- return url_helper.readurl(url,
- timeout=timeout,
- retries=retries,
- headers=headers,
- headers_cb=headers_cb,
- data=data,
- sec_between=sec_between,
- ssl_details=ssl_details,
- exception_cb=exception_cb)
-
-
def load_yaml(blob, default=None, allowed=(dict,)):
loaded = default
blob = decode_binary(blob)
@@ -925,12 +894,14 @@ def read_seeded(base="", ext="", timeout=5, retries=10, file_retries=0):
ud_url = "%s%s%s" % (base, "user-data", ext)
md_url = "%s%s%s" % (base, "meta-data", ext)
- md_resp = read_file_or_url(md_url, timeout, retries, file_retries)
+ md_resp = url_helper.read_file_or_url(md_url, timeout, retries,
+ file_retries)
md = None
if md_resp.ok():
md = load_yaml(decode_binary(md_resp.contents), default={})
- ud_resp = read_file_or_url(ud_url, timeout, retries, file_retries)
+ ud_resp = url_helper.read_file_or_url(ud_url, timeout, retries,
+ file_retries)
ud = None
if ud_resp.ok():
ud = ud_resp.contents
diff --git a/tests/unittests/test__init__.py b/tests/unittests/test__init__.py
index 25878d7..2f2e42c 100644
--- a/tests/unittests/test__init__.py
+++ b/tests/unittests/test__init__.py
@@ -182,7 +182,7 @@ class TestCmdlineUrl(CiTestCase):
self.assertEqual(
('url', 'http://example.com'), main.parse_cmdline_url(cmdline))
- @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
+ @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
def test_invalid_content(self, m_read):
key = "cloud-config-url"
url = 'http://example.com/foo'
@@ -196,7 +196,7 @@ class TestCmdlineUrl(CiTestCase):
self.assertIn(url, msg)
self.assertFalse(os.path.exists(fpath))
- @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
+ @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
def test_valid_content(self, m_read):
url = "http://example.com/foo";
payload = b"#cloud-config\nmydata: foo\nbar: wark\n"
@@ -210,7 +210,7 @@ class TestCmdlineUrl(CiTestCase):
self.assertEqual(logging.INFO, lvl)
self.assertIn(url, msg)
- @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
+ @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
def test_no_key_found(self, m_read):
cmdline = "ro mykey=http://example.com/foo root=foo"
fpath = self.tmp_path("ccpath")
@@ -221,7 +221,7 @@ class TestCmdlineUrl(CiTestCase):
self.assertFalse(os.path.exists(fpath))
self.assertEqual(logging.DEBUG, lvl)
- @mock.patch('cloudinit.cmd.main.util.read_file_or_url')
+ @mock.patch('cloudinit.cmd.main.url_helper.read_file_or_url')
def test_exception_warns(self, m_read):
url = "http://example.com/foo";
cmdline = "ro cloud-config-url=%s root=LABEL=bar" % url
diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py
index b42b073..af9d3e1 100644
--- a/tests/unittests/test_datasource/test_azure_helper.py
+++ b/tests/unittests/test_datasource/test_azure_helper.py
@@ -195,7 +195,7 @@ class TestAzureEndpointHttpClient(CiTestCase):
self.addCleanup(patches.close)
self.read_file_or_url = patches.enter_context(
- mock.patch.object(azure_helper.util, 'read_file_or_url'))
+ mock.patch.object(azure_helper.url_helper, 'read_file_or_url'))
def test_non_secure_get(self):
client = azure_helper.AzureEndpointHttpClient(mock.MagicMock())
diff --git a/tests/unittests/test_handler/test_handler_apt_conf_v1.py b/tests/unittests/test_handler/test_handler_apt_conf_v1.py
index 83f962a..6a4b03e 100644
--- a/tests/unittests/test_handler/test_handler_apt_conf_v1.py
+++ b/tests/unittests/test_handler/test_handler_apt_conf_v1.py
@@ -12,10 +12,6 @@ import shutil
import tempfile
-def load_tfile_or_url(*args, **kwargs):
- return(util.decode_binary(util.read_file_or_url(*args, **kwargs).contents))
-
-
class TestAptProxyConfig(TestCase):
def setUp(self):
super(TestAptProxyConfig, self).setUp()
@@ -36,7 +32,7 @@ class TestAptProxyConfig(TestCase):
self.assertTrue(os.path.isfile(self.pfile))
self.assertFalse(os.path.isfile(self.cfile))
- contents = load_tfile_or_url(self.pfile)
+ contents = util.load_file(self.pfile)
self.assertTrue(self._search_apt_config(contents, "http", "myproxy"))
def test_apt_http_proxy_written(self):
@@ -46,7 +42,7 @@ class TestAptProxyConfig(TestCase):
self.assertTrue(os.path.isfile(self.pfile))
self.assertFalse(os.path.isfile(self.cfile))
- contents = load_tfile_or_url(self.pfile)
+ contents = util.load_file(self.pfile)
self.assertTrue(self._search_apt_config(contents, "http", "myproxy"))
def test_apt_all_proxy_written(self):
@@ -64,7 +60,7 @@ class TestAptProxyConfig(TestCase):
self.assertTrue(os.path.isfile(self.pfile))
self.assertFalse(os.path.isfile(self.cfile))
- contents = load_tfile_or_url(self.pfile)
+ contents = util.load_file(self.pfile)
for ptype, pval in values.items():
self.assertTrue(self._search_apt_config(contents, ptype, pval))
@@ -80,7 +76,7 @@ class TestAptProxyConfig(TestCase):
cc_apt_configure.apply_apt_config({'proxy': "foo"},
self.pfile, self.cfile)
self.assertTrue(os.path.isfile(self.pfile))
- contents = load_tfile_or_url(self.pfile)
+ contents = util.load_file(self.pfile)
self.assertTrue(self._search_apt_config(contents, "http", "foo"))
def test_config_written(self):
@@ -92,14 +88,14 @@ class TestAptProxyConfig(TestCase):
self.assertTrue(os.path.isfile(self.cfile))
self.assertFalse(os.path.isfile(self.pfile))
- self.assertEqual(load_tfile_or_url(self.cfile), payload)
+ self.assertEqual(util.load_file(self.cfile), payload)
def test_config_replaced(self):
util.write_file(self.pfile, "content doesnt matter")
cc_apt_configure.apply_apt_config({'conf': "foo"},
self.pfile, self.cfile)
self.assertTrue(os.path.isfile(self.cfile))
- self.assertEqual(load_tfile_or_url(self.cfile), "foo")
+ self.assertEqual(util.load_file(self.cfile), "foo")
def test_config_deleted(self):
# if no 'conf' is provided, delete any previously written file
diff --git a/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py b/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py
index d2b96f0..23bd6e1 100644
--- a/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py
+++ b/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py
@@ -64,13 +64,6 @@ deb-src http://archive.ubuntu.com/ubuntu/ fakerelease main restricted
""")
-def load_tfile_or_url(*args, **kwargs):
- """load_tfile_or_url
- load file and return content after decoding
- """
- return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents)
-
-
class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase):
"""TestAptSourceConfigSourceList
Main Class to test sources list rendering
diff --git a/tests/unittests/test_handler/test_handler_apt_source_v1.py b/tests/unittests/test_handler/test_handler_apt_source_v1.py
index 46ca4ce..a3132fb 100644
--- a/tests/unittests/test_handler/test_handler_apt_source_v1.py
+++ b/tests/unittests/test_handler/test_handler_apt_source_v1.py
@@ -39,13 +39,6 @@ S0ORP6HXET3+jC8BMG4tBWCTK/XEZw==
ADD_APT_REPO_MATCH = r"^[\w-]+:\w"
-def load_tfile_or_url(*args, **kwargs):
- """load_tfile_or_url
- load file and return content after decoding
- """
- return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents)
-
-
class FakeDistro(object):
"""Fake Distro helper object"""
def update_package_sources(self):
@@ -125,7 +118,7 @@ class TestAptSourceConfig(TestCase):
self.assertTrue(os.path.isfile(filename))
- contents = load_tfile_or_url(filename)
+ contents = util.load_file(filename)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", "http://archive.ubuntu.com/ubuntu";,
"karmic-backports",
@@ -157,13 +150,13 @@ class TestAptSourceConfig(TestCase):
self.apt_src_basic(self.aptlistfile, cfg)
# extra verify on two extra files of this test
- contents = load_tfile_or_url(self.aptlistfile2)
+ contents = util.load_file(self.aptlistfile2)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", "http://archive.ubuntu.com/ubuntu";,
"precise-backports",
"main universe multiverse restricted"),
contents, flags=re.IGNORECASE))
- contents = load_tfile_or_url(self.aptlistfile3)
+ contents = util.load_file(self.aptlistfile3)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", "http://archive.ubuntu.com/ubuntu";,
"lucid-backports",
@@ -220,7 +213,7 @@ class TestAptSourceConfig(TestCase):
self.assertTrue(os.path.isfile(filename))
- contents = load_tfile_or_url(filename)
+ contents = util.load_file(filename)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", params['MIRROR'], params['RELEASE'],
"multiverse"),
@@ -241,12 +234,12 @@ class TestAptSourceConfig(TestCase):
# extra verify on two extra files of this test
params = self._get_default_params()
- contents = load_tfile_or_url(self.aptlistfile2)
+ contents = util.load_file(self.aptlistfile2)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", params['MIRROR'], params['RELEASE'],
"main"),
contents, flags=re.IGNORECASE))
- contents = load_tfile_or_url(self.aptlistfile3)
+ contents = util.load_file(self.aptlistfile3)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", params['MIRROR'], params['RELEASE'],
"universe"),
@@ -296,7 +289,7 @@ class TestAptSourceConfig(TestCase):
self.assertTrue(os.path.isfile(filename))
- contents = load_tfile_or_url(filename)
+ contents = util.load_file(filename)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb",
('http://ppa.launchpad.net/smoser/'
@@ -336,14 +329,14 @@ class TestAptSourceConfig(TestCase):
'filename': self.aptlistfile3}
self.apt_src_keyid(self.aptlistfile, [cfg1, cfg2, cfg3], 3)
- contents = load_tfile_or_url(self.aptlistfile2)
+ contents = util.load_file(self.aptlistfile2)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb",
('http://ppa.launchpad.net/smoser/'
'cloud-init-test/ubuntu'),
"xenial", "universe"),
contents, flags=re.IGNORECASE))
- contents = load_tfile_or_url(self.aptlistfile3)
+ contents = util.load_file(self.aptlistfile3)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb",
('http://ppa.launchpad.net/smoser/'
@@ -375,7 +368,7 @@ class TestAptSourceConfig(TestCase):
self.assertTrue(os.path.isfile(filename))
- contents = load_tfile_or_url(filename)
+ contents = util.load_file(filename)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb",
('http://ppa.launchpad.net/smoser/'
diff --git a/tests/unittests/test_handler/test_handler_apt_source_v3.py b/tests/unittests/test_handler/test_handler_apt_source_v3.py
index 7bb1b7c..904b912 100644
--- a/tests/unittests/test_handler/test_handler_apt_source_v3.py
+++ b/tests/unittests/test_handler/test_handler_apt_source_v3.py
@@ -49,13 +49,6 @@ ADD_APT_REPO_MATCH = r"^[\w-]+:\w"
TARGET = None
-def load_tfile(*args, **kwargs):
- """load_tfile_or_url
- load file and return content after decoding
- """
- return util.decode_binary(util.read_file_or_url(*args, **kwargs).contents)
-
-
class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
"""TestAptSourceConfig
Main Class to test apt configs
@@ -119,7 +112,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
self.assertTrue(os.path.isfile(filename))
- contents = load_tfile(filename)
+ contents = util.load_file(filename)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", "http://test.ubuntu.com/ubuntu";,
"karmic-backports",
@@ -151,13 +144,13 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
self._apt_src_basic(self.aptlistfile, cfg)
# extra verify on two extra files of this test
- contents = load_tfile(self.aptlistfile2)
+ contents = util.load_file(self.aptlistfile2)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", "http://test.ubuntu.com/ubuntu";,
"precise-backports",
"main universe multiverse restricted"),
contents, flags=re.IGNORECASE))
- contents = load_tfile(self.aptlistfile3)
+ contents = util.load_file(self.aptlistfile3)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", "http://test.ubuntu.com/ubuntu";,
"lucid-backports",
@@ -174,7 +167,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
self.assertTrue(os.path.isfile(filename))
- contents = load_tfile(filename)
+ contents = util.load_file(filename)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", params['MIRROR'], params['RELEASE'],
"multiverse"),
@@ -201,12 +194,12 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
# extra verify on two extra files of this test
params = self._get_default_params()
- contents = load_tfile(self.aptlistfile2)
+ contents = util.load_file(self.aptlistfile2)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", params['MIRROR'], params['RELEASE'],
"main"),
contents, flags=re.IGNORECASE))
- contents = load_tfile(self.aptlistfile3)
+ contents = util.load_file(self.aptlistfile3)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb", params['MIRROR'], params['RELEASE'],
"universe"),
@@ -240,7 +233,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
self.assertTrue(os.path.isfile(filename))
- contents = load_tfile(filename)
+ contents = util.load_file(filename)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb",
('http://ppa.launchpad.net/smoser/'
@@ -277,14 +270,14 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
'keyid': "03683F77"}}
self._apt_src_keyid(self.aptlistfile, cfg, 3)
- contents = load_tfile(self.aptlistfile2)
+ contents = util.load_file(self.aptlistfile2)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb",
('http://ppa.launchpad.net/smoser/'
'cloud-init-test/ubuntu'),
"xenial", "universe"),
contents, flags=re.IGNORECASE))
- contents = load_tfile(self.aptlistfile3)
+ contents = util.load_file(self.aptlistfile3)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb",
('http://ppa.launchpad.net/smoser/'
@@ -310,7 +303,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
self.assertTrue(os.path.isfile(self.aptlistfile))
- contents = load_tfile(self.aptlistfile)
+ contents = util.load_file(self.aptlistfile)
self.assertTrue(re.search(r"%s %s %s %s\n" %
("deb",
('http://ppa.launchpad.net/smoser/'
diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
index 1b3ca57..f52a595 100644
--- a/tests/unittests/test_handler/test_handler_ntp.py
+++ b/tests/unittests/test_handler/test_handler_ntp.py
@@ -1,8 +1,7 @@
# This file is part of cloud-init. See LICENSE file for license information.
-
from cloudinit.config import cc_ntp
from cloudinit.sources import DataSourceNone
-from cloudinit import (distros, helpers, cloud, util)
+from cloudinit import (distros, helpers, cloud, util, url_helper)
from cloudinit.tests.helpers import (
CiTestCase, FilesystemMockingTestCase, mock, skipUnlessJsonSchema)
@@ -143,6 +142,7 @@ class TestNtp(FilesystemMockingTestCase):
self.assertFalse(os.path.exists("{0}.dist".format(ntpconf)))
self.assertFalse(os.path.exists(ntpconf))
+<<<<<<< tests/unittests/test_handler/test_handler_ntp.py
def test_write_ntp_config_template_uses_ntp_conf_distro_no_servers(self):
"""write_ntp_config_template reads from $client.conf.distro.tmpl"""
servers = []
@@ -156,6 +156,27 @@ class TestNtp(FilesystemMockingTestCase):
template_fn=template_fn,
template=None)
content = util.read_file_or_url('file://' + confpath).contents
+=======
+ def test_write_ntp_config_template_from_ntp_conf_tmpl_with_servers(self):
+ """write_ntp_config_template reads content from ntp.conf.tmpl.
+
+ It reads ntp.conf.tmpl if present and renders the value from servers
+ key. When no pools key is defined, template is rendered using an empty
+ list for pools.
+ """
+ distro = 'ubuntu'
+ cfg = {
+ 'servers': ['192.168.2.1', '192.168.2.2']
+ }
+ mycloud = self._get_cloud(distro)
+ ntp_conf = self.tmp_path("ntp.conf", self.new_root) # Doesn't exist
+ # Create ntp.conf.tmpl
+ with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
+ stream.write(NTP_TEMPLATE)
+ with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+ cc_ntp.write_ntp_config_template(cfg, mycloud, ntp_conf)
+ content = url_helper.read_file_or_url('file://' + ntp_conf).contents
+>>>>>>> tests/unittests/test_handler/test_handler_ntp.py
self.assertEqual(
"servers []\npools ['10.0.0.1', '10.0.0.2']\n", content.decode())
@@ -166,6 +187,7 @@ class TestNtp(FilesystemMockingTestCase):
configured.
"""
distro = 'ubuntu'
+<<<<<<< tests/unittests/test_handler/test_handler_ntp.py
pools = cc_ntp.generate_server_names(distro)
servers = []
(confpath, template_fn) = self._generate_template()
@@ -177,6 +199,22 @@ class TestNtp(FilesystemMockingTestCase):
template_fn=template_fn,
template=None)
content = util.read_file_or_url('file://' + confpath).contents
+=======
+ cfg = {
+ 'pools': ['10.0.0.1', '10.0.0.2']
+ }
+ mycloud = self._get_cloud(distro)
+ ntp_conf = self.tmp_path('ntp.conf', self.new_root) # Doesn't exist
+ # Create ntp.conf.tmpl which isn't read
+ with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
+ stream.write(b'NOT READ: ntp.conf.<distro>.tmpl is primary')
+ # Create ntp.conf.tmpl.<distro>
+ with open('{0}.{1}.tmpl'.format(ntp_conf, distro), 'wb') as stream:
+ stream.write(NTP_TEMPLATE)
+ with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+ cc_ntp.write_ntp_config_template(cfg, mycloud, ntp_conf)
+ content = url_helper.read_file_or_url('file://' + ntp_conf).contents
+>>>>>>> tests/unittests/test_handler/test_handler_ntp.py
self.assertEqual(
"servers []\npools {0}\n".format(pools),
content.decode())
@@ -187,6 +225,7 @@ class TestNtp(FilesystemMockingTestCase):
When both pools and servers are empty, default NR_POOL_SERVERS get
configured.
"""
+<<<<<<< tests/unittests/test_handler/test_handler_ntp.py
distro = 'sles'
default_pools = cc_ntp.generate_server_names(distro)
(confpath, template_fn) = self._generate_template()
@@ -199,6 +238,20 @@ class TestNtp(FilesystemMockingTestCase):
content = util.read_file_or_url('file://' + confpath).contents
for pool in default_pools:
self.assertIn('opensuse', pool)
+=======
+ distro = 'ubuntu'
+ mycloud = self._get_cloud(distro)
+ ntp_conf = self.tmp_path('ntp.conf', self.new_root) # Doesn't exist
+ # Create ntp.conf.tmpl
+ with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
+ stream.write(NTP_TEMPLATE)
+ with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+ cc_ntp.write_ntp_config_template({}, mycloud, ntp_conf)
+ content = url_helper.read_file_or_url('file://' + ntp_conf).contents
+ default_pools = [
+ "{0}.{1}.pool.ntp.org".format(x, distro)
+ for x in range(0, cc_ntp.NR_POOL_SERVERS)]
+>>>>>>> tests/unittests/test_handler/test_handler_ntp.py
self.assertEqual(
"servers []\npools {0}\n".format(default_pools), content.decode())
self.assertIn(
@@ -206,6 +259,7 @@ class TestNtp(FilesystemMockingTestCase):
",".join(default_pools)),
self.logs.getvalue())
+<<<<<<< tests/unittests/test_handler/test_handler_ntp.py
def test_timesyncd_template(self):
"""Test timesycnd template is correct"""
pools = ['0.mycompany.pool.ntp.org', '3.mycompany.pool.ntp.org']
@@ -218,6 +272,59 @@ class TestNtp(FilesystemMockingTestCase):
template_fn=template_fn,
template=None)
content = util.read_file_or_url('file://' + confpath).contents
+=======
+ @mock.patch("cloudinit.config.cc_ntp.ntp_installable")
+ def test_ntp_handler_mocked_template(self, m_ntp_install):
+ """Test ntp handler renders ubuntu ntp.conf template."""
+ pools = ['0.mycompany.pool.ntp.org', '3.mycompany.pool.ntp.org']
+ servers = ['192.168.23.3', '192.168.23.4']
+ cfg = {
+ 'ntp': {
+ 'pools': pools,
+ 'servers': servers
+ }
+ }
+ mycloud = self._get_cloud('ubuntu')
+ ntp_conf = self.tmp_path('ntp.conf', self.new_root) # Doesn't exist
+ m_ntp_install.return_value = True
+
+ # Create ntp.conf.tmpl
+ with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
+ stream.write(NTP_TEMPLATE)
+ with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+ with mock.patch.object(util, 'which', return_value=None):
+ cc_ntp.handle('notimportant', cfg, mycloud, None, None)
+
+ content = url_helper.read_file_or_url('file://' + ntp_conf).contents
+ self.assertEqual(
+ 'servers {0}\npools {1}\n'.format(servers, pools),
+ content.decode())
+
+ @mock.patch("cloudinit.config.cc_ntp.util")
+ def test_ntp_handler_mocked_template_snappy(self, m_util):
+ """Test ntp handler renders timesycnd.conf template on snappy."""
+ pools = ['0.mycompany.pool.ntp.org', '3.mycompany.pool.ntp.org']
+ servers = ['192.168.23.3', '192.168.23.4']
+ cfg = {
+ 'ntp': {
+ 'pools': pools,
+ 'servers': servers
+ }
+ }
+ mycloud = self._get_cloud('ubuntu')
+ m_util.system_is_snappy.return_value = True
+
+ # Create timesyncd.conf.tmpl
+ tsyncd_conf = self.tmp_path("timesyncd.conf", self.new_root)
+ template = '{0}.tmpl'.format(tsyncd_conf)
+ with open(template, 'wb') as stream:
+ stream.write(TIMESYNCD_TEMPLATE)
+
+ with mock.patch('cloudinit.config.cc_ntp.TIMESYNCD_CONF', tsyncd_conf):
+ cc_ntp.handle('notimportant', cfg, mycloud, None, None)
+
+ content = url_helper.read_file_or_url('file://' + tsyncd_conf).contents
+>>>>>>> tests/unittests/test_handler/test_handler_ntp.py
self.assertEqual(
"[Time]\nNTP=%s %s \n" % (" ".join(servers), " ".join(pools)),
content.decode())
@@ -244,6 +351,7 @@ class TestNtp(FilesystemMockingTestCase):
"""Test ntp handler renders the shipped distro ntp client templates."""
pools = ['0.mycompany.pool.ntp.org', '3.mycompany.pool.ntp.org']
servers = ['192.168.23.3', '192.168.23.4']
+<<<<<<< tests/unittests/test_handler/test_handler_ntp.py
for client in ['ntp', 'systemd-timesyncd', 'chrony']:
for distro in cc_ntp.distros:
distro_cfg = cc_ntp.distro_ntp_client_configs(distro)
@@ -287,6 +395,40 @@ class TestNtp(FilesystemMockingTestCase):
"[Time]\nNTP=%s %s \n" % (" ".join(servers),
" ".join(pools)))
self.assertEqual(expected_content, content.decode())
+=======
+ cfg = {
+ 'ntp': {
+ 'pools': pools,
+ 'servers': servers
+ }
+ }
+ ntp_conf = self.tmp_path('ntp.conf', self.new_root) # Doesn't exist
+ for distro in ('debian', 'ubuntu', 'fedora', 'rhel', 'sles'):
+ mycloud = self._get_cloud(distro)
+ root_dir = dirname(dirname(os.path.realpath(util.__file__)))
+ tmpl_file = os.path.join(
+ '{0}/templates/ntp.conf.{1}.tmpl'.format(root_dir, distro))
+ # Create a copy in our tmp_dir
+ shutil.copy(
+ tmpl_file,
+ os.path.join(self.new_root, 'ntp.conf.%s.tmpl' % distro))
+ with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+ with mock.patch.object(util, 'which', return_value=[True]):
+ cc_ntp.handle('notimportant', cfg, mycloud, None, None)
+
+ content = url_helper.read_file_or_url(
+ 'file://' + ntp_conf).contents
+ expected_servers = '\n'.join([
+ 'server {0} iburst'.format(server) for server in servers])
+ self.assertIn(
+ expected_servers, content.decode(),
+ 'failed to render ntp.conf for distro:{0}'.format(distro))
+ expected_pools = '\n'.join([
+ 'pool {0} iburst'.format(pool) for pool in pools])
+ self.assertIn(
+ expected_pools, content.decode(),
+ 'failed to render ntp.conf for distro:{0}'.format(distro))
+>>>>>>> tests/unittests/test_handler/test_handler_ntp.py
def test_no_ntpcfg_does_nothing(self):
"""When no ntp section is defined handler logs a warning and noops."""
@@ -415,6 +557,7 @@ class TestNtp(FilesystemMockingTestCase):
@mock.patch('cloudinit.config.cc_ntp.select_ntp_client')
def test_ntp_handler_timesyncd(self, m_select):
"""Test ntp handler configures timesyncd"""
+<<<<<<< tests/unittests/test_handler/test_handler_ntp.py
servers = ['192.168.2.1', '192.168.2.2']
pools = ['0.mypool.org']
cfg = {'ntp': {'servers': servers, 'pools': pools}}
@@ -503,13 +646,61 @@ class TestNtp(FilesystemMockingTestCase):
mycloud = self._get_cloud('ubuntu')
expected_client = mycloud.distro.preferred_ntp_clients[0]
self.assertEqual('ntp', expected_client)
+=======
+ m_ntp_install.return_value = False
+ distro = 'ubuntu'
+ cfg = {
+ 'servers': ['192.168.2.1', '192.168.2.2'],
+ 'pools': ['0.mypool.org'],
+ }
+ mycloud = self._get_cloud(distro)
+ tsyncd_conf = self.tmp_path("timesyncd.conf", self.new_root)
+ # Create timesyncd.conf.tmpl
+ template = '{0}.tmpl'.format(tsyncd_conf)
+ print(template)
+ with open(template, 'wb') as stream:
+ stream.write(TIMESYNCD_TEMPLATE)
+ with mock.patch('cloudinit.config.cc_ntp.TIMESYNCD_CONF', tsyncd_conf):
+ cc_ntp.write_ntp_config_template(cfg, mycloud, tsyncd_conf,
+ template='timesyncd.conf')
+
+ content = url_helper.read_file_or_url('file://' + tsyncd_conf).contents
+ self.assertEqual(
+ "[Time]\nNTP=192.168.2.1 192.168.2.2 0.mypool.org \n",
+ content.decode())
+>>>>>>> tests/unittests/test_handler/test_handler_ntp.py
@mock.patch('cloudinit.config.cc_ntp.util.which')
def test_snappy_system_picks_timesyncd(self, m_which):
"""Test snappy systems prefer installed clients"""
+<<<<<<< tests/unittests/test_handler/test_handler_ntp.py
# we are on ubuntu-core here
self.m_snappy.return_value = True
+=======
+ When both pools and servers are empty, default NR_POOL_SERVERS get
+ configured.
+ """
+ distro = 'sles'
+ mycloud = self._get_cloud(distro)
+ ntp_conf = self.tmp_path('ntp.conf', self.new_root) # Doesn't exist
+ # Create ntp.conf.tmpl
+ with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
+ stream.write(NTP_TEMPLATE)
+ with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+ cc_ntp.write_ntp_config_template({}, mycloud, ntp_conf)
+ content = url_helper.read_file_or_url('file://' + ntp_conf).contents
+ default_pools = [
+ "{0}.opensuse.pool.ntp.org".format(x)
+ for x in range(0, cc_ntp.NR_POOL_SERVERS)]
+ self.assertEqual(
+ "servers []\npools {0}\n".format(default_pools),
+ content.decode())
+ self.assertIn(
+ "Adding distro default ntp pool servers: {0}".format(
+ ",".join(default_pools)),
+ self.logs.getvalue())
+>>>>>>> tests/unittests/test_handler/test_handler_ntp.py
# ubuntu core systems will have timesyncd installed
m_which.side_effect = iter([None, '/lib/systemd/systemd-timesyncd',
Follow ups
