)]}'
{"networking_ovn/common/acl.py":[{"author":{"_account_id":23458,"name":"Dong Jun","email":"dongjun.01@bytedance.com","username":"Dongj"},"change_message_id":"c49cc7b2881dbffa23527653490c492edce408c5","unresolved":false,"context_lines":[{"line_number":393,"context_line":"                                sg_id)"},{"line_number":394,"context_line":"        for r in sg[\u0027security_group_rules\u0027]:"},{"line_number":395,"context_line":"            acl \u003d _add_sg_rule_acl_for_port(port, r)"},{"line_number":396,"context_line":"            acl_list.append(acl)"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    # Remove ACL log name and severity if not supported,"},{"line_number":399,"context_line":"    if not _acl_columns_name_severity_supported(ovn):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f96bb07_5676e3da","line":396,"range":{"start_line":396,"start_character":0,"end_line":396,"end_character":32},"updated":"2018-01-11 12:23:02.000000000","message":"There\u0027s a question, when we delete a rule, all the duplicate rules will be deleted[1] because only \u0027match\u0027 is compared.\nI think we need to compare uuid or only compare uuid for new rule deleting. If uuid does not exist(old rule), then we should remove the rule from del_acl_matches after hitting comparison for backward. Maybe someone else has a better idea.\n\n[1]https://github.com/openstack/networking-ovn/blob/9bab8d0bca7b4355f3d7fe41d79cea311e608d74/networking_ovn/ovsdb/commands.py#L571","commit_id":"012f221b021c7fa8c81c494a48c1c584030648b8"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"9bddd7491b4a7c1d81010eeb2f9ffedc798f660e","unresolved":false,"context_lines":[{"line_number":393,"context_line":"                                sg_id)"},{"line_number":394,"context_line":"        for r in sg[\u0027security_group_rules\u0027]:"},{"line_number":395,"context_line":"            acl \u003d _add_sg_rule_acl_for_port(port, r)"},{"line_number":396,"context_line":"            acl_list.append(acl)"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    # Remove ACL log name and severity if not supported,"},{"line_number":399,"context_line":"    if not _acl_columns_name_severity_supported(ovn):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f96bb07_e22dd746","line":396,"range":{"start_line":396,"start_character":0,"end_line":396,"end_character":32},"in_reply_to":"7f96bb07_5676e3da","updated":"2018-01-12 00:12:12.000000000","message":"@Dong, two cases:\n1) When the ACL to be deleted has the external id referencing the SG rule in neutron. We want to compare the \u0027match\u0027 field and also the SG rule id. If both match, we\u0027ll append the ACL to \u0027acl_del_objs_dict\u0027 (and I guess we can break the loop here as there shouldn\u0027t be more matches?) \n\n2) When the ACL to be deleted is old (ie. doesn\u0027t have the external id). What do you suggest here? Append to \u0027acl_del_objs_dict\u0027 all those ACLs in the lswitch which match the \u0027match\u0027 field *and*  don\u0027t have the external id either?","commit_id":"012f221b021c7fa8c81c494a48c1c584030648b8"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"b4c70dd07d732927fadf8daa7e6f378c652c5770","unresolved":false,"context_lines":[{"line_number":393,"context_line":"                                sg_id)"},{"line_number":394,"context_line":"        for r in sg[\u0027security_group_rules\u0027]:"},{"line_number":395,"context_line":"            acl \u003d _add_sg_rule_acl_for_port(port, r)"},{"line_number":396,"context_line":"            acl_list.append(acl)"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    # Remove ACL log name and severity if not supported,"},{"line_number":399,"context_line":"    if not _acl_columns_name_severity_supported(ovn):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f96bb07_780d088f","line":396,"range":{"start_line":396,"start_character":0,"end_line":396,"end_character":32},"in_reply_to":"7f96bb07_5676e3da","updated":"2018-01-11 16:32:28.000000000","message":"I haven\u0027t tested it but it\u0027s a good point.\n\n@Daniel, we can try to create 2 SGs and a duplicated SG rule on each SGs. Then, delete the rule from one of the SGs and check if both were removed (apparently, looking at the code that @Dong pointed to us, both will be deleted).","commit_id":"012f221b021c7fa8c81c494a48c1c584030648b8"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"d9e7ff3cd1d1456dbecf9d0303c075527b9f7eec","unresolved":false,"context_lines":[{"line_number":393,"context_line":"                                sg_id)"},{"line_number":394,"context_line":"        for r in sg[\u0027security_group_rules\u0027]:"},{"line_number":395,"context_line":"            acl \u003d _add_sg_rule_acl_for_port(port, r)"},{"line_number":396,"context_line":"            acl_list.append(acl)"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    # Remove ACL log name and severity if not supported,"},{"line_number":399,"context_line":"    if not _acl_columns_name_severity_supported(ovn):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f96bb07_cceb34ae","line":396,"range":{"start_line":396,"start_character":0,"end_line":396,"end_character":32},"in_reply_to":"7f96bb07_780d088f","updated":"2018-01-11 23:16:45.000000000","message":"Thanks guys. Dong is absolutely right, I\u0027ll fix it.\nBTW, no need that you guys try it. It\u0027s clear from the code but I also tried it out and when I delete one SG rule, all duplicated ACL entries go away.","commit_id":"012f221b021c7fa8c81c494a48c1c584030648b8"},{"author":{"_account_id":23458,"name":"Dong Jun","email":"dongjun.01@bytedance.com","username":"Dongj"},"change_message_id":"a2843d8a16cc895a346c9c53adab2a5d740db075","unresolved":false,"context_lines":[{"line_number":280,"context_line":"    return (\u0027name\u0027 in columns) and (\u0027severity\u0027 in columns)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"def _filter_security_groups_by_rule(plugin, admin_context,"},{"line_number":284,"context_line":"                                    security_group_rule, security_groups):"},{"line_number":285,"context_line":"    if not security_groups:"},{"line_number":286,"context_line":"        return set()"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    # Need accurate match including value None"},{"line_number":289,"context_line":"    filters \u003d {key: security_group_rule.get(key)"},{"line_number":290,"context_line":"               for key in (\u0027direction\u0027, \u0027protocol\u0027, \u0027ethertype\u0027,"},{"line_number":291,"context_line":"                           \u0027port_range_min\u0027, \u0027port_range_max\u0027,"},{"line_number":292,"context_line":"                           \u0027remote_ip_prefix\u0027, \u0027remote_group_id\u0027)}"},{"line_number":293,"context_line":"    filters[\u0027security_group_id\u0027] \u003d set(security_groups)"},{"line_number":294,"context_line":"    rules \u003d plugin.get_security_group_rules(admin_context,"},{"line_number":295,"context_line":"                                            filters\u003dfilters)"},{"line_number":296,"context_line":"    return set([r[\u0027security_group_id\u0027] for r in rules])"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"def update_acls_for_security_group(plugin,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f96bb07_dad651c4","line":296,"range":{"start_line":283,"start_character":0,"end_line":296,"end_character":55},"updated":"2018-01-15 07:17:36.000000000","message":"This method can be removed","commit_id":"1dca7694b7c4b66f41f9b93112dc90e658dc7521"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"75faced1df8368cca86c6fc2010a2ede72c4bb79","unresolved":false,"context_lines":[{"line_number":280,"context_line":"    return (\u0027name\u0027 in columns) and (\u0027severity\u0027 in columns)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"def _filter_security_groups_by_rule(plugin, admin_context,"},{"line_number":284,"context_line":"                                    security_group_rule, security_groups):"},{"line_number":285,"context_line":"    if not security_groups:"},{"line_number":286,"context_line":"        return set()"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    # Need accurate match including value None"},{"line_number":289,"context_line":"    filters \u003d {key: security_group_rule.get(key)"},{"line_number":290,"context_line":"               for key in (\u0027direction\u0027, \u0027protocol\u0027, \u0027ethertype\u0027,"},{"line_number":291,"context_line":"                           \u0027port_range_min\u0027, \u0027port_range_max\u0027,"},{"line_number":292,"context_line":"                           \u0027remote_ip_prefix\u0027, \u0027remote_group_id\u0027)}"},{"line_number":293,"context_line":"    filters[\u0027security_group_id\u0027] \u003d set(security_groups)"},{"line_number":294,"context_line":"    rules \u003d plugin.get_security_group_rules(admin_context,"},{"line_number":295,"context_line":"                                            filters\u003dfilters)"},{"line_number":296,"context_line":"    return set([r[\u0027security_group_id\u0027] for r in rules])"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"def update_acls_for_security_group(plugin,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f96bb07_e0d6fac3","line":296,"range":{"start_line":283,"start_character":0,"end_line":296,"end_character":55},"in_reply_to":"7f96bb07_dad651c4","updated":"2018-01-15 09:05:32.000000000","message":"Right!","commit_id":"1dca7694b7c4b66f41f9b93112dc90e658dc7521"}],"networking_ovn/common/maintenance.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"b4c70dd07d732927fadf8daa7e6f378c652c5770","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        if row.revision_number \u003d\u003d ovn_const.INITIAL_REV_NUM:"},{"line_number":145,"context_line":"            self._ovn_client.create_security_group_rule(sgr_db_obj)"},{"line_number":146,"context_line":"        else:"},{"line_number":147,"context_line":"            LOG.error(\"SG rule %s found with a revision number while this \""},{"line_number":148,"context_line":"                      \"resource doesn\u0027t support updates.\", row.resource_uuid)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    @periodics.periodic(spacing\u003dDB_CONSISTENCY_CHECK_INTERVAL,"},{"line_number":151,"context_line":"                        run_immediately\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f96bb07_98d574bc","line":148,"range":{"start_line":147,"start_character":0,"end_line":148,"end_character":77},"updated":"2018-01-11 16:32:28.000000000","message":"Perhaps we should bump the revision number of the SG Rule in the ovn_revision_numbers table here ? Otherwise it will continue to be seen as an inconsistency until it\u0027s deleted.","commit_id":"012f221b021c7fa8c81c494a48c1c584030648b8"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"d9e7ff3cd1d1456dbecf9d0303c075527b9f7eec","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        if row.revision_number \u003d\u003d ovn_const.INITIAL_REV_NUM:"},{"line_number":145,"context_line":"            self._ovn_client.create_security_group_rule(sgr_db_obj)"},{"line_number":146,"context_line":"        else:"},{"line_number":147,"context_line":"            LOG.error(\"SG rule %s found with a revision number while this \""},{"line_number":148,"context_line":"                      \"resource doesn\u0027t support updates.\", row.resource_uuid)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    @periodics.periodic(spacing\u003dDB_CONSISTENCY_CHECK_INTERVAL,"},{"line_number":151,"context_line":"                        run_immediately\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f96bb07_ace6e8e7","line":148,"range":{"start_line":147,"start_character":0,"end_line":148,"end_character":77},"in_reply_to":"7f96bb07_98d574bc","updated":"2018-01-11 23:16:45.000000000","message":"This shouldn\u0027t ever happen right?  Not sure what\u0027s best... bumping the rev would just hide a situation that should\u0027ve not happened. If this happens maybe something went wrong with the maintenance and we want an operator to have an eye:? Thoughts?","commit_id":"012f221b021c7fa8c81c494a48c1c584030648b8"}],"networking_ovn/ovn_db_sync.py":[{"author":{"_account_id":23458,"name":"Dong Jun","email":"dongjun.01@bytedance.com","username":"Dongj"},"change_message_id":"49862a7adf40500b52734a56cc8cc69d7a24a324","unresolved":false,"context_lines":[{"line_number":248,"context_line":"                                              sg_cache,"},{"line_number":249,"context_line":"                                              subnet_cache,"},{"line_number":250,"context_line":"                                              self.l3_plugin._ovn)"},{"line_number":251,"context_line":"                for acl in acl_list:"},{"line_number":252,"context_line":"                    acl[\u0027external_ids\u0027].pop(const.OVN_SG_RULE_EXT_ID_KEY, None)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"                if port_id in neutron_acls:"},{"line_number":255,"context_line":"                    neutron_acls[port_id].extend(acl_list)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f96bb07_76c3c6ee","line":252,"range":{"start_line":251,"start_character":0,"end_line":252,"end_character":79},"updated":"2018-01-12 06:10:52.000000000","message":"Remove this for repair acl.","commit_id":"012f221b021c7fa8c81c494a48c1c584030648b8"}],"networking_ovn/ovsdb/commands.py":[{"author":{"_account_id":23458,"name":"Dong Jun","email":"dongjun.01@bytedance.com","username":"Dongj"},"change_message_id":"05aef7eb9d461e400a26fda9fcd82ed1b0f1e89d","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                for acl in acls:"},{"line_number":573,"context_line":"                    match \u003d getattr(acl, \u0027match\u0027)"},{"line_number":574,"context_line":"                    acl_extids \u003d {match: getattr(acl, \u0027external_ids\u0027)}"},{"line_number":575,"context_line":"                    if (match in del_acl_matches and ("},{"line_number":576,"context_line":"                            acl_extids in del_acl_extids)):"},{"line_number":577,"context_line":"                        acl_del_objs_dict[switch_name].append(acl)"},{"line_number":578,"context_line":"        return lswitch_ovsdb_dict, acl_del_objs_dict, acl_add_values_dict"},{"line_number":579,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7f96bb07_7ff05268","line":576,"range":{"start_line":575,"start_character":20,"end_line":576,"end_character":59},"updated":"2018-01-15 02:53:57.000000000","message":"Great catch my fault.\n\"match in del_acl_matches\" branch is no longer needed, right? Because \"acl_extids in del_acl_extids\" is the complete dict comparison.","commit_id":"1dca7694b7c4b66f41f9b93112dc90e658dc7521"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"75faced1df8368cca86c6fc2010a2ede72c4bb79","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                for acl in acls:"},{"line_number":573,"context_line":"                    match \u003d getattr(acl, \u0027match\u0027)"},{"line_number":574,"context_line":"                    acl_extids \u003d {match: getattr(acl, \u0027external_ids\u0027)}"},{"line_number":575,"context_line":"                    if (match in del_acl_matches and ("},{"line_number":576,"context_line":"                            acl_extids in del_acl_extids)):"},{"line_number":577,"context_line":"                        acl_del_objs_dict[switch_name].append(acl)"},{"line_number":578,"context_line":"        return lswitch_ovsdb_dict, acl_del_objs_dict, acl_add_values_dict"},{"line_number":579,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7f96bb07_a0e0f297","line":576,"range":{"start_line":575,"start_character":20,"end_line":576,"end_character":59},"in_reply_to":"7f96bb07_7ff05268","updated":"2018-01-15 09:05:32.000000000","message":"True, thanks!","commit_id":"1dca7694b7c4b66f41f9b93112dc90e658dc7521"}],"networking_ovn/ovsdb/impl_idl_ovn.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"b4c70dd07d732927fadf8daa7e6f378c652c5770","unresolved":false,"context_lines":[{"line_number":226,"context_line":"                           \u0027dnat_and_snats\u0027: dnat_and_snats})"},{"line_number":227,"context_line":"        return result"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def get_acl_by_id(self, acl_id):"},{"line_number":230,"context_line":"        try:"},{"line_number":231,"context_line":"            return self.lookup(\u0027ACL\u0027, uuid.UUID(acl_id))"},{"line_number":232,"context_line":"        except idlutils.RowNotFound:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f96bb07_b894f0e9","line":229,"updated":"2018-01-11 16:32:28.000000000","message":"We should update the ovn_api.py module to include this new method as well [0]\n\n[0] https://github.com/openstack/networking-ovn/blob/master/networking_ovn/ovsdb/ovn_api.py#L20","commit_id":"012f221b021c7fa8c81c494a48c1c584030648b8"}]}