)]}'
{"kuryr_kubernetes/controller/drivers/nested_vlan_vif.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b8e9ce5d6080b04b7957cfbc9dc5c39736ee9155","unresolved":true,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"DEFAULT_MAX_RETRY_COUNT \u003d 3"},{"line_number":35,"context_line":"DEFAULT_RETRY_INTERVAL \u003d 1"},{"line_number":36,"context_line":"ACTIVE_TIMEOUT \u003d 90"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"CONF \u003d cfg.CONF"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"76af417f_e5869832","line":36,"range":{"start_line":36,"start_character":0,"end_line":36,"end_character":19},"updated":"2021-06-08 07:19:06.000000000","message":"perhaps 90 second is not the \"right\" timeout. Perhaps good approach is to take the same timeout as nova has for waiting for the port to become active before failing","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"1f9758950b55d0df05fdbd1cc19c18a50933cc25","unresolved":true,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"DEFAULT_MAX_RETRY_COUNT \u003d 3"},{"line_number":35,"context_line":"DEFAULT_RETRY_INTERVAL \u003d 1"},{"line_number":36,"context_line":"ACTIVE_TIMEOUT \u003d 90"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"CONF \u003d cfg.CONF"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"f548f9fb_337847e5","line":36,"range":{"start_line":36,"start_character":0,"end_line":36,"end_character":19},"in_reply_to":"76af417f_e5869832","updated":"2021-06-08 08:04:25.000000000","message":"Sure, let me check it.","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"aa340bada2c09d68435854d2d645fc47952fe11f","unresolved":true,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"DEFAULT_MAX_RETRY_COUNT \u003d 3"},{"line_number":35,"context_line":"DEFAULT_RETRY_INTERVAL \u003d 1"},{"line_number":36,"context_line":"ACTIVE_TIMEOUT \u003d 90"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"CONF \u003d cfg.CONF"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"80b326ae_1b1d4bf2","line":36,"range":{"start_line":36,"start_character":0,"end_line":36,"end_character":19},"in_reply_to":"f548f9fb_337847e5","updated":"2021-06-08 10:00:58.000000000","message":"Okay, so this is 300 seconds in Nova by default [1] and our RetryHandler times out after 500 seconds of wait. I think I\u0027d rather make it lower than 300 seconds anyway. Note that in case of Nova it\u0027s not about subports being attached to trunk but regular ports plugged on the node. I guess we can expect more from Neutron. Plus subports will already sit a while in the pool in some cases.\n\n[1] https://github.com/openstack/nova/blob/50fdbc752a9ca9c31488140ef2997ed59d861a41/nova/conf/compute.py#L160","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b8e9ce5d6080b04b7957cfbc9dc5c39736ee9155","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                            \u0027reattach the subport to trunk %s using VLAN ID %s\u0027"},{"line_number":136,"context_line":"                            \u0027 to fix it.\u0027, vif.id, retry_info[\u0027elapsed\u0027],"},{"line_number":137,"context_line":"                            trunk_id, vif.vlan_id)"},{"line_number":138,"context_line":"                self._remove_subport(trunk_id, vif.id)"},{"line_number":139,"context_line":"                self._add_subport(trunk_id, vif.id,"},{"line_number":140,"context_line":"                                  requested_vlan_id\u003dvif.vlan_id)"},{"line_number":141,"context_line":"                LOG.warning(\"Reattached subport %s, it\u0027s state will be \""},{"line_number":142,"context_line":"                            \"rechecked when event will be retried.\", vif.id)"},{"line_number":143,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"2d6e33ca_2f8ed525","line":140,"range":{"start_line":138,"start_character":1,"end_line":140,"end_character":64},"updated":"2021-06-08 07:19:06.000000000","message":"is there a chance of \"collision\" due to making this 2 calls one after another? meaning different neutron server will handled the remove and add, and they won\u0027t be yet in sync and the add_subport fails due to \"port already attached\" or the like","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"1f9758950b55d0df05fdbd1cc19c18a50933cc25","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                            \u0027reattach the subport to trunk %s using VLAN ID %s\u0027"},{"line_number":136,"context_line":"                            \u0027 to fix it.\u0027, vif.id, retry_info[\u0027elapsed\u0027],"},{"line_number":137,"context_line":"                            trunk_id, vif.vlan_id)"},{"line_number":138,"context_line":"                self._remove_subport(trunk_id, vif.id)"},{"line_number":139,"context_line":"                self._add_subport(trunk_id, vif.id,"},{"line_number":140,"context_line":"                                  requested_vlan_id\u003dvif.vlan_id)"},{"line_number":141,"context_line":"                LOG.warning(\"Reattached subport %s, it\u0027s state will be \""},{"line_number":142,"context_line":"                            \"rechecked when event will be retried.\", vif.id)"},{"line_number":143,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"aa4c5ac0_ac347924","line":140,"range":{"start_line":138,"start_character":1,"end_line":140,"end_character":64},"in_reply_to":"2d6e33ca_2f8ed525","updated":"2021-06-08 08:04:25.000000000","message":"Oh my, I don\u0027t know, do we really need to code around such deficiencies? I\u0027ll try asking Neutron folks.","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"22225e47321b06fc465ccc8e7e9072e344a2c716","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                            \u0027reattach the subport to trunk %s using VLAN ID %s\u0027"},{"line_number":136,"context_line":"                            \u0027 to fix it.\u0027, vif.id, retry_info[\u0027elapsed\u0027],"},{"line_number":137,"context_line":"                            trunk_id, vif.vlan_id)"},{"line_number":138,"context_line":"                self._remove_subport(trunk_id, vif.id)"},{"line_number":139,"context_line":"                self._add_subport(trunk_id, vif.id,"},{"line_number":140,"context_line":"                                  requested_vlan_id\u003dvif.vlan_id)"},{"line_number":141,"context_line":"                LOG.warning(\"Reattached subport %s, it\u0027s state will be \""},{"line_number":142,"context_line":"                            \"rechecked when event will be retried.\", vif.id)"},{"line_number":143,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"425f865b_7782ded7","line":140,"range":{"start_line":138,"start_character":1,"end_line":140,"end_character":64},"in_reply_to":"2d6e33ca_2f8ed525","updated":"2021-06-08 08:05:21.000000000","message":"Yes, I think that this can happen, looking at neutron server code, subport is created in https://github.com/openstack/neutron/blob/master/neutron/services/trunk/plugin.py#L358 and OVO\u0027s create() method will raise DuplicateSubPort exception in such case: https://github.com/openstack/neutron/blob/master/neutron/objects/trunk.py#L80\nThat exception will be returned directly to the client as it\u0027s not handled anywhere in the neutron.","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"6bc2e85337089e715250947a53e3e39d436f64b0","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                            \u0027reattach the subport to trunk %s using VLAN ID %s\u0027"},{"line_number":136,"context_line":"                            \u0027 to fix it.\u0027, vif.id, retry_info[\u0027elapsed\u0027],"},{"line_number":137,"context_line":"                            trunk_id, vif.vlan_id)"},{"line_number":138,"context_line":"                self._remove_subport(trunk_id, vif.id)"},{"line_number":139,"context_line":"                self._add_subport(trunk_id, vif.id,"},{"line_number":140,"context_line":"                                  requested_vlan_id\u003dvif.vlan_id)"},{"line_number":141,"context_line":"                LOG.warning(\"Reattached subport %s, it\u0027s state will be \""},{"line_number":142,"context_line":"                            \"rechecked when event will be retried.\", vif.id)"},{"line_number":143,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"4ffd0547_dc72bb2a","line":140,"range":{"start_line":138,"start_character":1,"end_line":140,"end_character":64},"in_reply_to":"31163289_eaf4dea8","updated":"2021-06-08 15:15:47.000000000","message":"It seems that it might do that, yes.","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"aa340bada2c09d68435854d2d645fc47952fe11f","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                            \u0027reattach the subport to trunk %s using VLAN ID %s\u0027"},{"line_number":136,"context_line":"                            \u0027 to fix it.\u0027, vif.id, retry_info[\u0027elapsed\u0027],"},{"line_number":137,"context_line":"                            trunk_id, vif.vlan_id)"},{"line_number":138,"context_line":"                self._remove_subport(trunk_id, vif.id)"},{"line_number":139,"context_line":"                self._add_subport(trunk_id, vif.id,"},{"line_number":140,"context_line":"                                  requested_vlan_id\u003dvif.vlan_id)"},{"line_number":141,"context_line":"                LOG.warning(\"Reattached subport %s, it\u0027s state will be \""},{"line_number":142,"context_line":"                            \"rechecked when event will be retried.\", vif.id)"},{"line_number":143,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"f556f191_d2f39574","line":140,"range":{"start_line":138,"start_character":1,"end_line":140,"end_character":64},"in_reply_to":"425f865b_7782ded7","updated":"2021-06-08 10:00:58.000000000","message":"Continuation of discussion on IRC [1] - Galera should not be out of sync between neutron-servers, so this is pretty implausible. I\u0027m going to protect from this in another way - I\u0027ll ignore 404 on _remove_subport() to make sure that if _remove_subport() succeeded but _add_subport() failed we\u0027ll still have a way to recover from this once the KuryrPort is handled again.\n\n[1] https://meetings.opendev.org/irclogs/%23openstack-neutron/%23openstack-neutron.2021-06-08.log.html#t2021-06-08T08:05:45","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"d41f6de9f3cc139adb44857de8b59cab9d99e6af","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                            \u0027reattach the subport to trunk %s using VLAN ID %s\u0027"},{"line_number":136,"context_line":"                            \u0027 to fix it.\u0027, vif.id, retry_info[\u0027elapsed\u0027],"},{"line_number":137,"context_line":"                            trunk_id, vif.vlan_id)"},{"line_number":138,"context_line":"                self._remove_subport(trunk_id, vif.id)"},{"line_number":139,"context_line":"                self._add_subport(trunk_id, vif.id,"},{"line_number":140,"context_line":"                                  requested_vlan_id\u003dvif.vlan_id)"},{"line_number":141,"context_line":"                LOG.warning(\"Reattached subport %s, it\u0027s state will be \""},{"line_number":142,"context_line":"                            \"rechecked when event will be retried.\", vif.id)"},{"line_number":143,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"31163289_eaf4dea8","line":140,"range":{"start_line":138,"start_character":1,"end_line":140,"end_character":64},"in_reply_to":"f556f191_d2f39574","updated":"2021-06-08 14:50:07.000000000","message":"If _add_subport fail with an exception would it likely crash the controller and only then retry the event?","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":3153,"name":"Emilien Macchi","email":"emilien@redhat.com","username":"emilienm"},"change_message_id":"b97a5a0d3afa6c980107b47ddb0d1a66e00e815c","unresolved":true,"context_lines":[{"line_number":138,"context_line":"                self._remove_subport(trunk_id, vif.id)"},{"line_number":139,"context_line":"                self._add_subport(trunk_id, vif.id,"},{"line_number":140,"context_line":"                                  requested_vlan_id\u003dvif.vlan_id)"},{"line_number":141,"context_line":"                LOG.warning(\"Reattached subport %s, it\u0027s state will be \""},{"line_number":142,"context_line":"                            \"rechecked when event will be retried.\", vif.id)"},{"line_number":143,"context_line":"            raise"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"c45225ce_7c5e6296","line":141,"updated":"2021-06-07 20:38:07.000000000","message":"s/it\u0027s/its/ ?","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"1f9758950b55d0df05fdbd1cc19c18a50933cc25","unresolved":false,"context_lines":[{"line_number":138,"context_line":"                self._remove_subport(trunk_id, vif.id)"},{"line_number":139,"context_line":"                self._add_subport(trunk_id, vif.id,"},{"line_number":140,"context_line":"                                  requested_vlan_id\u003dvif.vlan_id)"},{"line_number":141,"context_line":"                LOG.warning(\"Reattached subport %s, it\u0027s state will be \""},{"line_number":142,"context_line":"                            \"rechecked when event will be retried.\", vif.id)"},{"line_number":143,"context_line":"            raise"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"4287b8d0_371a002a","line":141,"in_reply_to":"c45225ce_7c5e6296","updated":"2021-06-08 08:04:25.000000000","message":"I guess I\u0027ll never stop making mistakes with words being homophones to me. ;)","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"}],"kuryr_kubernetes/controller/handlers/kuryrport.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b8e9ce5d6080b04b7957cfbc9dc5c39736ee9155","unresolved":true,"context_lines":[{"line_number":58,"context_line":"                                .get_instance())"},{"line_number":59,"context_line":"        self.k8s \u003d clients.get_kubernetes_client()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def on_present(self, kuryrport_crd, *args, retry_info\u003dNone, **kwargs):"},{"line_number":62,"context_line":"        if not kuryrport_crd[\u0027status\u0027][\u0027vifs\u0027]:"},{"line_number":63,"context_line":"            # Get vifs"},{"line_number":64,"context_line":"            if not self.get_vifs(kuryrport_crd):"}],"source_content_type":"text/x-python","patch_set":2,"id":"72c2a8c1_89867cc1","line":61,"range":{"start_line":61,"start_character":40,"end_line":61,"end_character":72},"updated":"2021-06-08 07:19:06.000000000","message":"this is a bit \"ugly\", perhaps can be part of the kwargs and check if it is defined","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"1f9758950b55d0df05fdbd1cc19c18a50933cc25","unresolved":true,"context_lines":[{"line_number":58,"context_line":"                                .get_instance())"},{"line_number":59,"context_line":"        self.k8s \u003d clients.get_kubernetes_client()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def on_present(self, kuryrport_crd, *args, retry_info\u003dNone, **kwargs):"},{"line_number":62,"context_line":"        if not kuryrport_crd[\u0027status\u0027][\u0027vifs\u0027]:"},{"line_number":63,"context_line":"            # Get vifs"},{"line_number":64,"context_line":"            if not self.get_vifs(kuryrport_crd):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3818c984_b15013ba","line":61,"range":{"start_line":61,"start_character":40,"end_line":61,"end_character":72},"in_reply_to":"72c2a8c1_89867cc1","updated":"2021-06-08 08:04:25.000000000","message":"It feels different to me, but now that I think of it, I never seen it written like this elsewhere.","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b8e9ce5d6080b04b7957cfbc9dc5c39736ee9155","unresolved":true,"context_lines":[{"line_number":86,"context_line":"                    # add information to CRD instead of the pod."},{"line_number":87,"context_line":"                    driver_utils.update_port_pci_info(pod_node, data[\u0027vif\u0027])"},{"line_number":88,"context_line":"                if not data[\u0027vif\u0027].active:"},{"line_number":89,"context_line":"                    if not pod:"},{"line_number":90,"context_line":"                        pod \u003d self._get_pod(kuryrport_crd)"},{"line_number":91,"context_line":"                    try:"},{"line_number":92,"context_line":"                        self._drv_vif_pool.activate_vif(data[\u0027vif\u0027], pod\u003dpod,"},{"line_number":93,"context_line":"                                                        retry_info\u003dretry_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"33150062_2b080c1e","line":90,"range":{"start_line":89,"start_character":1,"end_line":90,"end_character":58},"updated":"2021-06-08 07:19:06.000000000","message":"why not simply have this at line 78?","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"1f9758950b55d0df05fdbd1cc19c18a50933cc25","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                    # add information to CRD instead of the pod."},{"line_number":87,"context_line":"                    driver_utils.update_port_pci_info(pod_node, data[\u0027vif\u0027])"},{"line_number":88,"context_line":"                if not data[\u0027vif\u0027].active:"},{"line_number":89,"context_line":"                    if not pod:"},{"line_number":90,"context_line":"                        pod \u003d self._get_pod(kuryrport_crd)"},{"line_number":91,"context_line":"                    try:"},{"line_number":92,"context_line":"                        self._drv_vif_pool.activate_vif(data[\u0027vif\u0027], pod\u003dpod,"},{"line_number":93,"context_line":"                                                        retry_info\u003dretry_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"8ce3f3f7_b764a860","line":90,"range":{"start_line":89,"start_character":1,"end_line":90,"end_character":58},"in_reply_to":"33150062_2b080c1e","updated":"2021-06-08 08:04:25.000000000","message":"Ah right, seems like we\u0027ll return early on line 74 in case all ports are ACTIVE. Okay!","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"}],"kuryr_kubernetes/handlers/retry.py":[{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"ea093c1a49e103bc89970a1cc5ad262247035c8f","unresolved":true,"context_lines":[{"line_number":79,"context_line":"                                      \"execution.\")"},{"line_number":80,"context_line":"            try:"},{"line_number":81,"context_line":"                info \u003d {"},{"line_number":82,"context_line":"                    \u0027attempt\u0027: attempt,"},{"line_number":83,"context_line":"                    \u0027elapsed\u0027: time.time() - start_time"},{"line_number":84,"context_line":"                }"},{"line_number":85,"context_line":"                self._handler(event, *args, retry_info\u003dinfo, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"087ca044_9a09ea07","line":82,"updated":"2021-06-08 08:38:44.000000000","message":"it seems the attempt info is not used anywhere, only the elapsed is used at nested_vlan_vif.py. Should it get removed?","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"aa340bada2c09d68435854d2d645fc47952fe11f","unresolved":true,"context_lines":[{"line_number":79,"context_line":"                                      \"execution.\")"},{"line_number":80,"context_line":"            try:"},{"line_number":81,"context_line":"                info \u003d {"},{"line_number":82,"context_line":"                    \u0027attempt\u0027: attempt,"},{"line_number":83,"context_line":"                    \u0027elapsed\u0027: time.time() - start_time"},{"line_number":84,"context_line":"                }"},{"line_number":85,"context_line":"                self._handler(event, *args, retry_info\u003dinfo, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e288173c_196edaef","line":82,"in_reply_to":"087ca044_9a09ea07","updated":"2021-06-08 10:00:58.000000000","message":"I added it just because it might be useful one day, but yeah, maybe no need to add it now.","commit_id":"8f5d02cf13a8243c2250d171f9caddb12f2bb66d"}]}