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
WIP: Migrate some more YAML schedules using PoC for enhanced scheduler #14562
Conversation
Great PR! Please pay attention to the following items before merging: Files matching
This is an automatically generated QA checklist based on modified files |
@@ -58,7 +58,7 @@ test-compile-changed: os-autoinst/ | |||
|
|||
.PHONY: test-yaml-valid | |||
test-yaml-valid: | |||
$(eval YAMLS=$(shell sh -c "git ls-files schedule/ test_data/ | grep '\\.ya\?ml$$'")) | |||
$(eval YAMLS=$(shell sh -c "git ls-files schedule/ test_data/ | grep -v '/defaults/' | grep '\\.ya\?ml$$'")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong in a first sight. It makes the CI obsolete and probe to give false positives
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem here is that the enhancement for the scheduler uses a new format for the default files which differs from the current schema used for YAML-Schedules. See my documentation on Confluence for details. So "schedule:" is no longer an array of type string, but rather a hash with sections that are arrays of string.
Therefore the YAML checking for the /defaults/ directory is omitted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but @b10n1k is right, in my initial PoC I just wrote that pass the CI, we will plan some ticket for it or I will improve it directly there, we need some specific path not so generic :) so no worries, was in my mind! CI/Documentation/Schema check that will come later, we are not going to merge anything with todos
:)
7ab62fa
to
0fb86c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to test xfs with all the flavors not just x86_64, at least we need an additional test suite for zVM if I'm not wrong, could you please try all or at least that one to see how does it look?
- installation/registration/skip_registration | ||
extension_module_selection: | ||
- installation/module_selection/skip_module_selection | ||
add_on_product: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be more explicit here:
add_on_product: []
is not processed and now
add_on_product_installation:
- installation/add_on_product_installation/accept_add_on_installation
takes over, which is a different screen
for which we need a hook in default.yaml
- console/system_prepare | ||
- console/hostname | ||
- console/force_scheduled_tasks | ||
- shutdown/grub_set_bootargs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test module we could unschedule it, it is not needed.
system_preparation: | ||
- console/system_prepare | ||
- console/hostname | ||
- console/force_scheduled_tasks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only the threes tests above are considered preparation, the rest is system_validation
.
default_systemd_target: | ||
- installation/installation_settings/validate_default_target | ||
grub: | ||
- installation/grub_test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed, handle_reboot
will do the job, but we should find in the future some way to make it more readable, as there is not reconnection or any remote in this case for example, but afir there was some controversy with the name in PR creating that test module and got that handle_reboot
instead of handle_grub
, I would say let's use the default here for now.
Not to be merged in any way. |
This is just a PoC, do not merge!
Few more tests migrated to the enhanced scheduler.
Diversion in USBInstall (Full medium or Online Medium) is resovled by creating a default for Full medium.