Clear residual qos rules after l2-agent restarts.

During the l2-agent stop, if the policy rule is cleared,
after the l2-agent is started, the qos rule that has been applied should be cleared.

Change-Id: Iaaff10dfa8ac6ab8c9dead3124e2bb3caa03a665
Closes-Bug: #1810025

neutron/agent/l2/extensions/qos.py

@@ -271,6 +271,12 @@ class QosAgentExtension(l2_extension.L2AgentExtension):
             if old_qos_policy:
                 self.qos_driver.delete(port, old_qos_policy)
                 self.qos_driver.update(port, qos_policy)
+            elif not qos_policy.rules:
+                # There are no rules in new qos_policy,
+                # so it should be deleted.
+                # But new policy has a relationship with the port,
+                # so reseting should not be called.
+                self.qos_driver.delete(port)
             else:
                 self.qos_driver.create(port, qos_policy)
 

neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py

@@ -64,30 +64,33 @@ class QosOVSAgentDriver(qos.QosLinuxAgentDriver):
 
     def delete_bandwidth_limit(self, port):
         port_id = port.get('port_id')
+        vif_port = port.get('vif_port')
         port = self.ports[port_id].pop((qos_consts.RULE_TYPE_BANDWIDTH_LIMIT,
                                         constants.EGRESS_DIRECTION),
                                        None)
-        if not port:
+
+        if not port and not vif_port:
             LOG.debug("delete_bandwidth_limit was received "
                       "for port %s but port was not found. "
                       "It seems that bandwidth_limit is already deleted",
                       port_id)
             return
-        vif_port = port.get('vif_port')
+        vif_port = vif_port or port.get('vif_port')
         self.br_int.delete_egress_bw_limit_for_port(vif_port.port_name)
 
     def delete_bandwidth_limit_ingress(self, port):
         port_id = port.get('port_id')
+        vif_port = port.get('vif_port')
         port = self.ports[port_id].pop((qos_consts.RULE_TYPE_BANDWIDTH_LIMIT,
                                         constants.INGRESS_DIRECTION),
                                        None)
-        if not port:
+        if not port and not vif_port:
             LOG.debug("delete_bandwidth_limit_ingress was received "
                       "for port %s but port was not found. "
                       "It seems that bandwidth_limit is already deleted",
                       port_id)
             return
-        vif_port = port.get('vif_port')
+        vif_port = vif_port or port.get('vif_port')
         self.br_int.delete_ingress_bw_limit_for_port(vif_port.port_name)
 
     def create_dscp_marking(self, port, rule):
@@ -135,15 +138,19 @@ class QosOVSAgentDriver(qos.QosLinuxAgentDriver):
                                      actions=actions)
 
     def delete_dscp_marking(self, port):
+        vif_port = port.get('vif_port')
         dscp_port = self.ports[port['port_id']].pop(qos_consts.
                                                     RULE_TYPE_DSCP_MARKING, 0)
-        if dscp_port:
-            port_num = dscp_port['vif_port'].ofport
-            self.br_int.uninstall_flows(in_port=port_num, table_id=0, reg2=0)
-        else:
+
+        if not dscp_port and not vif_port:
             LOG.debug("delete_dscp_marking was received for port %s but "
                       "no port information was stored to be deleted",
                       port['port_id'])
+            return
+
+        vif_port = vif_port or dscp_port.get('vif_port')
+        port_num = vif_port.ofport
+        self.br_int.uninstall_flows(in_port=port_num, table_id=0, reg2=0)
 
     def _update_egress_bandwidth_limit(self, vif_port, rule):
         max_kbps = rule.max_kbps

neutron/tests/fullstack/test_qos.py

@@ -64,6 +64,7 @@ class BaseQoSRuleTestCase(object):
                 l2_agent_type=self.l2_agent_type
             ) for _ in range(self.number_of_hosts)]
         env_desc = environment.EnvironmentDescription(
+            agent_down_time=10,
             qos=True)
         env = environment.Environment(env_desc, host_desc)
         super(BaseQoSRuleTestCase, self).setUp(env)
@@ -190,6 +191,47 @@ class _TestBwLimitQoS(BaseQoSRuleTestCase):
         self._wait_for_bw_rule_applied(
             vm, BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.reverse_direction)
 
