Merge ~chad.smith/cloud-init:set-hostname-before-network into cloud-init:master

Proposed by Chad Smith
Status: Merged
Approved by: Scott Moser
Approved revision: 148b8ab1ae0a21bb9e58a94818a234d0aaaf3548
Merge reported by: Chad Smith
Merged at revision: 133ad2cb327ad17b7b81319fac8f9f14577c04df
Proposed branch: ~chad.smith/cloud-init:set-hostname-before-network
Merge into: cloud-init:master
Diff against target: 668 lines (+449/-22)
9 files modified
cloudinit/cloud.py (+3/-2)
cloudinit/cmd/main.py (+25/-0)
cloudinit/cmd/tests/test_main.py (+161/-0)
cloudinit/config/cc_set_hostname.py (+35/-6)
cloudinit/sources/__init__.py (+17/-4)
cloudinit/sources/tests/test_init.py (+69/-1)
cloudinit/tests/test_util.py (+74/-0)
cloudinit/util.py (+12/-5)
tests/unittests/test_handler/test_handler_set_hostname.py (+53/-4)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+339720@code.launchpad.net

Commit message

set_hostname: When present in metadata, set it before network bringup.

When instance meta-data provides hostname information, run
cc_set_hostname in the init-local or init-net stage before network
comes up.

Prevent an initial DHCP request which leaks the stock cloud-image default
hostname before the meta-data provided hostname was processed.
A leaked cloud-image hostname adversely affects Dynamic DNS which
would reallocate 'ubuntu' hostname in DNS to every instance brought up by
cloud-init. These instances would only update DNS to the cloud-init
configured hostname upon DHCP lease renewal.

This branch extends the get_hostname methods in datasource, cloud and
util to limit results to metadata_only to avoid extra cost of querying
the distro for hostname information if metadata does not provide that
information.

LP: #1746455

Description of the change

See commit message

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:a015a634f18594be2bcf78824eb64311d1d52845
https://jenkins.ubuntu.com/server/job/cloud-init-ci/790/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/790/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Scott Moser (smoser) :
Revision history for this message
Ryan Harper (raharper) wrote :

I've a bit of a quibble with the description. Mostly because cloud-init has _no_ control over how DNS or DHCP servers react to dhcp client requests coming from an instance.

What we (cloud-init) can do is set instance specified hostnames as _soon_ as we know it, which may be of help to some situations where Dynamic DNS via dhcp client hostname value.

This is an environment specific issue and having cloud-init set hostname as soon as possible does not ensure that there won't be issues with DNS, DHCP outside of cloud-init control.

Some inline comments too.

8211b97... by Chad Smith

lints and flake8

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:2baccbbf6c4421ef8e166e7a35f20be442d73578
https://jenkins.ubuntu.com/server/job/cloud-init-ci/793/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/793/rebuild

review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) :
Revision history for this message
Chad Smith (chad.smith) :
Revision history for this message
Scott Moser (smoser) wrote :

My only comment at this point is wheter or not we should bother with calling the config module and writing the marker file and such at that point.

the other option is to just set it.

The other thoughts (i guess i did have more than one)
a.) we have had issues with setting hostname not being possible early in boot due to that being a systemd service (hostnamectl) .
b.) we want to somehow make sure that we only set this hostname on first instance boot. We don't want to overwrite what a user configured manually.

I think that your use of config module probably handles that, but just want to write those thoughts down.

Revision history for this message
Ryan Harper (raharper) wrote :

Sorry for the noise on the get_hostname part; I missed it getting sent through the datasource object.

6bd8875... by Chad Smith

SethostnameError is now raised by cc_set_hostname. Handle that exception in init-local main. Call cc_set_hostname if hostname changed in init-net.

37d5b61... by Chad Smith

UserDataProcess class now returns a tuple of (processed_messages, processing_errors). Datsources now requery userdata or vendordata on previous errors instead of using the cache.

227b353... by Chad Smith

flakes

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:2b8f6fa52c9f4dbd21a0f6fa572c6660c78f8972
https://jenkins.ubuntu.com/server/job/cloud-init-ci/798/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/798/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

hm...
Does UserDataProcessor.process() actually get part handlers and such called?

If so, we're absolutely changing their contract. Now they'd be running without a network.

I'm really sorry this is so hard. :-(

6ebb0d1... by Chad Smith

address review comments

cc8a6ec... by Chad Smith

revert UserDataProcess changes

92d62c6... by Chad Smith

don't call update in local stage as it tries to process userdata. Only react to metadata in local stage

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:254d7a22e42b2179f9a4450bcfe6feed056828ea
https://jenkins.ubuntu.com/server/job/cloud-init-ci/811/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/811/rebuild

review: Approve (continuous-integration)
e51081d... by Chad Smith

unit tests for cc_set_hostname and get_hostname_fqdn

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:cf810a94cbfe3cb46b21e40f7c50d6a77ea22d0f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/812/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/812/rebuild

