search

canonical / microcloud

Automated private cloud based on LXD, Ceph and OVN
https://microcloud.is
GNU Affero General Public License v3.0
275 stars 42 forks source link

Collect system information prior to asking any questions #332

Closed masnax closed 2 months ago

masnax commented 3 months ago

It was getting very hard and inefficient to modify or extend the MicroCloud setup logic without devolving into lots of temporary maps and slices, and redundantly calling back over the network to the other systems to fetch their configuration.

This PR attempts to address that by immediately collecting all relevant system information for the cluster setup in a SystemInformation for each system. Each question helper can then reference the fields and methods of the SystemInformation to check what is and isn't supported, rather than fetching them over the network each time.

In order to pass around SystemInformation as well as other common state information that were previously passed directly as arguments to each function, this PR also adds an initConfig type, of which all question functions are a method of.

Sadly, this does result in quite a large refactor, but given how deeply everything here is coupled together, I don't see a way around it. I've tried to at least split the commits by the functions that are being changed.

Due to this refactor, several edge cases around detecting already configured storage pools and networks have been addressed (tests for these will come later, I didn't want to make this PR any bigger). As well, the ceph cluster network questions are now part of the distributed storage set of questions, rather than the OVN network questions.

roosterfish commented 3 months ago

@masnax ready for review? Then I'll focus on this one next so that we can pave the way for putting the explicit trust establishment into place.

masnax commented 3 months ago

@masnax ready for review? Then I'll focus on this one next so that we can pave the way for putting the explicit trust establishment into place.

This is ready for review, but will likely see rebases once #308 is merged.

masnax commented 3 months ago

Looks like there is some sort of cloud-init issue with the test VMs cc @simondeziel

simondeziel commented 3 months ago

Looks like there is some sort of cloud-init issue with the test VMs cc @simondeziel

I think this PR should be rebased now that #335 was merged.

masnax commented 2 months ago

@roosterfish Freshly rebased and ready for review, thanks :)

masnax commented 2 months ago

Thanks, looks great! I guess the test failures aren't related to any of the changes in this PR?

Yeah, the instance tests appear to be broken with either 24.04 or the new runners.

masnax commented 2 months ago

As for the interactive tests, those appear broken due to some other unrelated reason, so I'm going to go ahead and merge this PR now.

I've tested that they all pass locally.