cloud-init

Merge ~smoser/cloud-init:bug/1692028-empty-macs-for-bymac into cloud-init:master

  1. Git
  2. lp:~smoser/cloud-init
  3. bug/1692028-empty-macs-for-bymac
  4. Merge into master
Proposed by Scott Moser
Status: Merged
Merged at revision: 2c0655feb9a194b5fbdfe90a5f847c16f1e15409
Proposed branch: ~smoser/cloud-init:bug/1692028-empty-macs-for-bymac
Merge into: cloud-init:master
Diff against target: 107 lines (+22/-8)
2 files modified
cloudinit/net/__init__.py (+3/-0)
tests/unittests/test_net.py (+19/-8)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Ryan Harper Approve
Review via email: mp+324332@code.launchpad.net

Commit message

Fix get_interfaces_by_mac for empty macs

Some interfaces (greptap0 in the bug) have a mac address of
'00:00:00:00:00:00'. That was causing a duplicate mac detection
as the 'lo' device also has that mac.

The change here is to just ignore macs other than 'lo' that have that.

LP: #1692028

To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :

I'm not sure where this 'gretap0' device comes from or if it would ever appear in the network data. I'd suggest that it would be bad for it to...

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

FAILED: Continuous integration, rev:2cb58ff6eb4d267b6f63ec6a983cc27d7a6bc187
https://jenkins.ubuntu.com/server/job/cloud-init-ci/360/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/360/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/360/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/360/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/360/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/360/console

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

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

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

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

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

GRE tap is an overlay network, I believe it's supported in openstack;
Google also found some AWS setup instructions mentioning gretap's

https://aws.amazon.com/articles/6234671078671125

Patch looks good to me.

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

Yeah, my question really was if we'd ever be expected to configure this device, or if its already configured...

Ie, i want to see network_json for this. It seems like there will be issues further down the road if we simply ignore the device in get_interfaces_by_mac but we're supposed to write a network configuration for it.

James, could you provide the config drive contents on the system you saw this on ?

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

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

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

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

As ryan has approved, and we've opened nova-lxd/lxd bugs at
 https://bugs.launchpad.net/cloud-archive/+bug/1692545
and
 https://github.com/lxc/lxd/issues/3338

