)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"bf4925fca89408327fbbe6f04a42faeccf13a7a0","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Copy existing IPv6 leases to generated lease file"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Because the DHCP agent does not know the IAID (identity association"},{"line_number":10,"context_line":"identifier) of assigned IPv6 addresses it\u0027s not possible to generate the"},{"line_number":11,"context_line":"lease file including IPv6 leases. Because of this IPv6 addresses are excluded"},{"line_number":12,"context_line":"when generating the lease file in case of DHCP agent restarts. This causes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"90a19754_1bdd333a","line":9,"updated":"2021-06-08 08:45:47.000000000","message":"nit: please, reduce the commit message lines size to max 72, it is easier to read here.","commit_id":"47a0fa87280954ef857301bd27c8df00a5f914fd"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"943c257408976cca157946f01a731007dde38b2e","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Copy existing IPv6 leases to generated lease file"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Because the DHCP agent does not know the IAID (identity association"},{"line_number":10,"context_line":"identifier) of assigned IPv6 addresses it\u0027s not possible to generate the"},{"line_number":11,"context_line":"lease file including IPv6 leases. Because of this IPv6 addresses are excluded"},{"line_number":12,"context_line":"when generating the lease file in case of DHCP agent restarts. This causes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7eb42e41_620eeb03","line":9,"in_reply_to":"90a19754_1bdd333a","updated":"2021-06-09 08:58:10.000000000","message":"Ack","commit_id":"47a0fa87280954ef857301bd27c8df00a5f914fd"}],"neutron/agent/linux/dhcp.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2e0649cd2082053d2023e534ebc831a2b5a81005","unresolved":false,"context_lines":[{"line_number":771,"context_line":"        if os.path.isfile(filename):"},{"line_number":772,"context_line":"            LOG.debug(\u0027Skipping initial lease file creation as a lease file \u0027"},{"line_number":773,"context_line":"                      \u0027already exists.\u0027)"},{"line_number":774,"context_line":"            return filename"},{"line_number":775,"context_line":""},{"line_number":776,"context_line":"        buf \u003d io.StringIO()"},{"line_number":777,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f621f24_4e945237","line":774,"updated":"2020-11-16 17:03:09.000000000","message":"What if the agent has been down a while and the lease file is stale?  If there is a bug in the logic below let\u0027s fix it but I don\u0027t think this is foolproof otherwise.","commit_id":"e92683d6436dac63d359e323988e0906d7a7766c"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"d43b5edec386e0c5c23f96c5e451db522aa8f4fa","unresolved":false,"context_lines":[{"line_number":771,"context_line":"        if os.path.isfile(filename):"},{"line_number":772,"context_line":"            LOG.debug(\u0027Skipping initial lease file creation as a lease file \u0027"},{"line_number":773,"context_line":"                      \u0027already exists.\u0027)"},{"line_number":774,"context_line":"            return filename"},{"line_number":775,"context_line":""},{"line_number":776,"context_line":"        buf \u003d io.StringIO()"},{"line_number":777,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f621f24_c75cc04c","line":774,"in_reply_to":"1f621f24_02806bf4","updated":"2020-11-17 07:51:49.000000000","message":"I don\u0027t think this is a concern. The static IP assignment is still in the `host` file. If a client does send a broadcast the lease has already expired. DHCP RENEW requests are sent by unicast to the the server that originally offered the lease. So any other instances will not see them anyway. There will be no NAKs due to missing entries in a lease file as a response to a DHCP DISCOVER request. The lease file is only relevant if a client want\u0027s to extend it\u0027s lease lifetime before it expires.\nThe only reason I\u0027m aware of why Neutron cares about this file at all is for the case where a DHCP agent is migrated to a different host. Then this ensures that at least for IPv4 the agent knows about potential leases which are still active because the migrated dnsmasq instance lost it\u0027s state (ie. the lease file).\nThe reason why this needs fixing is not to optimize the code but to avoid hiccups in the IPv6 network connectivity when the client has to go through a full renewal because the lease state on the dnsmasq side got discarded. This is especially relevant as systemd-networkd had a bug which resulted in long time without IPv6 address: https://github.com/systemd/systemd/issues/12623 While this is clearly a (now fixed) bug in systemd-networkd it can be worked around by not discarding the lease state.","commit_id":"e92683d6436dac63d359e323988e0906d7a7766c"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"cde63898a5d275fbf847498a78bff28352aec9bf","unresolved":false,"context_lines":[{"line_number":771,"context_line":"        if os.path.isfile(filename):"},{"line_number":772,"context_line":"            LOG.debug(\u0027Skipping initial lease file creation as a lease file \u0027"},{"line_number":773,"context_line":"                      \u0027already exists.\u0027)"},{"line_number":774,"context_line":"            return filename"},{"line_number":775,"context_line":""},{"line_number":776,"context_line":"        buf \u003d io.StringIO()"},{"line_number":777,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f621f24_ae336661","line":774,"in_reply_to":"1f621f24_4e945237","updated":"2020-11-16 17:11:50.000000000","message":"I don\u0027t think this is a problem as the lease file contains a timestamp when the lease will expire. So the stale leases will just be disregarded and pruned by dnsmasq. And as the clients which received these leases also know that they have expired, they either already received a new lease form another agent or should not expect to be able to renew the lease without going through a full DHCPDISCOVER/DHCPREQUEST cycle.","commit_id":"e92683d6436dac63d359e323988e0906d7a7766c"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0e0966f3d986e753d2fc19fdd4f19ec5b3dc79c7","unresolved":false,"context_lines":[{"line_number":771,"context_line":"        if os.path.isfile(filename):"},{"line_number":772,"context_line":"            LOG.debug(\u0027Skipping initial lease file creation as a lease file \u0027"},{"line_number":773,"context_line":"                      \u0027already exists.\u0027)"},{"line_number":774,"context_line":"            return filename"},{"line_number":775,"context_line":""},{"line_number":776,"context_line":"        buf \u003d io.StringIO()"},{"line_number":777,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f621f24_02806bf4","line":774,"in_reply_to":"1f621f24_ae336661","updated":"2020-11-16 22:19:38.000000000","message":"I guess we would have to check that when a failover occurs when one agent has been offline, i.e. the other dhcp-agent is stopped and a second started, that a client asking this dnsmasq instance (i think broadcast in that case), doesn\u0027t get a NAK since it\u0027s lease won\u0027t be here.  That\u0027s what we\u0027re trying to avoid.\n\nDoes the file write save us a lot of time?","commit_id":"e92683d6436dac63d359e323988e0906d7a7766c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6df32f99b28810621731563af473d158b8cb156b","unresolved":false,"context_lines":[{"line_number":771,"context_line":"        if os.path.isfile(filename):"},{"line_number":772,"context_line":"            LOG.debug(\u0027Skipping initial lease file creation as a lease file \u0027"},{"line_number":773,"context_line":"                      \u0027already exists.\u0027)"},{"line_number":774,"context_line":"            return filename"},{"line_number":775,"context_line":""},{"line_number":776,"context_line":"        buf \u003d io.StringIO()"},{"line_number":777,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f621f24_dbcdb566","line":774,"in_reply_to":"1f621f24_c75cc04c","updated":"2020-11-17 11:41:01.000000000","message":"This could work for non HA environments where the dnsmasq process is restarted and the \"leases\" file preserves all ipv4/6 addresses.\n\nBut in case of failover, the now active dnsmasq host won\u0027t have any information about any lease. That will not only provoke a hiccup for the IPv6 but also for the IPv4 addresses.\n\nIn order to correctly populate the leases file to prevent this issue should be to capture the IAID and the DUID (but this last parameter could be \"*\"). The client IAID could be read only from the leases file, when dnsmasq populates the value and then store it in the DB, associated to the port (maybe to the IPAllocation child register).","commit_id":"e92683d6436dac63d359e323988e0906d7a7766c"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"af8940845e939b55673ff4b0a47081e1e04f6a59","unresolved":true,"context_lines":[{"line_number":771,"context_line":"        if os.path.isfile(filename):"},{"line_number":772,"context_line":"            LOG.debug(\u0027Skipping initial lease file creation as a lease file \u0027"},{"line_number":773,"context_line":"                      \u0027already exists.\u0027)"},{"line_number":774,"context_line":"            return filename"},{"line_number":775,"context_line":""},{"line_number":776,"context_line":"        buf \u003d io.StringIO()"},{"line_number":777,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"92010224_f3bc320d","line":774,"in_reply_to":"1f621f24_dbcdb566","updated":"2020-11-24 08:14:41.000000000","message":"I have an alternate proposal: Instead of just skipping the lease file generation to read the existing lease file and to copy all IPv6 leases and the DUID to the generated lease file. This would solve the common case where the DHCP agent is just restarted (regardless of HA configuration). What do you think?","commit_id":"e92683d6436dac63d359e323988e0906d7a7766c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"bf4925fca89408327fbbe6f04a42faeccf13a7a0","unresolved":true,"context_lines":[{"line_number":783,"context_line":"        epoch-timestamp mac_addr ip_addr hostname client-ID"},{"line_number":784,"context_line":"        \"\"\""},{"line_number":785,"context_line":"        filename \u003d self.get_conf_file_name(\u0027leases\u0027)"},{"line_number":786,"context_line":""},{"line_number":787,"context_line":"        buf \u003d io.StringIO()"},{"line_number":788,"context_line":""},{"line_number":789,"context_line":"        LOG.debug(\u0027Building initial lease file: %s\u0027, filename)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9796ea36_76e437e1","line":786,"updated":"2021-06-08 08:45:47.000000000","message":"unrelated change","commit_id":"47a0fa87280954ef857301bd27c8df00a5f914fd"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"943c257408976cca157946f01a731007dde38b2e","unresolved":false,"context_lines":[{"line_number":783,"context_line":"        epoch-timestamp mac_addr ip_addr hostname client-ID"},{"line_number":784,"context_line":"        \"\"\""},{"line_number":785,"context_line":"        filename \u003d self.get_conf_file_name(\u0027leases\u0027)"},{"line_number":786,"context_line":""},{"line_number":787,"context_line":"        buf \u003d io.StringIO()"},{"line_number":788,"context_line":""},{"line_number":789,"context_line":"        LOG.debug(\u0027Building initial lease file: %s\u0027, filename)"}],"source_content_type":"text/x-python","patch_set":3,"id":"e7e55b20_17ea48ad","line":786,"in_reply_to":"9796ea36_76e437e1","updated":"2021-06-09 08:58:10.000000000","message":"Ack","commit_id":"47a0fa87280954ef857301bd27c8df00a5f914fd"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"bf4925fca89408327fbbe6f04a42faeccf13a7a0","unresolved":true,"context_lines":[{"line_number":815,"context_line":"            # The IPv6 leases can\u0027t be generated as their DUID is unknown. To"},{"line_number":816,"context_line":"            # not loose active leases, read the existing leases and add them to"},{"line_number":817,"context_line":"            # the generated file."},{"line_number":818,"context_line":"            LOG.debug(\u0027Reading IPv6 leases from existing leasfile.\u0027)"},{"line_number":819,"context_line":"            with open(filename) as leasefile:"},{"line_number":820,"context_line":"                for line in leasefile:"},{"line_number":821,"context_line":"                    if line.startswith(\u0027duid \u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"6c0b577a_322cc8aa","line":818,"range":{"start_line":818,"start_character":57,"end_line":818,"end_character":65},"updated":"2021-06-08 08:45:47.000000000","message":"leases file","commit_id":"47a0fa87280954ef857301bd27c8df00a5f914fd"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"943c257408976cca157946f01a731007dde38b2e","unresolved":false,"context_lines":[{"line_number":815,"context_line":"            # The IPv6 leases can\u0027t be generated as their DUID is unknown. To"},{"line_number":816,"context_line":"            # not loose active leases, read the existing leases and add them to"},{"line_number":817,"context_line":"            # the generated file."},{"line_number":818,"context_line":"            LOG.debug(\u0027Reading IPv6 leases from existing leasfile.\u0027)"},{"line_number":819,"context_line":"            with open(filename) as leasefile:"},{"line_number":820,"context_line":"                for line in leasefile:"},{"line_number":821,"context_line":"                    if line.startswith(\u0027duid \u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"eead85f1_842ca3ea","line":818,"range":{"start_line":818,"start_character":57,"end_line":818,"end_character":65},"in_reply_to":"6c0b577a_322cc8aa","updated":"2021-06-09 08:58:10.000000000","message":"Ack","commit_id":"47a0fa87280954ef857301bd27c8df00a5f914fd"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"bf4925fca89408327fbbe6f04a42faeccf13a7a0","unresolved":true,"context_lines":[{"line_number":822,"context_line":"                        buf.write(line)"},{"line_number":823,"context_line":"                        continue"},{"line_number":824,"context_line":"                    try:"},{"line_number":825,"context_line":"                        ts, mac, ip, host, duid \u003d line.split(\u0027 \u0027, 5)"},{"line_number":826,"context_line":"                    except ValueError:"},{"line_number":827,"context_line":"                        # not the correct format for a lease, skip this line"},{"line_number":828,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":3,"id":"bddaa836_121f1b55","line":825,"range":{"start_line":825,"start_character":66,"end_line":825,"end_character":67},"updated":"2021-06-08 08:45:47.000000000","message":"that will find 5 split separators (in this case \" \"); that means it will return 6 elements if present\n\nBecause you have a try/catch with ValueError, there is not need to add this parameter","commit_id":"47a0fa87280954ef857301bd27c8df00a5f914fd"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"943c257408976cca157946f01a731007dde38b2e","unresolved":false,"context_lines":[{"line_number":822,"context_line":"                        buf.write(line)"},{"line_number":823,"context_line":"                        continue"},{"line_number":824,"context_line":"                    try:"},{"line_number":825,"context_line":"                        ts, mac, ip, host, duid \u003d line.split(\u0027 \u0027, 5)"},{"line_number":826,"context_line":"                    except ValueError:"},{"line_number":827,"context_line":"                        # not the correct format for a lease, skip this line"},{"line_number":828,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":3,"id":"db691b3e_3981cfa0","line":825,"range":{"start_line":825,"start_character":66,"end_line":825,"end_character":67},"in_reply_to":"bddaa836_121f1b55","updated":"2021-06-09 08:58:10.000000000","message":"Ack","commit_id":"47a0fa87280954ef857301bd27c8df00a5f914fd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"5cca3d116a30690f21b1e8438b4705cb158779fc","unresolved":true,"context_lines":[{"line_number":814,"context_line":"            # The IPv6 leases can\u0027t be generated as their DUID is unknown. To"},{"line_number":815,"context_line":"            # not loose active leases, read the existing leases and add them to"},{"line_number":816,"context_line":"            # the generated file."},{"line_number":817,"context_line":"            LOG.debug(\u0027Reading IPv6 leases from existing leasesfile.\u0027)"},{"line_number":818,"context_line":"            with open(filename) as leasefile:"},{"line_number":819,"context_line":"                for line in leasefile:"},{"line_number":820,"context_line":"                    if line.startswith(\u0027duid \u0027):"}],"source_content_type":"text/x-python","patch_set":4,"id":"8b655421_a1ffc60b","line":817,"range":{"start_line":817,"start_character":57,"end_line":817,"end_character":67},"updated":"2021-06-08 16:20:00.000000000","message":"\"lease file\" - I think Rodolfo mentioned this in the last PS too","commit_id":"0a2a68d093f079db6b9c8dd68fc38acc2234d2b6"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"943c257408976cca157946f01a731007dde38b2e","unresolved":false,"context_lines":[{"line_number":814,"context_line":"            # The IPv6 leases can\u0027t be generated as their DUID is unknown. To"},{"line_number":815,"context_line":"            # not loose active leases, read the existing leases and add them to"},{"line_number":816,"context_line":"            # the generated file."},{"line_number":817,"context_line":"            LOG.debug(\u0027Reading IPv6 leases from existing leasesfile.\u0027)"},{"line_number":818,"context_line":"            with open(filename) as leasefile:"},{"line_number":819,"context_line":"                for line in leasefile:"},{"line_number":820,"context_line":"                    if line.startswith(\u0027duid \u0027):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dabcb7e2_6ba4f1f4","line":817,"range":{"start_line":817,"start_character":57,"end_line":817,"end_character":67},"in_reply_to":"8b655421_a1ffc60b","updated":"2021-06-09 08:58:10.000000000","message":"Ack","commit_id":"0a2a68d093f079db6b9c8dd68fc38acc2234d2b6"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2d8f6ae987934e2a781560ffc757954c6e54acb9","unresolved":true,"context_lines":[{"line_number":827,"context_line":"                        continue"},{"line_number":828,"context_line":""},{"line_number":829,"context_line":"                    if netaddr.valid_ipv6(ip):"},{"line_number":830,"context_line":"                        buf.write(line)"},{"line_number":831,"context_line":""},{"line_number":832,"context_line":"        contents \u003d buf.getvalue()"},{"line_number":833,"context_line":"        file_utils.replace_file(filename, contents)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f1216a6_f542e337","line":830,"range":{"start_line":830,"start_character":24,"end_line":830,"end_character":39},"updated":"2021-06-08 16:30:21.000000000","message":"btw, now I recheck this patch. Shouldn\u0027t we check that the IPs we are adding belong to a valid IP? self._iter_hosts() should provide this info.","commit_id":"0a2a68d093f079db6b9c8dd68fc38acc2234d2b6"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"8f3175033c080b5fa4209c76e76ba30a7a465c78","unresolved":true,"context_lines":[{"line_number":836,"context_line":"                  alloc.ip_address in existing_ipv6_leases):"},{"line_number":837,"context_line":"                # Keep the existing IPv6 lease if the port still exists and is"},{"line_number":838,"context_line":"                # still configured for DHCPv6"},{"line_number":839,"context_line":"                buf.write(existing_ipv6_leases[alloc.ip_address])"},{"line_number":840,"context_line":""},{"line_number":841,"context_line":"        contents \u003d buf.getvalue()"},{"line_number":842,"context_line":"        file_utils.replace_file(filename, contents)"}],"source_content_type":"text/x-python","patch_set":5,"id":"4ab3a0e5_ce2dfdec","line":839,"updated":"2021-06-21 08:06:50.000000000","message":"nit: I would do something like:\n\n    if no_dhcp:\n        continue\n\nbefore L829 to remove this \"if not no_dhcp\" from other conditions","commit_id":"9cc40d86deecfbf290070dd45a096ff050af4301"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"b98d99496959d8e3c86ff7648076d95230927f94","unresolved":false,"context_lines":[{"line_number":836,"context_line":"                  alloc.ip_address in existing_ipv6_leases):"},{"line_number":837,"context_line":"                # Keep the existing IPv6 lease if the port still exists and is"},{"line_number":838,"context_line":"                # still configured for DHCPv6"},{"line_number":839,"context_line":"                buf.write(existing_ipv6_leases[alloc.ip_address])"},{"line_number":840,"context_line":""},{"line_number":841,"context_line":"        contents \u003d buf.getvalue()"},{"line_number":842,"context_line":"        file_utils.replace_file(filename, contents)"}],"source_content_type":"text/x-python","patch_set":5,"id":"0f66209a_bd8dd682","line":839,"in_reply_to":"4ab3a0e5_ce2dfdec","updated":"2021-06-21 12:57:44.000000000","message":"Ack","commit_id":"9cc40d86deecfbf290070dd45a096ff050af4301"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1e289de1a572b42f4296853be7a6f55aa44a1fca","unresolved":true,"context_lines":[{"line_number":835,"context_line":"                buf.write(\u0027%s %s %s * *\\n\u0027 %"},{"line_number":836,"context_line":"                          (timestamp, port.mac_address, alloc.ip_address))"},{"line_number":837,"context_line":"            elif (alloc.subnet_id in dhcpv6_enabled_subnet_ids and"},{"line_number":838,"context_line":"                  alloc.ip_address in existing_ipv6_leases):"},{"line_number":839,"context_line":"                # Keep the existing IPv6 lease if the port still exists and is"},{"line_number":840,"context_line":"                # still configured for DHCPv6"},{"line_number":841,"context_line":"                buf.write(existing_ipv6_leases[alloc.ip_address])"}],"source_content_type":"text/x-python","patch_set":6,"id":"88079d35_011dc2ca","line":838,"range":{"start_line":838,"start_character":18,"end_line":838,"end_character":58},"updated":"2021-06-22 10:45:15.000000000","message":"Just a heads-up: IPv6 has multiple formats. Instead of comparing strings, I would use IPAddress:\n\n                    if netaddr.valid_ipv6(ip):\n                        existing_ipv6_leases[netaddr.IPAddress(ip)] \u003d line\n\n                    ...\n\n            elif (alloc.subnet_id in dhcpv6_enabled_subnet_ids and\n                  netaddr.IPAddress(alloc.ip_address) in existing_ipv6_leases)","commit_id":"cf20743b5612b60fcc30841a5d4f1a306c17ec8e"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"e9d6d36f5deb847229e53a7d88ea7c8f5bc3c059","unresolved":false,"context_lines":[{"line_number":835,"context_line":"                buf.write(\u0027%s %s %s * *\\n\u0027 %"},{"line_number":836,"context_line":"                          (timestamp, port.mac_address, alloc.ip_address))"},{"line_number":837,"context_line":"            elif (alloc.subnet_id in dhcpv6_enabled_subnet_ids and"},{"line_number":838,"context_line":"                  alloc.ip_address in existing_ipv6_leases):"},{"line_number":839,"context_line":"                # Keep the existing IPv6 lease if the port still exists and is"},{"line_number":840,"context_line":"                # still configured for DHCPv6"},{"line_number":841,"context_line":"                buf.write(existing_ipv6_leases[alloc.ip_address])"}],"source_content_type":"text/x-python","patch_set":6,"id":"a646030b_66425cd0","line":838,"range":{"start_line":838,"start_character":18,"end_line":838,"end_character":58},"in_reply_to":"88079d35_011dc2ca","updated":"2021-06-24 20:50:54.000000000","message":"Ack","commit_id":"cf20743b5612b60fcc30841a5d4f1a306c17ec8e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9adbe3eabfb1e51c54073569cb878f679b6a3299","unresolved":true,"context_lines":[{"line_number":820,"context_line":"                        # not the correct format for a lease, skip this line"},{"line_number":821,"context_line":"                        continue"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"                    if netaddr.valid_ipv6(ip):"},{"line_number":824,"context_line":"                        existing_ipv6_leases[netaddr.IPAddress(ip)] \u003d line"},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"        for host_tuple in self._iter_hosts():"}],"source_content_type":"text/x-python","patch_set":7,"id":"28e74ad2_4e1ea373","line":823,"range":{"start_line":823,"start_character":23,"end_line":823,"end_character":41},"updated":"2021-06-25 14:55:02.000000000","message":"nit: Other checks like this in-tree use netutils.is_valid_ipv6() which can deal with scoping (addr%eth) but I don\u0027t think we have that here.  But the one thing it does do is catch any address formatting issues that would have triggered an exception, which shouldn\u0027t ever happen here in reality.  Would be a simple change if you needed to update, but not required.","commit_id":"6bc1c00d66ced82dd142739790222e8408684696"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"f86bfa68f7e494d7ca7e3ba9e83989c22910993a","unresolved":true,"context_lines":[{"line_number":820,"context_line":"                        # not the correct format for a lease, skip this line"},{"line_number":821,"context_line":"                        continue"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"                    if netaddr.valid_ipv6(ip):"},{"line_number":824,"context_line":"                        existing_ipv6_leases[netaddr.IPAddress(ip)] \u003d line"},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"        for host_tuple in self._iter_hosts():"}],"source_content_type":"text/x-python","patch_set":7,"id":"3dbda3d2_0c36aed0","line":823,"range":{"start_line":823,"start_character":23,"end_line":823,"end_character":41},"in_reply_to":"28e74ad2_4e1ea373","updated":"2021-06-28 15:19:53.000000000","message":"+1","commit_id":"6bc1c00d66ced82dd142739790222e8408684696"}],"neutron/tests/unit/agent/linux/test_dhcp.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"5cca3d116a30690f21b1e8438b4705cb158779fc","unresolved":true,"context_lines":[{"line_number":1574,"context_line":"            \u00271623133362 3042863103 2001:db8::20 host-2001-db8--20 00:02:00:00:ab:11:f9:cc:99:ad:ce:54:22:69\u0027, # noqa"},{"line_number":1575,"context_line":"            \u00271623143299 3042863103 2001:db8::45 host-2001-db8--45 00:02:00:00:ab:11:fa:c9:0e:0f:3d:90:73:f0\u0027, # noqa"},{"line_number":1576,"context_line":"            \u00271623134168 3042863103 2001:db8::12 host-2001-db8--12 00:02:00:00:ab:11:f6:f2:aa:cb:94:c1:b4:86\u0027, # noqa"},{"line_number":1577,"context_line":""},{"line_number":1578,"context_line":"        ))"},{"line_number":1579,"context_line":"        existing_leases \u003d \u0027\\n\u0027.join((ipv4_leases, ipv6_leases))"},{"line_number":1580,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9fd83cc6_e6d29171","line":1577,"updated":"2021-06-08 16:20:00.000000000","message":"I think the above can be done without all the wrapping and #noqa based on the tests below this one.","commit_id":"0a2a68d093f079db6b9c8dd68fc38acc2234d2b6"},{"author":{"_account_id":11290,"name":"Gaudenz Steinlin","email":"gaudenz.steinlin@cloudscale.ch","username":"gaudenz"},"change_message_id":"943c257408976cca157946f01a731007dde38b2e","unresolved":false,"context_lines":[{"line_number":1574,"context_line":"            \u00271623133362 3042863103 2001:db8::20 host-2001-db8--20 00:02:00:00:ab:11:f9:cc:99:ad:ce:54:22:69\u0027, # noqa"},{"line_number":1575,"context_line":"            \u00271623143299 3042863103 2001:db8::45 host-2001-db8--45 00:02:00:00:ab:11:fa:c9:0e:0f:3d:90:73:f0\u0027, # noqa"},{"line_number":1576,"context_line":"            \u00271623134168 3042863103 2001:db8::12 host-2001-db8--12 00:02:00:00:ab:11:f6:f2:aa:cb:94:c1:b4:86\u0027, # noqa"},{"line_number":1577,"context_line":""},{"line_number":1578,"context_line":"        ))"},{"line_number":1579,"context_line":"        existing_leases \u003d \u0027\\n\u0027.join((ipv4_leases, ipv6_leases))"},{"line_number":1580,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"a9fa310f_ebb1e04e","line":1577,"in_reply_to":"9fd83cc6_e6d29171","updated":"2021-06-09 08:58:10.000000000","message":"To address the comment made by Adolfo to not include IPv6 addresses for ports which no longer exist I had to reformat the test quite a bit. I also formatted the strings similar to the tests below. But I\u0027m not really convinced that this is more readable than the one line per line in the expected file with #noqa added where lines get too long, but YMMV.","commit_id":"0a2a68d093f079db6b9c8dd68fc38acc2234d2b6"}]}