Skip to content

Commit

Permalink
ovs: Fix error in patch_ports_on_bridge (juju#509)
Browse files Browse the repository at this point in the history
The unit test for patch_ports_on_bridge was not quite good enough
and hid the fact that the function now always raises ValueError.

This patch fixes both.
  • Loading branch information
fnordahl authored Sep 3, 2020
1 parent c0576c7 commit b7414c4
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
5 changes: 5 additions & 0 deletions charmhelpers/contrib/network/ovs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,11 @@ def patch_ports_on_bridge(bridge):
interface['options']['peer'])),
interface['options']['peer'])
yield(Patch(this_end, other_end))
# We expect one result and it is ok if it turns out to be a port
# for a different bridge. However we need a break here to satisfy
# the for/else check which is in place to detect interface refering
# to non-existent port.
break
else:
raise ValueError('Port for interface named "{}" does unexpectedly '
'not exist.'.format(interface['name']))
Expand Down
20 changes: 16 additions & 4 deletions tests/contrib/network/ovs/test_ovs.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,15 @@ def test_bridge_for_port(self):
def test_patch_ports_on_bridge(self):
self.patch_object(ovs.ch_ovsdb, 'SimpleOVSDB')
self.patch_object(ovs, 'bridge_for_port')
self.patch_object(ovs, 'uuid_for_port')
ovsdb = mock.MagicMock()
ovsdb.interface.find.return_value = [
{
'name': 'fake-interface-with-port-for-other-bridge',
'options': {
'peer': 'fake-peer'
},
},
{
'name': 'fake-interface',
'options': {
Expand All @@ -229,14 +236,18 @@ def test_patch_ports_on_bridge(self):
},
]
port_uuid = uuid.UUID('0d43905b-f80e-4eaa-9feb-a9017da8c6bc')
ovsdb.port.find.return_value = [
{
ovsdb.port.find.side_effect = [
[{
'_uuid': port_uuid,
'name': 'port-on-other-bridge',
}],
[{
'_uuid': port_uuid,
'name': 'fake-port',
},
}],
]
self.SimpleOVSDB.return_value = ovsdb
self.bridge_for_port.side_effect = ['fake-bridge', 'fake-peer-bridge']
self.bridge_for_port.side_effect = ['some-other-bridge', 'fake-bridge', 'fake-peer-bridge']
for patch in ovs.patch_ports_on_bridge('fake-bridge'):
self.assertEquals(
patch,
Expand All @@ -251,6 +262,7 @@ def test_patch_ports_on_bridge(self):
break
else:
assert 0, 'Expected generator to provide output'
ovsdb.port.find.side_effect = None
ovsdb.port.find.return_value = []
with self.assertRaises(ValueError):
for patch in ovs.patch_ports_on_bridge('fake-bridge'):
Expand Down

0 comments on commit b7414c4

Please sign in to comment.