Ship a default hardware config file #3663

skycastlelily · 2025-04-13T07:28:45Z

We should have tmt ship a default hardware config file containing complex and not-implemented-yet translations, and allow users override it with theirs if they want.

Pull Request Checklist

We should have tmt ship a default hardware config file containing complex and not-implemented-yet translations, and allow users override it with theirs if they want.

happz · 2025-04-13T09:22:41Z

tmt/steps/provision/hardware.fmf

+#      {% elif (BEAKER_OPERATOR, BEAKER_VALUE) in [("==", "uefi"), ("!=", "bios")] %}
+#        {"or": [{"key_value": {"_key": "NETBOOT_METHOD", "_value": "grub2"},}, {"key_value": {"_key": "NETBOOT_METHOD", "_value": "efigrub"},}]}
+#      {% endif %}
+#      '


Spread the template into multiple lines, the output is supposed to be a YAML, so we can avoid { and } and make the structure be more visible and readable - we are not in the business of saving whitespace, we can afford to use spaces and empty lines to improve things for humans:

template: | {% if (BEAKER_OPERATOR, BEAKER_VALUE) in [("==", "bios"), ("!=", "uefi")] %} key_value: _key: NETBOOT_METHOD _value: pxe {% elif (BEAKER_OPERATOR, BEAKER_VALUE) in [("==", "uefi"), ("!=", "bios")] %} or: - key_value: _key: NETBOOT_METHOD _value: grub2 - key_value: _key: NETBOOT_METHOD _value: efigrub {% endif %}

happz · 2025-04-13T09:23:12Z

tmt/config/__init__.py

@@ -19,6 +19,7 @@

 # Config directory
 DEFAULT_CONFIG_DIR = Path('~/.config/tmt')
+DEFAULT_HARDWARE_FILE = Path('/etc/tmt/hardware.fmf')


"Hardware" file is part of configuration, and I don't think it's a good idea to single out one piece of it like this and make it special in such a way. Instead, we should probably think about whether tmt could load a "global" configuration, e.g., /etc/tmt/config, if "local" ~/.config/tmt is missing. That way, we could ship the initial configuration as a whole, not just for hardware - it is not special enough to deserve this kind of exceptional handling.

Let's discuss that in Tuesday's meeting.

Instead, we should probably think about whether tmt could load a "global" configuration, e.g., /etc/tmt/config, >if "local" ~/.config/tmt is missing. That way, we could ship the initial configuration as a whole, not just for >hardware

ah, yes, we should 😅

happz · 2025-04-13T09:25:48Z

tmt/steps/provision/mrack.py

@@ -79,6 +79,21 @@ def _get_constraint_translations(logger: tmt.log.Logger) -> list[MrackTranslatio
    )


+def _get_default_constraint_translations(logger: tmt.log.Logger) -> list[MrackTranslation]:


See? Adding special handling for one part of the configuration because we need to define the defaults we could ship for tmt users. We also want to ship themes, and maybe also Jira configuration, and so on, so we cannot start adding special functions for each node in the config fmf tree.

yes,no need to add special handling, maybe we could rename /etc/tmt/hardware.fmf to /etc/tmt/config.fmf or similar, then put all the content of hardware.fmf under hardware: and we could also add link: and theme: part, gonna to modify the code to accordingly see how it gose^^

Done: dc84426
WDYT? 😃

I think you still have too much special handling despite acknowledging it's not needed :) DefaultConfig, default_config, DEFAULT_HARDWARE_FILE, _get_default_config, ...

Is there any difference between the "user" and the "default" config? What's different? They are both fmf trees; they have exactly the same structure, same keys, keys are of the same types, and so on. Should plugins care whether they are working with "user" or "default" configuration? To them, it's the same container class, just loaded from a different location. I'd say that is the only difference, the location, and which piece of code is responsible for loading configuration? Config class. So the logic of loading "user" or "default" configuration should be there, transparent to its users who would not need to know the difference. Plugins care about the content, not the origin, which is completely out of their scope, and they should not be responsible for deciding which configuration to load - the inner working of tmt.config.Config(logger) should take care of loading the right content and presenting it further.

