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

Interactive address selection #79

Merged
merged 3 commits into from
Mar 29, 2023
Merged

Conversation

masnax
Copy link
Contributor

@masnax masnax commented Mar 29, 2023

Closes #76

Now shows a table of addresses and their interfaces on the local machine. The user's selection will be used as the listen address and to lookup peers.

After this step, the user will be prompted with a binary yes/no for whether or not to filter on the subnet of the listen address.

  • If --address is given, that will be used in lieu of the interactive step.
  • If --auto, the first address will be used and peer lookup will be limited to its subnet.
  • If only one address is available, the interactive step will be skipped and we will go right to asking for subnet filtering.
  • The --subnet flag is gone now

Additionally, in response to the comments in #74, if there are too few disks for distributed storage after selecting local storage, the distributed storage selection step will be entirely skipped with a warning about insufficient disks, rather than prompting the user to add disks as normal.

@masnax masnax requested a review from stgraber March 29, 2023 20:19
Comment on lines 118 to 123
if net.ParseIP(listenAddr).To4() != nil {
_, subnet, err = net.ParseCIDR(listenAddr + "/24")
} else {
_, subnet, err = net.ParseCIDR(listenAddr + "/64")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get the CIDR from the address instead of assuming /24 for v4 and /64 for v6 as that can be very wrong?

I'd probably have gone with net.InterfaceAddrs() which gets you CIDR of all addresses on the system.
You'd then need to filter our anything that's not a global address from that by using ip.ParseIP and only going for GlobalUnicast.

Copy link
Contributor

Choose a reason for hiding this comment

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

	addrs, err := net.InterfaceAddrs()
	if err != nil {
		os.Exit(1)
	}

	for _, addr := range addrs {
		ip, _, err := net.ParseCIDR(addr.String())
		if err != nil {
			continue
		}

		if !ip.IsGlobalUnicast() {
			continue
		}

		fmt.Printf("%s\n", addr.String())
	}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's a good point. I was less concerned about invalid subnets since I had added the --subnet flag, but since that's gone it can be problematic.

I've added a Subnet field to the NetworkInfo struct now. So any selected address will have its associated subnet be used. If the user specifies their own address with --address, init will fail if that address doesn't fit in any of the found subnets.

Adds a helper for asking for the listen address and subnet to use
for finding peers.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
@stgraber stgraber merged commit 69eddd6 into canonical:main Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ambiguous initial networking questions
2 participants