openstack/kuryr-kubernetes
Code Issues Proposed changes

Make CNI Registry Plugin namespace aware

Browse Source 
As with the k8sCNIRegistryPlugin the watching is for the
complete node, instead of per pod and namespace, we need
to make registry information to account for the namespace
where the pod is created to differentiate between different
containers running on the same node, with the same name, but
in a different namespace

Related-Bug: 1731486
Change-Id: I26e1dec6ae613c5316a45f93563c4a015df59441
This commit is contained in:
Luis Tomas Bolivar 2018-02-27 14:46:40 +00:00
parent bcacff60c9
commit 7061f4abac
4 changed files with 40 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
kuryr_kubernetes/cni/daemon/service.py
View File

@ -36,11 +36,12 @@ from kuryr_kubernetes import clients
from kuryr_kubernetes.cni import handlers as h_cni
from kuryr_kubernetes.cni import health
from kuryr_kubernetes.cni.plugins import k8s_cni_registry
from kuryr_kubernetes.cni import utils
from kuryr_kubernetes.cni import utils as cni_utils
from kuryr_kubernetes import config
from kuryr_kubernetes import constants as k_const
from kuryr_kubernetes import exceptions
from kuryr_kubernetes import objects
from kuryr_kubernetes import utils
from kuryr_kubernetes import watcher as k_watcher
LOG = logging.getLogger(__name__)
@ -63,7 +64,7 @@ class DaemonServer(object):
'Connection': 'close'}
def _prepare_request(self):
params = utils.CNIParameters(flask.request.get_json())
params = cni_utils.CNIParameters(flask.request.get_json())
LOG.debug('Received %s request. CNI Params: %s',
params.CNI_COMMAND, params)
return params
@ -209,7 +210,7 @@ class CNIDaemonWatcherService(cotyledon.Service):
time.sleep(HEALTH_CHECKER_DELAY)
def on_done(self, pod, vif):
pod_name = pod['metadata']['name']
pod_name = utils.get_pod_unique_name(pod)
vif_dict = vif.obj_to_primitive()
# NOTE(dulek): We need a lock when modifying shared self.registry dict
# to prevent race conditions with other processes/threads.
@ -228,7 +229,7 @@ class CNIDaemonWatcherService(cotyledon.Service):
self.registry[pod_name] = pod_dict
def on_deleted(self, pod):
pod_name = pod['metadata']['name']
pod_name = utils.get_pod_unique_name(pod)
try:
if pod_name in self.registry:
# NOTE(dulek): del on dict is atomic as long as we use standard

13
kuryr_kubernetes/cni/plugins/k8s_cni_registry.py
View File

@ -41,13 +41,16 @@ class K8sCNIRegistryPlugin(base_cni.CNIPlugin):
self.healthy = healthy
self.registry = registry
def _get_name(self, pod):
return pod['metadata']['name']
def _get_pod_name(self, params):
return "%(namespace)s/%(name)s" % {
'namespace': params.args.K8S_POD_NAMESPACE,
'name': params.args.K8S_POD_NAME}
def add(self, params):
vif = self._do_work(params, b_base.connect)
pod_name = params.args.K8S_POD_NAME
pod_name = self._get_pod_name(params)
# NOTE(dulek): Saving containerid to be able to distinguish old DEL
# requests that we should ignore. We need a lock to
# prevent race conditions and replace whole object in the
@ -76,7 +79,7 @@ class K8sCNIRegistryPlugin(base_cni.CNIPlugin):
return vif
def delete(self, params):
pod_name = params.args.K8S_POD_NAME
pod_name = self._get_pod_name(params)
try:
reg_ci = self.registry[pod_name]['containerid']
LOG.debug('Read containerid = %s for pod %s', reg_ci, pod_name)
@ -98,7 +101,7 @@ class K8sCNIRegistryPlugin(base_cni.CNIPlugin):
self.healthy.value = driver_healthy
def _do_work(self, params, fn):
pod_name = params.args.K8S_POD_NAME
pod_name = self._get_pod_name(params)
timeout = CONF.cni_daemon.vif_annotation_timeout

