Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dev] use cidr based ip notation when merging lld addrs #11000

Conversation

achilleasa
Copy link
Contributor

Description of change

To create a new link layer device address, we are required to populate an object which among other things includes the IP address in CIDR notation. The code in the state package breaks this down into an IP and CIDR value which is then persisted to the DB.

When we retrieve a link layer device address from disk, we can only access the IP and subnet CIDR values.

The changes introduced by #10979 (this commit in particular) included additional logic to backfill the provider-specific IDs to existing link layer devices and their addresses. The introduced merge code would create a new set of link layer device addresses by re-using the original values (reported by the machiner) plus the provider-specific IDs.

Unfortunately, that code contained a bug: instead of converting the existing address into CIDR format, it pulled the data from the SubnetCIDR method of the link layer device address. By passing that value back into the state, we ended up with invalid IP addresses for the machines (following x.y.z.0 and x.y.0.0 patterns due to the way that the state code works).

This PR contributes a small fix where we basically recombine the existing IP and CIDR into an IP-in-CIDR-format value which we can then pass to the state code and get the correct address values persisted to the DB.

QA steps

Run nw-network-health-aws with this PR (this is how we caught the bug in the first place)

Copy link
Member

@manadart manadart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

@achilleasa
Copy link
Contributor Author

$$merge$$

@jujubot jujubot merged commit cd599b2 into juju:develop Dec 9, 2019
@achilleasa achilleasa deleted the dev-use-cidr-based-ip-notation-when-merging-lld-addrs branch December 9, 2019 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants