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

Grow a MicroCloud cluster #48

Merged
merged 31 commits into from
Mar 16, 2023
Merged

Grow a MicroCloud cluster #48

merged 31 commits into from
Mar 16, 2023

Conversation

masnax
Copy link
Contributor

@masnax masnax commented Mar 8, 2023

Closes #31

Adds microcloud add which allows for rescanning for new cluster members. The process works similarly to init.

To facilitate communication with a system outside the cluster, microcloud now by default will listen on :9443 and will serve thee /1.0 and /cluster/1.0 endpoints. This listener will get replaced with a proper one once the peer has joined.

Additionally the token broadcast has been updated to send a struct, which additionally includes a LXDConfig for assigning MemberConfig to the join request made by the peer.

The /services/disks endpoint is now removed, to be replaced with the source.wipe config field for LXD storage pools.

As per the comments in #46, I've also reworked the join logic to be through the temporary network listener instead of mDNS.

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Do we need to add a version to the mdns advert/response so we can ignore responses from earlier/incompatible machines on the network (now that we are evolving the mdns advert format).

@masnax
Copy link
Contributor Author

masnax commented Mar 9, 2023

@tomponline I've made some changes now according to your feedback.

Do we need to add a version to the mdns advert/response so we can ignore responses from earlier/incompatible machines on the network (now that we are evolving the mdns advert format).

I think this is a really good idea. I've added a version flag that's appended to the keyword that tells us an mDNS record is from MicroCloud (_microcloud_v1.0). Adding it to the payload wouldn't have worked as older systems would fail the json.Unmarshal call in the first place.

@tomponline
Copy link
Member

Adding it to the payload wouldn't have worked as older systems would fail the json.Unmarshal call in the first place.

I don't think thats true. json.Unmarshal will ignore unknown fields unless https://pkg.go.dev/encoding/json#Decoder.DisallowUnknownFields is enabled.

@masnax
Copy link
Contributor Author

masnax commented Mar 9, 2023

Adding it to the payload wouldn't have worked as older systems would fail the json.Unmarshal call in the first place.

I don't think thats true. json.Unmarshal will ignore unknown fields unless https://pkg.go.dev/encoding/json#Decoder.DisallowUnknownFields is enabled.

However in upstream MicroCloud, the response type is map[string]string which will fail to convert to map[string]struct{} with the error json: cannot unmarshal string into Go value of type <type>

@tomponline
Copy link
Member

Adding it to the payload wouldn't have worked as older systems would fail the json.Unmarshal call in the first place.

I don't think thats true. json.Unmarshal will ignore unknown fields unless https://pkg.go.dev/encoding/json#Decoder.DisallowUnknownFields is enabled.

However in upstream MicroCloud, the response type is map[string]string which will fail to convert to map[string]struct{} with the error json: cannot unmarshal string into Go value of type <type>

Ah I see you've changed the type not just the fields. Is map[string]struct{} the old type?

@masnax
Copy link
Contributor Author

masnax commented Mar 9, 2023

Adding it to the payload wouldn't have worked as older systems would fail the json.Unmarshal call in the first place.

I don't think thats true. json.Unmarshal will ignore unknown fields unless https://pkg.go.dev/encoding/json#Decoder.DisallowUnknownFields is enabled.

However in upstream MicroCloud, the response type is map[string]string which will fail to convert to map[string]struct{} with the error json: cannot unmarshal string into Go value of type <type>

Ah I see you've changed the type not just the fields. Is map[string]struct{} the old type?

Oh actually forget everything I said, you're right. I thought I was still unmarshaling a whole map of structs (I was at some point but not anymore). It's actually one struct at a time now so I can totally just add Version to the ServerInfo struct.

@tomponline
Copy link
Member

thanks for the changes @masnax

@stgraber
Copy link
Contributor

@masnax oops, sorry, looks like I broke your branch with my gomod update

@masnax
Copy link
Contributor Author

masnax commented Mar 15, 2023

@stgraber Fixed now.

@stgraber
Copy link
Contributor

@masnax needs a rebase post MicroOVN

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
…cket

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
Additionally sets up to broadcast a JoinConfig type instead of a
map[string]string

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
…ge config

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
…torage actions

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
…ster

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
…d systems

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>
Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
…tems found

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>
…er info

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>
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>
This helper sends a request to all to-be peers over the temporary
listener at :9443. Rather than blocking on each join request, each
request has its peer name sent through a channel, which is the same
size as the number of expected 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>
…names

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>
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>
Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
@masnax
Copy link
Contributor Author

masnax commented Mar 16, 2023

@stgraber Fixed

EDIT: Just added one last commit that enables MicroOVN on the add command too.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
@stgraber stgraber merged commit 644e17d into canonical:main Mar 16, 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.

Allow growing an existing cluster through microcloud init
3 participants