QA (public) » openQA Project (public)

The keys call defines $options{group} as an empty hash ref, as we can see with this version of the test script:

#!/bin/perl

use Data::Dump 'dd';

my %options;
dd $options{group};
my $group = (keys %{$options{group}})[0];
dd $options{group};

Ugh, so this is getting fun. If I fix all the bugs, the 're-imported groups' test in 40-script_load_dump_templates.t unexpectedly starts failing with an error:

Job group 'opensuse' already exists. Use --update to modify existing entries. at /builddir/build/BUILD/openqa-5_20250430git10b1e43-build/openQA-10b1e43840bddcc9839be91ebb46fb422effe601/script/openqa-load-templates line 127, <> line 388.

That seems weird because the test intentionally clears all the database tables immediately before running the command:

# Clear the data in relevant tables
$schema->resultset($_)->delete for qw(Machines TestSuites Products JobTemplates JobGroups);

The problem seems to happen because job templates are loaded before job groups. I can't quite see how, but I think that somehow, now we actually have job templates to load, loading them somehow causes at least one job group to be created; so when we get to loading the job groups, we fail the check that it doesn't already exist. (At first I thought it was odd I'd never run into this in the real world, but then I realized this check is quite new - it was added in 97d34ec to fix https://progress.opensuse.org/issues/174808 ). I tested this by flipping the loading order, so we load job groups before job templates:

my @tables = (qw(Machines TestSuites Products JobGroups JobTemplates));

and now it doesn't fail that way any more, but it still fails. Now, somehow, the repeated dump doesn't give identical data:

#   Failed test 'both dumps match'
#   at t/40-script_load_dump_templates.t line 109.
#     Structures begin differing at:
#          $got->{JobTemplates}[0]{test_suite}{name} = 'textmode'
#     $expected->{JobTemplates}[0]{test_suite}{name} = 'client1'

I assume this is just because we don't work hard enough to make sure the job templates array is always ordered the same in the output, or something, but I kinda ran out of time at this point. Will get back to it tomorrow.

oh, god, every time I think I have this fixed, something else rears up.

So...this kinda runs into the whole "job template YAML migration" thing, right? https://progress.opensuse.org/issues/44360 .

With that, we have two kinda-competing ways of defining job groups and job templates. Job groups can kinda encode job templates: a job group can include a YAML template string which defines the templates. When load-templates runs (sigh, the word 'templates' is now overloaded...), if a job group definition includes a templates string, then loading the job group also implicitly loads the templates it defines - the template string gets POSTed to the job_templates_scheduling API endpoint, which is connected to the JobTemplate controller's update method, which reads the template string as YAML and loads the resulting templates.

However, you can still define and load job templates directly by posting to job_templates, which is connected to the JobTemplate controller's create method, and load-templates still supports this; if the template file has a JobTemplates object, it will be read and loaded. You can still load a template file containing JobGroups with no YAML template strings, and some JobTemplates, and that works fine, but gives you the 'deprecated' webUI Job Groups admin view (this is how things currently work in Fedora).

If a template file has both JobGroups with YAML template strings and JobTemplates, then load-templates will try and load both, which seems maybe surprising? As it currently stands, it tries to load the job templates first, then the job groups. Loading the templates seems to somehow implicitly create the groups they refer to, and then when it tries to load the groups, it chokes on the check added in 97d34ec . If I reverse this so it loads the groups first, then it will create the job templates they describe, then try to load the job templates explicitly defined in the templates file. If the two sets of templates match, then all the explicitly-defined ones just get skipped, and things are fine. I'm not sure what happens if the explicitly-defined templates are a superset of the ones defined in YAML in the groups - I suspect it'd die on the "Group foo must be updated through the YAML template" error check in JobTemplate create.

Due to the bugs I found at first, dump-templates currently almost never actually dumps any job templates, which goes a long way to hiding all of this if you only ever load template files created by dump-templates. But it does seem clearly intended to dump job templates, the issues that make it not do so really do look like bugs, nobody ever committed anything that said 'make dump-templates not write JobTemplates by default' or anything like that.

But it also unconditionally writes the YAML template strings, if it's dumping job group data at all, and the mechanism for this means you'll get a YAML template string in the output data even if there is no YAML template string in the job group as it currently exists on the server. That's because dump-templates GETs the job_templates_scheduling endpoint, which is connected to JobTemplate schedules method, which winds up iterating over the groups in the DB and calling $group->to_yaml on each, and that returns the existing YAML template string if there is one, but if there isn't one, it calls $self->to_template which tries to auto-generate it from the existing legacy templates.

Now I've traced this all the way out, that feels...odd. Let's say you have a 'legacy' instance, and you run dump-templates on it. The fact that it unconditionally auto-generates and writes YAML template strings implies that this will be a 'conversion' operation - if you load the dumped templates, you'll be converting the instance to the 'modern' style with job groups containing YAML template strings. But in this case, why is it dumping the legacy templates at all? It doesn't make sense.

At this point I kinda feel like dump-templates should only ever dump 'legacy' data (groups with no template string, explicit templates) or 'modern' data (groups with template strings, no templates), and load-templates should only accept data in that format: it should intentionally exit if the data it's passed contains both groups with template strings and a JobTemplates object.

(Alternatively we could do a band-aid rip and give up on the 'legacy' format entirely, which would require Fedora and any other legacy deployments to convert).

Does anyone see any problems with this plan?

hmm, indeed, looking at https://github.com/os-autoinst/openQA/commit/323246dc9f597c0efa137e910ad038594489ce83 - which added the initial YAML support to dump/load-templates - I remember that before that, job groups weren't explicitly dumped or loaded at all. They were always implied by the templates. If a template referenced a group that didn't exist, it would be created. Other properties of job groups could only (and still can only) be set via the web UI, they don't exist so far as the template dump/load stuff is concerned.

So...I think we could just make this either/or in both dump and load. Job template and job group data should never co-exist in the same template file. dump-templates should only ever create files with one or the other, and load-templates should only ever load files with one or the other, and error out if you try and feed it a file with both.

ETA: ugh...do we possibly consider a 'hybrid' setup valid? where some groups are YAML-converted but some are not, and this is OK so long as they don't mix?

If anyone wants to peer down my rabbit hole, the git branch is https://github.com/AdamWill/openQA/tree/template-dump-load-bugs . I'm really hoping to get it to be a PR tomorrow.

okay, after another solid day slogging at this I have a pretty clear view of where I'm going at last. The current end state of my git branch is mostly good, but I want to rearrange it a bit - moving some of the later test changes earlier would avoid the need for the "load-templates: load job groups before job templates" commit from the middle of the series, now I have a better understanding of exactly why I got errors without it (it's because if a legacy job group has a job template with settings in it, we can dump them, but we cannot restore them via the legacy API).

I need to add an explicit test of dumping and reloading YAML templates with settings, and then there are two potential follow-ups I want to look at: making --clean work for YAML templates (see https://progress.opensuse.org/issues/182063 ), and making the limit options in dump-templates (--group, --test, --machine, --product, --full) make sense, because there's a lot that's broken about them now, I'm pretty sure. But I may send those as separate follow-up PRs. Not sure yet.

https://github.com/os-autoinst/openQA/pull/6446 addresses most of the issues discussed here.

https://github.com/os-autoinst/openQA/pull/6446 is merged since some days. @AdamWill In case you want to look into the rest of "most of the issues" be my guest to do it as part of this ticket. If you need help from others then I suggest another ticket with clear description of the still missing parts.