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: {
       '<ifname>': VIFBase,
       ...
       '<ifname>': 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

@@ -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']

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)
+        state_annotation = jsonutils.loads(state_annotation)
-        vifs_dict = {
+        state = obj_vif.base.VersionedObject.obj_from_primitive(
-            ifname: obj_vif.vif.VIFBase.obj_from_primitive(vif)
+            state_annotation)
-            for ifname, vif in vifs_annotation.items()
+        vifs_dict = state.vifs
-        }
         LOG.debug("Got VIFs from annotation: %r", vifs_dict)
         return vifs_dict
 

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

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)
+        state = self._get_pod_state(pod)
-
-        if not vifs:
-            vifs = {}
 
+        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)
+        state = self._get_pod_state(pod)
-        for ifname, vif in vifs.items():
+        if state:
-            self._drv_for_vif(vif).release_vif(pod, vif, project_id,
+            for ifname, vif in state.vifs.items():
-                                               security_groups)
+                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:
+        if not state:
-            LOG.debug("Removing VIFs annotation: %r", vifs)
+            LOG.debug("Removing VIFs annotation: %r", state)
             annotation = None
         else:
-            vifs_dict = {}
+            state_dict = state.obj_to_primitive()
-            for ifname, vif in vifs.items():
+            annotation = jsonutils.dumps(state_dict, sort_keys=True)
-                vif.obj_reset_changes(recursive=True)
-                vifs_dict[ifname] = vif.obj_to_primitive()
-
-            annotation = jsonutils.dumps(vifs_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 {}
+            return None
-        vif_annotation = jsonutils.loads(vif_annotation)
+
-        vifs = {
+        state_annotation = jsonutils.loads(state_annotation)
-            ifname: obj_vif.vif.VIFBase.obj_from_primitive(vif_obj) for
+        state = base.VersionedObject.obj_from_primitive(state_annotation)
-            ifname, vif_obj in vif_annotation.items()
+        # TODO(dulek): Here goes backward compatibility code.
-        }
+        LOG.debug("Got VIFs from annotation: %r", state)
-        LOG.debug("Got VIFs from annotation: %r", vifs)
+        return state
-        return vifs

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))

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):

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._get_pod_state = self._handler._get_pod_state
-        self._set_vifs = self._handler._set_vifs
+        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._get_pod_state.return_value = None
-        self._request_additional_vifs.return_value = [mock.Mock()]
+        additional_vif = os_obj.vif.VIFBase()
-        vifs = self._vifs.copy()
+        self._state.additional_vifs = {'eth1': additional_vif}
-        vifs.update({k_const.ADDITIONAL_IFNAME_PREFIX+'1':
+        self._request_additional_vifs.return_value = [additional_vif]
-                    self._request_additional_vifs.return_value[0]})
 
         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._get_pod_state.return_value = None
-        self._set_vifs.side_effect = k_exc.K8sClientException
+        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()]
+        additional_vif = os_obj.vif.VIFBase()
-        self._get_vifs.return_value = self._vifs.copy()
+        self._state.additional_vifs = {'eth1': additional_vif}
-        self._get_vifs.return_value.update(
+        self._get_pod_state.return_value = self._state
-            {k_const.ADDITIONAL_IFNAME_PREFIX+'1': self._additioan_vifs[0]})
 
         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()