Fix KeyError in OVS firewall

When merging port ranges, the code never assumed the
conjunction ID might not be present in the set due to
already being removed.

In this case there were two security groups, both using
the same remote security group, but the first security
group does not define a port range and the second one does.
Or more generally, the first SG port range is a subset
of the second, as no port-range means the full range.

Change-Id: I17ab643abbd2ec21eda4ae1dfb9abf2d4b0657f2
Closes-bug: #1813007

neutron/agent/linux/openvswitch_firewall/rules.py

@@ -121,26 +121,32 @@ def merge_port_ranges(rule_conj_list):
     # item means a removal.
     result = []
     rule_tmpl = rule_conj_list[0][0]
-    cur_conj = set()
+    cur_conj = {}
     cur_range_min = None
     for port, m, conj_id in port_ranges:
         if m == 'min':
+            if conj_id in cur_conj:
+                cur_conj[conj_id] += 1
+                continue
             if cur_conj and cur_range_min != port:
                 rule = rule_tmpl.copy()
                 rule['port_range_min'] = cur_range_min
                 rule['port_range_max'] = port - 1
-                result.append((rule, list(cur_conj)))
+                result.append((rule, list(cur_conj.keys())))
             cur_range_min = port
-            cur_conj.add(conj_id)
+            cur_conj[conj_id] = 1
         else:
+            if cur_conj[conj_id] > 1:
+                cur_conj[conj_id] -= 1
+                continue
             if cur_range_min <= port:
                 rule = rule_tmpl.copy()
                 rule['port_range_min'] = cur_range_min
                 rule['port_range_max'] = port
-                result.append((rule, list(cur_conj)))
+                result.append((rule, list(cur_conj.keys())))
                 # The next port range without 'port' starts from (port + 1)
                 cur_range_min = port + 1
-            cur_conj.remove(conj_id)
+            del cur_conj[conj_id]
 
     if (len(result) == 1 and result[0][0]['port_range_min'] == 1 and
             result[0][0]['port_range_max'] == 65535):

neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py

@@ -420,8 +420,8 @@ class TestMergeRules(base.BaseTestCase):
         self.assertEqual(len(expected), len(result))
         for (range_min, range_max, conj_ids), result1 in zip(
                 expected, result):
-            self.assertEqual(range_min, result1[0]['port_range_min'])
-            self.assertEqual(range_max, result1[0]['port_range_max'])
+            self.assertEqual(range_min, result1[0].get('port_range_min'))
+            self.assertEqual(range_max, result1[0].get('port_range_max'))
             self.assertEqual(conj_ids, set(result1[1]))
 
     def test__assert_mergeable_rules(self):
@@ -489,6 +489,15 @@ class TestMergeRules(base.BaseTestCase):
                 (30, 40, {10, 12, 4}),
                 (41, 65535, {10, 12})], result)
 
+    def test_merge_port_ranges_no_port_ranges_same_conj_id(self):
+        result = rules.merge_port_ranges(
+            [(dict(self.rule_tmpl), 10),
+             (dict(self.rule_tmpl), 12),
+             (dict([('port_range_min', 30), ('port_range_max', 30)] +
+                   self.rule_tmpl), 10)])
+        self._test_merge_port_ranges_helper([
+                (None, None, {10, 12})], result)
+
     def test_merge_port_ranges_nonoverlapping(self):
         result = rules.merge_port_ranges(
             [(dict([('port_range_min', 30), ('port_range_max', 40)] +