openstack
/
kuryr-kubernetes
Code Issues Proposed changes

Fixes race cond. during updates of neutron resources 

When macvlan driver updates neutron resources during pods creation
in short period of time, only last address pair remains in Neutron.
We need to utilize revision_number feature of neutron api, to
detect such race condition and attempt to resend updated api request
in such case.

Co-Authored-By: Michał Dulko <mdulko@redhat.com>
Closes-Bug: 1845307
Change-Id: I01c544cc284f4e7bc339ac4f422e90a0c6cb31fa
(cherry picked from commit 3208b192ad)
This commit is contained in:
Kamil Madac 2019-04-24 12:08:20 +02:00 committed by Michał Dulko
parent 8567c9a951
commit 2b56458959
3 changed files with 140 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
kuryr_kubernetes/config.py
View File

@ -224,7 +224,7 @@ neutron_defaults = [
"This can be used to identify and garbage-collect "
"them when Kubernetes cluster Kuryr was serving is no "
"longer needed."),
default=[])
default=[]),
]
octavia_defaults = [
@ -262,6 +262,10 @@ nested_vif_driver_opts = [
cfg.StrOpt('worker_nodes_subnet',
help=_("Neutron subnet ID for k8s worker node vms."),
default=''),
cfg.IntOpt('rev_update_attempts',
help=_("How many time to try to re-update the neutron resource "
"when revision has been changed by other thread"),
default=3),
]
DEFAULT_PHYSNET_SUBNET_MAPPINGS = {}

81
kuryr_kubernetes/controller/drivers/nested_macvlan_vif.py
View File

@ -18,6 +18,7 @@ from neutronclient.common import exceptions as n_exc
from oslo_log import log as logging
from kuryr_kubernetes import clients
from kuryr_kubernetes import config as kuryr_config
from kuryr_kubernetes.controller.drivers import nested_vif
from kuryr_kubernetes.controller.drivers import utils
from kuryr_kubernetes import exceptions as k_exc
@ -36,17 +37,22 @@ class NestedMacvlanPodVIFDriver(nested_vif.NestedPodVIFDriver):
neutron = clients.get_neutron_client()
req = self._get_port_request(pod, project_id, subnets,
security_groups)
vm_port = self._get_parent_port(neutron, pod)
container_port = neutron.create_port(req).get('port')
utils.tag_neutron_resources('ports', [container_port['id']])
attempts = kuryr_config.CONF.pod_vif_nested.rev_update_attempts
container_port = None
while attempts > 0:
vm_port = self._get_parent_port(neutron, pod)
container_mac = container_port['mac_address']
container_ips = frozenset(entry['ip_address'] for entry in
container_port['fixed_ips'])
if not container_port:
container_port = neutron.create_port(req).get('port')
utils.tag_neutron_resources('ports', [container_port['id']])
with self.lock:
self._add_to_allowed_address_pairs(neutron, vm_port,
container_ips, container_mac)
container_mac = container_port['mac_address']
container_ips = frozenset(entry['ip_address'] for entry in
container_port['fixed_ips'])
attempts = self._try_update_port(
attempts, self._add_to_allowed_address_pairs, neutron, vm_port,
container_ips, container_mac)
return ovu.neutron_to_osvif_vif_nested_macvlan(container_port, subnets)
@ -57,17 +63,18 @@ class NestedMacvlanPodVIFDriver(nested_vif.NestedPodVIFDriver):
def release_vif(self, pod, vif, project_id=None, security_groups=None):
neutron = clients.get_neutron_client()
container_port = neutron.show_port(vif.id).get('port')
container_mac = container_port['mac_address']
container_ips = frozenset(entry['ip_address'] for entry in
container_port['fixed_ips'])
attempts = kuryr_config.CONF.pod_vif_nested.rev_update_attempts
while attempts > 0:
container_port = neutron.show_port(vif.id).get('port')
with self.lock:
container_mac = container_port['mac_address']
container_ips = frozenset(entry['ip_address'] for entry in
container_port['fixed_ips'])
vm_port = self._get_parent_port(neutron, pod)
self._remove_from_allowed_address_pairs(neutron, vm_port,
container_ips,
container_mac)
attempts = self._try_update_port(
attempts, self._remove_from_allowed_address_pairs, neutron,
vm_port, container_ips, container_mac)
try:
neutron.delete_port(vif.id)
@ -112,7 +119,12 @@ class NestedMacvlanPodVIFDriver(nested_vif.NestedPodVIFDriver):
for ip in ip_addresses:
address_pairs.append({'ip_address': ip, 'mac_address': mac})
self._update_port_address_pairs(neutron, port['id'], address_pairs)
self._update_port_address_pairs(
neutron, port['id'], address_pairs,
revision_number=port['revision_number'])
LOG.debug("Added allowed_address_pair %s %s" %
(str(ip_addresses,), mac_address))
def _remove_from_allowed_address_pairs(self, neutron, port, ip_addresses,
mac_address=None):
@ -136,15 +148,30 @@ class NestedMacvlanPodVIFDriver(nested_vif.NestedPodVIFDriver):
"trying to remove it.", ip, mac)
if updated:
self._update_port_address_pairs(neutron, port['id'], address_pairs)
self._update_port_address_pairs(
neutron, port['id'],
address_pairs,
revision_number=port['revision_number'])
def _update_port_address_pairs(self, neutron, port_id, address_pairs):
def _update_port_address_pairs(self, neutron, port_id, address_pairs,
revision_number=None):
neutron.update_port(
port_id,
{'port': {'allowed_address_pairs': address_pairs}},
revision_number=revision_number
)
def _try_update_port(self, attempts, f,
neutron, vm_port, container_ips, container_mac):
try:
neutron.update_port(
port_id,
{'port': {'allowed_address_pairs': address_pairs}}
)
with self.lock:
f(neutron, vm_port, container_ips, container_mac)
attempts = 0
except n_exc.NeutronClientException:
LOG.exception("Error happened during updating Neutron port %s",
port_id)
raise
attempts -= 1
if attempts == 0:
LOG.exception("Error happened during updating port %s",
vm_port['id'] if vm_port else None)
raise
return attempts

