openstack
/
kuryr-kubernetes
Code Issues Proposed changes

Cleanup minor thing for KuryrPort feature. 

There was outstanding review comments for some minor things. In this
patch they are resolved.

Change-Id: If950900293cae1d1b0c57492d5dfa62770c08a7c
This commit is contained in:
Roman Dobosz 2020-07-30 16:06:35 +02:00 committed by Michał Dulko
parent 1aa6753d80
commit ebf55415b6
5 changed files with 24 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
kuryr_kubernetes/controller/handlers/kuryrport.py
View File

@ -135,10 +135,6 @@ class KuryrPortHandler(k8s_base.ResourceEventHandler):
self.k8s.remove_finalizer(kuryrport_crd, constants.POD_FINALIZER)
raise
if (driver_utils.is_host_network(pod) or
not pod['spec'].get('nodeName')):
return
project_id = self._drv_project.get_project(pod)
try:
crd_pod_selectors = self._drv_sg.delete_sg_rules(pod)

6
kuryr_kubernetes/controller/handlers/pod_label.py
View File

@ -52,12 +52,6 @@ class PodLabelHandler(k8s_base.ResourceEventHandler):
# annotates the pod with the pod state.
return
if (constants.K8S_ANNOTATION_VIF in
pod['metadata'].get('annotations', {})):
# NOTE(dulek): This might happen on upgrade, we need to wait for
# annotation to be moved to KuryrPort CRD.
return
current_pod_info = (pod['metadata'].get('labels'),
pod['status'].get('podIP'))
previous_pod_info = self._get_pod_info(pod)

16
kuryr_kubernetes/controller/handlers/vif.py
View File

@ -63,6 +63,14 @@ class VIFHandler(k8s_base.ResourceEventHandler):
drivers.ServiceSecurityGroupsDriver.get_instance())
def on_present(self, pod):
if (driver_utils.is_host_network(pod) or
not self._is_pod_scheduled(pod)):
# REVISIT(ivc): consider an additional configurable check that
# would allow skipping pods to enable heterogeneous environments
# where certain pods/namespaces/nodes can be managed by other
# networking solutions/CNI drivers.
return
# NOTE(gryf): Set the finalizer as soon, as we have pod created. On
# subsequent updates of the pod, add_finalizer will ignore this if
# finalizer exists.
@ -82,14 +90,6 @@ class VIFHandler(k8s_base.ResourceEventHandler):
" Skipping")
return
if (driver_utils.is_host_network(pod) or
not self._is_pod_scheduled(pod)):
# REVISIT(ivc): consider an additional configurable check that
# would allow skipping pods to enable heterogeneous environments
# where certain pods/namespaces/nodes can be managed by other
# networking solutions/CNI drivers.
return
LOG.debug("Got KuryrPort: %r", kp)
if not kp:
try:

52
kuryr_kubernetes/tests/unit/controller/handlers/test_kuryrport.py
View File