28
kuryr_kubernetes/tests/unit/cni/plugins/test_k8s_cni_registry.py
View File

@ -25,13 +25,15 @@ from kuryr_kubernetes.tests import fake
class TestK8sCNIRegistryPlugin(base.TestCase):
def setUp(self):
super(TestK8sCNIRegistryPlugin, self).setUp()
self.pod = {'metadata': {'name': 'foo', 'uid': 'bar'}}
self.pod = {'metadata': {'name': 'foo', 'uid': 'bar',
'namespace': 'default'}}
self.vif = fake._fake_vif_dict()
registry = {'foo': {'pod': self.pod, 'vif': self.vif,
'containerid': None}}
registry = {'default/foo': {'pod': self.pod, 'vif': self.vif,
'containerid': None}}
healthy = mock.Mock()
self.plugin = k8s_cni_registry.K8sCNIRegistryPlugin(registry, healthy)
self.params = mock.Mock(args=mock.Mock(K8S_POD_NAME='foo'),
self.params = mock.Mock(args=mock.Mock(K8S_POD_NAME='foo',
K8S_POD_NAMESPACE='default'),
CNI_IFNAME='baz', CNI_NETNS=123,
CNI_CONTAINERID='cont_id')
@ -40,9 +42,10 @@ class TestK8sCNIRegistryPlugin(base.TestCase):
def test_add_present(self, m_connect, m_lock):
self.plugin.add(self.params)
m_lock.assert_called_with('foo', external=True)
m_lock.assert_called_with('default/foo', external=True)
m_connect.assert_called_with(mock.ANY, mock.ANY, 'baz', 123, mock.ANY)
self.assertEqual('cont_id', self.plugin.registry['foo']['containerid'])
self.assertEqual('cont_id',
self.plugin.registry['default/foo']['containerid'])
@mock.patch('kuryr_kubernetes.cni.binding.base.disconnect')
def test_del_present(self, m_disconnect):
@ -53,8 +56,8 @@ class TestK8sCNIRegistryPlugin(base.TestCase):
@mock.patch('kuryr_kubernetes.cni.binding.base.disconnect')
def test_del_wrong_container_id(self, m_disconnect):
registry = {'foo': {'pod': self.pod, 'vif': self.vif,
'containerid': 'different'}}
registry = {'default/foo': {'pod': self.pod, 'vif': self.vif,
'containerid': 'different'}}
healthy = mock.Mock()
self.plugin = k8s_cni_registry.K8sCNIRegistryPlugin(registry, healthy)
self.plugin.delete(self.params)
@ -75,10 +78,11 @@ class TestK8sCNIRegistryPlugin(base.TestCase):
self.plugin.registry = m_registry
self.plugin.add(self.params)
m_lock.assert_called_with('foo', external=True)
m_setitem.assert_called_once_with('foo', {'pod': self.pod,
'vif': self.vif,
'containerid': 'cont_id'})
m_lock.assert_called_with('default/foo', external=True)
m_setitem.assert_called_once_with('default/foo',
{'pod': self.pod,
'vif': self.vif,
'containerid': 'cont_id'})
m_connect.assert_called_with(mock.ANY, mock.ANY, 'baz', 123, mock.ANY)
@mock.patch('time.sleep', mock.Mock())

11
kuryr_kubernetes/utils.py
View File

@ -39,3 +39,14 @@ def convert_netns(netns):
return netns.replace('/proc', CONF.cni_daemon.netns_proc_dir)
else:
return netns
def get_pod_unique_name(pod):
"""Returns a unique name for the pod.
It returns a pod unique name for the pod composed of its name and the
namespace it is running on.
:returns: String with namespace/name of the pod
"""
return "%(namespace)s/%(name)s" % pod['metadata']