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

Don't use mDNS payload address for local node validation #289

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

masnax
Copy link
Contributor

@masnax masnax commented Apr 19, 2024

The local node does not have an mDNS record (the struct is blank) so to validate its cluster address we need to grab it from the Handler.

@masnax
Copy link
Contributor Author

masnax commented Apr 19, 2024

Error: Failed to issue LXD token for peer "micro03": There are no online cluster members

That's a fun one, looks like the first LXD didn't bootstrap fast enough before we attempted to generate tokens.

I would think bootstrapping LXD would only return after the node is actually operational.

@masnax
Copy link
Contributor Author

masnax commented Apr 19, 2024

I would think bootstrapping LXD would only return after the node is actually operational.

Looking at the corresponding code block: https://github.com/canonical/lxd/blob/9bc2974a84d15fb9c16de04666dc79041a8a19ca/lxd/api_cluster.go#L1323-L1339

It looks like we're considering nodes to be offline if they haven't received a heartbeat, but shouldn't the local node always be online to itself?

@simondeziel
Copy link
Member

It looks like we're considering nodes to be offline if they haven't received a heartbeat, but shouldn't the local node always be online to itself?

I guess it depends on what you define as being online. Having received a heartbeat is IMHO a very good indication of being online but maybe it could be more flexible for itself? I don't know the code, but that definition of online seems to be a consistent way to treat everyone the same with regards to connectivity.

Copy link
Contributor

@MggMuggins MggMuggins left a comment

Choose a reason for hiding this comment

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

I won't speak to the CI failure. Thanks for correcting my assumption.

@tomponline tomponline merged commit 6719d9e into canonical:main Apr 23, 2024
12 checks passed
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.

4 participants