Update validate-autoinstall-user-data script #1901
Conversation
Chris-Peterson444
changed the title
Chris-Peterson444
force-pushed
the
user-data-validation
branch
2 times, most recently
from
722fc30
to
d8d9f0c
Compare
Chris-Peterson444
force-pushed
the
user-data-validation
branch
3 times, most recently
from
691476d
to
6a3abc7
Compare
|
I think this is ready. It's not perfect but it's a good start. I think I would like to punt writing tests for this since it uses the dry-run logic that is already tested. Eventually I think we could use this in CI for some more testing, which at that point we could write some more tests for this particular script if we wanted. Eventually I would like to package this (and subiquity as a whole) in a way that users could just install the subiquity snap and invoke the validator from there, which would reduce the setup here. |
doc/howto/autoinstall-validation.rst
Outdated
| finish: subiquity/load_autoinstall_config: FAIL: Malformed autoinstall in 'version or interactive-sections' section | ||
| Malformed autoinstall in 'version or interactive-sections' section | ||
| Traceback (most recent call last): | ||
| File "/home/cpete/software/subiquity/scripts/../subiquity/server/server.py", line 654, in validate_autoinstall |
There was a problem hiding this comment.
I'd typically drop the "/home/cpete/software/subiquity" prefix from the tracebacks but it's up to you
There was a problem hiding this comment.
Good catch, I hadn't thought about that. I replaced the real prefix with ellipses. Does that seem better?
|
|
||
|
|
||
| SUCCESS_MSG = "Success: The provided autoinstall config validated succesfully" | ||
| FAILURE_MSG = "Failure: The provided autoinstall config did not validate succesfully" |
There was a problem hiding this comment.
nitpick: I'd reword just a bit so that the last words written are not "validate successfully". I'm a lazy reader 😆
The provided autoinstall config failed validation?
There was a problem hiding this comment.
Also, I'm tempted to suggest rich (https://rich.readthedocs.io/en/stable/introduction.html) but it's probably difficult to justify for only one message.
There was a problem hiding this comment.
lazy reader
Me too. I couldn't come up with something at the time but I like it. I used your suggested text.
I'm tempted to suggest
rich
I do like the idea. And it's in Main, which is good. Although maybe once (if) we start having more error messaging we could use it?
There was a problem hiding this comment.
yeah, let's think about it later. This is not blocking.
Chris-Peterson444
force-pushed
the
user-data-validation
branch
2 times, most recently
from
17ace31
to
77ab65d
Compare
|
Updated + rebased onto main. Thanks for the feedback @ogayot |
Chris-Peterson444
force-pushed
the
user-data-validation
branch
2 times, most recently
from
f31629b
to
46c3981
Compare
./scripts/validate-autoinstall-user-data is used by the integration tests to verify the written user data validates against the combined JSON schema, but we have introduced run-time checks for more things than can be caught by simple JSON validation (e.g. warns/errors on unknown keys or strict top-level key checking for supporting a top-level "autoinstall" keyword in the non-cloud-config delivery scenario). This changes the validation script to rely on the logic from the server directly to perform pre-validation of the the supplied autoinstall configuration. Additionally, this adds an argparser to make it more user-friendly. Now we can advertise this script as something for users to pre-validate their autoinstall configurations.
Chris-Peterson444
force-pushed
the
user-data-validation
branch
from
46c3981
to
5413f4d
Compare
./scripts/validate-autoinstall-user-data is used by the integration tests to verify the written user data is correct, but we can additionally advertise this script as something for users to pre-validate their autoinstall data.
Changes include some refactoring and trying to catch some likely common mistakes.
Todo:
[ ] Tests