Merge ~darkmuggle-deactivatedaccount/cloud-init:lp-1676908-resolver_workaround into cloud-init:master

Proposed by Ben Howard
Status: Merged
Approved by: Scott Moser
Approved revision: 9bf82bec81455037235c7064d5b63378b32ded3b
Merged at revision: 493f6c3e923902d5d4f3d87e1cc4c726ea90ada4
Proposed branch: ~darkmuggle-deactivatedaccount/cloud-init:lp-1676908-resolver_workaround
Merge into: cloud-init:master
Diff against target: 135 lines (+34/-30)
2 files modified
cloudinit/sources/helpers/digitalocean.py (+7/-11)
tests/unittests/test_datasource/test_digitalocean.py (+27/-19)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+322379@code.launchpad.net

Description of the change

DigitalOcean update to bind the resovlers to the loopback interface. This is a workaround to the resolvconf bug. However, this change brings the DigitalOcean DS in line with OpenStack and the SmartOS DS in the configuration of resolvers.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

drop the reference to 1676908 in the commit message, as it will just mean that is "fixed". You can put "this is part of bug XXXXXX" to avoid that.

Please fix the debug message as suggested.

(if a commit message has 'LP: #' at the beginning of the line, its treated like 'bzr --fixes='.
and since this is one of three that get there, thats kind of odd.

I suggest for commit message:

DigitalOcean: attach dns information to loopback interface

This change makes the DigitalOcean datasource consistent with OpenStack and
Joyent by binding the resolver addresses to the loopback interface. This _is_
a work-around to LP: #1675571.

Part of bug 1676908.

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

given the updated commit message, i've updated the log message and will just push.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/sources/helpers/digitalocean.py b/cloudinit/sources/helpers/digitalocean.py
2index 72f7bde..ca290cf 100644
3--- a/cloudinit/sources/helpers/digitalocean.py
4+++ b/cloudinit/sources/helpers/digitalocean.py
5@@ -107,15 +107,12 @@ def convert_network_configuration(config, dns_servers):
6 }
7 """
8
9- def _get_subnet_part(pcfg, nameservers=None):
10+ def _get_subnet_part(pcfg):
11 subpart = {'type': 'static',
12 'control': 'auto',
13 'address': pcfg.get('ip_address'),
14 'gateway': pcfg.get('gateway')}
15
16- if nameservers:
17- subpart['dns_nameservers'] = nameservers
18-
19 if ":" in pcfg.get('ip_address'):
20 subpart['address'] = "{0}/{1}".format(pcfg.get('ip_address'),
21 pcfg.get('cidr'))
22@@ -157,13 +154,8 @@ def convert_network_configuration(config, dns_servers):
23 continue
24
25 sub_part = _get_subnet_part(raw_subnet)
26- if nic_type == 'public' and 'anchor' not in netdef:
27- # add DNS resolvers to the public interfaces only
28- sub_part = _get_subnet_part(raw_subnet, dns_servers)
29- else:
30- # remove the gateway any non-public interfaces
31- if 'gateway' in sub_part:
32- del sub_part['gateway']
33+ if netdef in ('private', 'anchor_ipv4', 'anchor_ipv6'):
34+ del sub_part['gateway']
35
36 subnets.append(sub_part)
37
38@@ -171,6 +163,10 @@ def convert_network_configuration(config, dns_servers):
39 nic_configs.append(ncfg)
40 LOG.debug("nic '%s' configuration: %s", if_name, ncfg)
41
42+ if dns_servers:
43+ LOG.debug("added dns servers")
44+ nic_configs.append({'type': 'nameserver', 'address': dns_servers})
45+
46 return {'version': 1, 'config': nic_configs}
47
48
49diff --git a/tests/unittests/test_datasource/test_digitalocean.py b/tests/unittests/test_datasource/test_digitalocean.py
50index 61d6e00..a11166a 100644
51--- a/tests/unittests/test_datasource/test_digitalocean.py
52+++ b/tests/unittests/test_datasource/test_digitalocean.py
53@@ -197,7 +197,8 @@ class TestNetworkConvert(TestCase):
54 @mock.patch('cloudinit.net.get_interfaces_by_mac')
55 def _get_networking(self, m_get_by_mac):
56 m_get_by_mac.return_value = {
57- '04:01:57:d1:9e:01': 'ens1', '04:01:57:d1:9e:02': 'ens2',
58+ '04:01:57:d1:9e:01': 'ens1',
59+ '04:01:57:d1:9e:02': 'ens2',
60 'b8:ae:ed:75:5f:9a': 'enp0s25',
61 'ae:cc:08:7c:88:00': 'meta2p1'}
62 netcfg = digitalocean.convert_network_configuration(
63@@ -208,18 +209,33 @@ class TestNetworkConvert(TestCase):
64 def test_networking_defined(self):
65 netcfg = self._get_networking()
66 self.assertIsNotNone(netcfg)
67+ dns_defined = False
68
69- for nic_def in netcfg.get('config'):
70- print(json.dumps(nic_def, indent=3))
71- n_type = nic_def.get('type')
72- n_subnets = nic_def.get('type')
73- n_name = nic_def.get('name')
74- n_mac = nic_def.get('mac_address')
75+ for part in netcfg.get('config'):
76+ n_type = part.get('type')
77+ print("testing part ", n_type, "\n", json.dumps(part, indent=3))
78+
79+ if n_type == 'nameserver':
80+ n_address = part.get('address')
81+ self.assertIsNotNone(n_address)
82+ self.assertEqual(len(n_address), 3)
83+
84+ dns_resolvers = DO_META["dns"]["nameservers"]
85+ for x in n_address:
86+ self.assertIn(x, dns_resolvers)
87+ dns_defined = True
88
89- self.assertIsNotNone(n_type)
90- self.assertIsNotNone(n_subnets)
91- self.assertIsNotNone(n_name)
92- self.assertIsNotNone(n_mac)
93+ else:
94+ n_subnets = part.get('type')
95+ n_name = part.get('name')
96+ n_mac = part.get('mac_address')
97+
98+ self.assertIsNotNone(n_type)
99+ self.assertIsNotNone(n_subnets)
100+ self.assertIsNotNone(n_name)
101+ self.assertIsNotNone(n_mac)
102+
103+ self.assertTrue(dns_defined)
104
105 def _get_nic_definition(self, int_type, expected_name):
106 """helper function to return if_type (i.e. public) and the expected
107@@ -260,12 +276,6 @@ class TestNetworkConvert(TestCase):
108 self.assertEqual(meta_def.get('mac'), nic_def.get('mac_address'))
109 self.assertEqual('physical', nic_def.get('type'))
110
111- def _check_dns_nameservers(self, subn_def):
112- self.assertIn('dns_nameservers', subn_def)
113- expected_nameservers = DO_META['dns']['nameservers']
114- nic_nameservers = subn_def.get('dns_nameservers')
115- self.assertEqual(expected_nameservers, nic_nameservers)
116-
117 def test_public_interface_ipv6(self):
118 """test public ipv6 addressing"""
119 (nic_def, meta_def) = self._get_nic_definition('public', 'eth0')
120@@ -280,7 +290,6 @@ class TestNetworkConvert(TestCase):
121
122 self.assertEqual(cidr_notated_address, subn_def.get('address'))
123 self.assertEqual(ipv6_def.get('gateway'), subn_def.get('gateway'))
124- self._check_dns_nameservers(subn_def)
125
126 def test_public_interface_ipv4(self):
127 """test public ipv4 addressing"""
128@@ -293,7 +302,6 @@ class TestNetworkConvert(TestCase):
129
130 self.assertEqual(ipv4_def.get('netmask'), subn_def.get('netmask'))
131 self.assertEqual(ipv4_def.get('gateway'), subn_def.get('gateway'))
132- self._check_dns_nameservers(subn_def)
133
134 def test_public_interface_anchor_ipv4(self):
135 """test public ipv4 addressing"""

Subscribers

People subscribed via source and target branches