Merge ~smoser/cloud-init:fix/smartos-container-nics-have-own-mac into cloud-init:master

Proposed by Scott Moser
Status: Merged
Approved by: Chad Smith
Approved revision: 8f5a47f312c017cfe12e9d976d68998e570cb6f2
Merge reported by: Chad Smith
Merged at revision: 23a84d2ce4ec44a0a0c2edbc3b1948c59bb0afbb
Proposed branch: ~smoser/cloud-init:fix/smartos-container-nics-have-own-mac
Merge into: cloud-init:master
Diff against target: 83 lines (+46/-8)
2 files modified
cloudinit/net/__init__.py (+6/-2)
tests/unittests/test_net.py (+40/-6)
Reviewer Review Type Date Requested Status
Chad Smith Approve
Server Team CI bot continuous-integration Approve
Mike Gerdts (community) Approve
Review via email: mp+343739@code.launchpad.net

Commit message

SmartOS: fix get_interfaces for nics that do not have addr_assign_type.

When attempting to apply network configuration for SmartOS's container
platform, cloud-init would not identify nics. The nics on provided
in this container service do not have 'addr_assign_type'. That
was being interpreted as being a "stolen" mac, and would be filtered
out by get_interfaces.

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 :

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

review: Approve (continuous-integration)
Revision history for this message
Mike Gerdts (mgerdts) wrote :

I've approved this, but do think a comment update to explain None will reduce future confusion.

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

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

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

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

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

+1 on this LGTM!

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=23a84d2c

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
2index 43226bd..3ffde52 100644
3--- a/cloudinit/net/__init__.py
4+++ b/cloudinit/net/__init__.py
5@@ -359,8 +359,12 @@ def interface_has_own_mac(ifname, strict=False):
6 1: randomly generated 3: set using dev_set_mac_address"""
7
8 assign_type = read_sys_net_int(ifname, "addr_assign_type")
9- if strict and assign_type is None:
10- raise ValueError("%s had no addr_assign_type.")
11+ if assign_type is None:
12+ # None is returned if this nic had no 'addr_assign_type' entry.
13+ # if strict, raise an error, if not return True.
14+ if strict:
15+ raise ValueError("%s had no addr_assign_type.")
16+ return True
17 return assign_type in (0, 1, 3)
18
19
20diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
21index fac8267..31807e1 100644
22--- a/tests/unittests/test_net.py
23+++ b/tests/unittests/test_net.py
24@@ -2,12 +2,9 @@
25
26 from cloudinit import net
27 from cloudinit.net import cmdline
28-from cloudinit.net import eni
29-from cloudinit.net import natural_sort_key
30-from cloudinit.net import netplan
31-from cloudinit.net import network_state
32-from cloudinit.net import renderers
33-from cloudinit.net import sysconfig
34+from cloudinit.net import (
35+ eni, interface_has_own_mac, natural_sort_key, netplan, network_state,
36+ renderers, sysconfig)
37 from cloudinit.sources.helpers import openstack
38 from cloudinit import temp_utils
39 from cloudinit import util
40@@ -2691,6 +2688,43 @@ class TestGetInterfaces(CiTestCase):
41 any_order=True)
42
43
44+class TestInterfaceHasOwnMac(CiTestCase):
45+ """Test interface_has_own_mac. This is admittedly a bit whitebox."""
46+
47+ @mock.patch('cloudinit.net.read_sys_net_int', return_value=None)
48+ def test_non_strict_with_no_addr_assign_type(self, m_read_sys_net_int):
49+ """If nic does not have addr_assign_type, it is not "stolen".
50+
51+ SmartOS containers do not provide the addr_assign_type in /sys.
52+
53+ $ ( cd /sys/class/net/eth0/ && grep -r . *)
54+ address:90:b8:d0:20:e1:b0
55+ addr_len:6
56+ flags:0x1043
57+ ifindex:2
58+ mtu:1500
59+ tx_queue_len:1
60+ type:1
61+ """
62+ self.assertTrue(interface_has_own_mac("eth0"))
63+
64+ @mock.patch('cloudinit.net.read_sys_net_int', return_value=None)
65+ def test_strict_with_no_addr_assign_type_raises(self, m_read_sys_net_int):
66+ with self.assertRaises(ValueError):
67+ interface_has_own_mac("eth0", True)
68+
69+ @mock.patch('cloudinit.net.read_sys_net_int')
70+ def test_expected_values(self, m_read_sys_net_int):
71+ msg = "address_assign_type=%d said to not have own mac"
72+ for address_assign_type in (0, 1, 3):
73+ m_read_sys_net_int.return_value = address_assign_type
74+ self.assertTrue(
75+ interface_has_own_mac("eth0", msg % address_assign_type))
76+
77+ m_read_sys_net_int.return_value = 2
78+ self.assertFalse(interface_has_own_mac("eth0"))
79+
80+
81 class TestGetInterfacesByMac(CiTestCase):
82 _data = {'bonds': ['bond1'],
83 'bridges': ['bridge1'],

Subscribers

People subscribed via source and target branches