cloud-init

Merge ~raharper/cloud-init:fix/netplan-subnet-route-metric-lp1805871 into cloud-init:master

  1. Git
  2. lp:~raharper/cloud-init
  3. fix/netplan-subnet-route-metric-lp1805871
  4. Merge into master
Proposed by Ryan Harper
Status: Merged
Approved by: Chad Smith
Approved revision: 275ba70ee70dc429a72195245939e95b11d5f299
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~raharper/cloud-init:fix/netplan-subnet-route-metric-lp1805871
Merge into: cloud-init:master
Diff against target: 255 lines (+83/-27)
4 files modified
cloudinit/net/eni.py (+15/-14)
cloudinit/net/netplan.py (+3/-3)
cloudinit/net/sysconfig.py (+21/-4)
tests/unittests/test_net.py (+44/-6)
Reviewer Review Type Date Requested Status
Chad Smith Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+359876@code.launchpad.net

Commit message

net: render 'metric' values in per-subnet routes

It is possible to have a metric value in a per-subnet route.
This is currently missing in all renderers. Update each
renderer to emit the correct metric value from the config.

LP: #1805871

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:ffcf46ba146b1e5c0ec02a731938db35106fd804
https://jenkins.ubuntu.com/server/job/cloud-init-ci/473/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

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

PASSED: Continuous integration, rev:54d0044222a20b6855c9290bc7e54ddd893e517a
https://jenkins.ubuntu.com/server/job/cloud-init-ci/475/
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/475/rebuild

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

A couple of questions/comments. but looks pretty good.

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

Thanks for review, will fix those up.

~raharper/cloud-init:fix/netplan-subnet-route-metric-lp1805871 updated
cfdbb6a... by Ryan Harper

eni: simplify generating metric string if present in config

ca25952... by Ryan Harper

eni: simplify construction of the route string

275ba70... by Ryan Harper

net/sysconfig: fix routes with metric handling, add ipv6 scenarios, fix unittest

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

PASSED: Continuous integration, rev:275ba70ee70dc429a72195245939e95b11d5f299
https://jenkins.ubuntu.com/server/job/cloud-init-ci/482/
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/482/rebuild

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

