)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"3f8e788cc8cf54df5d5ac892441e2a3c7f5d7031","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"baaf2f32_cf4eebce","updated":"2022-03-04 07:52:35.000000000","message":"oh my... great catch! Perhaps this would need some docs update though, and either a lp bug or blueprint, depending on how it wants to be treated (bug or feature) ","commit_id":"039fe55dbf68dc1b01f3a232418d70e6ed06c543"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"8bfb8705de81f9fe40953e80e44ec1dad2c8c2b2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"323e86cb_676e79f6","in_reply_to":"baaf2f32_cf4eebce","updated":"2022-03-04 11:54:30.000000000","message":"Bug added. I haven\u0027t found docs that are in need of critical update with this addition.","commit_id":"039fe55dbf68dc1b01f3a232418d70e6ed06c543"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"db68d5ebec53cef393abfa5d5f74ed29745e28a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"607f2f28_ec3396c6","updated":"2022-03-07 12:48:19.000000000","message":"Nicely handled! Just to raise discussion: another possible solution would be to only watch for Pod events and make sure the KuryrPort CRD is fetched right before handling the event. I don\u0027t have a strong opinion about it.\n\nBTW, regarding the docs, maybe a small note can be added here about the new watcher\nhttps://docs.openstack.org/kuryr-kubernetes/latest/devref/kuryr_kubernetes_design.html#cni-daemon\n\n","commit_id":"b76348435f142274506d37d56477df15d2055902"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"9af06b53d234c0c90fc2226923ac1617e3008ff3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"de6c7d0f_86b893b5","in_reply_to":"607f2f28_ec3396c6","updated":"2022-03-07 16:51:23.000000000","message":"The problem with that approach is - what do we do if KuryrPort is not there? Do we wait and repeat the call? Do we fail the CNI call? What if UID doesn\u0027t match? Do we wait? Do we drop the request? I\u0027m not sure if this solves much comparing to the present approach.\n\nA simpler approach is taken by ovn-kube, it\u0027s just waiting while polling until everything is set up by the the main service. The thing is that for us it\u0027s not much else than waiting for an event IMO.","commit_id":"b76348435f142274506d37d56477df15d2055902"}],"kuryr_kubernetes/cni/handlers.py":[{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"db68d5ebec53cef393abfa5d5f74ed29745e28a0","unresolved":true,"context_lines":[{"line_number":44,"context_line":"                                          \u0027vifs\u0027: vifs,"},{"line_number":45,"context_line":"                                          \u0027containerid\u0027: None,"},{"line_number":46,"context_line":"                                          \u0027vif_unplugged\u0027: False,"},{"line_number":47,"context_line":"                                          \u0027del_received\u0027: False}"},{"line_number":48,"context_line":"            else:"},{"line_number":49,"context_line":"                old_vifs \u003d self.registry[kp_name][\u0027vifs\u0027]"},{"line_number":50,"context_line":"                for iface in vifs:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f1fba2d_b47756e7","line":47,"range":{"start_line":47,"start_character":58,"end_line":47,"end_character":63},"updated":"2022-03-07 12:48:19.000000000","message":"should it be True?","commit_id":"b76348435f142274506d37d56477df15d2055902"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"9af06b53d234c0c90fc2226923ac1617e3008ff3","unresolved":true,"context_lines":[{"line_number":44,"context_line":"                                          \u0027vifs\u0027: vifs,"},{"line_number":45,"context_line":"                                          \u0027containerid\u0027: None,"},{"line_number":46,"context_line":"                                          \u0027vif_unplugged\u0027: False,"},{"line_number":47,"context_line":"                                          \u0027del_received\u0027: False}"},{"line_number":48,"context_line":"            else:"},{"line_number":49,"context_line":"                old_vifs \u003d self.registry[kp_name][\u0027vifs\u0027]"},{"line_number":50,"context_line":"                for iface in vifs:"}],"source_content_type":"text/x-python","patch_set":3,"id":"48eeb0e8_bda8bd93","line":47,"range":{"start_line":47,"start_character":58,"end_line":47,"end_character":63},"in_reply_to":"3f1fba2d_b47756e7","updated":"2022-03-07 16:51:23.000000000","message":"Not really, this is just a regular event when we\u0027ve just got a new ADD or MODIFIED KuryrPort event, so it\u0027s not deleted yet.","commit_id":"b76348435f142274506d37d56477df15d2055902"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"db68d5ebec53cef393abfa5d5f74ed29745e28a0","unresolved":true,"context_lines":[{"line_number":56,"context_line":"    def on_deleted(self, kuryrport, *args, **kwargs):"},{"line_number":57,"context_line":"        kp_name \u003d utils.get_res_unique_name(kuryrport)"},{"line_number":58,"context_line":"        try:"},{"line_number":59,"context_line":"            if (kp_name in self.registry and self.registry[kp_name]"},{"line_number":60,"context_line":"                    !\u003d k_const.CNI_DELETED_POD_SENTINEL):"},{"line_number":61,"context_line":"                # NOTE(ndesh): We need to lock here to avoid race condition"},{"line_number":62,"context_line":"                #              with the deletion code for CNI DEL so that"},{"line_number":63,"context_line":"                #              we delete the registry entry exactly once"}],"source_content_type":"text/x-python","patch_set":3,"id":"9e0d32ee_6935a7e1","line":60,"range":{"start_line":59,"start_character":41,"end_line":60,"end_character":57},"updated":"2022-03-07 12:48:19.000000000","message":"Isn\u0027t the dict entry supposed to be deleted regardless if the KuryrPort was created or not?\nOr it\u0027s being cleaned up here https://review.opendev.org/c/openstack/kuryr-kubernetes/+/831822/3/kuryr_kubernetes/cni/plugins/k8s_cni_registry.py?","commit_id":"b76348435f142274506d37d56477df15d2055902"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"9af06b53d234c0c90fc2226923ac1617e3008ff3","unresolved":true,"context_lines":[{"line_number":56,"context_line":"    def on_deleted(self, kuryrport, *args, **kwargs):"},{"line_number":57,"context_line":"        kp_name \u003d utils.get_res_unique_name(kuryrport)"},{"line_number":58,"context_line":"        try:"},{"line_number":59,"context_line":"            if (kp_name in self.registry and self.registry[kp_name]"},{"line_number":60,"context_line":"                    !\u003d k_const.CNI_DELETED_POD_SENTINEL):"},{"line_number":61,"context_line":"                # NOTE(ndesh): We need to lock here to avoid race condition"},{"line_number":62,"context_line":"                #              with the deletion code for CNI DEL so that"},{"line_number":63,"context_line":"                #              we delete the registry entry exactly once"}],"source_content_type":"text/x-python","patch_set":3,"id":"403ef190_41e5557e","line":60,"range":{"start_line":59,"start_character":41,"end_line":60,"end_character":57},"in_reply_to":"9e0d32ee_6935a7e1","updated":"2022-03-07 16:51:23.000000000","message":"Oh, even more interestingly, in case we get CNI_DELETED_POD_SENTINEL here, this should never be called as KuryrPort never got created, so certainly we shouldn\u0027t rely on deleting it here. It doesn\u0027t hurt to just make sure we\u0027re not touching anything if it\u0027s a sentinel though.","commit_id":"b76348435f142274506d37d56477df15d2055902"}],"kuryr_kubernetes/cni/plugins/k8s_cni_registry.py":[{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"db68d5ebec53cef393abfa5d5f74ed29745e28a0","unresolved":true,"context_lines":[{"line_number":31,"context_line":"CONF \u003d cfg.CONF"},{"line_number":32,"context_line":"RETRY_DELAY \u003d 1000  # 1 second in milliseconds"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# TODO(dulek, gryf): Another corner case is (and was) when pod is deleted"},{"line_number":35,"context_line":"# before it\u0027s corresponding CRD was created and populated by vifs by"},{"line_number":36,"context_line":"# controller or even noticed by any watcher. Kubelet will try to delete such"},{"line_number":37,"context_line":"# vif, but we will have no data about it. This is currently worked around by"}],"source_content_type":"text/x-python","patch_set":3,"id":"a901775f_09f4d54c","line":34,"updated":"2022-03-07 12:48:19.000000000","message":"looks like this TODO can be removed.","commit_id":"b76348435f142274506d37d56477df15d2055902"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"9af06b53d234c0c90fc2226923ac1617e3008ff3","unresolved":false,"context_lines":[{"line_number":31,"context_line":"CONF \u003d cfg.CONF"},{"line_number":32,"context_line":"RETRY_DELAY \u003d 1000  # 1 second in milliseconds"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# TODO(dulek, gryf): Another corner case is (and was) when pod is deleted"},{"line_number":35,"context_line":"# before it\u0027s corresponding CRD was created and populated by vifs by"},{"line_number":36,"context_line":"# controller or even noticed by any watcher. Kubelet will try to delete such"},{"line_number":37,"context_line":"# vif, but we will have no data about it. This is currently worked around by"}],"source_content_type":"text/x-python","patch_set":3,"id":"3b4a662c_2270f620","line":34,"in_reply_to":"a901775f_09f4d54c","updated":"2022-03-07 16:51:23.000000000","message":"Ack","commit_id":"b76348435f142274506d37d56477df15d2055902"}]}