+    def test_bw_limit_qos_l2_agent_restart(self):
+        self.l2_agent_process = self.environment.hosts[0].l2_agent
+        self.l2_agent = self.safe_client.client.list_agents(
+            agent_type=self.l2_agent_type)['agents'][0]
+
+        # Create port with qos policy attached, with different direction
+        vm, qos_policy = self._prepare_vm_with_qos_policy(
+            [functools.partial(
+                self._add_bw_limit_rule,
+                BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.direction),
+             functools.partial(
+                self._add_bw_limit_rule,
+                BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.reverse_direction)])
+
+        bw_rule_1 = qos_policy['rules'][0]
+        bw_rule_2 = qos_policy['rules'][1]
+        qos_policy_id = qos_policy['id']
+
+        self._wait_for_bw_rule_applied(
+            vm, BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.direction)
+        self._wait_for_bw_rule_applied(
+            vm, BANDWIDTH_LIMIT, BANDWIDTH_BURST, self.reverse_direction)
+
+        # Stop l2_agent and clear these rules
+        self.l2_agent_process.stop()
+        self._wait_until_agent_down(self.l2_agent['id'])
+
+        self.client.delete_bandwidth_limit_rule(bw_rule_1['id'],
+                                                qos_policy_id)
+        self.client.delete_bandwidth_limit_rule(bw_rule_2['id'],
+                                                qos_policy_id)
+
+        # Start l2_agent to check if these rules is cleared
+        self.l2_agent_process.start()
+        self._wait_until_agent_up(self.l2_agent['id'])
+
+        self._wait_for_bw_rule_applied(
+            vm, None, None, self.direction)
+        self._wait_for_bw_rule_applied(
+            vm, None, None, self.reverse_direction)
+
 
 class TestBwLimitQoSOvs(_TestBwLimitQoS, base.BaseFullStackTestCase):
     l2_agent_type = constants.AGENT_TYPE_OVS

neutron/tests/unit/agent/l2/extensions/test_qos.py

@@ -234,6 +234,7 @@ class QosExtensionBaseTestCase(base.BaseTestCase):
             manager.NeutronManager, 'load_class_for_provider',
             return_value=lambda: mock.Mock(
                 spec=qos_linux.QosLinuxAgentDriver)).start()
+        setattr(TEST_POLICY, 'rules', [])
 
 
 class QosExtensionRpcTestCase(QosExtensionBaseTestCase):
@@ -262,6 +263,12 @@ class QosExtensionRpcTestCase(QosExtensionBaseTestCase):
         port = self._create_test_port_dict()
         qos_policy_id = port['qos_policy_id']
         port_id = port['port_id']
+
+        TEST_POLICY.rules = [rule.QosBandwidthLimitRule(
+            context=None, id=FAKE_RULE_ID,
+            qos_policy_id=TEST_POLICY.id,
+            max_kbps=100, max_burst_kbps=200,
+            direction=common_constants.EGRESS_DIRECTION)]
         self.qos_ext.handle_port(self.context, port)
         # we make sure the underlying qos driver is called with the
         # right parameters
@@ -273,6 +280,29 @@ class QosExtensionRpcTestCase(QosExtensionBaseTestCase):
         self.assertEqual(TEST_POLICY,
             self.qos_ext.policy_map.known_policies[qos_policy_id])
 
+    def test_handle_unknown_port_with_no_rules(self):
+        test_policy_with_rules = {'context': None,
+                                  'name': 'test1',
+                                  'id': uuidutils.generate_uuid()}
+
+        test_policy = policy.QosPolicy(**test_policy_with_rules)
+        test_policy.rules = []
+
+        port = self._create_test_port_dict(test_policy.id)
+        qos_policy_id = port['qos_policy_id']
+        port_id = port['port_id']
+
+        self.pull_mock.return_value = test_policy
+        self.qos_ext.handle_port(self.context, port)
+        # we make sure the underlying qos driver is called with the
+        # right parameters
+        self.qos_ext.qos_driver.delete.assert_called_once_with(port)
+        self.assertEqual(port,
+            self.qos_ext.policy_map.qos_policy_ports[qos_policy_id][port_id])
+        self.assertIn(port_id, self.qos_ext.policy_map.port_policies)
+        self.assertEqual(test_policy,
+            self.qos_ext.policy_map.known_policies[qos_policy_id])
+
     def test_handle_known_port(self):
         port_obj1 = self._create_test_port_dict()
         port_obj2 = dict(port_obj1)
@@ -285,7 +315,13 @@ class QosExtensionRpcTestCase(QosExtensionBaseTestCase):
         port = self._create_test_port_dict()
         self.qos_ext.handle_port(self.context, port)
         self.qos_ext.resource_rpc.pull.reset_mock()
-        port['qos_policy_id'] = uuidutils.generate_uuid()
+        test_policy_with_rules = {'context': None,
+                                  'name': 'test1',
+                                  'id': uuidutils.generate_uuid()}
+
+        test_policy = policy.QosPolicy(**test_policy_with_rules)
+        self.pull_mock.return_value = test_policy
+        port['qos_policy_id'] = test_policy.id
         self.qos_ext.handle_port(self.context, port)
         self.pull_mock.assert_called_once_with(
              self.context, resources.QOS_POLICY,

neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py

@@ -156,8 +156,8 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
             return_value=(self.rules[1].max_kbps,
                           self.rules[1].max_burst_kbps))
         self.qos_driver.delete(self.port)
-        self.delete_egress.assert_not_called()
-        self.delete_ingress.assert_not_called()
+        self.delete_egress.assert_called_once_with(self.port_name)
+        self.delete_ingress.assert_called_once_with(self.port_name)
 
     def test_delete_rules(self):
         self._test_delete_rules(self.qos_policy)