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

Preseed yaml #164

Merged
merged 6 commits into from
Sep 26, 2023
Merged

Preseed yaml #164

merged 6 commits into from
Sep 26, 2023

Conversation

masnax
Copy link
Contributor

@masnax masnax commented Sep 20, 2023

Adds support for preseed init

@masnax
Copy link
Contributor Author

masnax commented Sep 20, 2023

I've just dropped a somewhat descriptive yaml under the doc directory, but we probably want some more comprehensive documentation of this feature.

@tomponline
Copy link
Member

@masnax did you want a review of this now?

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 Sep 25, 2023

@tomponline Yeah, that would be great thanks.

// - If `autoSetup` is true, all systems found in the first 5s will be recorded, and no other input is required.
// - `expectedSystems` is a list of expected hostnames. If given, the behaviour is similar to `autoSetup`,
// except it will wait up to a minute for exclusively these systems to be recorded.
func lookupPeers(s *service.Handler, autoSetup bool, subnet *net.IPNet, expectedSystems []string, systems map[string]InitSystem) error {
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the comment, this explains it well.

This isn't a blocker, just a nit, but the coupling of different timeouts with different modes feels quite peculiar for a utility function like this.

For future reference pushing the timeout decision to the caller and just having a ctx or timeout argument here would make the function description cleaner to understand and separate the concerns of what the function should do by when.

@tomponline tomponline merged commit 09bd911 into canonical:main Sep 26, 2023
8 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.

None yet

2 participants