cloud-init

Merge ~xnox/cloud-init:cidr into cloud-init:master

  1. Git
  2. lp:~xnox/cloud-init
  3. cidr
  4. Merge into master
Proposed by Dimitri John Ledkov
Status: Merged
Merged at revision: 16a7302f6acb69adb0aee75eaf12392fa3688853
Proposed branch: ~xnox/cloud-init:cidr
Merge into: cloud-init:master
Diff against target: 81 lines (+12/-10)
4 files modified
cloudinit/net/netplan.py (+8/-6)
cloudinit/net/network_state.py (+2/-2)
tests/unittests/test_distros/test_netconfig.py (+1/-1)
tests/unittests/test_net.py (+1/-1)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Ryan Harper Needs Fixing
Review via email: mp+324010@code.launchpad.net

Commit message

netplan: generate cidr routes and netmasks.

LP: #1689346

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:678db0c7f5f031725ba72625504e50c5c8f55936
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~xnox/cloud-init/+git/cloud-init/+merge/324010/+edit-commit-message

https://jenkins.ubuntu.com/server/job/cloud-init-ci/330/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/330
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/330
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/330
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/330

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

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

In both cases, use of mask2cidr when not a dotted quad will return the original string sent, but that will end up embedded strings instead of integers:

>>> network_state.mask2cidr('24')
'24'
>>> network_state.mask2cidr('255.255.255.0')
24

I think we either need to modify mask2cidr to do something else (convert to integer) so it it's consistent across input invocations, or if we don't change mask2cidr, then the callers need to handle all of the following

address = 192.168.1.2/24 , netmask: None
address = 192.168.1.2/255.255.255.0 , netmask: None
address = 192.168.1.2 , netmask: 24
address = 192.168.1.2 , netmask: 255.255.255.0

https://code.launchpad.net/~raharper/curtin/trunk.vmtest-reuse-output/+merge/318150

review: Needs Fixing
~xnox/cloud-init:cidr updated
9e5cdbb... by Dimitri John Ledkov

nplan: correctly generate ipv6 and ipv4 netmask addresses

Insure that only cidr used when outputing netmasks in netplan.

LP: #1689346
LP: #1691100

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

PASSED: Continuous integration, rev:bd2954bc992fcd10fdcecaf79cbaecea8ec6a735
https://jenkins.ubuntu.com/server/job/cloud-init-ci/337/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/337
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/337
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/337
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/337

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

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

PASSED: Continuous integration, rev:bf84def335f088fc71d5cb6245b3eb886584efac
https://jenkins.ubuntu.com/server/job/cloud-init-ci/358/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/358
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/358
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/358
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/358

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

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

FAILED: Continuous integration, rev:9e5cdbbd2d49bd3034075a355bb0632f37b0a1ce
https://jenkins.ubuntu.com/server/job/cloud-init-ci/359/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/359
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/359
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/359
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/359/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Any idea why test_handler_apt_source_v3.py is failing on the i386?

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

PASSED: Continuous integration, rev:9e5cdbbd2d49bd3034075a355bb0632f37b0a1ce
https://jenkins.ubuntu.com/server/job/cloud-init-ci/366/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/366
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/366
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/366
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/366
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/366

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

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

I just re-built this at
 https://jenkins.ubuntu.com/server/job/cloud-init-ci/366/
and the i386 ran fine. obviously it wasn't a problem with Dimitri's code that triggered it.