@ -345,38 +345,6 @@ class TestKuryrPortHandler(test_base.TestCase):
k8s.remove_finalizer.assert_called_once_with(
self._kp, constants.POD_FINALIZER)
@mock.patch('kuryr_kubernetes.controller.drivers.utils.is_host_network')
@mock.patch('kuryr_kubernetes.clients.get_kubernetes_client')
@mock.patch('kuryr_kubernetes.controller.drivers.base.MultiVIFDriver.'
'get_enabled_drivers')
def test_on_finalize_host_net_or_no_nodename(self, ged, k8s,
is_host_network):
ged.return_value = [self._driver]
kp = kuryrport.KuryrPortHandler()
self._kp['status']['vifs'] = self._vifs_primitive
is_host_network.return_value = False
_pod = dict(self._pod)
del _pod['spec']['nodeName']
with mock.patch.object(kp, 'k8s') as k8s:
k8s.get.return_value = _pod
kp.on_finalize(self._kp)
k8s.get.assert_called_once_with(self._pod_uri)
is_host_network.assert_called_once_with(self._pod)
is_host_network.reset_mock()
is_host_network.return_value = False
with mock.patch.object(kp, 'k8s') as k8s:
k8s.get.return_value = self._pod
kp.on_finalize(self._kp)
k8s.get.assert_called_once_with(self._pod_uri)
is_host_network.assert_called_once_with(self._pod)
@mock.patch('kuryr_kubernetes.controller.drivers.vif_pool.MultiVIFPool.'
'release_vif')
@mock.patch('kuryr_kubernetes.controller.drivers.default_security_groups.'
@ -385,17 +353,15 @@ class TestKuryrPortHandler(test_base.TestCase):
'DefaultPodSecurityGroupsDriver.delete_sg_rules')
@mock.patch('kuryr_kubernetes.controller.drivers.default_project.'
'DefaultPodProjectDriver.get_project')
@mock.patch('kuryr_kubernetes.controller.drivers.utils.is_host_network')
@mock.patch('kuryr_kubernetes.clients.get_kubernetes_client')
@mock.patch('kuryr_kubernetes.controller.drivers.base.MultiVIFDriver.'
'get_enabled_drivers')
def test_on_finalize_crd_sg_exceptions(self, ged, k8s, is_host_network,
get_project, delete_sg_rules,
get_sg, release_vif):
def test_on_finalize_crd_sg_exceptions(self, ged, k8s, get_project,
delete_sg_rules, get_sg,
release_vif):
ged.return_value = [self._driver]
kp = kuryrport.KuryrPortHandler()
self._kp['status']['vifs'] = self._vifs_primitive
is_host_network.return_value = False
get_project.return_value = self._project_id
delete_sg_rules.side_effect = k_exc.ResourceNotReady(self._pod)
get_sg.side_effect = k_exc.ResourceNotReady(self._pod)
@ -409,7 +375,6 @@ class TestKuryrPortHandler(test_base.TestCase):
k8s.remove_finalizer.assert_has_calls(
[mock.call(self._pod, constants.POD_FINALIZER),
mock.call(self._kp, constants.KURYRPORT_FINALIZER)])
is_host_network.assert_called_once_with(self._pod)
delete_sg_rules.assert_called_once_with(self._pod)
get_sg.assert_called_once_with(self._pod, self._project_id)
release_vif.assert_has_calls([mock.call(self._pod, self._vif1,
@ -434,21 +399,19 @@ class TestKuryrPortHandler(test_base.TestCase):
'DefaultPodSecurityGroupsDriver.delete_sg_rules')
@mock.patch('kuryr_kubernetes.controller.drivers.default_project.'
'DefaultPodProjectDriver.get_project')
@mock.patch('kuryr_kubernetes.controller.drivers.utils.is_host_network')
@mock.patch('kuryr_kubernetes.clients.get_kubernetes_client')
@mock.patch('kuryr_kubernetes.controller.drivers.base.MultiVIFDriver.'
'get_enabled_drivers')
def test_on_finalize_np(self, ged, k8s, is_host_network, get_project,
delete_sg_rules, get_sg, release_vif,
is_np_enabled, get_lb_instance, get_sg_instance,
get_services, update_services):
def test_on_finalize_np(self, ged, k8s, get_project, delete_sg_rules,
get_sg, release_vif, is_np_enabled,
get_lb_instance, get_sg_instance, get_services,
update_services):
ged.return_value = [self._driver]
CONF.set_override('enforce_sg_rules', True, group='octavia_defaults')
self.addCleanup(CONF.clear_override, 'enforce_sg_rules',
group='octavia_defaults')
kp = kuryrport.KuryrPortHandler()
self._kp['status']['vifs'] = self._vifs_primitive
is_host_network.return_value = False
get_project.return_value = self._project_id
selector = mock.sentinel.selector
delete_sg_rules.return_value = selector
@ -465,7 +428,6 @@ class TestKuryrPortHandler(test_base.TestCase):
[mock.call(self._pod, constants.POD_FINALIZER),
mock.call(self._kp, constants.KURYRPORT_FINALIZER)])
is_host_network.assert_called_once_with(self._pod)
delete_sg_rules.assert_called_once_with(self._pod)
get_sg.assert_called_once_with(self._pod, self._project_id)
release_vif.assert_has_calls([mock.call(self._pod, self._vif1,

20
kuryr_kubernetes/tests/unit/controller/handlers/test_vif.py
View File

@ -181,10 +181,9 @@ class TestVIFHandler(test_base.TestCase):
h_vif.VIFHandler.on_present(self._handler, self._pod)
k8s.add_finalizer.assert_called_once_with(self._pod,
k_const.POD_FINALIZER)
self._matc.assert_called_once_with(self._pod)
m_get_kuryrport.assert_called_once()
k8s.add_finalizer.assert_not_called()
self._matc.assert_not_called()
m_get_kuryrport.assert_not_called()
self._request_vif.assert_not_called()
self._request_additional_vifs.assert_not_called()
self._activate_vif.assert_not_called()
@ -192,8 +191,8 @@ class TestVIFHandler(test_base.TestCase):
@mock.patch('kuryr_kubernetes.clients.get_kubernetes_client')
@mock.patch('kuryr_kubernetes.controller.drivers.utils.is_host_network')
@mock.patch('kuryr_kubernetes.controller.drivers.utils.get_kuryrport')
def test_on_present_not_pending(self, m_get_kuryrport, m_host_network,
m_get_k8s_client):
def test_on_present_not_scheduled(self, m_get_kuryrport, m_host_network,
m_get_k8s_client):
m_get_kuryrport.return_value = self._kp
m_host_network.return_value = False
self._is_pod_scheduled.return_value = False
@ -203,10 +202,9 @@ class TestVIFHandler(test_base.TestCase):
h_vif.VIFHandler.on_present(self._handler, self._pod)
k8s.add_finalizer.assert_called_once_with(self._pod,
k_const.POD_FINALIZER)
self._matc.assert_called_once_with(self._pod)
m_get_kuryrport.assert_called_once()
k8s.add_finalizer.assert_not_called()
self._matc.assert_not_called()
m_get_kuryrport.assert_not_called()
self._request_vif.assert_not_called()
self._request_additional_vifs.assert_not_called()
self._activate_vif.assert_not_called()
@ -295,7 +293,7 @@ class TestVIFHandler(test_base.TestCase):
def test_on_present_upgrade(self, m_get_kuryrport, m_host_network,
m_get_k8s_client):
m_get_kuryrport.return_value = self._kp
m_host_network.return_value = True
m_host_network.return_value = False
self._matc.return_value = True
k8s = mock.MagicMock()
m_get_k8s_client.return_value = k8s