review: Approve (continuous-integration)
29fc8fe... by Chad Smith

add unit test for SetHostnameError

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:711843e2e24f279ec7dbe38fef89ea95305011fc
https://jenkins.ubuntu.com/server/job/cloud-init-ci/813/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/813/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

Some questions inline.
I think your commit message also needs updating to address the
fact that you're (I htink) only looking at meta-data, not user-data.
Specifically:
  "When instance metadata/user-data provides hostname information,"

review: Needs Fixing
4a3814d... by Chad Smith

address review: fixup incorrect docstring comments on metadata_only parameter, comment about different uses of set-hostname artifact file versus prevvious-hostname used by cc_update_hostname

d9ff4f2... by Chad Smith

add unit tests for main_init calling set_hostname on metadata

148b8ab... by Chad Smith

flakes

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:fe9ba4daf5997fcbe5d6b81a59becc5d95f3dfc9
https://jenkins.ubuntu.com/server/job/cloud-init-ci/844/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/844/rebuild

review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:148b8ab1ae0a21bb9e58a94818a234d0aaaf3548
https://jenkins.ubuntu.com/server/job/cloud-init-ci/845/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/845/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

Approved. I'd like to get this into bionic tomorrow.

review: Approve
Revision history for this message
Chad Smith (chad.smith) wrote :

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=133ad2cb

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/cloud.py b/cloudinit/cloud.py
2index ba61678..6d12c43 100644
3--- a/cloudinit/cloud.py
4+++ b/cloudinit/cloud.py
5@@ -78,8 +78,9 @@ class Cloud(object):
6 def get_locale(self):
7 return self.datasource.get_locale()
8
9- def get_hostname(self, fqdn=False):
10- return self.datasource.get_hostname(fqdn=fqdn)
11+ def get_hostname(self, fqdn=False, metadata_only=False):
12+ return self.datasource.get_hostname(
13+ fqdn=fqdn, metadata_only=metadata_only)
14
15 def device_name_to_device(self, name):
16 return self.datasource.device_name_to_device(name)
17diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
18index fcddd75..3f2dbb9 100644
19--- a/cloudinit/cmd/main.py
20+++ b/cloudinit/cmd/main.py
21@@ -40,6 +40,7 @@ from cloudinit.settings import (PER_INSTANCE, PER_ALWAYS, PER_ONCE,
22
23 from cloudinit import atomic_helper
24
25+from cloudinit.config import cc_set_hostname
26 from cloudinit.dhclient_hook import LogDhclient
27
28
29@@ -352,6 +353,11 @@ def main_init(name, args):
30 LOG.debug("[%s] %s will now be targeting instance id: %s. new=%s",
31 mode, name, iid, init.is_new_instance())
32
33+ if mode == sources.DSMODE_LOCAL:
34+ # Before network comes up, set any configured hostname to allow
35+ # dhcp clients to advertize this hostname to any DDNS services
36+ # LP: #1746455.
37+ _maybe_set_hostname(init, stage='local', retry_stage='network')
38 init.apply_network_config(bring_up=bool(mode != sources.DSMODE_LOCAL))
39
40 if mode == sources.DSMODE_LOCAL:
41@@ -368,6 +374,7 @@ def main_init(name, args):
42 init.setup_datasource()
43 # update fully realizes user-data (pulling in #include if necessary)
44 init.update()
45+ _maybe_set_hostname(init, stage='init-net', retry_stage='modules:config')
46 # Stage 7
47 try:
48 # Attempt to consume the data per instance.
49@@ -681,6 +688,24 @@ def status_wrapper(name, args, data_d=None, link_d=None):
50 return len(v1[mode]['errors'])
51
52
53+def _maybe_set_hostname(init, stage, retry_stage):
54+ """Call set-hostname if metadata, vendordata or userdata provides it.
55+
56+ @param stage: String representing current stage in which we are running.
57+ @param retry_stage: String represented logs upon error setting hostname.
58+ """
59+ cloud = init.cloudify()
60+ (hostname, _fqdn) = util.get_hostname_fqdn(
61+ init.cfg, cloud, metadata_only=True)
62+ if hostname: # meta-data or user-data hostname content
63+ try:
64+ cc_set_hostname.handle('set-hostname', init.cfg, cloud, LOG, None)
65+ except cc_set_hostname.SetHostnameError as e:
66+ LOG.debug(
67+ 'Failed setting hostname in %s stage. Will'
68+ ' retry in %s stage. Error: %s.', stage, retry_stage, str(e))
69+
70+
71 def main_features(name, args):
72 sys.stdout.write('\n'.join(sorted(version.FEATURES)) + '\n')
73
74diff --git a/cloudinit/cmd/tests/test_main.py b/cloudinit/cmd/tests/test_main.py
75new file mode 100644
76index 0000000..dbe421c
77--- /dev/null
78+++ b/cloudinit/cmd/tests/test_main.py
79@@ -0,0 +1,161 @@
80+# This file is part of cloud-init. See LICENSE file for license information.
81+
82+from collections import namedtuple
83+import copy
84+import os
85+from six import StringIO
86+
87+from cloudinit.cmd import main
88+from cloudinit.util import (
89+ ensure_dir, load_file, write_file, yaml_dumps)
90+from cloudinit.tests.helpers import (
91+ FilesystemMockingTestCase, wrap_and_call)
92+
93+mypaths = namedtuple('MyPaths', 'run_dir')
94+myargs = namedtuple('MyArgs', 'debug files force local reporter subcommand')
95+
96+
97+class TestMain(FilesystemMockingTestCase):
98+
99+ with_logs = True
100+
101+ def setUp(self):
102+ super(TestMain, self).setUp()
103+ self.new_root = self.tmp_dir()
104+ self.cloud_dir = self.tmp_path('var/lib/cloud/', dir=self.new_root)
105+ os.makedirs(self.cloud_dir)
106+ self.replicateTestRoot('simple_ubuntu', self.new_root)
107+ self.cfg = {
108+ 'datasource_list': ['None'],
109+ 'runcmd': ['ls /etc'], # test ALL_DISTROS
110+ 'system_info': {'paths': {'cloud_dir': self.cloud_dir,
111+ 'run_dir': self.new_root}},
112+ 'write_files': [
113+ {
114+ 'path': '/etc/blah.ini',
115+ 'content': 'blah',
116+ 'permissions': 0o755,
117+ },
118+ ],
119+ 'cloud_init_modules': ['write-files', 'runcmd'],
120+ }
121+ cloud_cfg = yaml_dumps(self.cfg)
122+ ensure_dir(os.path.join(self.new_root, 'etc', 'cloud'))
123+ self.cloud_cfg_file = os.path.join(
124+ self.new_root, 'etc', 'cloud', 'cloud.cfg')
125+ write_file(self.cloud_cfg_file, cloud_cfg)
126+ self.patchOS(self.new_root)
127+ self.patchUtils(self.new_root)
128+ self.stderr = StringIO()
129+ self.patchStdoutAndStderr(stderr=self.stderr)
130+
131+ def test_main_init_run_net_stops_on_file_no_net(self):
132+ """When no-net file is present, main_init does not process modules."""
133+ stop_file = os.path.join(self.cloud_dir, 'data', 'no-net') # stop file
134+ write_file(stop_file, '')
135+ cmdargs = myargs(
136+ debug=False, files=None, force=False, local=False, reporter=None,
137+ subcommand='init')
138+ (item1, item2) = wrap_and_call(
139+ 'cloudinit.cmd.main',
140+ {'util.close_stdin': True,
141+ 'netinfo.debug_info': 'my net debug info',
142+ 'util.fixup_output': ('outfmt', 'errfmt')},
143+ main.main_init, 'init', cmdargs)
144+ # We should not run write_files module
145+ self.assertFalse(
146+ os.path.exists(os.path.join(self.new_root, 'etc/blah.ini')),
147+ 'Unexpected run of write_files module produced blah.ini')
148+ self.assertEqual([], item2)
149+ # Instancify is called
150+ instance_id_path = 'var/lib/cloud/data/instance-id'
151+ self.assertFalse(
152+ os.path.exists(os.path.join(self.new_root, instance_id_path)),
153+ 'Unexpected call to datasource.instancify produced instance-id')
154+ expected_logs = [
155+ "Exiting. stop file ['{stop_file}'] existed\n".format(
156+ stop_file=stop_file),
157+ 'my net debug info' # netinfo.debug_info
158+ ]
159+ for log in expected_logs:
160+ self.assertIn(log, self.stderr.getvalue())
161+
162+ def test_main_init_run_net_runs_modules(self):
163+ """Modules like write_files are run in 'net' mode."""
164+ cmdargs = myargs(
165+ debug=False, files=None, force=False, local=False, reporter=None,
166+ subcommand='init')
167+ (item1, item2) = wrap_and_call(
168+ 'cloudinit.cmd.main',
169+ {'util.close_stdin': True,
170+ 'netinfo.debug_info': 'my net debug info',
171+ 'util.fixup_output': ('outfmt', 'errfmt')},
172+ main.main_init, 'init', cmdargs)
173+ self.assertEqual([], item2)
174+ # Instancify is called
175+ instance_id_path = 'var/lib/cloud/data/instance-id'
176+ self.assertEqual(
177+ 'iid-datasource-none\n',
178+ os.path.join(load_file(
179+ os.path.join(self.new_root, instance_id_path))))
180+ # modules are run (including write_files)
181+ self.assertEqual(
182+ 'blah', load_file(os.path.join(self.new_root, 'etc/blah.ini')))
183+ expected_logs = [
184+ 'network config is disabled by fallback', # apply_network_config
185+ 'my net debug info', # netinfo.debug_info
186+ 'no previous run detected'
187+ ]
188+ for log in expected_logs:
189+ self.assertIn(log, self.stderr.getvalue())
190+
191+ def test_main_init_run_net_calls_set_hostname_when_metadata_present(self):
192+ """When local-hostname metadata is present, call cc_set_hostname."""
193+ self.cfg['datasource'] = {
194+ 'None': {'metadata': {'local-hostname': 'md-hostname'}}}
195+ cloud_cfg = yaml_dumps(self.cfg)
196+ write_file(self.cloud_cfg_file, cloud_cfg)
197+ cmdargs = myargs(
198+ debug=False, files=None, force=False, local=False, reporter=None,
199+ subcommand='init')
200+
201+ def set_hostname(name, cfg, cloud, log, args):
202+ self.assertEqual('set-hostname', name)
203+ updated_cfg = copy.deepcopy(self.cfg)
204+ updated_cfg.update(
205+ {'def_log_file': '/var/log/cloud-init.log',
206+ 'log_cfgs': [],
207+ 'syslog_fix_perms': ['syslog:adm', 'root:adm', 'root:wheel'],
208+ 'vendor_data': {'enabled': True, 'prefix': []}})
209+ updated_cfg.pop('system_info')
210+
211+ self.assertEqual(updated_cfg, cfg)
212+ self.assertEqual(main.LOG, log)
213+ self.assertIsNone(args)
214+
215+ (item1, item2) = wrap_and_call(
216+ 'cloudinit.cmd.main',
217+ {'util.close_stdin': True,
218+ 'netinfo.debug_info': 'my net debug info',
219+ 'cc_set_hostname.handle': {'side_effect': set_hostname},
220+ 'util.fixup_output': ('outfmt', 'errfmt')},
221+ main.main_init, 'init', cmdargs)
222+ self.assertEqual([], item2)
223+ # Instancify is called
224+ instance_id_path = 'var/lib/cloud/data/instance-id'
225+ self.assertEqual(
226+ 'iid-datasource-none\n',
227+ os.path.join(load_file(
228+ os.path.join(self.new_root, instance_id_path))))
229+ # modules are run (including write_files)
230+ self.assertEqual(
231+ 'blah', load_file(os.path.join(self.new_root, 'etc/blah.ini')))
232+ expected_logs = [
233+ 'network config is disabled by fallback', # apply_network_config
234+ 'my net debug info', # netinfo.debug_info
235+ 'no previous run detected'
236+ ]
237+ for log in expected_logs:
238+ self.assertIn(log, self.stderr.getvalue())
239+
240+# vi: ts=4 expandtab
241diff --git a/cloudinit/config/cc_set_hostname.py b/cloudinit/config/cc_set_hostname.py
242index aa3dfe5..3d2b2da 100644
243--- a/cloudinit/config/cc_set_hostname.py
244+++ b/cloudinit/config/cc_set_hostname.py
245@@ -32,22 +32,51 @@ will be used.
246 hostname: <fqdn/hostname>
247 """
248
249+import os
250+
251+
252+from cloudinit.atomic_helper import write_json
253 from cloudinit import util
254
255
256+class SetHostnameError(Exception):
257+ """Raised when the distro runs into an exception when setting hostname.
258+
259+ This may happen if we attempt to set the hostname early in cloud-init's
260+ init-local timeframe as certain services may not be running yet.
261+ """
262+ pass
263+
264+
265 def handle(name, cfg, cloud, log, _args):
266 if util.get_cfg_option_bool(cfg, "preserve_hostname", False):
267 log.debug(("Configuration option 'preserve_hostname' is set,"
268 " not setting the hostname in module %s"), name)
269 return
270-
271 (hostname, fqdn) = util.get_hostname_fqdn(cfg, cloud)
272+ # Check for previous successful invocation of set-hostname
273+
274+ # set-hostname artifact file accounts for both hostname and fqdn
275+ # deltas. As such, it's format is different than cc_update_hostname's
276+ # previous-hostname file which only contains the base hostname.
277+ # TODO consolidate previous-hostname and set-hostname artifact files and
278+ # distro._read_hostname implementation so we only validate one artifact.
279+ prev_fn = os.path.join(cloud.get_cpath('data'), "set-hostname")
280+ prev_hostname = {}
281+ if os.path.exists(prev_fn):
282+ prev_hostname = util.load_json(util.load_file(prev_fn))
283+ hostname_changed = (hostname != prev_hostname.get('hostname') or
284+ fqdn != prev_hostname.get('fqdn'))
285+ if not hostname_changed:
286+ log.debug('No hostname changes. Skipping set-hostname')
287+ return
288+ log.debug("Setting the hostname to %s (%s)", fqdn, hostname)
289 try:
290- log.debug("Setting the hostname to %s (%s)", fqdn, hostname)
291 cloud.distro.set_hostname(hostname, fqdn)
292- except Exception:
293- util.logexc(log, "Failed to set the hostname to %s (%s)", fqdn,
294- hostname)
295- raise
296+ except Exception as e:
297+ msg = "Failed to set the hostname to %s (%s)" % (fqdn, hostname)
298+ util.logexc(log, msg)
299+ raise SetHostnameError("%s: %s" % (msg, e))
300+ write_json(prev_fn, {'hostname': hostname, 'fqdn': fqdn})
301
302 # vi: ts=4 expandtab
303diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
304index a05ca2f..df0b374 100644
305--- a/cloudinit/sources/__init__.py
306+++ b/cloudinit/sources/__init__.py
307@@ -276,21 +276,34 @@ class DataSource(object):
308 return "iid-datasource"
309 return str(self.metadata['instance-id'])
310
311- def get_hostname(self, fqdn=False, resolve_ip=False):
312+ def get_hostname(self, fqdn=False, resolve_ip=False, metadata_only=False):
313+ """Get hostname or fqdn from the datasource. Look it up if desired.
314+
315+ @param fqdn: Boolean, set True to return hostname with domain.
316+ @param resolve_ip: Boolean, set True to attempt to resolve an ipv4
317+ address provided in local-hostname meta-data.
318+ @param metadata_only: Boolean, set True to avoid looking up hostname
319+ if meta-data doesn't have local-hostname present.
320+
321+ @return: hostname or qualified hostname. Optionally return None when
322+ metadata_only is True and local-hostname data is not available.
323+ """
324 defdomain = "localdomain"
325 defhost = "localhost"
326 domain = defdomain
327
328 if not self.metadata or 'local-hostname' not in self.metadata:
329+ if metadata_only:
330+ return None
331 # this is somewhat questionable really.
332 # the cloud datasource was asked for a hostname
333 # and didn't have one. raising error might be more appropriate
334 # but instead, basically look up the existing hostname
335 toks = []
336 hostname = util.get_hostname()
337- fqdn = util.get_fqdn_from_hosts(hostname)
338- if fqdn and fqdn.find(".") > 0:
339- toks = str(fqdn).split(".")
340+ hosts_fqdn = util.get_fqdn_from_hosts(hostname)
341+ if hosts_fqdn and hosts_fqdn.find(".") > 0:
342+ toks = str(hosts_fqdn).split(".")
343 elif hostname and hostname.find(".") > 0:
344 toks = str(hostname).split(".")
345 elif hostname:
346diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py
347index af15115..5065083 100644
348--- a/cloudinit/sources/tests/test_init.py
349+++ b/cloudinit/sources/tests/test_init.py
350@@ -7,7 +7,7 @@ import stat
351 from cloudinit.helpers import Paths
352 from cloudinit.sources import (
353 INSTANCE_JSON_FILE, DataSource)
354-from cloudinit.tests.helpers import CiTestCase, skipIf
355+from cloudinit.tests.helpers import CiTestCase, skipIf, mock
356 from cloudinit.user_data import UserDataProcessor
357 from cloudinit import util
358
359@@ -108,6 +108,74 @@ class TestDataSource(CiTestCase):
360 self.assertEqual('userdata_raw', datasource.userdata_raw)
361 self.assertEqual('vendordata_raw', datasource.vendordata_raw)
362
363+ def test_get_hostname_strips_local_hostname_without_domain(self):
364+ """Datasource.get_hostname strips metadata local-hostname of domain."""
365+ tmp = self.tmp_dir()
366+ datasource = DataSourceTestSubclassNet(
367+ self.sys_cfg, self.distro, Paths({'run_dir': tmp}))
368+ self.assertTrue(datasource.get_data())
369+ self.assertEqual(
370+ 'test-subclass-hostname', datasource.metadata['local-hostname'])
371+ self.assertEqual('test-subclass-hostname', datasource.get_hostname())
372+ datasource.metadata['local-hostname'] = 'hostname.my.domain.com'
373+ self.assertEqual('hostname', datasource.get_hostname())
374+
375+ def test_get_hostname_with_fqdn_returns_local_hostname_with_domain(self):
376+ """Datasource.get_hostname with fqdn set gets qualified hostname."""
377+ tmp = self.tmp_dir()
378+ datasource = DataSourceTestSubclassNet(
379+ self.sys_cfg, self.distro, Paths({'run_dir': tmp}))
380+ self.assertTrue(datasource.get_data())
381+ datasource.metadata['local-hostname'] = 'hostname.my.domain.com'
382+ self.assertEqual(
383+ 'hostname.my.domain.com', datasource.get_hostname(fqdn=True))
384+
385+ def test_get_hostname_without_metadata_uses_system_hostname(self):
386+ """Datasource.gethostname runs util.get_hostname when no metadata."""
387+ tmp = self.tmp_dir()
388+ datasource = DataSourceTestSubclassNet(
389+ self.sys_cfg, self.distro, Paths({'run_dir': tmp}))
390+ self.assertEqual({}, datasource.metadata)
391+ mock_fqdn = 'cloudinit.sources.util.get_fqdn_from_hosts'
392+ with mock.patch('cloudinit.sources.util.get_hostname') as m_gethost:
393+ with mock.patch(mock_fqdn) as m_fqdn:
394+ m_gethost.return_value = 'systemhostname.domain.com'
395+ m_fqdn.return_value = None # No maching fqdn in /etc/hosts
396+ self.assertEqual('systemhostname', datasource.get_hostname())
397+ self.assertEqual(
398+ 'systemhostname.domain.com',
399+ datasource.get_hostname(fqdn=True))
400+
401+ def test_get_hostname_without_metadata_returns_none(self):
402+ """Datasource.gethostname returns None when metadata_only and no MD."""
403+ tmp = self.tmp_dir()
404+ datasource = DataSourceTestSubclassNet(
405+ self.sys_cfg, self.distro, Paths({'run_dir': tmp}))
406+ self.assertEqual({}, datasource.metadata)
407+ mock_fqdn = 'cloudinit.sources.util.get_fqdn_from_hosts'
408+ with mock.patch('cloudinit.sources.util.get_hostname') as m_gethost:
409+ with mock.patch(mock_fqdn) as m_fqdn:
410+ self.assertIsNone(datasource.get_hostname(metadata_only=True))
411+ self.assertIsNone(
412+ datasource.get_hostname(fqdn=True, metadata_only=True))
413+ self.assertEqual([], m_gethost.call_args_list)
414+ self.assertEqual([], m_fqdn.call_args_list)
415+
416+ def test_get_hostname_without_metadata_prefers_etc_hosts(self):
417+ """Datasource.gethostname prefers /etc/hosts to util.get_hostname."""
418+ tmp = self.tmp_dir()
419+ datasource = DataSourceTestSubclassNet(
420+ self.sys_cfg, self.distro, Paths({'run_dir': tmp}))
421+ self.assertEqual({}, datasource.metadata)
422+ mock_fqdn = 'cloudinit.sources.util.get_fqdn_from_hosts'
423+ with mock.patch('cloudinit.sources.util.get_hostname') as m_gethost:
424+ with mock.patch(mock_fqdn) as m_fqdn:
425+ m_gethost.return_value = 'systemhostname.domain.com'
426+ m_fqdn.return_value = 'fqdnhostname.domain.com'
427+ self.assertEqual('fqdnhostname', datasource.get_hostname())
428+ self.assertEqual('fqdnhostname.domain.com',
429+ datasource.get_hostname(fqdn=True))
430+
431 def test_get_data_write_json_instance_data(self):
432 """get_data writes INSTANCE_JSON_FILE to run_dir as readonly root."""
433 tmp = self.tmp_dir()
434diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py
435index c3e2e40..d30643d 100644
436--- a/cloudinit/tests/test_util.py
437+++ b/cloudinit/tests/test_util.py
438@@ -16,6 +16,25 @@ MOUNT_INFO = [
439 ]
440
441
442+class FakeCloud(object):
443+
444+ def __init__(self, hostname, fqdn):
445+ self.hostname = hostname
446+ self.fqdn = fqdn
447+ self.calls = []
448+
449+ def get_hostname(self, fqdn=None, metadata_only=None):
450+ myargs = {}
451+ if fqdn is not None:
452+ myargs['fqdn'] = fqdn
453+ if metadata_only is not None:
454+ myargs['metadata_only'] = metadata_only
455+ self.calls.append(myargs)
456+ if fqdn:
457+ return self.fqdn
458+ return self.hostname
459+
460+
461 class TestUtil(CiTestCase):
462
463 def test_parse_mount_info_no_opts_no_arg(self):
464@@ -67,3 +86,58 @@ class TestShellify(CiTestCase):
465 "'echo' 'hi' 'sis'", ""]),
466 util.shellify(["echo hi mom", ["echo", "hi dad"],
467 ('echo', 'hi', 'sis')]))
468+
469+
470+class TestGetHostnameFqdn(CiTestCase):
471+
472+ def test_get_hostname_fqdn_from_only_cfg_fqdn(self):
473+ """When cfg only has the fqdn key, derive hostname and fqdn from it."""
474+ hostname, fqdn = util.get_hostname_fqdn(
475+ cfg={'fqdn': 'myhost.domain.com'}, cloud=None)
476+ self.assertEqual('myhost', hostname)
477+ self.assertEqual('myhost.domain.com', fqdn)
478+
479+ def test_get_hostname_fqdn_from_cfg_fqdn_and_hostname(self):
480+ """When cfg has both fqdn and hostname keys, return them."""
481+ hostname, fqdn = util.get_hostname_fqdn(
482+ cfg={'fqdn': 'myhost.domain.com', 'hostname': 'other'}, cloud=None)
483+ self.assertEqual('other', hostname)
484+ self.assertEqual('myhost.domain.com', fqdn)
485+
486+ def test_get_hostname_fqdn_from_cfg_hostname_with_domain(self):
487+ """When cfg has only hostname key which represents a fqdn, use that."""
488+ hostname, fqdn = util.get_hostname_fqdn(
489+ cfg={'hostname': 'myhost.domain.com'}, cloud=None)
490+ self.assertEqual('myhost', hostname)
491+ self.assertEqual('myhost.domain.com', fqdn)
492+
493+ def test_get_hostname_fqdn_from_cfg_hostname_without_domain(self):
494+ """When cfg has a hostname without a '.' query cloud.get_hostname."""
495+ mycloud = FakeCloud('cloudhost', 'cloudhost.mycloud.com')
496+ hostname, fqdn = util.get_hostname_fqdn(
497+ cfg={'hostname': 'myhost'}, cloud=mycloud)
498+ self.assertEqual('myhost', hostname)
499+ self.assertEqual('cloudhost.mycloud.com', fqdn)
500+ self.assertEqual(
501+ [{'fqdn': True, 'metadata_only': False}], mycloud.calls)
502+
503+ def test_get_hostname_fqdn_from_without_fqdn_or_hostname(self):
504+ """When cfg has neither hostname nor fqdn cloud.get_hostname."""
505+ mycloud = FakeCloud('cloudhost', 'cloudhost.mycloud.com')
506+ hostname, fqdn = util.get_hostname_fqdn(cfg={}, cloud=mycloud)
507+ self.assertEqual('cloudhost', hostname)
508+ self.assertEqual('cloudhost.mycloud.com', fqdn)
509+ self.assertEqual(
510+ [{'fqdn': True, 'metadata_only': False},
511+ {'metadata_only': False}], mycloud.calls)
512+
513+ def test_get_hostname_fqdn_from_passes_metadata_only_to_cloud(self):
514+ """Calls to cloud.get_hostname pass the metadata_only parameter."""
515+ mycloud = FakeCloud('cloudhost', 'cloudhost.mycloud.com')
516+ hostname, fqdn = util.get_hostname_fqdn(
517+ cfg={}, cloud=mycloud, metadata_only=True)
518+ self.assertEqual(
519+ [{'fqdn': True, 'metadata_only': True},
520+ {'metadata_only': True}], mycloud.calls)
521+
522+# vi: ts=4 expandtab
523diff --git a/cloudinit/util.py b/cloudinit/util.py
524index 083a8ef..4504f05 100644
525--- a/cloudinit/util.py
526+++ b/cloudinit/util.py
527@@ -1025,9 +1025,16 @@ def dos2unix(contents):
528 return contents.replace('\r\n', '\n')
529
530
531-def get_hostname_fqdn(cfg, cloud):
532- # return the hostname and fqdn from 'cfg'. If not found in cfg,
533- # then fall back to data from cloud
534+def get_hostname_fqdn(cfg, cloud, metadata_only=False):
535+ """Get hostname and fqdn from config if present and fallback to cloud.
536+
537+ @param cfg: Dictionary of merged user-data configuration (from init.cfg).
538+ @param cloud: Cloud instance from init.cloudify().
539+ @param metadata_only: Boolean, set True to only query cloud meta-data,
540+ returning None if not present in meta-data.
541+ @return: a Tuple of strings <hostname>, <fqdn>. Values can be none when
542+ metadata_only is True and no cfg or metadata provides hostname info.
543+ """
544 if "fqdn" in cfg:
545 # user specified a fqdn. Default hostname then is based off that
546 fqdn = cfg['fqdn']
547@@ -1041,11 +1048,11 @@ def get_hostname_fqdn(cfg, cloud):
548 else:
549 # no fqdn set, get fqdn from cloud.
550 # get hostname from cfg if available otherwise cloud
551- fqdn = cloud.get_hostname(fqdn=True)
552+ fqdn = cloud.get_hostname(fqdn=True, metadata_only=metadata_only)
553 if "hostname" in cfg:
554 hostname = cfg['hostname']
555 else:
556- hostname = cloud.get_hostname()
557+ hostname = cloud.get_hostname(metadata_only=metadata_only)
558 return (hostname, fqdn)
559
560
561diff --git a/tests/unittests/test_handler/test_handler_set_hostname.py b/tests/unittests/test_handler/test_handler_set_hostname.py
562index abdc17e..d09ec23 100644
563--- a/tests/unittests/test_handler/test_handler_set_hostname.py
564+++ b/tests/unittests/test_handler/test_handler_set_hostname.py
565@@ -11,6 +11,7 @@ from cloudinit.tests import helpers as t_help
566
567 from configobj import ConfigObj
568 import logging
569+import os
570 import shutil
571 from six import BytesIO
572 import tempfile
573@@ -19,14 +20,18 @@ LOG = logging.getLogger(__name__)
574
575
576 class TestHostname(t_help.FilesystemMockingTestCase):
577+
578+ with_logs = True
579+
580 def setUp(self):
581 super(TestHostname, self).setUp()
582 self.tmp = tempfile.mkdtemp()
583+ util.ensure_dir(os.path.join(self.tmp, 'data'))
584 self.addCleanup(shutil.rmtree, self.tmp)
585
586 def _fetch_distro(self, kind):
587 cls = distros.fetch(kind)
588- paths = helpers.Paths({})
589+ paths = helpers.Paths({'cloud_dir': self.tmp})
590 return cls(kind, {}, paths)
591
592 def test_write_hostname_rhel(self):
593@@ -34,7 +39,7 @@ class TestHostname(t_help.FilesystemMockingTestCase):
594 'hostname': 'blah.blah.blah.yahoo.com',
595 }
596 distro = self._fetch_distro('rhel')
597- paths = helpers.Paths({})
598+ paths = helpers.Paths({'cloud_dir': self.tmp})
599 ds = None
600 cc = cloud.Cloud(ds, paths, {}, distro, None)
601 self.patchUtils(self.tmp)
602@@ -51,7 +56,7 @@ class TestHostname(t_help.FilesystemMockingTestCase):
603 'hostname': 'blah.blah.blah.yahoo.com',
604 }
605 distro = self._fetch_distro('debian')
606- paths = helpers.Paths({})
607+ paths = helpers.Paths({'cloud_dir': self.tmp})
608 ds = None
609 cc = cloud.Cloud(ds, paths, {}, distro, None)
610 self.patchUtils(self.tmp)
611@@ -65,7 +70,7 @@ class TestHostname(t_help.FilesystemMockingTestCase):
612 'hostname': 'blah.blah.blah.suse.com',
613 }
614 distro = self._fetch_distro('sles')
615- paths = helpers.Paths({})
616+ paths = helpers.Paths({'cloud_dir': self.tmp})
617 ds = None
618 cc = cloud.Cloud(ds, paths, {}, distro, None)
619 self.patchUtils(self.tmp)
620@@ -74,4 +79,48 @@ class TestHostname(t_help.FilesystemMockingTestCase):
621 contents = util.load_file(distro.hostname_conf_fn)
622 self.assertEqual('blah', contents.strip())
623
624+ def test_multiple_calls_skips_unchanged_hostname(self):
625+ """Only new hostname or fqdn values will generate a hostname call."""
626+ distro = self._fetch_distro('debian')
627+ paths = helpers.Paths({'cloud_dir': self.tmp})
628+ ds = None
629+ cc = cloud.Cloud(ds, paths, {}, distro, None)
630+ self.patchUtils(self.tmp)
631+ cc_set_hostname.handle(
632+ 'cc_set_hostname', {'hostname': 'hostname1.me.com'}, cc, LOG, [])
633+ contents = util.load_file("/etc/hostname")
634+ self.assertEqual('hostname1', contents.strip())
635+ cc_set_hostname.handle(
636+ 'cc_set_hostname', {'hostname': 'hostname1.me.com'}, cc, LOG, [])
637+ self.assertIn(
638+ 'DEBUG: No hostname changes. Skipping set-hostname\n',
639+ self.logs.getvalue())
640+ cc_set_hostname.handle(
641+ 'cc_set_hostname', {'hostname': 'hostname2.me.com'}, cc, LOG, [])
642+ contents = util.load_file("/etc/hostname")
643+ self.assertEqual('hostname2', contents.strip())
644+ self.assertIn(
645+ 'Non-persistently setting the system hostname to hostname2',
646+ self.logs.getvalue())
647+
648+ def test_error_on_distro_set_hostname_errors(self):
649+ """Raise SetHostnameError on exceptions from distro.set_hostname."""
650+ distro = self._fetch_distro('debian')
651+
652+ def set_hostname_error(hostname, fqdn):
653+ raise Exception("OOPS on: %s" % fqdn)
654+
655+ distro.set_hostname = set_hostname_error
656+ paths = helpers.Paths({'cloud_dir': self.tmp})
657+ ds = None
658+ cc = cloud.Cloud(ds, paths, {}, distro, None)
659+ self.patchUtils(self.tmp)
660+ with self.assertRaises(cc_set_hostname.SetHostnameError) as ctx_mgr:
661+ cc_set_hostname.handle(
662+ 'somename', {'hostname': 'hostname1.me.com'}, cc, LOG, [])
663+ self.assertEqual(
664+ 'Failed to set the hostname to hostname1.me.com (hostname1):'
665+ ' OOPS on: hostname1.me.com',
666+ str(ctx_mgr.exception))
667+
668 # vi: ts=4 expandtab

Subscribers

People subscribed via source and target branches