Remap beaker group key for mrack to beaker_job_group

[dav-pascual]

Fix #4198

Remap beaker key from group to beaker_job_group in mrack module
to incorporate new changes in neoave/mrack#325

This amends feature introduced in #3397

This doesn't imply any API changes

[LecrisUT]

Marking it as blocked because the mrack version is not in dist-git. But for testing, you can make a simple copr with the new mrack build and add it to

tmt/.packit.yaml

Lines 119 to 120 in 64be40c

	- <<: *copr-under-packit
	trigger: pull_request

as additional_repos: copr://user/project (as a temp commit)

[skycastlelily]

With this change and the mrack mr , you will get fail: 'group' error all the time, that is because tmt will not pass group, and you use host["group"] in your mrack mr, after change it to host.get("group"), the error will be gone, but you will find that the beaker-job-group is not passed to the beaker job, because you use

"job_group": host.get(CONFIG_KEY, {}).get(
                "beaker_job_group"

where you should use

"job_group": host.get(
                "beaker_job_group"

In conclusion, you need revert your merged mrack mr. A little sorry for failing to check this mr earlier, I was really pretty busy with other stuff.

[dav-pascual]

@skycastlelily

With this change and the mrack mr , you will get fail: 'group' error all the time, that is because tmt will not pass group, and you use host["group"] in your mrack mr, after change it to host.get("group"), the error will be gone, but you will find that the beaker-job-group is not passed to the beaker job, because you use

"job_group": host.get(CONFIG_KEY, {}).get(
                "beaker_job_group"

where you should use

"job_group": host.get(
                "beaker_job_group"

In conclusion, you need revert your merged mrack mr. A little sorry for failing to check this mr earlier, I was really pretty busy with other stuff.

The mrack API change is on purpose. It's a Beaker specific configuration (not host specific), therefore it should be under "beaker" dict. It follows the same logic as 'beaker_job_owner'

e.g.

  - group: server
    name: bkr-fedora-latest.test
    os: fedora-42
    provider: beaker
    beaker:
        beaker_job_group: devel

instead of

  - group: server
    name: bkr-fedora-latest.test
    os: fedora-42
    provider: beaker
    beaker_job_group: devel

I already tested the mrack changes successfully with an internal CI (not tmt). If you can, please test with tmt to confirm this works

[skycastlelily]

The mrack API change is on purpose. It's a Beaker specific configuration (not host specific), therefore it should be under "beaker" dict. It follows the same logic as 'beaker_job_owner'

If so, then you need to change tmt code accordingly in this mr, or it will definitely break tmt users

I already tested the mrack changes successfully with an internal CI (not tmt). If you can, please test with tmt to confirm this works

I tested,and it does NOT work, that's what my last comment said.

I extended the test and tried to commit to your branch, but failed through a quick try. So I put it in a new branch, and here is the commit
You need make this test pass to NOT break tmt user.

[LecrisUT]

If you could make a copr repo and add it here as mentioned in #4199 (comment) that would help a bunch. Alternatively mrack could run the tmt tests, but those are not well filtered right now to expose them, so please try the copr+packit approach instead.

[skycastlelily]

If you could make a copr repo and add it here as mentioned in #4199 (comment) that would help a bunch

yeah, so that other reviewers don't need to verify this mr by looking into your mrack mr and changing the local mrack code manually,as I did,which is not convenient.

[skycastlelily]

Suddenly realized that if we don't have this mr merged within 1.60, it will take another two weeks to have it landed.

@dav-pascual 5f0c520 handles the api change, but the 'group' fatal error will happen whenever user run tmt,with your mrack mr, really seriously...

[dav-pascual]

I just added a commit to add the missing pieces of the remapping.
"group" key is always expected in mrack, so I'm reverting back to the "hardcoded" value we had before (I will look on mrack side if we can make this key optional in the future). Also assigning "job_group" to "beaker_job_group" correctly

[skycastlelily]

"group" key is always expected in mrack,

it will not needed if you change host["group"] to host.get("group") ....

in your commit 0cfe46a

you take reference from

     if host.beaker_job_owner:
                req['job_owner'] = host.beaker_job_owner

which is Milos' walk around for mrack's beaker_job_owner doesn't work well issue.

Any reason, why don't take 5f0c520? If not, please change it back, thanks. And also, please extend the test, feel free to take this commit ...

[dav-pascual]

@skycastlelily Before I look at your suggestion, please understand that mrack is an independent project integrated in more CIs apart from tmt, especially an internal CI that runs hundreds of jobs a day on multiple clouds (like artemis) and relies on mrack heavily.

I cannot always satisfy API changes requests but I do my best. Here I'm fixing a regression that slipped past us and trying minimize collateral damage on tmt. I understand that it's inconvenient for you too but let's find common ground

[skycastlelily]

Please address my last comment(just to be clear, group one could be addressed in the future,since you already landed the new mrack) thanks,otherwise ,LGTM

[dav-pascual]

in your commit 0cfe46a

you take reference from

     if host.beaker_job_owner:
                req['job_owner'] = host.beaker_job_owner

which is Milos' walk around for mrack's beaker_job_owner doesn't work well issue.

Any reason, why don't take 5f0c520? If not, please change it back, thanks. And also, please extend the test, feel free to take this commit ...

Thanks. Updated with your suggestion

[dav-pascual]

/packit test failed

[dav-pascual]

All runs succesful except for F43 (expected as there's a freeze https://bodhi.fedoraproject.org/updates/FEDORA-2025-abbb40385b)

[LecrisUT]

All runs succesful except for F43 (expected as there's a freeze https://bodhi.fedoraproject.org/updates/FEDORA-2025-abbb40385b)

Thanks. But the freeze period could be quite problematic for that since it would mean our CI would break until the freeze period ends, unless you could make it work for both versions. Currently the CI is not broken because it seems it does not properly test this.

A course of action here would be to just cherry-pick downstream to fix this issue as soon as the current release is finished and we have a green light from the next release monitor.