From 324c3528ec7726fb6c69f70473b75b14357ff14a Mon Sep 17 00:00:00 2001 From: Gregory Thiemonge Date: Wed, 1 Mar 2023 21:51:19 +0100 Subject: [PATCH] Remove specific handling of amphorav1 parameters Some functions in the drivers accepted 2 different types for the same parameters: provider dict or provider data models. Now that amphorav1 is removed, we can remove the cases that handle both types. Change-Id: Ibeb9132e65706b50706249df3e8b6c455863513c --- octavia/amphorae/drivers/driver_base.py | 3 +- .../drivers/haproxy/rest_api_driver.py | 27 ++------- .../drivers/keepalived/jinja/jinja_cfg.py | 15 +---- .../amphorae/drivers/noop_driver/driver.py | 8 +-- .../worker/v2/tasks/amphora_driver_tasks.py | 4 +- .../haproxy/test_rest_api_driver_1_0.py | 55 +++++++++++++------ .../keepalived/jinja/test_jinja_cfg.py | 8 +-- .../drivers/noop_driver/test_driver.py | 5 +- .../v2/tasks/test_amphora_driver_tasks.py | 19 +++---- 9 files changed, 69 insertions(+), 75 deletions(-) diff --git a/octavia/amphorae/drivers/driver_base.py b/octavia/amphorae/drivers/driver_base.py index 1886cb4cf8..20ddde8470 100644 --- a/octavia/amphorae/drivers/driver_base.py +++ b/octavia/amphorae/drivers/driver_base.py @@ -151,8 +151,7 @@ class AmphoraLoadBalancerDriver(object, metaclass=abc.ABCMeta): """ def post_vip_plug(self, amphora, load_balancer, amphorae_network_config, - vrrp_port=None, vip_subnet=None, - additional_vip_data=None): + vrrp_port, vip_subnet, additional_vip_data=None): """Called after network driver has allocated and plugged the VIP :param amphora: diff --git a/octavia/amphorae/drivers/haproxy/rest_api_driver.py b/octavia/amphorae/drivers/haproxy/rest_api_driver.py index 0be632e45d..cdefb7da37 100644 --- a/octavia/amphorae/drivers/haproxy/rest_api_driver.py +++ b/octavia/amphorae/drivers/haproxy/rest_api_driver.py @@ -40,7 +40,6 @@ from octavia.common import utils from octavia.db import api as db_apis from octavia.db import models as db_models from octavia.db import repositories as repo -from octavia.network import data_models as network_models LOG = logging.getLogger(__name__) @@ -358,26 +357,16 @@ class HaproxyAmphoraLoadBalancerDriver( return net_info def post_vip_plug(self, amphora, load_balancer, amphorae_network_config, - vrrp_port=None, vip_subnet=None, - additional_vip_data=None): + vrrp_port, vip_subnet, additional_vip_data=None): if amphora.status != consts.DELETED: self._populate_amphora_api_version(amphora) - if vip_subnet is None: - vip_subnet = amphorae_network_config.get(amphora.id).vip_subnet - if vrrp_port is None: - port = amphorae_network_config.get(amphora.id).vrrp_port - mtu = port.network.mtu - else: - port = vrrp_port - mtu = port.network['mtu'] + port = vrrp_port.to_dict(recurse=True) + mtu = port[consts.NETWORK][consts.MTU] LOG.debug("Post-VIP-Plugging with vrrp_ip %s vrrp_port %s", - amphora.vrrp_ip, port.id) + amphora.vrrp_ip, port[consts.ID]) net_info = self._build_net_info( - port.to_dict(recurse=True), amphora.to_dict(), + port, amphora.to_dict(), vip_subnet.to_dict(recurse=True), mtu) - if additional_vip_data is None: - additional_vip_data = amphorae_network_config.get( - amphora.id).additional_vip_data for add_vip in additional_vip_data: add_host_routes = [{'nexthop': hr.nexthop, 'destination': hr.destination} @@ -393,7 +382,7 @@ class HaproxyAmphoraLoadBalancerDriver( except exc.Conflict: LOG.warning('VIP with MAC %(mac)s already exists on amphora, ' 'skipping post_vip_plug', - {'mac': port.mac_address}) + {'mac': port[consts.MAC_ADDRESS]}) def post_network_plug(self, amphora, port, amphora_network_config): fixed_ips = [] @@ -410,10 +399,6 @@ class HaproxyAmphoraLoadBalancerDriver( 'fixed_ips': fixed_ips, 'mtu': port.network.mtu} if port.id == amphora.vrrp_port_id: - if isinstance(amphora_network_config, - network_models.AmphoraNetworkConfig): - amphora_network_config = amphora_network_config.to_dict( - recurse=True) # We have to special-case sharing the vrrp port and pass through # enough extra information to populate the whole VIP port net_info = self._build_net_info( diff --git a/octavia/amphorae/drivers/keepalived/jinja/jinja_cfg.py b/octavia/amphorae/drivers/keepalived/jinja/jinja_cfg.py index 9b396fd3cf..dce571f9f5 100644 --- a/octavia/amphorae/drivers/keepalived/jinja/jinja_cfg.py +++ b/octavia/amphorae/drivers/keepalived/jinja/jinja_cfg.py @@ -60,9 +60,7 @@ class KeepalivedJinjaTemplater(object): :param loadbalancer: A loadbalancer object :param amphora: An amphora object - :param amp_net_config: The amphora network config, - an AmphoraeNetworkConfig object in amphorav1, - a dict in amphorav2 + :param amp_net_config: The amphora network config, a dict """ # Note on keepalived configuration: The current base configuration # enforced Master election whenever a high priority VRRP instance @@ -74,15 +72,8 @@ class KeepalivedJinjaTemplater(object): peers_ips = [] # Get the VIP subnet for the amphora - # For amphorav2 amphorae_network_config will be list of dicts - if isinstance(amp_net_config, dict): - additional_vip_data = amp_net_config['additional_vip_data'] - vip_subnet = amp_net_config[constants.VIP_SUBNET] - else: - additional_vip_data = [ - add_vip.to_dict(recurse=True) - for add_vip in amp_net_config.additional_vip_data] - vip_subnet = amp_net_config.vip_subnet.to_dict() + additional_vip_data = amp_net_config['additional_vip_data'] + vip_subnet = amp_net_config[constants.VIP_SUBNET] # Sort VIPs by their IP so we can guarantee interface_index matching sorted_add_vips = sorted(additional_vip_data, diff --git a/octavia/amphorae/drivers/noop_driver/driver.py b/octavia/amphorae/drivers/noop_driver/driver.py index 97ccbf0756..fd467a8586 100644 --- a/octavia/amphorae/drivers/noop_driver/driver.py +++ b/octavia/amphorae/drivers/noop_driver/driver.py @@ -93,8 +93,7 @@ class NoopManager(object): 'post_network_plug') def post_vip_plug(self, amphora, load_balancer, amphorae_network_config, - vrrp_port=None, vip_subnet=None, - additional_vip_data=None): + vrrp_port, vip_subnet, additional_vip_data=None): LOG.debug("Amphora %s no-op, post vip plug load balancer %s", self.__class__.__name__, load_balancer.id) self.amphoraconfig[(load_balancer.id, id(amphorae_network_config))] = ( @@ -167,12 +166,11 @@ class NoopAmphoraLoadBalancerDriver( self.driver.post_network_plug(amphora, port, amphora_network_config) def post_vip_plug(self, amphora, load_balancer, amphorae_network_config, - vrrp_port=None, vip_subnet=None, - additional_vip_data=None): + vrrp_port, vip_subnet, additional_vip_data=None): self.driver.post_vip_plug(amphora, load_balancer, amphorae_network_config, - vrrp_port=vrrp_port, vip_subnet=vip_subnet, + vrrp_port, vip_subnet, additional_vip_data=additional_vip_data) def upload_cert_amp(self, amphora, pem_file): diff --git a/octavia/controller/worker/v2/tasks/amphora_driver_tasks.py b/octavia/controller/worker/v2/tasks/amphora_driver_tasks.py index ee0359c9b1..c79d553dba 100644 --- a/octavia/controller/worker/v2/tasks/amphora_driver_tasks.py +++ b/octavia/controller/worker/v2/tasks/amphora_driver_tasks.py @@ -367,8 +367,8 @@ class AmphoraPostVIPPlug(BaseAmphoraTask): subnet=subnet)) self.amphora_driver.post_vip_plug( - db_amp, db_lb, amphorae_network_config, vrrp_port=vrrp_port, - vip_subnet=vip_subnet, additional_vip_data=additional_vip_data) + db_amp, db_lb, amphorae_network_config, vrrp_port, + vip_subnet, additional_vip_data=additional_vip_data) LOG.debug("Notified amphora of vip plug") def revert(self, result, amphora, loadbalancer, *args, **kwargs): diff --git a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py index ef286f1331..094d70c94f 100644 --- a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py +++ b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py @@ -123,7 +123,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): vip_subnet=network_models.Subnet( id=self.lb.vip.subnet_id, cidr=FAKE_VIP_SUBNET, - host_routes=[])) + host_routes=[])).to_dict(recurse=True) @mock.patch('octavia.amphorae.drivers.haproxy.rest_api_driver.' 'HaproxyAmphoraLoadBalancerDriver._process_secret') @@ -698,43 +698,64 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): self.assertIsNone(result) def test_post_vip_plug(self): - amphorae_network_config = mock.MagicMock() - amphorae_network_config.get().vip_subnet.cidr = FAKE_CIDR - amphorae_network_config.get().vip_subnet.gateway_ip = FAKE_GATEWAY - amphorae_network_config.get().vip_subnet.host_routes = self.host_routes - amphorae_network_config.get().vip_subnet.to_dict.return_value = { + vip_subnet = mock.MagicMock() + vip_subnet.cidr = FAKE_CIDR + vip_subnet.gateway_ip = FAKE_GATEWAY + vip_subnet.host_routes = self.host_routes + vip_subnet.to_dict.return_value = { 'cidr': FAKE_CIDR, 'gateway_ip': FAKE_GATEWAY, 'host_routes': [ hr.to_dict(recurse=True) for hr in self.host_routes] } + + amphorae_network_config = mock.MagicMock() + amphorae_network_config.get().vip_subnet = vip_subnet amphorae_network_config.get().vrrp_port = self.port - self.driver.post_vip_plug(self.amp, self.lb, amphorae_network_config) + self.driver.post_vip_plug(self.amp, self.lb, amphorae_network_config, + self.port, vip_subnet, + additional_vip_data=[]) self.driver.clients[API_VERSION].plug_vip.assert_called_once_with( self.amp, self.lb.vip.ip_address, self.subnet_info) def test_post_vip_plug_additional_vips(self): - amphorae_network_config = mock.MagicMock() - amphorae_network_config.get().vip_subnet.cidr = FAKE_CIDR - amphorae_network_config.get().vip_subnet.gateway_ip = FAKE_GATEWAY - amphorae_network_config.get().vip_subnet.host_routes = self.host_routes - amphorae_network_config.get().vip_subnet.to_dict.return_value = { + vip_subnet = mock.MagicMock() + vip_subnet.cidr = FAKE_CIDR + vip_subnet.gateway_ip = FAKE_GATEWAY + vip_subnet.host_routes = self.host_routes + vip_subnet.to_dict.return_value = { 'cidr': FAKE_CIDR, 'gateway_ip': FAKE_GATEWAY, 'host_routes': [ hr.to_dict(recurse=True) for hr in self.host_routes] } + + amphorae_network_config = mock.MagicMock() + amphorae_network_config.get().vip_subnet = vip_subnet amphorae_network_config.get().vrrp_port = self.port + + vip1_subnet = mock.MagicMock() + vip1_subnet.cidr = mock.Mock() + vip1_subnet.gateway_ip = mock.Mock() + vip1_subnet.host_routes = self.host_routes + vip1_subnet.to_dict.return_value = { + 'cidr': vip1_subnet.cidr, + 'gateway_ip': vip1_subnet.gateway_ip, + 'host_routes': [ + hr.to_dict(recurse=True) + for hr in self.host_routes] + } additional_vip1 = mock.MagicMock() additional_vip1.ip_address = mock.Mock() - additional_vip1.subnet.cidr = mock.Mock() - additional_vip1.subnet.gateway_ip = mock.Mock() - additional_vip1.subnet.host_routes = self.host_routes + additional_vip1.subnet = vip1_subnet additional_vip_data = [additional_vip1] - self.driver.post_vip_plug(self.amp, self.lb, amphorae_network_config, - additional_vip_data=additional_vip_data) + self.driver.post_vip_plug( + self.amp, self.lb, + amphorae_network_config, + self.port, vip_subnet, + additional_vip_data=additional_vip_data) netinfo = self.subnet_info.copy() netinfo['additional_vips'] = [ { diff --git a/octavia/tests/unit/amphorae/drivers/keepalived/jinja/test_jinja_cfg.py b/octavia/tests/unit/amphorae/drivers/keepalived/jinja/test_jinja_cfg.py index 15dd2e942b..14d4b90c9e 100644 --- a/octavia/tests/unit/amphorae/drivers/keepalived/jinja/test_jinja_cfg.py +++ b/octavia/tests/unit/amphorae/drivers/keepalived/jinja/test_jinja_cfg.py @@ -267,7 +267,7 @@ class TestVRRPRestDriver(base.TestCase): mock_subnet.gateway_ip = '10.1.0.1' mock_subnet.host_routes = [] amp_net_config = n_data_models.AmphoraNetworkConfig( - vip_subnet=mock_subnet) + vip_subnet=mock_subnet).to_dict(recurse=True) config = self.templater.build_keepalived_config( self.lb, self.amphora1, amp_net_config) @@ -279,7 +279,7 @@ class TestVRRPRestDriver(base.TestCase): mock_subnet.gateway_ip = '2001:db8::ff' mock_subnet.host_routes = [] amp_net_config = n_data_models.AmphoraNetworkConfig( - vip_subnet=mock_subnet) + vip_subnet=mock_subnet).to_dict(recurse=True) config = self.templater.build_keepalived_config( self.lbv6, self.amphora1v6, amp_net_config) @@ -302,7 +302,7 @@ class TestVRRPRestDriver(base.TestCase): ) amp_net_config = n_data_models.AmphoraNetworkConfig( vip_subnet=mock_subnet1, - additional_vip_data=[additional_vip]) + additional_vip_data=[additional_vip]).to_dict(recurse=True) config = self.templater.build_keepalived_config( self.lb, self.amphora1, amp_net_config) @@ -315,7 +315,7 @@ class TestVRRPRestDriver(base.TestCase): ) amp_net_config = n_data_models.AmphoraNetworkConfig( vip_subnet=mock_subnet2, - additional_vip_data=[additional_vip]) + additional_vip_data=[additional_vip]).to_dict(recurse=True) config = self.templater.build_keepalived_config( self.lbv6, self.amphora1v6, amp_net_config) diff --git a/octavia/tests/unit/amphorae/drivers/noop_driver/test_driver.py b/octavia/tests/unit/amphorae/drivers/noop_driver/test_driver.py index 8f05d05389..da0058414f 100644 --- a/octavia/tests/unit/amphorae/drivers/noop_driver/test_driver.py +++ b/octavia/tests/unit/amphorae/drivers/noop_driver/test_driver.py @@ -123,8 +123,11 @@ class TestNoopAmphoraLoadBalancerDriver(base.TestCase): self.amphora.id, self.port.id)]) def test_post_vip_plug(self): + port = network_models.Port(id=uuidutils.generate_uuid()) + subnet = network_models.Subnet(id=uuidutils.generate_uuid()) self.driver.post_vip_plug(self.amphora, self.load_balancer, - self.amphorae_net_configs) + self.amphorae_net_configs, + port, subnet) expected_method_and_args = (self.load_balancer.id, self.amphorae_net_configs, 'post_vip_plug') diff --git a/octavia/tests/unit/controller/worker/v2/tasks/test_amphora_driver_tasks.py b/octavia/tests/unit/controller/worker/v2/tasks/test_amphora_driver_tasks.py index 4fd0a947a2..9fcbd50d3f 100644 --- a/octavia/tests/unit/controller/worker/v2/tasks/test_amphora_driver_tasks.py +++ b/octavia/tests/unit/controller/worker/v2/tasks/test_amphora_driver_tasks.py @@ -560,8 +560,7 @@ class TestAmphoraDriverTasks(base.TestCase): mock_driver.post_vip_plug.assert_called_once_with( _db_amphora_mock, _db_load_balancer_mock, amphorae_net_config_mock, - vip_subnet=vip_subnet, vrrp_port=vrrp_port, - additional_vip_data=[]) + vrrp_port, vip_subnet, additional_vip_data=[]) # Test revert amp = amphora_post_vip_plug_obj.revert(None, _amphora_mock, _LB_mock) @@ -628,11 +627,10 @@ class TestAmphoraDriverTasks(base.TestCase): mock_driver.post_vip_plug.assert_called_once_with( _db_amphora_mock, _db_load_balancer_mock, amphorae_net_config_mock, - vip_subnet=vip_subnet, vrrp_port=vrrp_port, - additional_vip_data=[]) + vrrp_port, vip_subnet, additional_vip_data=[]) - call_kwargs = mock_driver.post_vip_plug.call_args[1] - vip_subnet_arg = call_kwargs.get(constants.VIP_SUBNET) + call_args = mock_driver.post_vip_plug.call_args[0] + vip_subnet_arg = call_args[4] self.assertEqual(2, len(vip_subnet_arg.host_routes)) for hr1, hr2 in zip(host_routes, vip_subnet_arg.host_routes): self.assertEqual(hr1['destination'], hr2.destination) @@ -691,11 +689,11 @@ class TestAmphoraDriverTasks(base.TestCase): mock_driver.post_vip_plug.assert_called_once_with( _db_amphora_mock, _db_load_balancer_mock, amphorae_net_config_mock, - vip_subnet=vip_subnet, vrrp_port=vrrp_port, - additional_vip_data=additional_vip_data) + vrrp_port, vip_subnet, additional_vip_data=additional_vip_data) + call_args = mock_driver.post_vip_plug.call_args[0] call_kwargs = mock_driver.post_vip_plug.call_args[1] - vip_subnet_arg = call_kwargs.get(constants.VIP_SUBNET) + vip_subnet_arg = call_args[4] self.assertEqual(2, len(vip_subnet_arg.host_routes)) for hr1, hr2 in zip(host_routes, vip_subnet_arg.host_routes): self.assertEqual(hr1['destination'], hr2.destination) @@ -741,8 +739,7 @@ class TestAmphoraDriverTasks(base.TestCase): mock_driver.post_vip_plug.assert_called_once_with( _db_amphora_mock, _db_load_balancer_mock, amphorae_net_config_mock, - vip_subnet=vip_subnet, vrrp_port=vrrp_port, - additional_vip_data=[]) + vrrp_port, vip_subnet, additional_vip_data=[]) def test_amphora_cert_upload(self, mock_driver,