Merge ~raharper/cloud-init:add-netplan-bridge-port-priority into cloud-init:master

Proposed by Ryan Harper
Status: Merged
Approved by: Scott Moser
Approved revision: a02036eaf847aaec2fce2ad3e7fbdd4ef967bb97
Merge reported by: Scott Moser
Merged at revision: b7b7331b9c308d8e1cb0b3dfd4398e6e7cb1b60f
Proposed branch: ~raharper/cloud-init:add-netplan-bridge-port-priority
Merge into: cloud-init:master
Diff against target: 49 lines (+9/-6)
3 files modified
cloudinit/net/netplan.py (+5/-5)
cloudinit/net/network_state.py (+1/-1)
tests/unittests/test_net.py (+3/-0)
Reviewer Review Type Date Requested Status
Scott Moser Needs Fixing
Server Team CI bot continuous-integration Approve
Review via email: mp+334611@code.launchpad.net

Commit message

netplan: render bridge port-priority values

Update netplan renderer to write out bridge port-priority values
now that netplan supports the feature.

LP: #1735821

Description of the change

netplan: render bridge port-priority values

Update netplan renderer to write out bridge port-priority values
now that netplan supports the feature.

LP: #1735821

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

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

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

We decided to mark this as work in progress until the fixes in netplan arrive.
As it is right now, if the user provided data with port-priority values,
cloud-init would render netplan config that netplan would fail on.

Resurrect this when netplan is fixed.

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

As of nplan 0.33 netplan supports bridge port-priority.

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

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

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

This looks good, we'll pull it.
I would like a statement from Ryan as to whether anything is made
worse off if the netplan fix is not present.

netplan is fixed in bionic, so cloud-init would be fine there.
It is being SRU'd (in -proposed) in artful and xenial also.
artful *uses* netplan, so that is really where this could fail.

So, Ryan:

Does this make anything worse on artful if we do not have netplan fix?

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

Thank you for your merge proposal.

Your branch has been set to 'Work in progress'.
Please set the branch back to 'Needs Review' after resolving the issues below.

Thanks again,
Your friendly neighborhood cloud-init robot.

Please fix the following issues:
------------------------------
No commit message in Launchpad
------------------------------

For more information, see commit message guidelines at
https://cloudinit.readthedocs.io/en/latest/topics/hacking.html#do-these-things-for-each-feature-or-bug

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

An upstream commit landed for this bug.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
2index d3788af..6bee3d3 100644
3--- a/cloudinit/net/netplan.py
4+++ b/cloudinit/net/netplan.py
5@@ -311,12 +311,12 @@ class Renderer(renderer.Renderer):
6 if newname is None:
7 continue
8 br_config.update({newname: value})
9- if newname == 'path-cost':
10- # <interface> <cost> -> <interface>: int(<cost>)
11+ if newname in ['path-cost', 'port-priority']:
12+ # <interface> <value> -> <interface>: int(<value>)
13 newvalue = {}
14- for costval in value:
15- (port, cost) = costval.split()
16- newvalue[port] = int(cost)
17+ for val in value:
18+ (port, portval) = val.split()
19+ newvalue[port] = int(portval)
20 br_config.update({newname: newvalue})
21
22 if len(br_config) > 0:
23diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
24index fe667d8..1dd7ded 100644
25--- a/cloudinit/net/network_state.py
26+++ b/cloudinit/net/network_state.py
27@@ -47,7 +47,7 @@ NET_CONFIG_TO_V2 = {
28 'bridge_maxage': 'max-age',
29 'bridge_maxwait': None,
30 'bridge_pathcost': 'path-cost',
31- 'bridge_portprio': None,
32+ 'bridge_portprio': 'port-priority',
33 'bridge_stp': 'stp',
34 'bridge_waitport': None}}
35
36diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
37index ac33e8e..9cf11f2 100644
38--- a/tests/unittests/test_net.py
39+++ b/tests/unittests/test_net.py
40@@ -758,6 +758,9 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
41 path-cost:
42 eth3: 50
43 eth4: 75
44+ port-priority:
45+ eth3: 28
46+ eth4: 14
47 priority: 22
48 stp: false
49 routes:

Subscribers

People subscribed via source and target branches