This is good, thanks for the fixups here. LGTM!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py
2index c6f631a..6423632 100644
3--- a/cloudinit/net/eni.py
4+++ b/cloudinit/net/eni.py
5@@ -371,22 +371,23 @@ class Renderer(renderer.Renderer):
6 'gateway': 'gw',
7 'metric': 'metric',
8 }
9+
10+ default_gw = ''
11 if route['network'] == '0.0.0.0' and route['netmask'] == '0.0.0.0':
12- default_gw = " default gw %s" % route['gateway']
13- content.append(up + default_gw + or_true)
14- content.append(down + default_gw + or_true)
15+ default_gw = ' default'
16 elif route['network'] == '::' and route['prefix'] == 0:
17- # ipv6!
18- default_gw = " -A inet6 default gw %s" % route['gateway']
19- content.append(up + default_gw + or_true)
20- content.append(down + default_gw + or_true)
21- else:
22- route_line = ""
23- for k in ['network', 'netmask', 'gateway', 'metric']:
24- if k in route:
25- route_line += " %s %s" % (mapping[k], route[k])
26- content.append(up + route_line + or_true)
27- content.append(down + route_line + or_true)
28+ default_gw = ' -A inet6 default'
29+
30+ route_line = ''
31+ for k in ['network', 'netmask', 'gateway', 'metric']:
32+ if default_gw and k in ['network', 'netmask']:
33+ continue
34+ if k == 'gateway':
35+ route_line += '%s %s %s' % (default_gw, mapping[k], route[k])
36+ elif k in route:
37+ route_line += ' %s %s' % (mapping[k], route[k])
38+ content.append(up + route_line + or_true)
39+ content.append(down + route_line + or_true)
40 return content
41
42 def _render_iface(self, iface, render_hwaddress=False):
43diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
44index bc1087f..21517fd 100644
45--- a/cloudinit/net/netplan.py
46+++ b/cloudinit/net/netplan.py
47@@ -114,13 +114,13 @@ def _extract_addresses(config, entry, ifname):
48 for route in subnet.get('routes', []):
49 to_net = "%s/%s" % (route.get('network'),
50 route.get('prefix'))
51- route = {
52+ new_route = {
53 'via': route.get('gateway'),
54 'to': to_net,
55 }
56 if 'metric' in route:
57- route.update({'metric': route.get('metric', 100)})
58- routes.append(route)
59+ new_route.update({'metric': route.get('metric', 100)})
60+ routes.append(new_route)
61
62 addresses.append(addr)
63
64diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
65index 9c16d3a..17293e1 100644
66--- a/cloudinit/net/sysconfig.py
67+++ b/cloudinit/net/sysconfig.py
68@@ -156,13 +156,23 @@ class Route(ConfigMap):
69 _quote_value(gateway_value)))
70 buf.write("%s=%s\n" % ('NETMASK' + str(reindex),
71 _quote_value(netmask_value)))
72+ metric_key = 'METRIC' + index
73+ if metric_key in self._conf:
74+ metric_value = str(self._conf['METRIC' + index])
75+ buf.write("%s=%s\n" % ('METRIC' + str(reindex),
76+ _quote_value(metric_value)))
77 elif proto == "ipv6" and self.is_ipv6_route(address_value):
78 netmask_value = str(self._conf['NETMASK' + index])
79 gateway_value = str(self._conf['GATEWAY' + index])
80- buf.write("%s/%s via %s dev %s\n" % (address_value,
81- netmask_value,
82- gateway_value,
83- self._route_name))
84+ metric_value = (
85+ 'metric ' + str(self._conf['METRIC' + index])
86+ if 'METRIC' + index in self._conf else '')
87+ buf.write(
88+ "%s/%s via %s %s dev %s\n" % (address_value,
89+ netmask_value,
90+ gateway_value,
91+ metric_value,
92+ self._route_name))
93
94 return buf.getvalue()
95
96@@ -370,6 +380,9 @@ class Renderer(renderer.Renderer):
97 else:
98 iface_cfg['GATEWAY'] = subnet['gateway']
99
100+ if 'metric' in subnet:
101+ iface_cfg['METRIC'] = subnet['metric']
102+
103 if 'dns_search' in subnet:
104 iface_cfg['DOMAIN'] = ' '.join(subnet['dns_search'])
105
106@@ -414,15 +427,19 @@ class Renderer(renderer.Renderer):
107 else:
108 iface_cfg['GATEWAY'] = route['gateway']
109 route_cfg.has_set_default_ipv4 = True
110+ if 'metric' in route:
111+ iface_cfg['METRIC'] = route['metric']
112
113 else:
114 gw_key = 'GATEWAY%s' % route_cfg.last_idx
115 nm_key = 'NETMASK%s' % route_cfg.last_idx
116 addr_key = 'ADDRESS%s' % route_cfg.last_idx
117+ metric_key = 'METRIC%s' % route_cfg.last_idx
118 route_cfg.last_idx += 1
119 # add default routes only to ifcfg files, not
120 # to route-* or route6-*
121 for (old_key, new_key) in [('gateway', gw_key),
122+ ('metric', metric_key),
123 ('netmask', nm_key),
124 ('network', addr_key)]:
125 if old_key in route:
126diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
127index 8e38373..195f261 100644
128--- a/tests/unittests/test_net.py
129+++ b/tests/unittests/test_net.py
130@@ -488,8 +488,8 @@ NETWORK_CONFIGS = {
131 address 192.168.21.3/24
132 dns-nameservers 8.8.8.8 8.8.4.4
133 dns-search barley.maas sach.maas
134- post-up route add default gw 65.61.151.37 || true
135- pre-down route del default gw 65.61.151.37 || true
136+ post-up route add default gw 65.61.151.37 metric 10000 || true
137+ pre-down route del default gw 65.61.151.37 metric 10000 || true
138 """).rstrip(' '),
139 'expected_netplan': textwrap.dedent("""
140 network:
141@@ -513,7 +513,8 @@ NETWORK_CONFIGS = {
142 - barley.maas
143 - sach.maas
144 routes:
145- - to: 0.0.0.0/0
146+ - metric: 10000
147+ to: 0.0.0.0/0
148 via: 65.61.151.37
149 set-name: eth99
150 """).rstrip(' '),
151@@ -537,6 +538,7 @@ NETWORK_CONFIGS = {
152 HWADDR=c0:d6:9f:2c:e8:80
153 IPADDR=192.168.21.3
154 NETMASK=255.255.255.0
155+ METRIC=10000
156 NM_CONTROLLED=no
157 ONBOOT=yes
158 TYPE=Ethernet
159@@ -561,7 +563,7 @@ NETWORK_CONFIGS = {
160 - gateway: 65.61.151.37
161 netmask: 0.0.0.0
162 network: 0.0.0.0
163- metric: 2
164+ metric: 10000
165 - type: physical
166 name: eth1
167 mac_address: "cf:d6:af:48:e8:80"
168@@ -1161,6 +1163,13 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
169 - gateway: 192.168.0.3
170 netmask: 255.255.255.0
171 network: 10.1.3.0
172+ - gateway: 2001:67c:1562:1
173+ network: 2001:67c:1
174+ netmask: ffff:ffff:0
175+ - gateway: 3001:67c:1562:1
176+ network: 3001:67c:1
177+ netmask: ffff:ffff:0
178+ metric: 10000
179 - type: static
180 address: 192.168.1.2/24
181 - type: static
182@@ -1197,6 +1206,11 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
183 routes:
184 - to: 10.1.3.0/24
185 via: 192.168.0.3
186+ - to: 2001:67c:1/32
187+ via: 2001:67c:1562:1
188+ - metric: 10000
189+ to: 3001:67c:1/32
190+ via: 3001:67c:1562:1
191 """),
192 'yaml-v2': textwrap.dedent("""
193 version: 2
194@@ -1228,6 +1242,11 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
195 routes:
196 - to: 10.1.3.0/24
197 via: 192.168.0.3
198+ - to: 2001:67c:1562:8007::1/64
199+ via: 2001:67c:1562:8007::aac:40b2
200+ - metric: 10000
201+ to: 3001:67c:1562:8007::1/64
202+ via: 3001:67c:1562:8007::aac:40b2
203 """),
204 'expected_netplan-v2': textwrap.dedent("""
205 network:
206@@ -1249,6 +1268,11 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
207 routes:
208 - to: 10.1.3.0/24
209 via: 192.168.0.3
210+ - to: 2001:67c:1562:8007::1/64
211+ via: 2001:67c:1562:8007::aac:40b2
212+ - metric: 10000
213+ to: 3001:67c:1562:8007::1/64
214+ via: 3001:67c:1562:8007::aac:40b2
215 ethernets:
216 eth0:
217 match:
218@@ -1349,6 +1373,10 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
219 USERCTL=no
220 """),
221 'route6-bond0': textwrap.dedent("""\
222+ # Created by cloud-init on instance boot automatically, do not edit.
223+ #
224+ 2001:67c:1/ffff:ffff:0 via 2001:67c:1562:1 dev bond0
225+ 3001:67c:1/ffff:ffff:0 via 3001:67c:1562:1 metric 10000 dev bond0
226 """),
227 'route-bond0': textwrap.dedent("""\
228 ADDRESS0=10.1.3.0
229@@ -1879,14 +1907,24 @@ class TestRhelSysConfigRendering(CiTestCase):
230 return dir2dict(dir)
231
232 def _compare_files_to_expected(self, expected, found):
233+
234+ def _try_load(f):
235+ ''' Attempt to load shell content, otherwise return as-is '''
236+ try:
237+ return util.load_shell_content(f)
238+ except ValueError:
239+ pass
240+ # route6- * files aren't shell content, but iproute2 params
241+ return f
242+
243 orig_maxdiff = self.maxDiff
244 expected_d = dict(
245- (os.path.join(self.scripts_dir, k), util.load_shell_content(v))
246+ (os.path.join(self.scripts_dir, k), _try_load(v))
247 for k, v in expected.items())
248
249 # only compare the files in scripts_dir
250 scripts_found = dict(
251- (k, util.load_shell_content(v)) for k, v in found.items()
252+ (k, _try_load(v)) for k, v in found.items()
253 if k.startswith(self.scripts_dir))
254 try:
255 self.maxDiff = None

Subscribers

People subscribed via source and target branches