I tried to flush my git stash into a draft PR, I was thinking about something like this: #3664 - untested, but the idea is there.

Dear mentor, I misunderstood you 😅 , I thought by "special handling" you mean we could put all the default config(hardware,theme,etc) in one file, and one function to parse, I failed to get that depth as shown in
your mr, however, I feel a little confused, if seems that we need suitable_translations_default,as even if we have user config,it may not contains the suitable_translations ,right? 🤔
more info is put in:
https://github.com/teemtee/tmt/pull/3664/files#r2041652906

we should probably think about whether tmt could load a "global" configuration, e.g., /etc/tmt/config, if "local" ~/.config/tmt is missing. That way, we could ship the initial configuration as a whole, not just for hardware - it is not special enough to deserve this kind of exceptional handling.

After check your mr,it seems that I misunderstood your above comment, I thought by saying that you mean put hardware ,theme, and link configure into one file ie, /etc/tmt/config,fmf vs /etc/tmt/config/*.fmf, that's why there are many unwanted "special handling":)

I guess by "leaking the "local vs default" logic into plugins" you mean, I put get_default_config function into mrack.py, turns out that function is not needed anymore 🎉 😁 1145d44

Just to be clear, with this approach, we don't need to worry about/find solution for the following problem mentioned in your mr^^

IIUIC, you are worried that we add some new HW transition to our global configuration, but users will miss it because they already have some custom content in their local configuration. That is a valid concern, I do not have the solution yet,

we should probably think about whether tmt could load a "global" configuration, e.g., /etc/tmt/config, if "local" ~/.config/tmt is missing. That way, we could ship the initial configuration as a whole, not just for hardware - it is not special enough to deserve this kind of exceptional handling.

After check your mr,it seems that I misunderstood your above comment, I thought by saying that you mean put hardware ,theme, and link configure into one file ie, /etc/tmt/config,fmf vs /etc/tmt/config/*.fmf, that's why there are many unwanted "special handling":)

I guess by "leaking the "local vs default" logic into plugins" you mean, I put get_default_config function into mrack.py, turns out that function is not needed anymore 🎉 😁 1145d44

Not just that particular function, but also the concept of "default" configuration in general. For example:

user_translations = _get_constraint_translations('user', logger) default_translations = _get_constraint_translations('default', logger)

Because it's not just "user" and "default", it's also "in repository" config we want to add, to e.g. silence some linter checks just in that repository. We want this to be combined with the global and user config too. So, the list of config types is not just two items. I don't want all plugins to start adding repo_translations = _get_constraint_translations('repository', logger) :/

I don't want this to be a thing in tmt plugins. I don't want every plugin to become polluted with "user" and "default" configuration, because I believe it's not their job to worry about what is set where. We want them to be as simple as possible, work with a configuration which itself might be combined from different sources. How exactly it would behave, or how it would be implemented, we'll see after todays' meeting - but I, for one, don't want plugins to be aware of different kinds of configurations. This is a problem Config should solve internally, plugins should not be tasked with implementing part of the logic.

Just to be clear, with this approach, we don't need to worry about/find solution for the following problem mentioned in your mr^^

IIUIC, you are worried that we add some new HW transition to our global configuration, but users will miss it because they already have some custom content in their local configuration. That is a valid concern, I do not have the solution yet,

I think we do worry about it, not necessarily in my patch, but in some PR for sure, because I strongly believe plugins distinguishing between various config sources is not the right way forward. So I do want to solve that under the cover of Config.

hmm, TBO, I think the approach in your mr are,if I may say, too..., default configure will be ignored entirely ALA there is something is custom path, ie if there is a link.fmf, hardware.fmf in default configure path will be ignored.
I'm thinking maybe we could add a function in config/init.py, but out of class Config , say def hardware_combine, combine configure from user, default,whatever together, but if user have translations for A, then the default translations for A will be ignored, of course, WDYT?:)

hmm, TBO, I think the approach in your mr are,if I may say, too..., default configure will be ignored entirely ALA there is something is custom path, ie if there is a link.fmf, hardware.fmf in default configure path will be ignored.

Correct, but that's the destiny of all defaults, right? User provided their own configuration; who are we to say "nope, that's not good, we will use some of the defaults"? The user does not have /hardware/beaker/translations/boot.method, should we use the default? What if the user removed it on purpose? Again, who are we to say the user's configuration is useless and we know better? We must avoid the temptation to guess.

I'm thinking maybe we could add a function in config/init.py, but out of class Config , say def hardware_combine, combine configure from user, default,whatever together, but if user have translations for A, then the default translations for A will be ignored, of course, WDYT?:)

It must be more generic than that, it cannot be a one-shot thing just for hardware. We have CLI themes, we have linter config - is /etc/tmt/config the default or the global? Is it to be used when use lacks some nodes, or is it completely overriden by user config? I'd recommend we leave the question of hardware aside, because this is bigger than hardware and Beaker. If the user does not have /theme/active-theme key, what should we do? It's the same question, and we cannot and will not add tools just for hardware configuration.

and we cannot and will not add tools just for hardware configuration.

yeah, agree:)

happz · 2025-04-13T09:26:45Z

tmt/steps/provision/mrack.py

+        if translation.requirement == constraint.printable_name
+    ]
+
+    if not (suitable_translations or suitable_translations_default):


Nope, I think this is not a good way to approach "config has defaults shipped with tmt" problem :)

How about this one^^ :2b9ab2d

Ship a default hardware config file

3c34bb0

We should have tmt ship a default hardware config file containing complex and not-implemented-yet translations, and allow users override it with theirs if they want.

skycastlelily requested review from psss, lukaszachy, happz, thrix, janhavlin and martinhoyer as code owners April 13, 2025 07:28

skycastlelily mentioned this pull request Apr 13, 2025

Support boot.method in the mrack plugin #2671

Draft

8 tasks

happz reviewed Apr 13, 2025

View reviewed changes

squash:whole config

dc84426

skycastlelily added the status | blocking other work An important pull request, blocking other pull requests or issues label Apr 14, 2025

skycastlelily added this to the 1.47 milestone Apr 14, 2025

happz added the area | config User configuration label Apr 14, 2025

skycastlelily requested a review from dav-pascual as a code owner April 15, 2025 04:16

squash: polish

1145d44

skycastlelily force-pushed the default-config branch from 2b9ab2d to 1145d44 Compare April 15, 2025 07:07

skycastlelily added the status | blocked The merging of PR is blocked on some other issue label Apr 15, 2025

happz removed this from the 1.47 milestone Apr 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ship a default hardware config file #3663

Ship a default hardware config file #3663

skycastlelily commented Apr 13, 2025

happz Apr 13, 2025

happz Apr 13, 2025

skycastlelily Apr 14, 2025

happz Apr 13, 2025

skycastlelily Apr 14, 2025

skycastlelily Apr 14, 2025

happz Apr 14, 2025

skycastlelily Apr 14, 2025

skycastlelily Apr 15, 2025 •

edited

Loading

happz Apr 15, 2025

skycastlelily Apr 15, 2025

happz Apr 15, 2025

skycastlelily Apr 15, 2025

happz Apr 13, 2025

skycastlelily Apr 15, 2025

		@@ -79,6 +79,21 @@ def _get_constraint_translations(logger: tmt.log.Logger) -> list[MrackTranslatio
		)


		def _get_default_constraint_translations(logger: tmt.log.Logger) -> list[MrackTranslation]:

Ship a default hardware config file #3663

Are you sure you want to change the base?

Ship a default hardware config file #3663

Conversation

skycastlelily commented Apr 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skycastlelily Apr 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skycastlelily Apr 15, 2025 •

edited

Loading