From 7cc187806b42fee5ea660f86d33ad2f59b009754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dulko?= Date: Thu, 19 Jul 2018 12:47:06 +0200 Subject: [PATCH] Change Pod annotations format to o.vo With start of multi-vif support we've merged a new format of Pods annotations i.e. dictionary with interface names as keys and VIF o.vo's as values. This is inflexible, as we don't get versioning of the dictionary. This commit switches that dictionary to oslo.versionedobject of following structure: PodState - default_vif: VIFBase - additional_vifs: { '': VIFBase, ... '': VIFBase, } This should help us if we ever decide to change this format. Related-Bug: 1782366 Depends-On: Ied6b5883cf4bfe6a499813c65b49257de040c4b5 Change-Id: I5bce85029f9edfed56ca216f465226b9bcc0d21b --- contrib/kubectl_plugins/kuryr/kuryr | 5 +- kuryr_kubernetes/cni/handlers.py | 11 ++- .../cni/plugins/k8s_cni_registry.py | 2 +- kuryr_kubernetes/controller/handlers/vif.py | 72 +++++++++--------- kuryr_kubernetes/objects/fields.py | 6 ++ kuryr_kubernetes/objects/vif.py | 28 +++++++ .../unit/controller/handlers/test_vif.py | 74 +++++++++---------- 7 files changed, 118 insertions(+), 80 deletions(-) diff --git a/contrib/kubectl_plugins/kuryr/kuryr b/contrib/kubectl_plugins/kuryr/kuryr index af90f5287..9bd1dfb3c 100755 --- a/contrib/kubectl_plugins/kuryr/kuryr +++ b/contrib/kubectl_plugins/kuryr/kuryr @@ -132,7 +132,10 @@ def vifs(session, server, namespace, args): else: vif = json.loads(vif) - vif = vif.get('eth0', vif) + if vif['versioned_object.name'] == 'PodState': + # This is new format, fetch only default_vif from there. + vif = vif['versioned_object.data']['default_vif'] + network = (vif['versioned_object.data']['network'] ['versioned_object.data']) first_subnet = (network['subnets']['versioned_object.data'] diff --git a/kuryr_kubernetes/cni/handlers.py b/kuryr_kubernetes/cni/handlers.py index 258b97517..c4f439917 100644 --- a/kuryr_kubernetes/cni/handlers.py +++ b/kuryr_kubernetes/cni/handlers.py @@ -71,14 +71,13 @@ class CNIHandlerBase(k8s_base.ResourceEventHandler): # TODO(ivc): same as VIFHandler._get_vif try: annotations = pod['metadata']['annotations'] - vifs_annotation = annotations[k_const.K8S_ANNOTATION_VIF] + state_annotation = annotations[k_const.K8S_ANNOTATION_VIF] except KeyError: return {} - vifs_annotation = jsonutils.loads(vifs_annotation) - vifs_dict = { - ifname: obj_vif.vif.VIFBase.obj_from_primitive(vif) - for ifname, vif in vifs_annotation.items() - } + state_annotation = jsonutils.loads(state_annotation) + state = obj_vif.base.VersionedObject.obj_from_primitive( + state_annotation) + vifs_dict = state.vifs LOG.debug("Got VIFs from annotation: %r", vifs_dict) return vifs_dict diff --git a/kuryr_kubernetes/cni/plugins/k8s_cni_registry.py b/kuryr_kubernetes/cni/plugins/k8s_cni_registry.py index ef19f7acc..b00f38584 100644 --- a/kuryr_kubernetes/cni/plugins/k8s_cni_registry.py +++ b/kuryr_kubernetes/cni/plugins/k8s_cni_registry.py @@ -70,7 +70,7 @@ class K8sCNIRegistryPlugin(base_cni.CNIPlugin): # vif is not active. @retrying.retry(stop_max_delay=timeout * 1000, wait_fixed=RETRY_DELAY, retry_on_result=lambda x: any( - map(lambda y: not y[1].active, x.items()))) + map(lambda y: not y.active, x.values()))) def wait_for_active(pod_name): return { ifname: base.VersionedObject.obj_from_primitive(vif_obj) for diff --git a/kuryr_kubernetes/controller/handlers/vif.py b/kuryr_kubernetes/controller/handlers/vif.py index 253be021c..a22337c76 100644 --- a/kuryr_kubernetes/controller/handlers/vif.py +++ b/kuryr_kubernetes/controller/handlers/vif.py @@ -13,7 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. -from os_vif import objects as obj_vif +from os_vif.objects import base from oslo_log import log as logging from oslo_serialization import jsonutils @@ -22,6 +22,8 @@ from kuryr_kubernetes import constants from kuryr_kubernetes.controller.drivers import base as drivers from kuryr_kubernetes import exceptions as k_exc from kuryr_kubernetes.handlers import k8s_base +from kuryr_kubernetes import objects + LOG = logging.getLogger(__name__) @@ -61,11 +63,9 @@ class VIFHandler(k8s_base.ResourceEventHandler): # where certain pods/namespaces/nodes can be managed by other # networking solutions/CNI drivers. return - vifs = self._get_vifs(pod) - - if not vifs: - vifs = {} + state = self._get_pod_state(pod) + if not state: project_id = self._drv_project.get_project(pod) security_groups = self._drv_sg.get_security_groups(pod, project_id) subnets = self._drv_subnets.get_subnets(pod, project_id) @@ -73,7 +73,8 @@ class VIFHandler(k8s_base.ResourceEventHandler): # Request the default interface of pod main_vif = self._drv_vif_pool.request_vif( pod, project_id, subnets, security_groups) - vifs[constants.DEFAULT_IFNAME] = main_vif + + state = objects.vif.PodState(default_vif=main_vif) # Request the additional interfaces from multiple dirvers additional_vifs = [] @@ -82,26 +83,28 @@ class VIFHandler(k8s_base.ResourceEventHandler): driver.request_additional_vifs( pod, project_id, security_groups)) if additional_vifs: + state.additional_vifs = {} for i, vif in enumerate(additional_vifs, start=1): - vifs[constants.ADDITIONAL_IFNAME_PREFIX + str(i)] = vif + k = constants.ADDITIONAL_IFNAME_PREFIX + str(i) + state.additional_vifs[k] = vif try: - self._set_vifs(pod, vifs) + self._set_pod_state(pod, state) except k_exc.K8sClientException as ex: LOG.debug("Failed to set annotation: %s", ex) # FIXME(ivc): improve granularity of K8sClient exceptions: # only resourceVersion conflict should be ignored - for ifname, vif in vifs.items(): + for ifname, vif in state.vifs.items(): self._drv_for_vif(vif).release_vif(pod, vif, project_id, security_groups) else: changed = False - for ifname, vif in vifs.items(): + for ifname, vif in state.vifs.items(): if not vif.active: self._drv_for_vif(vif).activate_vif(pod, vif) changed = True if changed: - self._set_vifs(pod, vifs) + self._set_pod_state(pod, state) def on_deleted(self, pod): if self._is_host_network(pod): @@ -109,10 +112,11 @@ class VIFHandler(k8s_base.ResourceEventHandler): project_id = self._drv_project.get_project(pod) security_groups = self._drv_sg.get_security_groups(pod, project_id) - vifs = self._get_vifs(pod) - for ifname, vif in vifs.items(): - self._drv_for_vif(vif).release_vif(pod, vif, project_id, - security_groups) + state = self._get_pod_state(pod) + if state: + for ifname, vif in state.vifs.items(): + self._drv_for_vif(vif).release_vif(pod, vif, project_id, + security_groups) def _drv_for_vif(self, vif): # TODO(danil): a better polymorphism is required here @@ -131,36 +135,34 @@ class VIFHandler(k8s_base.ResourceEventHandler): except KeyError: return False - def _set_vifs(self, pod, vifs): + def _set_pod_state(self, pod, state): # TODO(ivc): extract annotation interactions - if not vifs: - LOG.debug("Removing VIFs annotation: %r", vifs) + if not state: + LOG.debug("Removing VIFs annotation: %r", state) annotation = None else: - vifs_dict = {} - for ifname, vif in vifs.items(): - vif.obj_reset_changes(recursive=True) - vifs_dict[ifname] = vif.obj_to_primitive() - - annotation = jsonutils.dumps(vifs_dict, - sort_keys=True) + state_dict = state.obj_to_primitive() + annotation = jsonutils.dumps(state_dict, sort_keys=True) LOG.debug("Setting VIFs annotation: %r", annotation) + + # TODO(dulek): Here goes backward compatiblity code. Probably we can + # just ignore this case. + k8s = clients.get_kubernetes_client() k8s.annotate(pod['metadata']['selfLink'], {constants.K8S_ANNOTATION_VIF: annotation}, resource_version=pod['metadata']['resourceVersion']) - def _get_vifs(self, pod): + def _get_pod_state(self, pod): # TODO(ivc): same as '_set_vif' try: annotations = pod['metadata']['annotations'] - vif_annotation = annotations[constants.K8S_ANNOTATION_VIF] + state_annotation = annotations[constants.K8S_ANNOTATION_VIF] except KeyError: - return {} - vif_annotation = jsonutils.loads(vif_annotation) - vifs = { - ifname: obj_vif.vif.VIFBase.obj_from_primitive(vif_obj) for - ifname, vif_obj in vif_annotation.items() - } - LOG.debug("Got VIFs from annotation: %r", vifs) - return vifs + return None + + state_annotation = jsonutils.loads(state_annotation) + state = base.VersionedObject.obj_from_primitive(state_annotation) + # TODO(dulek): Here goes backward compatibility code. + LOG.debug("Got VIFs from annotation: %r", state) + return state diff --git a/kuryr_kubernetes/objects/fields.py b/kuryr_kubernetes/objects/fields.py index 6fca84dcc..df19295b3 100644 --- a/kuryr_kubernetes/objects/fields.py +++ b/kuryr_kubernetes/objects/fields.py @@ -13,8 +13,14 @@ # License for the specific language governing permissions and limitations # under the License. +from os_vif.objects import vif from oslo_versionedobjects import fields as obj_fields class ListOfUUIDField(obj_fields.AutoTypedField): AUTO_TYPE = obj_fields.List(obj_fields.UUID()) + + +class DictOfVIFsField(obj_fields.AutoTypedField): + AUTO_TYPE = obj_fields.Dict(obj_fields.Object(vif.VIFBase.__name__, + subclasses=True)) diff --git a/kuryr_kubernetes/objects/vif.py b/kuryr_kubernetes/objects/vif.py index 9ad859dbd..730c0e8fa 100644 --- a/kuryr_kubernetes/objects/vif.py +++ b/kuryr_kubernetes/objects/vif.py @@ -15,6 +15,34 @@ from oslo_versionedobjects import fields as obj_fields from os_vif.objects import vif as obj_osvif +from kuryr_kubernetes import constants +from kuryr_kubernetes.objects import base +from kuryr_kubernetes.objects import fields + + +@obj_base.VersionedObjectRegistry.register +class PodState(base.KuryrK8sObjectBase): + VERSION = '1.0' + + # FIXME(dulek): I know it's an ugly hack, but turns out you cannot + # serialize-deserialize objects containing objects from + # different namespaces, so we need 'os_vif' namespace here. + OBJ_PROJECT_NAMESPACE = 'os_vif' + + fields = { + 'default_vif': obj_fields.ObjectField(obj_osvif.VIFBase.__name__, + subclasses=True, nullable=False), + 'additional_vifs': fields.DictOfVIFsField(default={}), + } + + @property + def vifs(self): + d = { + constants.DEFAULT_IFNAME: self.default_vif, + } + d.update(self.additional_vifs) + return d + @obj_base.VersionedObjectRegistry.register class VIFVlanNested(obj_osvif.VIFBase): diff --git a/kuryr_kubernetes/tests/unit/controller/handlers/test_vif.py b/kuryr_kubernetes/tests/unit/controller/handlers/test_vif.py index f5137d6dd..385df3977 100644 --- a/kuryr_kubernetes/tests/unit/controller/handlers/test_vif.py +++ b/kuryr_kubernetes/tests/unit/controller/handlers/test_vif.py @@ -15,10 +15,13 @@ import mock +from os_vif import objects as os_obj + from kuryr_kubernetes import constants as k_const from kuryr_kubernetes.controller.drivers import base as drivers from kuryr_kubernetes.controller.handlers import vif as h_vif from kuryr_kubernetes import exceptions as k_exc +from kuryr_kubernetes.objects import vif from kuryr_kubernetes.tests import base as test_base @@ -30,12 +33,12 @@ class TestVIFHandler(test_base.TestCase): self._project_id = mock.sentinel.project_id self._subnets = mock.sentinel.subnets self._security_groups = mock.sentinel.security_groups - self._vif = mock.Mock() + self._vif = os_obj.vif.VIFBase() self._vif.active = True self._vif_serialized = mock.sentinel.vif_serialized - self._vifs = {k_const.DEFAULT_IFNAME: self._vif} self._multi_vif_drv = mock.MagicMock(spec=drivers.MultiVIFDriver) self._additioan_vifs = [] + self._state = vif.PodState(default_vif=self._vif) self._pod_version = mock.sentinel.pod_version self._pod_link = mock.sentinel.pod_link @@ -63,8 +66,8 @@ class TestVIFHandler(test_base.TestCase): self._request_vif = self._handler._drv_vif_pool.request_vif self._release_vif = self._handler._drv_vif_pool.release_vif self._activate_vif = self._handler._drv_vif_pool.activate_vif - self._get_vifs = self._handler._get_vifs - self._set_vifs = self._handler._set_vifs + self._get_pod_state = self._handler._get_pod_state + self._set_pod_state = self._handler._set_pod_state self._is_host_network = self._handler._is_host_network self._is_pending_node = self._handler._is_pending_node self._request_additional_vifs = \ @@ -72,7 +75,7 @@ class TestVIFHandler(test_base.TestCase): self._request_vif.return_value = self._vif self._request_additional_vifs.return_value = self._additioan_vifs - self._get_vifs.return_value = self._vifs + self._get_pod_state.return_value = self._state self._is_host_network.return_value = False self._is_pending_node.return_value = True self._get_project.return_value = self._project_id @@ -145,87 +148,86 @@ class TestVIFHandler(test_base.TestCase): def test_on_present(self): h_vif.VIFHandler.on_present(self._handler, self._pod) - self._get_vifs.assert_called_once_with(self._pod) + self._get_pod_state.assert_called_once_with(self._pod) self._request_vif.assert_not_called() self._request_additional_vifs.assert_not_called() self._activate_vif.assert_not_called() - self._set_vifs.assert_not_called() + self._set_pod_state.assert_not_called() def test_on_present_host_network(self): self._is_host_network.return_value = True h_vif.VIFHandler.on_present(self._handler, self._pod) - self._get_vifs.assert_not_called() + self._get_pod_state.assert_not_called() self._request_vif.assert_not_called() self._request_additional_vifs.assert_not_called() self._activate_vif.assert_not_called() - self._set_vifs.assert_not_called() + self._set_pod_state.assert_not_called() def test_on_present_not_pending(self): self._is_pending_node.return_value = False h_vif.VIFHandler.on_present(self._handler, self._pod) - self._get_vifs.assert_not_called() + self._get_pod_state.assert_not_called() self._request_vif.assert_not_called() self._request_additional_vifs.assert_not_called() self._activate_vif.assert_not_called() - self._set_vifs.assert_not_called() + self._set_pod_state.assert_not_called() def test_on_present_activate(self): self._vif.active = False h_vif.VIFHandler.on_present(self._handler, self._pod) - self._get_vifs.assert_called_once_with(self._pod) + self._get_pod_state.assert_called_once_with(self._pod) self._activate_vif.assert_called_once_with(self._pod, self._vif) - self._set_vifs.assert_called_once_with(self._pod, self._vifs) + self._set_pod_state.assert_called_once_with(self._pod, self._state) self._request_vif.assert_not_called() self._request_additional_vifs.assert_not_called() def test_on_present_create(self): - self._get_vifs.return_value = {} + self._get_pod_state.return_value = None h_vif.VIFHandler.on_present(self._handler, self._pod) - self._get_vifs.assert_called_once_with(self._pod) + self._get_pod_state.assert_called_once_with(self._pod) self._request_vif.assert_called_once_with( self._pod, self._project_id, self._subnets, self._security_groups) self._request_additional_vifs.assert_called_once_with( self._pod, self._project_id, self._security_groups) - self._set_vifs.assert_called_once_with(self._pod, self._vifs) + self._set_pod_state.assert_called_once_with(self._pod, self._state) self._activate_vif.assert_not_called() def test_on_present_create_with_additional_vifs(self): - self._get_vifs.return_value = {} - self._request_additional_vifs.return_value = [mock.Mock()] - vifs = self._vifs.copy() - vifs.update({k_const.ADDITIONAL_IFNAME_PREFIX+'1': - self._request_additional_vifs.return_value[0]}) + self._get_pod_state.return_value = None + additional_vif = os_obj.vif.VIFBase() + self._state.additional_vifs = {'eth1': additional_vif} + self._request_additional_vifs.return_value = [additional_vif] h_vif.VIFHandler.on_present(self._handler, self._pod) - self._get_vifs.assert_called_once_with(self._pod) + self._get_pod_state.assert_called_once_with(self._pod) self._request_vif.assert_called_once_with( self._pod, self._project_id, self._subnets, self._security_groups) self._request_additional_vifs.assert_called_once_with( self._pod, self._project_id, self._security_groups) - self._set_vifs.assert_called_once_with(self._pod, vifs) + self._set_pod_state.assert_called_once_with(self._pod, self._state) self._activate_vif.assert_not_called() def test_on_present_rollback(self): - self._get_vifs.return_value = {} - self._set_vifs.side_effect = k_exc.K8sClientException + self._get_pod_state.return_value = None + self._set_pod_state.side_effect = k_exc.K8sClientException h_vif.VIFHandler.on_present(self._handler, self._pod) - self._get_vifs.assert_called_once_with(self._pod) + self._get_pod_state.assert_called_once_with(self._pod) self._request_vif.assert_called_once_with( self._pod, self._project_id, self._subnets, self._security_groups) self._request_additional_vifs.assert_called_once_with( self._pod, self._project_id, self._security_groups) - self._set_vifs.assert_called_once_with(self._pod, self._vifs) + self._set_pod_state.assert_called_once_with(self._pod, self._state) self._release_vif.assert_called_once_with(self._pod, self._vif, self._project_id, self._security_groups) @@ -234,24 +236,22 @@ class TestVIFHandler(test_base.TestCase): def test_on_deleted(self): h_vif.VIFHandler.on_deleted(self._handler, self._pod) - self._get_vifs.assert_called_once_with(self._pod) + self._get_pod_state.assert_called_once_with(self._pod) self._release_vif.assert_called_once_with(self._pod, self._vif, self._project_id, self._security_groups) def test_on_deleted_with_additional_vifs(self): - self._additioan_vifs = [mock.Mock()] - self._get_vifs.return_value = self._vifs.copy() - self._get_vifs.return_value.update( - {k_const.ADDITIONAL_IFNAME_PREFIX+'1': self._additioan_vifs[0]}) + additional_vif = os_obj.vif.VIFBase() + self._state.additional_vifs = {'eth1': additional_vif} + self._get_pod_state.return_value = self._state h_vif.VIFHandler.on_deleted(self._handler, self._pod) - self._get_vifs.assert_called_once_with(self._pod) self._release_vif.assert_any_call(self._pod, self._vif, self._project_id, self._security_groups) - self._release_vif.assert_any_call(self._pod, self._additioan_vifs[0], + self._release_vif.assert_any_call(self._pod, additional_vif, self._project_id, self._security_groups) @@ -260,13 +260,13 @@ class TestVIFHandler(test_base.TestCase): h_vif.VIFHandler.on_deleted(self._handler, self._pod) - self._get_vifs.assert_not_called() + self._get_pod_state.assert_not_called() self._release_vif.assert_not_called() def test_on_deleted_no_annotation(self): - self._get_vifs.return_value = {} + self._get_pod_state.return_value = None h_vif.VIFHandler.on_deleted(self._handler, self._pod) - self._get_vifs.assert_called_once_with(self._pod) + self._get_pod_state.assert_called_once_with(self._pod) self._release_vif.assert_not_called()