Allow MicroOVN init to take values from stdin. - #353
Merged
Conversation
matperin
requested changes
Jul 15, 2026
matperin
left a comment
Contributor
There was a problem hiding this comment.
Thanks @crypticC0der for the changes.
There is nothing worth flagging in what is currently present, the implementation is very well done, imho.
I think there are a few things missing before calling this feature complete, though:
- While the main functionality testing is covered (end-to-end bootstrap + join of a 3-node cluster via preseed YAML) we are still missing coverage for some added paths.
I think adding one preseed test that setsservicesandcustom_encapsulation_ip(and, ideally, acustom_cavariant mirroringinit_cluster_user_ca.bats) and one test usingadditional_serversmight be worthwhile. Also, while not entirely needed, one could also argue to add a couple of negative validation tests (invalid mode / bad YAML / join without token). - Since this is a new user/machine-facing interface, I would advise against leaving it undocumented. Leaving no mentions of preseed or the YAML fields on the docs risks making the feature undiscoverable by the charm authors who are the stated audience (mostly us, but still good for future-proofing).
I'd suggest a new how-to (e.g.docs/how-to/preseed-init.rst) that documents the fullInitValuesYAML schema and which fields apply to bootstrap vs join (i.e. the validation rules, token required for join and forbidden for bootstrap are non-obvious and should be spelled out), theadditional_serversbehavior (issues join tokens, printed to stdout) and maybe a note that if stdin is a TTY the command stays interactive. Also, a bootstrap example and a join example piping YAML intomicroovn initcould be useful.
Other than this everything looks good.
The microovn init functionality is not very machine usable, which is a problem because we like machines and the operator charm could gain a lot of extra functionality through the init options. The simple solution is to allow passing values though init as a yaml object. Signed-off-by: MJ Ponsonby <mj.ponsonby@canonical.com>
matperin
approved these changes
Jul 22, 2026
matperin
left a comment
Contributor
There was a problem hiding this comment.
Thank you for the added tests.
I see that the documentation is still missing but we can add it in a later commit, if needed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The microovn init functionality is not very machine usable, which is a problem because we like machines and the operator charm could gain a lot of extra functionality through the init options.
The simple solution is to allow passing values though init as a yaml object.