I'm going to pull the change.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index a072a8d..8c6cd05 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -393,6 +393,7 @@ def get_interfaces_by_mac():
393 else:393 else:
394 raise394 raise
395 ret = {}395 ret = {}
396 empty_mac = '00:00:00:00:00:00'
396 for name in devs:397 for name in devs:
397 if not interface_has_own_mac(name):398 if not interface_has_own_mac(name):
398 continue399 continue
@@ -404,6 +405,8 @@ def get_interfaces_by_mac():
404 # some devices may not have a mac (tun0)405 # some devices may not have a mac (tun0)
405 if not mac:406 if not mac:
406 continue407 continue
408 if mac == empty_mac and name != 'lo':
409 continue
407 if mac in ret:410 if mac in ret:
408 raise RuntimeError(411 raise RuntimeError(
409 "duplicate mac found! both '%s' and '%s' have mac '%s'" %412 "duplicate mac found! both '%s' and '%s' have mac '%s'" %
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 68a0157..8bd3f43 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -1542,24 +1542,24 @@ class TestNetRenderers(CiTestCase):
15421542
15431543
1544class TestGetInterfacesByMac(CiTestCase):1544class TestGetInterfacesByMac(CiTestCase):
1545 _data = {'devices': ['enp0s1', 'enp0s2', 'bond1', 'bridge1',1545 _data = {'bonds': ['bond1'],
1546 'bridge1-nic', 'tun0', 'bond1.101'],
1547 'bonds': ['bond1'],
1548 'bridges': ['bridge1'],1546 'bridges': ['bridge1'],
1549 'vlans': ['bond1.101'],1547 'vlans': ['bond1.101'],
1550 'own_macs': ['enp0s1', 'enp0s2', 'bridge1-nic', 'bridge1',1548 'own_macs': ['enp0s1', 'enp0s2', 'bridge1-nic', 'bridge1',
1551 'bond1.101'],1549 'bond1.101', 'lo'],
1552 'macs': {'enp0s1': 'aa:aa:aa:aa:aa:01',1550 'macs': {'enp0s1': 'aa:aa:aa:aa:aa:01',
1553 'enp0s2': 'aa:aa:aa:aa:aa:02',1551 'enp0s2': 'aa:aa:aa:aa:aa:02',
1554 'bond1': 'aa:aa:aa:aa:aa:01',1552 'bond1': 'aa:aa:aa:aa:aa:01',
1555 'bond1.101': 'aa:aa:aa:aa:aa:01',1553 'bond1.101': 'aa:aa:aa:aa:aa:01',
1556 'bridge1': 'aa:aa:aa:aa:aa:03',1554 'bridge1': 'aa:aa:aa:aa:aa:03',
1557 'bridge1-nic': 'aa:aa:aa:aa:aa:03',1555 'bridge1-nic': 'aa:aa:aa:aa:aa:03',
1556 'lo': '00:00:00:00:00:00',
1557 'greptap0': '00:00:00:00:00:00',
1558 'tun0': None}}1558 'tun0': None}}
1559 data = {}1559 data = {}
15601560
1561 def _se_get_devicelist(self):1561 def _se_get_devicelist(self):
1562 return self.data['devices']1562 return list(self.data['devices'])
15631563
1564 def _se_get_interface_mac(self, name):1564 def _se_get_interface_mac(self, name):
1565 return self.data['macs'][name]1565 return self.data['macs'][name]
@@ -1575,6 +1575,7 @@ class TestGetInterfacesByMac(CiTestCase):
15751575
1576 def _mock_setup(self):1576 def _mock_setup(self):
1577 self.data = copy.deepcopy(self._data)1577 self.data = copy.deepcopy(self._data)
1578 self.data['devices'] = set(list(self.data['macs'].keys()))
1578 mocks = ('get_devicelist', 'get_interface_mac', 'is_bridge',1579 mocks = ('get_devicelist', 'get_interface_mac', 'is_bridge',
1579 'interface_has_own_mac', 'is_vlan')1580 'interface_has_own_mac', 'is_vlan')
1580 self.mocks = {}1581 self.mocks = {}
@@ -1602,7 +1603,7 @@ class TestGetInterfacesByMac(CiTestCase):
1602 [mock.call('enp0s1'), mock.call('bond1')], any_order=True)1603 [mock.call('enp0s1'), mock.call('bond1')], any_order=True)
1603 self.assertEqual(1604 self.assertEqual(
1604 {'aa:aa:aa:aa:aa:01': 'enp0s1', 'aa:aa:aa:aa:aa:02': 'enp0s2',1605 {'aa:aa:aa:aa:aa:01': 'enp0s1', 'aa:aa:aa:aa:aa:02': 'enp0s2',
1605 'aa:aa:aa:aa:aa:03': 'bridge1-nic'},1606 'aa:aa:aa:aa:aa:03': 'bridge1-nic', '00:00:00:00:00:00': 'lo'},
1606 ret)1607 ret)
16071608
1608 def test_excludes_bridges(self):1609 def test_excludes_bridges(self):
@@ -1611,7 +1612,7 @@ class TestGetInterfacesByMac(CiTestCase):
1611 # set everything other than 'b1' to be a bridge.1612 # set everything other than 'b1' to be a bridge.
1612 # then expect b1 is the only thing left.1613 # then expect b1 is the only thing left.
1613 self.data['macs']['b1'] = 'aa:aa:aa:aa:aa:b1'1614 self.data['macs']['b1'] = 'aa:aa:aa:aa:aa:b1'
1614 self.data['devices'].append('b1')1615 self.data['devices'].add('b1')
1615 self.data['bonds'] = []1616 self.data['bonds'] = []
1616 self.data['own_macs'] = self.data['devices']1617 self.data['own_macs'] = self.data['devices']
1617 self.data['bridges'] = [f for f in self.data['devices'] if f != "b1"]1618 self.data['bridges'] = [f for f in self.data['devices'] if f != "b1"]
@@ -1628,7 +1629,7 @@ class TestGetInterfacesByMac(CiTestCase):
1628 # set everything other than 'b1' to be a vlan.1629 # set everything other than 'b1' to be a vlan.
1629 # then expect b1 is the only thing left.1630 # then expect b1 is the only thing left.
1630 self.data['macs']['b1'] = 'aa:aa:aa:aa:aa:b1'1631 self.data['macs']['b1'] = 'aa:aa:aa:aa:aa:b1'
1631 self.data['devices'].append('b1')1632 self.data['devices'].add('b1')
1632 self.data['bonds'] = []1633 self.data['bonds'] = []
1633 self.data['bridges'] = []1634 self.data['bridges'] = []
1634 self.data['own_macs'] = self.data['devices']1635 self.data['own_macs'] = self.data['devices']
@@ -1640,6 +1641,16 @@ class TestGetInterfacesByMac(CiTestCase):
1640 mock.call('b1')],1641 mock.call('b1')],
1641 any_order=True)1642 any_order=True)
16421643
1644 def test_duplicates_of_empty_mac_are_ok(self):
1645 """Duplicate macs of 00:00:00:00:00:00 should be skipped."""
1646 self._mock_setup()
1647 empty_mac = "00:00:00:00:00:00"
1648 addnics = ('greptap1', 'lo', 'greptap2')
1649 self.data['macs'].update(dict((k, empty_mac) for k in addnics))
1650 self.data['devices'].update(set(addnics))
1651 ret = net.get_interfaces_by_mac()
1652 self.assertEqual('lo', ret[empty_mac])
1653
16431654
1644def _gzip_data(data):1655def _gzip_data(data):
1645 with io.BytesIO() as iobuf:1656 with io.BytesIO() as iobuf:

Subscribers

People subscribed via source and target branches