It feels like a bug in cheetah or python. :(.

I've put the following diff in place:
diff --git a/cloudinit/templater.py b/cloudinit/templater.py
index b3ea64e..c24265a 100644
--- a/cloudinit/templater.py
+++ b/cloudinit/templater.py
@@ -76,6 +76,8 @@ def basic_render(content, params):
 def detect_template(text):

     def cheetah_render(content, params):
+ print("Calling CTemplate(%r, searchList=[%r]).respond()" %
+ (content, params))
         return CTemplate(content, searchList=[params]).respond()

     def jinja_render(content, params):

And then run the failing test case like:
 tox-venv py27 nosetests --nocapture -v tests/unittests/test_handler/test_handl
er_apt_source_v3.py:TestAptSourceConfig.test_apt_v3_disable_suites

It shows:
test_disable_suites - disable_suites with many configurations ... Calling CTemplate('$RELEASE', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('$RELEASE-updates', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('$RELEASE-updates', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('$RELEASE-security', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('$RELEASE-updates', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('$RELEASE-security', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('$RELEASE-updates', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('$RELEASE-security', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('foobar', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('foobar', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('$RELEASE-updates', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('$RELEASE-updates', searchList=[{'RELEASE': 'xenial'}]).respond()
Calling CTemplate('$RELEASE-security', searchList=[{'RELEASE': 'xenial'}]).respond()

This is pretty straight forward usage of cheetah, the CTemplate is just
 from Cheetah.Template import Template as CTemplate

I've also tried calling CTemplate with None in either or both positions and you end up with a different stack trace than above. All of this on both trusty and xenial i386.

I was running in lxc where as c-i runs in a vm.

Revision history for this message
Scott Moser (smoser) wrote :

I really want to clean up the 'network state' object.
It really should store only address and network_prefix and then renderers would render that to either a cidr or address and netmask.

note, i think that <address>/<netmask> is an abomination.
Ie:
 good: 192.168.1.2/24
 good: address=192.168.1.2 netmask: 255.255.255.0
 bad: 192.168.1.2/255.255.255.0

Wherever possible i want to warn or raise error on the bad above.

Rant aside, I'll grab this.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
index 56b41be..1c0cec9 100644
--- a/cloudinit/net/netplan.py
+++ b/cloudinit/net/netplan.py
@@ -4,7 +4,7 @@ import copy
4import os4import os
55
6from . import renderer6from . import renderer
7from .network_state import subnet_is_ipv67from .network_state import mask2cidr, subnet_is_ipv6
88
9from cloudinit import log as logging9from cloudinit import log as logging
10from cloudinit import util10from cloudinit import util
@@ -118,9 +118,10 @@ def _extract_addresses(config, entry):
118 sn_type += '4'118 sn_type += '4'
119 entry.update({sn_type: True})119 entry.update({sn_type: True})
120 elif sn_type in ['static']:120 elif sn_type in ['static']:
121 addr = "%s" % subnet.get('address')121 addr = '%s' % subnet.get('address')
122 if 'netmask' in subnet:122 netmask = subnet.get('netmask')
123 addr += "/%s" % subnet.get('netmask')123 if netmask and '/' not in addr:
124 addr += '/%s' % mask2cidr(netmask)
124 if 'gateway' in subnet and subnet.get('gateway'):125 if 'gateway' in subnet and subnet.get('gateway'):
125 gateway = subnet.get('gateway')126 gateway = subnet.get('gateway')
126 if ":" in gateway:127 if ":" in gateway:
@@ -137,8 +138,9 @@ def _extract_addresses(config, entry):
137 mtukey += '6'138 mtukey += '6'
138 entry.update({mtukey: subnet.get('mtu')})139 entry.update({mtukey: subnet.get('mtu')})
139 for route in subnet.get('routes', []):140 for route in subnet.get('routes', []):
140 to_net = "%s/%s" % (route.get('network'),141 network = route.get('network')
141 route.get('netmask'))142 netmask = route.get('netmask')
143 to_net = '%s/%s' % (network, mask2cidr(netmask))
142 route = {144 route = {
143 'via': route.get('gateway'),145 'via': route.get('gateway'),
144 'to': to_net,146 'to': to_net,
diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
index db3c357..9e9c05a 100644
--- a/cloudinit/net/network_state.py
+++ b/cloudinit/net/network_state.py
@@ -734,9 +734,9 @@ def ipv6mask2cidr(mask):
734734
735735
736def mask2cidr(mask):736def mask2cidr(mask):
737 if ':' in mask:737 if ':' in str(mask):
738 return ipv6mask2cidr(mask)738 return ipv6mask2cidr(mask)
739 elif '.' in mask:739 elif '.' in str(mask):
740 return ipv4mask2cidr(mask)740 return ipv4mask2cidr(mask)
741 else:741 else:
742 return mask742 return mask
diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py
index 1e10a33..4fe16a9 100644
--- a/tests/unittests/test_distros/test_netconfig.py
+++ b/tests/unittests/test_distros/test_netconfig.py
@@ -127,7 +127,7 @@ network:
127 ethernets:127 ethernets:
128 eth0:128 eth0:
129 addresses:129 addresses:
130 - 192.168.1.5/255.255.255.0130 - 192.168.1.5/24
131 gateway4: 192.168.1.254131 gateway4: 192.168.1.254
132 eth1:132 eth1:
133 dhcp4: true133 dhcp4: true
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index d36d0e7..951a071 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -441,7 +441,7 @@ NETWORK_CONFIGS = {
441 - sach.maas441 - sach.maas
442 - wark.maas442 - wark.maas
443 routes:443 routes:
444 - to: 0.0.0.0/0.0.0.0444 - to: 0.0.0.0/0
445 via: 65.61.151.37445 via: 65.61.151.37
446 set-name: eth99446 set-name: eth99
447 """).rstrip(' '),447 """).rstrip(' '),

Subscribers

People subscribed via source and target branches