100
kuryr_kubernetes/tests/unit/controller/drivers/test_nested_macvlan_vif.py
View File

@ -49,6 +49,7 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
m_to_vif.return_value = vif
m_driver._get_port_request.return_value = port_request
m_driver._get_parent_port.return_value = vm_port
m_driver._try_update_port.return_value = 0
m_driver.lock = mock.MagicMock(spec=threading.Lock())
neutron.create_port.return_value = container_port
@ -59,8 +60,7 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
pod, project_id, subnets, security_groups)
neutron.create_port.assert_called_once_with(port_request)
m_driver._get_parent_port.assert_called_once_with(neutron, pod)
m_driver._add_to_allowed_address_pairs.assert_called_once_with(
neutron, vm_port, frozenset([container_ip]), container_mac)
m_driver._try_update_port.assert_called_once()
m_to_vif.assert_called_once_with(container_port['port'], subnets)
@mock.patch(
@ -84,7 +84,7 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
m_driver._get_port_request.assert_called_once_with(
pod, project_id, subnets, security_groups)
neutron.create_port.assert_called_once_with(port_request)
m_driver._add_to_allowed_address_pairs.assert_not_called()
m_driver._try_update_port.assert_not_called()
m_to_vif.assert_not_called()
@mock.patch(
@ -115,7 +115,7 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
pod, project_id, subnets, security_groups)
neutron.create_port.assert_not_called()
m_driver._get_parent_port.assert_called_once_with(neutron, pod)
m_driver._add_to_allowed_address_pairs.assert_not_called()
m_driver._try_update_port.assert_not_called()
m_to_vif.assert_not_called()
def test_release_vif(self):
@ -136,14 +136,14 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
vm_port = self._get_fake_port()
m_driver._get_parent_port.return_value = vm_port
m_driver._try_update_port.return_value = 0
m_driver.lock = mock.MagicMock(spec=threading.Lock())
cls.release_vif(m_driver, pod, vif)
neutron.show_port.assert_called_once_with(port_id)
m_driver._get_parent_port.assert_called_once_with(neutron, pod)
m_driver._remove_from_allowed_address_pairs.assert_called_once_with(
neutron, vm_port, frozenset([container_ip]), container_mac)
m_driver._try_update_port.assert_called_once()
neutron.delete_port.assert_called_once_with(vif.id)
def test_release_vif_not_found(self):
@ -183,8 +183,10 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
self.assertRaises(n_exc.NeutronClientException, cls.release_vif,
m_driver, pod, vif)
neutron.show_port.assert_called_once_with(port_id)
m_driver._get_parent_port.assert_called_once_with(neutron, pod)
neutron.show_port.assert_called_with(port_id)
self.assertEqual(neutron.show_port.call_count, 1)
m_driver._get_parent_port.assert_called_with(neutron, pod)
self.assertEqual(m_driver._get_parent_port.call_count, 1)
m_driver._remove_from_allowed_address_pairs.assert_not_called()
neutron.delete_port.assert_not_called()
@ -207,14 +209,14 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
vm_port = self._get_fake_port()
m_driver._get_parent_port.return_value = vm_port
m_driver._try_update_port.return_value = 0
m_driver.lock = mock.MagicMock(spec=threading.Lock())
cls.release_vif(m_driver, pod, vif)
neutron.show_port.assert_called_once_with(port_id)
m_driver._get_parent_port.assert_called_once_with(neutron, pod)
m_driver._remove_from_allowed_address_pairs.assert_called_once_with(
neutron, vm_port, frozenset([container_ip]), container_mac)
m_driver._try_update_port.assert_called_once()
neutron.delete_port.assert_called_once_with(vif.id)
@ddt.data((False), (True))
@ -257,7 +259,7 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
frozenset([ip_addr]), m_mac)
m_driver._update_port_address_pairs.assert_called_once_with(
neutron, port_id, address_pairs)
neutron, port_id, address_pairs, revision_number=1)
def test_add_to_allowed_address_pairs_no_ip_addresses(self):
cls = nested_macvlan_vif.NestedMacvlanPodVIFDriver
@ -294,7 +296,7 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
frozenset([ip_addr]), mac_addr)
m_driver._update_port_address_pairs.assert_called_once_with(
neutron, port_id, address_pairs)
neutron, port_id, address_pairs, revision_number=1)
def test_add_to_allowed_address_pairs_already_present(self):
cls = nested_macvlan_vif.NestedMacvlanPodVIFDriver
@ -346,7 +348,7 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
m_driver, neutron, vm_port, frozenset([ip_addr]), m_mac)
m_driver._update_port_address_pairs.assert_called_once_with(
neutron, port_id, address_pairs)
neutron, port_id, address_pairs, revision_number=1)
def test_remove_from_allowed_address_pairs_no_ip_addresses(self):
cls = nested_macvlan_vif.NestedMacvlanPodVIFDriver
@ -386,7 +388,7 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
m_driver, neutron, vm_port, frozenset(ip_addr), m_mac)
m_driver._update_port_address_pairs.assert_called_once_with(
neutron, port_id, address_pairs)
neutron, port_id, address_pairs, revision_number=1)
@ddt.data((None), ('fa:16:3e:71:cb:80'))
def test_remove_from_allowed_address_pairs_no_update(self, m_mac):
@ -421,11 +423,13 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
port_id = lib_utils.get_hash()
pairs = mock.sentinel.allowed_address_pairs
cls._update_port_address_pairs(m_driver, neutron, port_id, pairs)
cls._update_port_address_pairs(m_driver, neutron, port_id, pairs,
revision_number=1)
neutron.update_port.assert_called_with(
port_id,
{'port': {'allowed_address_pairs': pairs}})
{'port': {'allowed_address_pairs': pairs}},
revision_number=1)
def test_update_port_address_pairs_failure(self):
cls = nested_macvlan_vif.NestedMacvlanPodVIFDriver
@ -438,11 +442,68 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
self.assertRaises(n_exc.NeutronClientException,
cls._update_port_address_pairs, m_driver, neutron,
port_id, pairs)
port_id, pairs, revision_number=1)
neutron.update_port.assert_called_with(
port_id,
{'port': {'allowed_address_pairs': pairs}})
{'port': {'allowed_address_pairs': pairs}},
revision_number=1)
@mock.patch('kuryr_kubernetes.controller.drivers.nested_macvlan_vif.'
'NestedMacvlanPodVIFDriver._add_to_allowed_address_pairs')
def test_try_update_port(self, aaapf_mock):
cls = nested_macvlan_vif.NestedMacvlanPodVIFDriver
m_driver = mock.Mock(spec=cls)
m_driver.lock = mock.MagicMock(spec=threading.Lock())
neutron = self.useFixture(k_fix.MockNeutronClient()).client
port_id = lib_utils.get_hash()
vm_port = self._get_fake_port(port_id)['port']
mac_addr = 'fa:16:3e:1b:30:00'
address_pairs = [
{'ip_address': '10.0.0.30',
'mac_address': mac_addr},
{'ip_address': 'fe80::f816:3eff:fe1c:36a9',
'mac_address': mac_addr},
]
vm_port['allowed_address_pairs'].extend(address_pairs)
ip_addr = ['10.0.0.29']
attempts = cls._try_update_port(m_driver, 3,
cls._add_to_allowed_address_pairs,
neutron, vm_port, frozenset(ip_addr),
mac_addr)
self.assertEqual(attempts, 0)
aaapf_mock.assert_called_once()
@mock.patch('kuryr_kubernetes.controller.drivers.nested_macvlan_vif.'
'NestedMacvlanPodVIFDriver._add_to_allowed_address_pairs')
def test_try_update_port_failure(self, aaapf_mock):
cls = nested_macvlan_vif.NestedMacvlanPodVIFDriver
m_driver = mock.Mock(spec=cls)
m_driver.lock = mock.MagicMock(spec=threading.Lock())
neutron = self.useFixture(k_fix.MockNeutronClient()).client
port_id = lib_utils.get_hash()
vm_port = self._get_fake_port(port_id)['port']
mac_addr = 'fa:16:3e:1b:30:00'
address_pairs = [
{'ip_address': '10.0.0.30',
'mac_address': mac_addr},
{'ip_address': 'fe80::f816:3eff:fe1c:36a9',
'mac_address': mac_addr},
]
vm_port['allowed_address_pairs'].extend(address_pairs)
ip_addr = ['10.0.0.29']
aaapf_mock.side_effect = n_exc.NeutronClientException
self.assertRaises(n_exc.NeutronClientException,
cls._try_update_port, m_driver, 1,
cls._add_to_allowed_address_pairs,
neutron, vm_port, frozenset(ip_addr), mac_addr)
# TODO(garyloug) consider exending and moving to a parent class
def _get_fake_port(self, port_id=None, ip_address=None, mac_address=None):
@ -451,7 +512,8 @@ class TestNestedMacvlanPodVIFDriver(test_base.TestCase):
"mac_address": "fa:16:3e:20:57:c4",
"fixed_ips": [],
"id": "07b21ebf-b105-4720-9f2e-95670c4032e4",
"allowed_address_pairs": []
"allowed_address_pairs": [],
"revision_number": 1
}
}