diff --git a/neutron/agent/l2/extensions/qos.py b/neutron/agent/l2/extensions/qos.py index 0a70fb5ea59..185ea298bd6 100644 --- a/neutron/agent/l2/extensions/qos.py +++ b/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) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py b/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py index 50e1ce5bc47..da9b1c6568f 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py +++ b/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 diff --git a/neutron/tests/fullstack/test_qos.py b/neutron/tests/fullstack/test_qos.py index de86015fd79..18d6bbcdcb1 100644 --- a/neutron/tests/fullstack/test_qos.py +++ b/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 diff --git a/neutron/tests/unit/agent/l2/extensions/test_qos.py b/neutron/tests/unit/agent/l2/extensions/test_qos.py index afe47cc13e3..1dff9af8b24 100644 --- a/neutron/tests/unit/agent/l2/extensions/test_qos.py +++ b/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, diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py index 55691c0db0c..14b30df72fe 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py +++ b/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)