)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"680cb2c5589009e5f7744e935fa5049292b87831","unresolved":false,"context_lines":[{"line_number":20,"context_line":"yet and passes an appropriate VF into container\u0027s"},{"line_number":21,"context_line":"network namespace."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Also this commit contains tools for cluster updrade."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Change-Id: I5b24981f715966369b05b8ab157f8bfe02afc2d4"},{"line_number":26,"context_line":"Closes-Bug: 1826865"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"7faddb67_908d5933","line":23,"range":{"start_line":23,"start_character":44,"end_line":23,"end_character":51},"updated":"2019-07-22 14:05:40.000000000","message":"upgrade","commit_id":"c7eda27280ce2b491a4137ff61659a3917631715"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"5ddd9135dd206e825b01a22424d2effa9a41800b","unresolved":false,"context_lines":[{"line_number":20,"context_line":"yet and passes an appropriate VF into container\u0027s"},{"line_number":21,"context_line":"network namespace."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Also this commit contains tools for cluster updrade."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Change-Id: I5b24981f715966369b05b8ab157f8bfe02afc2d4"},{"line_number":26,"context_line":"Closes-Bug: 1826865"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"7faddb67_dbd52645","line":23,"range":{"start_line":23,"start_character":44,"end_line":23,"end_character":51},"in_reply_to":"7faddb67_908d5933","updated":"2019-07-23 07:25:07.000000000","message":"Done","commit_id":"c7eda27280ce2b491a4137ff61659a3917631715"}],"doc/source/installation/sriov.rst":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d637d16610cabc00213d2945718f558ff59c3cf1","unresolved":false,"context_lines":[{"line_number":131,"context_line":"  device_plugin_resource_prefix \u003d samsung.com"},{"line_number":132,"context_line":"  physnet_resource_mappings \u003d physnet1:numa0"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"5. Enable Kubelet Pod Resources feature"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"To use SR-IOV functionality properly it is necessary to enable Kubelet Pod"},{"line_number":137,"context_line":"Resources feature. Pod Resources is a service provided by Kubelet via gRPC"}],"source_content_type":"text/x-rst","patch_set":21,"id":"7faddb67_37403ea2","line":134,"range":{"start_line":134,"start_character":1,"end_line":134,"end_character":39},"updated":"2019-08-21 09:03:03.000000000","message":"missing information about the new sriov config option: enable_pod_resource_service","commit_id":"329e7f165f35bd1151c4e52939ba3413de91778d"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"1b60269d874df1d8c49dd0edab1f43faa9f67724","unresolved":false,"context_lines":[{"line_number":131,"context_line":"  device_plugin_resource_prefix \u003d samsung.com"},{"line_number":132,"context_line":"  physnet_resource_mappings \u003d physnet1:numa0"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"5. Enable Kubelet Pod Resources feature"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"To use SR-IOV functionality properly it is necessary to enable Kubelet Pod"},{"line_number":137,"context_line":"Resources feature. Pod Resources is a service provided by Kubelet via gRPC"}],"source_content_type":"text/x-rst","patch_set":21,"id":"7faddb67_c9310ea4","line":134,"range":{"start_line":134,"start_character":1,"end_line":134,"end_character":39},"in_reply_to":"7faddb67_37403ea2","updated":"2019-08-21 10:02:29.000000000","message":"Done","commit_id":"329e7f165f35bd1151c4e52939ba3413de91778d"}],"kuryr_kubernetes/cmd/status.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"680cb2c5589009e5f7744e935fa5049292b87831","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        def update_fn(obj):"},{"line_number":244,"context_line":"            return obj.default_vif"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        def test_sriov(obj):"},{"line_number":247,"context_line":"            return True"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        def update_sriov(obj):"},{"line_number":250,"context_line":"            return obj"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        self._convert_annotations(test_fn, update_fn, test_sriov, update_sriov)"},{"line_number":253,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_4673a7b2","line":250,"range":{"start_line":246,"start_character":0,"end_line":250,"end_character":22},"updated":"2019-07-22 14:05:40.000000000","message":"So you\u0027re deliberately preventing any rollback if upgrade fails? Or we don\u0027t care as we only add fields and on rollback old Kuryr services will work just fine with new o.vo\u0027s? That at least deserves a comment.","commit_id":"c7eda27280ce2b491a4137ff61659a3917631715"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"5ddd9135dd206e825b01a22424d2effa9a41800b","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        def update_fn(obj):"},{"line_number":244,"context_line":"            return obj.default_vif"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        def test_sriov(obj):"},{"line_number":247,"context_line":"            return True"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        def update_sriov(obj):"},{"line_number":250,"context_line":"            return obj"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        self._convert_annotations(test_fn, update_fn, test_sriov, update_sriov)"},{"line_number":253,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_7bcaf2a1","line":250,"range":{"start_line":246,"start_character":0,"end_line":250,"end_character":22},"in_reply_to":"7faddb67_4673a7b2","updated":"2019-07-23 07:25:07.000000000","message":"I added a note regarding this code\nThanks for comments","commit_id":"c7eda27280ce2b491a4137ff61659a3917631715"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d637d16610cabc00213d2945718f558ff59c3cf1","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        def update_fn(obj):"},{"line_number":229,"context_line":"            return vif.PodState(default_vif\u003dobj)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        def test_sriov(obj):"},{"line_number":232,"context_line":"            return self._valid_sriov_annot(obj)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        def update_sriov(obj):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7faddb67_97e332d8","line":231,"range":{"start_line":231,"start_character":12,"end_line":231,"end_character":22},"updated":"2019-08-21 09:03:03.000000000","message":"maybe \"check_sriov\" or \"validate_sriov\"","commit_id":"329e7f165f35bd1151c4e52939ba3413de91778d"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"1b60269d874df1d8c49dd0edab1f43faa9f67724","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        def update_fn(obj):"},{"line_number":229,"context_line":"            return vif.PodState(default_vif\u003dobj)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        def test_sriov(obj):"},{"line_number":232,"context_line":"            return self._valid_sriov_annot(obj)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        def update_sriov(obj):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7faddb67_a92e1240","line":231,"range":{"start_line":231,"start_character":12,"end_line":231,"end_character":22},"in_reply_to":"7faddb67_97e332d8","updated":"2019-08-21 10:02:29.000000000","message":"Done","commit_id":"329e7f165f35bd1151c4e52939ba3413de91778d"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"20cccd604a70abde5a794a3f0702352ec4b64cc5","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        pods \u003d self.k8s.get(\u0027/api/v1/pods\u0027)[\u0027items\u0027]"},{"line_number":159,"context_line":"        for pod in pods:"},{"line_number":160,"context_line":"            try:"},{"line_number":161,"context_line":"                obj \u003d self._get_annotation(pod)"},{"line_number":162,"context_line":"                if not obj:"},{"line_number":163,"context_line":"                    # NOTE(dulek): We ignore pods without annotation, those"},{"line_number":164,"context_line":"                    # probably are hostNetworking."}],"source_content_type":"text/x-python","patch_set":22,"id":"7faddb67_4633b2c5","line":161,"range":{"start_line":161,"start_character":16,"end_line":161,"end_character":19},"updated":"2019-08-22 09:21:56.000000000","message":"perhaps worth to rename as pod_annotations","commit_id":"cfb046b83b016e540a886ad40f5184b6ab23e779"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"8411c2e49621f0f7243a3fdde9c788ffa8b0cd49","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        pods \u003d self.k8s.get(\u0027/api/v1/pods\u0027)[\u0027items\u0027]"},{"line_number":159,"context_line":"        for pod in pods:"},{"line_number":160,"context_line":"            try:"},{"line_number":161,"context_line":"                obj \u003d self._get_annotation(pod)"},{"line_number":162,"context_line":"                if not obj:"},{"line_number":163,"context_line":"                    # NOTE(dulek): We ignore pods without annotation, those"},{"line_number":164,"context_line":"                    # probably are hostNetworking."}],"source_content_type":"text/x-python","patch_set":22,"id":"7faddb67_09e62db2","line":161,"range":{"start_line":161,"start_character":16,"end_line":161,"end_character":19},"in_reply_to":"7faddb67_4633b2c5","updated":"2019-08-22 12:14:16.000000000","message":"I\u0027m not sure, that we should rename this, just because it more intuitive:\nWe gen an object (obj) and then we check the type of our object.\nAlso it doesn\u0027t relate to this patch","commit_id":"cfb046b83b016e540a886ad40f5184b6ab23e779"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"20cccd604a70abde5a794a3f0702352ec4b64cc5","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                malformed_count +\u003d 1"},{"line_number":168,"context_line":"                continue"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"            if not test_fn(obj):"},{"line_number":171,"context_line":"                if check_sriov(obj):"},{"line_number":172,"context_line":"                    continue"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"            obj \u003d update_fn(obj) if test_fn(obj) else update_sriov(obj)"}],"source_content_type":"text/x-python","patch_set":22,"id":"7faddb67_8650cad7","line":171,"range":{"start_line":170,"start_character":0,"end_line":171,"end_character":36},"updated":"2019-08-22 09:21:56.000000000","message":"It is not really intuitive to understand what this is doing, probably function naming could be more descriptive. In addition, any reason to have test and check separate? perhaps worth to have a check function that first test the name and then do the checking. And leave here just:\nif check_sriov(pod_annotations):\n   continue\n\nperhaps even \u0027check_sriov_annotations\u0027","commit_id":"cfb046b83b016e540a886ad40f5184b6ab23e779"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"8411c2e49621f0f7243a3fdde9c788ffa8b0cd49","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                malformed_count +\u003d 1"},{"line_number":168,"context_line":"                continue"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"            if not test_fn(obj):"},{"line_number":171,"context_line":"                if check_sriov(obj):"},{"line_number":172,"context_line":"                    continue"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"            obj \u003d update_fn(obj) if test_fn(obj) else update_sriov(obj)"}],"source_content_type":"text/x-python","patch_set":22,"id":"7faddb67_0f586d88","line":171,"range":{"start_line":170,"start_character":0,"end_line":171,"end_character":36},"in_reply_to":"7faddb67_8650cad7","updated":"2019-08-22 12:14:16.000000000","message":"Done","commit_id":"cfb046b83b016e540a886ad40f5184b6ab23e779"}],"kuryr_kubernetes/cni/binding/sriov.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"12c57c845f95a5c819e986f9dcb27d63c5efa05b","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        for pod_resource in resources:"},{"line_number":55,"context_line":"            LOG.info(\"pod_resource \u003d %s\", pod_resource)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        h_ipdb \u003d b_base.get_ipdb()"},{"line_number":58,"context_line":"        c_ipdb \u003d b_base.get_ipdb(netns)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        pci \u003d self._choose_pci(vif, resources, ifname, netns)"},{"line_number":61,"context_line":"        vf_name, vf_index, pf, pci_info \u003d self._get_vf_info(pci)"}],"source_content_type":"text/x-python","patch_set":10,"id":"bfb3d3c7_e8707d42","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":39},"updated":"2019-05-31 14:29:57.000000000","message":"You are not closing those. I don\u0027t understand why you need them? See line 72.","commit_id":"be490b382394b8657f35db7ba904c721642ee885"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"d4c1c9746a066a9c25e0567327d44ca883276663","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        for pod_resource in resources:"},{"line_number":55,"context_line":"            LOG.info(\"pod_resource \u003d %s\", pod_resource)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        h_ipdb \u003d b_base.get_ipdb()"},{"line_number":58,"context_line":"        c_ipdb \u003d b_base.get_ipdb(netns)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        pci \u003d self._choose_pci(vif, resources, ifname, netns)"},{"line_number":61,"context_line":"        vf_name, vf_index, pf, pci_info \u003d self._get_vf_info(pci)"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fb8cfa7_39ab2dd7","line":58,"range":{"start_line":57,"start_character":0,"end_line":58,"end_character":39},"in_reply_to":"bfb3d3c7_e8707d42","updated":"2019-06-03 08:40:22.000000000","message":"thanks for comment\nit\u0027s a mistake while rebase. These statements are not necessary here\nwill fix it","commit_id":"be490b382394b8657f35db7ba904c721642ee885"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"12c57c845f95a5c819e986f9dcb27d63c5efa05b","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        self._remove_pci_info(vif.id)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def _choose_pci(self, vif, resources, ifname, netns):"},{"line_number":93,"context_line":"        LOG.info(\"_choose_pci\")"},{"line_number":94,"context_line":"        pod_name \u003d vif.pod_name"},{"line_number":95,"context_line":"        pod_link \u003d vif.pod_link"},{"line_number":96,"context_line":"        physnet \u003d vif.physnet"}],"source_content_type":"text/x-python","patch_set":10,"id":"bfb3d3c7_c8aa596f","line":93,"range":{"start_line":93,"start_character":0,"end_line":93,"end_character":31},"updated":"2019-05-31 14:29:57.000000000","message":"This is way too vague as login message. Is that your debug statement? Same for below ones.","commit_id":"be490b382394b8657f35db7ba904c721642ee885"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"d4c1c9746a066a9c25e0567327d44ca883276663","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        self._remove_pci_info(vif.id)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def _choose_pci(self, vif, resources, ifname, netns):"},{"line_number":93,"context_line":"        LOG.info(\"_choose_pci\")"},{"line_number":94,"context_line":"        pod_name \u003d vif.pod_name"},{"line_number":95,"context_line":"        pod_link \u003d vif.pod_link"},{"line_number":96,"context_line":"        physnet \u003d vif.physnet"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fb8cfa7_593aa1e7","line":93,"range":{"start_line":93,"start_character":0,"end_line":93,"end_character":31},"in_reply_to":"bfb3d3c7_c8aa596f","updated":"2019-06-03 08:40:22.000000000","message":"Done","commit_id":"be490b382394b8657f35db7ba904c721642ee885"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"19bfcfe46c1e3a27a7c025da095f8a6f455d595b","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        self._remove_pci_info(vif.id)"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _choose_pci(self, vif, resources, ifname, netns):"},{"line_number":91,"context_line":"        pod_name \u003d vif.pod_name"},{"line_number":92,"context_line":"        pod_link \u003d vif.pod_link"},{"line_number":93,"context_line":"        physnet \u003d vif.physnet"},{"line_number":94,"context_line":"        resource_name \u003d self._get_resource_by_physnet(physnet)"},{"line_number":95,"context_line":"        resource \u003d self._make_resource(resource_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_ba1e2b6e","line":92,"range":{"start_line":91,"start_character":0,"end_line":92,"end_character":31},"updated":"2019-07-04 13:17:47.000000000","message":"Those are new fields, what happens if there\u0027s an annotation in older format (because someone just upgrade Kuryr)? Or we\u0027re sure nobody\u0027s using it as this feature wasn\u0027t released yet?","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"18f25fe649fe40215ab116d21052178ffbd05698","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        self._remove_pci_info(vif.id)"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _choose_pci(self, vif, resources, ifname, netns):"},{"line_number":91,"context_line":"        pod_name \u003d vif.pod_name"},{"line_number":92,"context_line":"        pod_link \u003d vif.pod_link"},{"line_number":93,"context_line":"        physnet \u003d vif.physnet"},{"line_number":94,"context_line":"        resource_name \u003d self._get_resource_by_physnet(physnet)"},{"line_number":95,"context_line":"        resource \u003d self._make_resource(resource_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_c2d99583","line":92,"range":{"start_line":91,"start_character":0,"end_line":92,"end_character":31},"in_reply_to":"7faddb67_107e30e4","updated":"2019-07-08 14:39:59.000000000","message":"\u003e I will start from the last point:\n \u003e 3. If we delete pods with vif objects in older version it is not\n \u003e necessary to take care about it on CNI side. But we should support\n \u003e this care somehow on controller side.\n \u003e 2. Good version. But how to register that kuryr was upgraded? Or we\n \u003e should create some kind of periodic task that will rewrite\n \u003e annotations constantly to the newest version? Or only when\n \u003e annotations will try to be get?\n\nPlease take a look at [1].\n\n[1] https://docs.openstack.org/kuryr-kubernetes/latest/installation/upgrades.html\n\n \u003e 1. Do you mean to check if kuryr-k8s was used to create pods\n \u003e previously or not? I mean if there pods with kuryr annotations?\n\nI mean that I don\u0027t know when the SR-IOV stuff was introduced and I don\u0027t know if we already released that code you\u0027re modifying. We only support upgrading release-to-release, no deployments from trunk, so if you added the code doing annotations in previous format during the Train release, then we\u0027re fine to change that format as Train is not released yet.","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"909b73ee6c3e256d12c5402d1b5e6e651d8d47c0","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        self._remove_pci_info(vif.id)"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _choose_pci(self, vif, resources, ifname, netns):"},{"line_number":91,"context_line":"        pod_name \u003d vif.pod_name"},{"line_number":92,"context_line":"        pod_link \u003d vif.pod_link"},{"line_number":93,"context_line":"        physnet \u003d vif.physnet"},{"line_number":94,"context_line":"        resource_name \u003d self._get_resource_by_physnet(physnet)"},{"line_number":95,"context_line":"        resource \u003d self._make_resource(resource_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_81aba21b","line":92,"range":{"start_line":91,"start_character":0,"end_line":92,"end_character":31},"in_reply_to":"7faddb67_107e30e4","updated":"2019-07-08 09:26:11.000000000","message":"I\u0027ve made some kind of testing for describes scenario:\n1. Run pod with sriov vif object of master branch  (old kuryr)\n2. Upgrade kuryr (both on controller and CNI sides)\n3. Try to delete previously created pod with sriov\n\nWhat I\u0027ve got from this experiment:\n1. CNI catches an exception:\n    NotImplementedError: Cannot load \u0027pod_link\u0027 in the base class\n\n2. After 3 attempts k8s deletes a pod finally","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"8c5928146965400001f8c65a48bc81976f018497","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        self._remove_pci_info(vif.id)"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _choose_pci(self, vif, resources, ifname, netns):"},{"line_number":91,"context_line":"        pod_name \u003d vif.pod_name"},{"line_number":92,"context_line":"        pod_link \u003d vif.pod_link"},{"line_number":93,"context_line":"        physnet \u003d vif.physnet"},{"line_number":94,"context_line":"        resource_name \u003d self._get_resource_by_physnet(physnet)"},{"line_number":95,"context_line":"        resource \u003d self._make_resource(resource_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_107e30e4","line":92,"range":{"start_line":91,"start_character":0,"end_line":92,"end_character":31},"in_reply_to":"7faddb67_2d91959a","updated":"2019-07-05 14:31:29.000000000","message":"I will start from the last point:\n3. If we delete pods with vif objects in older version it is not necessary to take care about it on CNI side. But we should support this care somehow on controller side.\n2. Good version. But how to register that kuryr was upgraded? Or we should create some kind of periodic task that will rewrite annotations constantly to the newest version? Or only when annotations will try to be get?\n1. Do you mean to check if kuryr-k8s was used to create pods previously or not? I mean if there pods with kuryr annotations?","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"9367ec91a9c5316363d827176c26009c8f4b9736","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        self._remove_pci_info(vif.id)"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _choose_pci(self, vif, resources, ifname, netns):"},{"line_number":91,"context_line":"        pod_name \u003d vif.pod_name"},{"line_number":92,"context_line":"        pod_link \u003d vif.pod_link"},{"line_number":93,"context_line":"        physnet \u003d vif.physnet"},{"line_number":94,"context_line":"        resource_name \u003d self._get_resource_by_physnet(physnet)"},{"line_number":95,"context_line":"        resource \u003d self._make_resource(resource_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_2d91959a","line":92,"range":{"start_line":91,"start_character":0,"end_line":92,"end_character":31},"in_reply_to":"7faddb67_68bc6969","updated":"2019-07-05 12:48:24.000000000","message":"We need to think the other way around. Annotations are our database. They will be preserved between upgrades, e.g.:\n\n1. User runs Kuryr that has VIFSriov 1.0, a bunch of pods gets created with those.\n2. Kuryr gets fully upgraded and restarted.\n3. User tries to delete those pods, Kuryr reads older annotations, fails and leaves some resources hanging.\n\nThat was my point, we should either:\n\n1. Be sure that the code with previous version wasn\u0027t released (we can assume nobody run it then).\n2. Provide a way to rewrite all the annotations (see that weird cmd/status.py).\n3. Code defensively to be able to work even if fields are not present.","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"18f25fe649fe40215ab116d21052178ffbd05698","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        self._remove_pci_info(vif.id)"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _choose_pci(self, vif, resources, ifname, netns):"},{"line_number":91,"context_line":"        pod_name \u003d vif.pod_name"},{"line_number":92,"context_line":"        pod_link \u003d vif.pod_link"},{"line_number":93,"context_line":"        physnet \u003d vif.physnet"},{"line_number":94,"context_line":"        resource_name \u003d self._get_resource_by_physnet(physnet)"},{"line_number":95,"context_line":"        resource \u003d self._make_resource(resource_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_02d1ed54","line":92,"range":{"start_line":91,"start_character":0,"end_line":92,"end_character":31},"in_reply_to":"7faddb67_81aba21b","updated":"2019-07-08 14:39:59.000000000","message":"\u003e I\u0027ve made some kind of testing for describes scenario:\n \u003e 1. Run pod with sriov vif object of master branch  (old kuryr)\n \u003e 2. Upgrade kuryr (both on controller and CNI sides)\n \u003e 3. Try to delete previously created pod with sriov\n \u003e \n \u003e What I\u0027ve got from this experiment:\n \u003e 1. CNI catches an exception:\n \u003e NotImplementedError: Cannot load \u0027pod_link\u0027 in the base class\n \u003e \n \u003e 2. After 3 attempts k8s deletes a pod finally\n\nIt\u0027s only deleted because kubelet will finally just ignore CNI failure, but it means that we possible orphaned resources.","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"15c22221e3ae48d94eefaa95aa37f4e7adf46f9a","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        self._remove_pci_info(vif.id)"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _choose_pci(self, vif, resources, ifname, netns):"},{"line_number":91,"context_line":"        pod_name \u003d vif.pod_name"},{"line_number":92,"context_line":"        pod_link \u003d vif.pod_link"},{"line_number":93,"context_line":"        physnet \u003d vif.physnet"},{"line_number":94,"context_line":"        resource_name \u003d self._get_resource_by_physnet(physnet)"},{"line_number":95,"context_line":"        resource \u003d self._make_resource(resource_name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_68bc6969","line":92,"range":{"start_line":91,"start_character":0,"end_line":92,"end_character":31},"in_reply_to":"7faddb67_ba1e2b6e","updated":"2019-07-04 14:55:43.000000000","message":"If this code gets an annotations in older format (from unupgraded kuryr-k8s-controller), then CNI will fail with an error:\n\n    NotImplementedError: Cannot load \u0027pod_link\u0027 in the base class\n\nThus cluster should be upgraded starting from controller. It will allow to avoid such kind of errors.\n\nBtw, from the other side , if kuryr-k8s-controller was upgraded and CNI wasn\u0027t upgraded, then CNI just will not parse an object and will notify that versions of objects are different:\n\n    IncompatibleObjectVersion: Version 1.1 of VIFSriov is not supported, supported version is 1.0","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"7269c0a6397013f88491d55ee79462674414e0c8","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        pod_resources_list \u003d pr_client.list()"},{"line_number":54,"context_line":"        resources \u003d pod_resources_list.pod_resources"},{"line_number":55,"context_line":"        for pod_resource in resources:"},{"line_number":56,"context_line":"            LOG.info(\"pod_resource \u003d %s\", pod_resource)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        pci \u003d self._choose_pci(vif, resources, ifname, netns)"},{"line_number":59,"context_line":"        vf_name, vf_index, pf, pci_info \u003d self._get_vf_info(pci)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7faddb67_51da11e9","line":56,"range":{"start_line":56,"start_character":11,"end_line":56,"end_character":55},"updated":"2019-07-30 07:26:31.000000000","message":"should this be just LOG.debug? Why is this even here? Perhaps worth to move it to choose_pci function","commit_id":"f4f1471810a2907bf943b7f55bab117e91334c33"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"2db2101534f596870a783360c606e7bc54ceb3b5","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        pod_resources_list \u003d pr_client.list()"},{"line_number":54,"context_line":"        resources \u003d pod_resources_list.pod_resources"},{"line_number":55,"context_line":"        for pod_resource in resources:"},{"line_number":56,"context_line":"            LOG.info(\"pod_resource \u003d %s\", pod_resource)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        pci \u003d self._choose_pci(vif, resources, ifname, netns)"},{"line_number":59,"context_line":"        vf_name, vf_index, pf, pci_info \u003d self._get_vf_info(pci)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7faddb67_ccc66617","line":56,"range":{"start_line":56,"start_character":11,"end_line":56,"end_character":55},"in_reply_to":"7faddb67_51da11e9","updated":"2019-07-30 10:47:34.000000000","message":"I think we do not need it at all, because resources are logged by pod resource client","commit_id":"f4f1471810a2907bf943b7f55bab117e91334c33"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"7269c0a6397013f88491d55ee79462674414e0c8","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        LOG.info(\"Trying to annotate pod %s with pci %s\", pod_link, pci)"},{"line_number":164,"context_line":"        k8s.annotate(pod_link, {annot_name: pod_devices})"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    def _get_vf_info(self, pci):"},{"line_number":167,"context_line":"        vf_sys_path \u003d \u0027/sys/bus/pci/devices/{}/net/\u0027.format(pci)"},{"line_number":168,"context_line":"        vf_names \u003d os.listdir(vf_sys_path)"},{"line_number":169,"context_line":"        vf_name \u003d vf_names[0]"}],"source_content_type":"text/x-python","patch_set":18,"id":"7faddb67_119259f7","line":166,"range":{"start_line":166,"start_character":0,"end_line":166,"end_character":32},"updated":"2019-07-30 07:26:31.000000000","message":"I suppose this will require to mount the kuryr-cni container with the right dirs mounted, right? Have you tested it with the containerized version?","commit_id":"f4f1471810a2907bf943b7f55bab117e91334c33"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"2db2101534f596870a783360c606e7bc54ceb3b5","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        LOG.info(\"Trying to annotate pod %s with pci %s\", pod_link, pci)"},{"line_number":164,"context_line":"        k8s.annotate(pod_link, {annot_name: pod_devices})"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    def _get_vf_info(self, pci):"},{"line_number":167,"context_line":"        vf_sys_path \u003d \u0027/sys/bus/pci/devices/{}/net/\u0027.format(pci)"},{"line_number":168,"context_line":"        vf_names \u003d os.listdir(vf_sys_path)"},{"line_number":169,"context_line":"        vf_name \u003d vf_names[0]"}],"source_content_type":"text/x-python","patch_set":18,"id":"7faddb67_16bc8c04","line":166,"range":{"start_line":166,"start_character":0,"end_line":166,"end_character":32},"in_reply_to":"7faddb67_119259f7","updated":"2019-07-30 10:47:34.000000000","message":"yes, sure. As I can see docker automatically mounts some directories (sysfs also) inside container. So there is no special need to mount it.\nI\u0027ve tested in both containerized and non-containerized scenarios","commit_id":"f4f1471810a2907bf943b7f55bab117e91334c33"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"fac74d616f1fa54d28877f2b81563166e4867def","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    @release_lock_object"},{"line_number":51,"context_line":"    def connect(self, vif, ifname, netns, container_id):"},{"line_number":52,"context_line":"        pr_client \u003d clients.get_pod_resources_client()"},{"line_number":53,"context_line":"        pod_resources_list \u003d pr_client.list()"},{"line_number":54,"context_line":"        resources \u003d pod_resources_list.pod_resources"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        pci \u003d self._choose_pci(vif, resources, ifname, netns)"},{"line_number":57,"context_line":"        vf_name, vf_index, pf, pci_info \u003d self._get_vf_info(pci)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_5ab8bec6","line":54,"range":{"start_line":52,"start_character":0,"end_line":54,"end_character":52},"updated":"2019-08-01 09:49:33.000000000","message":"this is only used inside choose_pci function. Better to move it there","commit_id":"7899619ee7bc24b897bc2226d4f6a9ef369ab6ff"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"1fed6c5ee8aeca2314d6b0cd0ded00dd07c8604b","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    @release_lock_object"},{"line_number":51,"context_line":"    def connect(self, vif, ifname, netns, container_id):"},{"line_number":52,"context_line":"        pr_client \u003d clients.get_pod_resources_client()"},{"line_number":53,"context_line":"        pod_resources_list \u003d pr_client.list()"},{"line_number":54,"context_line":"        resources \u003d pod_resources_list.pod_resources"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        pci \u003d self._choose_pci(vif, resources, ifname, netns)"},{"line_number":57,"context_line":"        vf_name, vf_index, pf, pci_info \u003d self._get_vf_info(pci)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_700599c1","line":54,"range":{"start_line":52,"start_character":0,"end_line":54,"end_character":52},"in_reply_to":"7faddb67_5ab8bec6","updated":"2019-08-01 13:51:13.000000000","message":"I agree, thanks","commit_id":"7899619ee7bc24b897bc2226d4f6a9ef369ab6ff"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d637d16610cabc00213d2945718f558ff59c3cf1","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        if not pod_resource:"},{"line_number":103,"context_line":"            raise exceptions.CNIError("},{"line_number":104,"context_line":"                \"No resources are discovered for pod {}\".format(pod_name))"},{"line_number":105,"context_line":"        LOG.info(\"Looking for PCI device used by kubelet service and not \""},{"line_number":106,"context_line":"                 \"used by pod %s yet ...\", pod_name)"},{"line_number":107,"context_line":"        for container in pod_resource.containers:"},{"line_number":108,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":21,"id":"7faddb67_37eb5ea4","line":105,"range":{"start_line":105,"start_character":12,"end_line":105,"end_character":16},"updated":"2019-08-21 09:03:03.000000000","message":"perhaps debug?","commit_id":"329e7f165f35bd1151c4e52939ba3413de91778d"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"1b60269d874df1d8c49dd0edab1f43faa9f67724","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        if not pod_resource:"},{"line_number":103,"context_line":"            raise exceptions.CNIError("},{"line_number":104,"context_line":"                \"No resources are discovered for pod {}\".format(pod_name))"},{"line_number":105,"context_line":"        LOG.info(\"Looking for PCI device used by kubelet service and not \""},{"line_number":106,"context_line":"                 \"used by pod %s yet ...\", pod_name)"},{"line_number":107,"context_line":"        for container in pod_resource.containers:"},{"line_number":108,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":21,"id":"7faddb67_098ac6ba","line":105,"range":{"start_line":105,"start_character":12,"end_line":105,"end_character":16},"in_reply_to":"7faddb67_37eb5ea4","updated":"2019-08-21 10:02:29.000000000","message":"Done","commit_id":"329e7f165f35bd1151c4e52939ba3413de91778d"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d637d16610cabc00213d2945718f558ff59c3cf1","unresolved":false,"context_lines":[{"line_number":119,"context_line":"                for pci in dev.device_ids:"},{"line_number":120,"context_line":"                    if pci in pod_devices:"},{"line_number":121,"context_line":"                        continue"},{"line_number":122,"context_line":"                    LOG.info(\"Appropriate PCI device %s is found\", pci)"},{"line_number":123,"context_line":"                    return pci"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    def _get_resource_by_physnet(self, physnet):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7faddb67_9731f25b","line":122,"range":{"start_line":122,"start_character":24,"end_line":122,"end_character":28},"updated":"2019-08-21 09:03:03.000000000","message":"ditto","commit_id":"329e7f165f35bd1151c4e52939ba3413de91778d"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"1b60269d874df1d8c49dd0edab1f43faa9f67724","unresolved":false,"context_lines":[{"line_number":119,"context_line":"                for pci in dev.device_ids:"},{"line_number":120,"context_line":"                    if pci in pod_devices:"},{"line_number":121,"context_line":"                        continue"},{"line_number":122,"context_line":"                    LOG.info(\"Appropriate PCI device %s is found\", pci)"},{"line_number":123,"context_line":"                    return pci"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    def _get_resource_by_physnet(self, physnet):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7faddb67_a9be1262","line":122,"range":{"start_line":122,"start_character":24,"end_line":122,"end_character":28},"in_reply_to":"7faddb67_9731f25b","updated":"2019-08-21 10:02:29.000000000","message":"Done","commit_id":"329e7f165f35bd1151c4e52939ba3413de91778d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"d501386d8b41eb3a737e5556622b6a57dbfa1308","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        return res_prefix + \u0027/\u0027 + res_name"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _get_pod_devices(self, pod_link):"},{"line_number":139,"context_line":"        annot_name \u003d \u0027pci_devices\u0027"},{"line_number":140,"context_line":"        k8s \u003d clients.get_kubernetes_client()"},{"line_number":141,"context_line":"        pod \u003d k8s.get(pod_link)"},{"line_number":142,"context_line":"        annotations \u003d pod[\u0027metadata\u0027][\u0027annotations\u0027]"}],"source_content_type":"text/x-python","patch_set":23,"id":"7faddb67_89117260","line":139,"range":{"start_line":139,"start_character":0,"end_line":139,"end_character":34},"updated":"2019-08-27 13:32:49.000000000","message":"Can we put that in the kuryr_kubernetes.constants? And make it \u0027openstack.org/kuryr-pci-devices\u0027?","commit_id":"a69c64965ae1b9bc04a679978b6157e49d574b08"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"d501386d8b41eb3a737e5556622b6a57dbfa1308","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        return devices"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    def _annotate_device(self, pod_link, pci):"},{"line_number":154,"context_line":"        annot_name \u003d \u0027pci_devices\u0027"},{"line_number":155,"context_line":"        k8s \u003d clients.get_kubernetes_client()"},{"line_number":156,"context_line":"        pod_devices \u003d self._get_pod_devices(pod_link)"},{"line_number":157,"context_line":"        pod_devices.append(pci)"}],"source_content_type":"text/x-python","patch_set":23,"id":"7faddb67_29007e02","line":154,"range":{"start_line":154,"start_character":0,"end_line":154,"end_character":34},"updated":"2019-08-27 13:32:49.000000000","message":"Same here.","commit_id":"a69c64965ae1b9bc04a679978b6157e49d574b08"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b43c368069f3c362d6d8869a94a7b8adbb704040","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        except KeyError:"},{"line_number":146,"context_line":"            devices \u003d []"},{"line_number":147,"context_line":"        except Exception:"},{"line_number":148,"context_line":"            import traceback"},{"line_number":149,"context_line":"            traceback.print_exc()"},{"line_number":150,"context_line":"        return devices"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"7faddb67_ba5d8eed","line":148,"updated":"2019-08-27 14:56:41.000000000","message":"This ia even allowed against PEP8? You can just use LOG.exception.","commit_id":"351a6025ea76a392eada7c77f98312f119cae2ce"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"fb627d1e7fdcdea8a9eaf70fbb7820a6ae0e45b5","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            devices \u003d jsonutils.loads(json_devices)"},{"line_number":145,"context_line":"        except KeyError:"},{"line_number":146,"context_line":"            devices \u003d []"},{"line_number":147,"context_line":"        except Exception as ex:"},{"line_number":148,"context_line":"            LOG.exception(\"Exception while getting annotations: %s\", ex)"},{"line_number":149,"context_line":"        return devices"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def _annotate_device(self, pod_link, pci):"}],"source_content_type":"text/x-python","patch_set":25,"id":"7faddb67_323534a9","line":148,"range":{"start_line":147,"start_character":0,"end_line":148,"end_character":72},"updated":"2019-08-28 09:46:19.000000000","message":"No need to pass the exception here, LOG.exception automatically adds the traceback getting the exception from the context.","commit_id":"5206717f084180c2d3cc994775f36f05f5475315"}],"kuryr_kubernetes/cni/daemon/service.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"e49caf3160ed5d3e427b25ce8c0cca8feb011e34","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"9fb8cfa7_3514bce8","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"updated":"2019-07-01 07:37:35.000000000","message":"should this be optional?","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"15c22221e3ae48d94eefaa95aa37f4e7adf46f9a","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_7daf4d94","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_1a03bf44","updated":"2019-07-04 14:55:43.000000000","message":"As described in doc for this patch it is necessary to run kubelet with additional argument to enable PodResources:\n\n    KUBELET_EXTRA_ARGS\u003d\"--feature-gates KubeletPodResources\u003dtrue\"\n\nBut there may be a situation when this argument is specified, but SriovBindingDriver is not used at all.\n\nI think, you\u0027re right. It worth to create client only if we call it. Or we can create it if instance of Sriov Binding Driver is created (for example in __init__)","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"361c40d2c09caa41a511635d2b5f88e03003b970","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_616546b5","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_1e8873e5","updated":"2019-07-08 08:50:30.000000000","message":"no, it was bad idea due to following reasons:\n1. Instance of binding driver is been created only to vif computing and been deleted right after binding was completed.\n2. Instance of binding driver is been created also for unbinding.and will be deleted as well\n3. It means that setup function for pod resource client will be called many times. A lot of instances of client will be created, and the previous ones will be deleted.\n\nGenerally it creates an additional load. So it worth to leave setup of pod resource client here, in kuryr_kubernetes/cni/daemon/service.py so that it will be called once for period of CNI work","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"0379f432afe3604eb1355bf86fdc15f356c88788","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_1e8873e5","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_3e9befc9","updated":"2019-07-08 08:08:06.000000000","message":"Ilya thanks for comments.\nIn this case I think the only one casse we can optimize is when PodResources is enabled and user doesn\u0027t try to use SRIOV.\nFor this case it worth to move initialization of PodResources client into initialization of sriov binding driver.","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"6dd76e246b98f3ec0ad07fdee1f51097f5c964ef","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_f76b5f1c","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_57afd393","updated":"2019-07-04 10:03:05.000000000","message":"sure, but enabled multivif driver doesn\u0027t guarantee that exactly sriov driver will be used","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":30247,"name":"Ilya Maximets","email":"i.maximets@ovn.org","username":"i.maximets"},"change_message_id":"18430e6067a665a61c27402bb866cb8f23e3aa72","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_3e9befc9","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_6db1cd8f","updated":"2019-07-08 07:17:15.000000000","message":"My 2 cents:\nclients.setup_pod_resources_client() will never fail, because gRPC client properly handles the missing unix socket case. Failure will happen if you\u0027ll try to use it, i.e. if you will try to get the list of pod resources.\nAnother point is that, IMHO, we should not allow using SRIOV if pod-resources is not available because this will lead to unpredictable device assignments as described in the bug.\nSo,\n* if user has PodResources disabled (or kubelet version \u003c 1.13) and doesn\u0027t try to use SRIOV, everything will be fine.\n* if user has PodResources disabled and will try to use SRIOV, there will be gRPC exception and CNI will fail the port binding. Isn\u0027t it a normal behavior?\n* if user has PodResources enabled and doesn\u0027t try to use SRIOV, everything is fine. Just a slight memory overhead on gRPC client.\n* if user has PodResources enabled and will try to use SRIOV, this is our main case, everything should be OK.","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"557ca15034f88d81211e462ebcf3fda59f926e58","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_cdd00154","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_7daf4d94","updated":"2019-07-05 12:54:30.000000000","message":"Sure, sure, but we now support multiple deployment tools that deploy Kuryr - e.g. openshift-ansible and openshift/installer, Magnum in the future. We cannot expect them to enable PodResources server, especially as we support OpenShift 3.11, which is based on K8s 1.11, which haven\u0027t had that feature according to [1].\n\nWe need to make sure enablement of KubeletPodResources is totally optional.\n\n[1] https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"19bfcfe46c1e3a27a7c025da095f8a6f455d595b","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_1a03bf44","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_9203190e","updated":"2019-07-04 13:17:47.000000000","message":"I have a feeling I asked about that in another review but can\u0027t find that at the moment. Generally I don\u0027t like creating it unconditionally as I\u0027m not sure if it\u0027ll always work (kubelet is run in various configurations, e.g. in containers, also isn\u0027t that PodResources server an option to kubelet command?).\n\nWould it be possible to just create it on first call to get_pod_resources_client()?","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"8c5928146965400001f8c65a48bc81976f018497","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_6db1cd8f","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_cdd00154","updated":"2019-07-05 14:31:29.000000000","message":"Do you mean that user can try to use sriov functionality without using k8s version \u003e\u003d 1.13 and face problems?\nLet\u0027s suppose that we try to setup pod_resources_client in first it\u0027s call in SriovBindingDriver.\n1. If k8s version \u003e\u003d 1.13 then it\u0027s ok\n2. If k8s version \u003c 1.13 and k8s has not feature according then obviously clients.setup_pod_resources_client() will fail.\n\nSo what do you suggest? How to ensure that feature is enabled?\nMaybe try to check version of k8s and check kubelet\u0027s arguments?","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"db4e16ccfafa378e72585e7b96193274d3a6bcaa","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_9203190e","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_f76b5f1c","updated":"2019-07-04 10:14:02.000000000","message":"yes, but at least the opposite is true, right? If the multivif is not enabled, SRIOV will not be used, right?\n\nPerhaps Michal has a better idea here","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"dd255fea32f721b1ea357b623f07a714ad268346","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"9fb8cfa7_d0776e9f","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"9fb8cfa7_3514bce8","updated":"2019-07-01 08:33:47.000000000","message":"I think this is a right place for clients initialization.\nActually, functionality of pod_resources_client is used in Sriov binding driver only.\nDo you think it worth to add it\u0027s initialization into Sriov binding driver initialization?","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"a9a51d3ccc8b7c498ca82ecfc42a10e8ac64e9b0","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"9fb8cfa7_5fba866e","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"9fb8cfa7_3f80f2bf","updated":"2019-07-03 07:01:06.000000000","message":"sounds good, I agree\nso do we have to check if instance of Sriov Binding Driver exists?\nBut as I understand initialization on line 282 happens before drivers initialization , isn\u0027t it?","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"e6fe25e38cb1b3c02d30a05a32727d93310badb3","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_57afd393","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"9fb8cfa7_5fba866e","updated":"2019-07-04 09:49:34.000000000","message":"umm, I wonder if we can at least check if the multivif driver is enabled. That is needed for SRIOV right?","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"1a0dedb5bb137919a834c947b9bbf2b46796f428","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":15,"id":"9fb8cfa7_3f80f2bf","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"9fb8cfa7_d0776e9f","updated":"2019-07-03 06:51:59.000000000","message":"that is why I\u0027m asking. This is the point where everything is initialized, but pod_resources_client is only needed for sriov driver. So, I would still leave it here but make it optional depending on the drivers configured, what do you think?","commit_id":"b3e22c6f1871106b527f709ec58ae8afb33ffe42"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"7269c0a6397013f88491d55ee79462674414e0c8","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":18,"id":"7faddb67_d1628116","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"updated":"2019-07-30 07:26:31.000000000","message":"I\u0027m still not completely happy with always initialiting this. Perhaps on the clients side we can check if sriov config options are set (i.e., the kubelet_root_dir) or have an specific disabled option), and if not just return None in this function instead of calling PodResourcesClient init","commit_id":"f4f1471810a2907bf943b7f55bab117e91334c33"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"2db2101534f596870a783360c606e7bc54ceb3b5","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":18,"id":"7faddb67_b6eb3876","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_d1628116","updated":"2019-07-30 10:47:34.000000000","message":"regarding you suggestion, Ilia left good comment\n\n    My 2 cents: clients.setup_pod_resources_client() will never fail, because gRPC client properly handles the missing unix socket case. Failure will happen if you\u0027ll try to use it, i.e. if you will try to get the list of pod resources. Another point is that, IMHO, we should not allow using SRIOV if pod-resources is not available because this will lead to unpredictable device assignments as described in the bug. So,\n\n    if user has PodResources disabled (or kubelet version \u003c 1.13) and doesn\u0027t try to use SRIOV, everything will be fine.\n    if user has PodResources disabled and will try to use SRIOV, there will be gRPC exception and CNI will fail the port binding. Isn\u0027t it a normal behavior?\n    if user has PodResources enabled and doesn\u0027t try to use SRIOV, everything is fine. Just a slight memory overhead on gRPC client.\n    if user has PodResources enabled and will try to use SRIOV, this is our main case, everything should be OK.\n\nso I\u0027m not sure if we should add special check one more time. If you really want to add check of kubelet_root_dir in config in client.py, we can do it","commit_id":"f4f1471810a2907bf943b7f55bab117e91334c33"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"8bf84168cb641a4d8d648dfb6bdf105ecce165ee","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":20,"id":"7faddb67_c2bc83bc","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"updated":"2019-08-06 09:50:04.000000000","message":"I would still prefer to have something on the setup_pod_resource_client() that skips the initialization is kubelet_root_dir is not set. This way we have a way to \u0027disable\u0027 it, not just having it enable always but failing if used","commit_id":"4dc6db3432aeee44e013df421e4eb15882f3e283"},{"author":{"_account_id":30247,"name":"Ilya Maximets","email":"i.maximets@ovn.org","username":"i.maximets"},"change_message_id":"d04091164c62a5fd8559860bf7ac91f89c5b5a3e","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        os_vif.initialize()"},{"line_number":281,"context_line":"        clients.setup_kubernetes_client()"},{"line_number":282,"context_line":"        clients.setup_pod_resources_client()"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        self.manager \u003d multiprocessing.Manager()"},{"line_number":285,"context_line":"        registry \u003d self.manager.dict()  # For Watcher-\u003eServer communication."}],"source_content_type":"text/x-python","patch_set":20,"id":"7faddb67_82d1abce","line":282,"range":{"start_line":282,"start_character":8,"end_line":282,"end_character":44},"in_reply_to":"7faddb67_c2bc83bc","updated":"2019-08-06 09:57:09.000000000","message":"kubelet_root_dir has a default value, so it\u0027s always set.","commit_id":"4dc6db3432aeee44e013df421e4eb15882f3e283"}],"kuryr_kubernetes/objects/vif.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"12c57c845f95a5c819e986f9dcb27d63c5efa05b","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        # physnet of the VIF"},{"line_number":81,"context_line":"        \u0027physnet\u0027: obj_fields.StringField(),"},{"line_number":82,"context_line":"        \u0027pod_name\u0027: obj_fields.StringField(),"},{"line_number":83,"context_line":"        \u0027pod_link\u0027: obj_fields.StringField()"},{"line_number":84,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":10,"id":"bfb3d3c7_88ba21be","line":83,"updated":"2019-05-31 14:29:57.000000000","message":"Please add \",\" at the end, as indicated in OpenStack\u0027s hacking guide.","commit_id":"be490b382394b8657f35db7ba904c721642ee885"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"d4c1c9746a066a9c25e0567327d44ca883276663","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        # physnet of the VIF"},{"line_number":81,"context_line":"        \u0027physnet\u0027: obj_fields.StringField(),"},{"line_number":82,"context_line":"        \u0027pod_name\u0027: obj_fields.StringField(),"},{"line_number":83,"context_line":"        \u0027pod_link\u0027: obj_fields.StringField()"},{"line_number":84,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fb8cfa7_192429c6","line":83,"in_reply_to":"bfb3d3c7_88ba21be","updated":"2019-06-03 08:40:22.000000000","message":"Done","commit_id":"be490b382394b8657f35db7ba904c721642ee885"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"7b061b934a2ef0db30023c8655c12bd313d7244e","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    fields \u003d {"},{"line_number":80,"context_line":"        # physnet of the VIF"},{"line_number":81,"context_line":"        \u0027physnet\u0027: obj_fields.StringField(),"},{"line_number":82,"context_line":"        \u0027pod_name\u0027: obj_fields.StringField(),"},{"line_number":83,"context_line":"        \u0027pod_link\u0027: obj_fields.StringField(),"},{"line_number":84,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_989f9d4f","line":83,"range":{"start_line":82,"start_character":0,"end_line":83,"end_character":45},"updated":"2019-06-04 07:21:41.000000000","message":"this will require a bump on the version","commit_id":"60071b1096706d0f3fd71bc981b65e773630e720"},{"author":{"_account_id":24604,"name":"Danil Golov","email":"d.golov@samsung.com","username":"d.golov"},"change_message_id":"e26ef51f3633e00e25844ca9ba2c1c43788a7124","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    fields \u003d {"},{"line_number":80,"context_line":"        # physnet of the VIF"},{"line_number":81,"context_line":"        \u0027physnet\u0027: obj_fields.StringField(),"},{"line_number":82,"context_line":"        \u0027pod_name\u0027: obj_fields.StringField(),"},{"line_number":83,"context_line":"        \u0027pod_link\u0027: obj_fields.StringField(),"},{"line_number":84,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_82272b65","line":83,"range":{"start_line":82,"start_character":0,"end_line":83,"end_character":45},"in_reply_to":"9fb8cfa7_989f9d4f","updated":"2019-06-04 09:44:38.000000000","message":"Done","commit_id":"60071b1096706d0f3fd71bc981b65e773630e720"}]}
