From 42edf6f8e9134af65cf3881cc63528facdf8f981 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Apr 18 2020 21:36:24 +0000 Subject: Backport PR #2955 to fix broken load_templates --clean --- diff --git a/0001-Fix-load_templates-clean-to-wipe-more-than-one-entry.patch b/0001-Fix-load_templates-clean-to-wipe-more-than-one-entry.patch new file mode 100644 index 0000000..97f04c7 --- /dev/null +++ b/0001-Fix-load_templates-clean-to-wipe-more-than-one-entry.patch @@ -0,0 +1,157 @@ +From edbef94836b6bfbc796819a0c3f59ae89de5f675 Mon Sep 17 00:00:00 2001 +From: Adam Williamson +Date: Sat, 18 Apr 2020 11:38:37 -0700 +Subject: [PATCH] Fix load_templates --clean to wipe more than one entry per + table + +1400d8a8dd inverted the logic here - instead of exiting the loop +on a *failure* we now exit it on *success*. So each time --clean +is run it will only wipe one entry per table. This obviously +breaks things badly (the clean phase doesn't clean things right +at all, so the subsequent load phase fails because we try to add +entries that already exist, and you wind up with a mess). + +This flips the logic back, and adds a test. + +Signed-off-by: Adam Williamson +--- + script/load_templates | 2 +- + t/01-compile-check-all.t | 3 ++- + t/40-script_load_templates.t | 32 ++++++++++++++++++++++++------- + t/data/40-templates-more.pl | 37 ++++++++++++++++++++++++++++++++++++ + 4 files changed, 65 insertions(+), 9 deletions(-) + create mode 100644 t/data/40-templates-more.pl + +diff --git a/script/load_templates b/script/load_templates +index 724a404f3..da089347c 100755 +--- a/script/load_templates ++++ b/script/load_templates +@@ -241,7 +241,7 @@ if ($options{'clean'}) { + my $id = $result->{$table}->[$i]->{id}; + my $table_url_id = $url->clone->path($options{'apibase'} . '/' . decamelize($table) . "/$id"); + $res = $client->delete($table_url_id)->res; +- last if $res->is_success; ++ last unless $res->is_success; + } + } + +diff --git a/t/01-compile-check-all.t b/t/01-compile-check-all.t +index 5b620e3d5..b6cebd48e 100644 +--- a/t/01-compile-check-all.t ++++ b/t/01-compile-check-all.t +@@ -27,8 +27,9 @@ $Test::Strict::TEST_WARNINGS = 1; + $Test::Strict::TEST_SKIP = [ + # skip test module which would require test API from os-autoinst to be present + 't/data/openqa/share/tests/opensuse/tests/installation/installer_timezone.pm', +- # Skip data file which is supposed to resemble generated output which has no 'use' statements ++ # Skip data files which are supposed to resemble generated output which has no 'use' statements + 't/data/40-templates.pl', ++ 't/data/40-templates-more.pl', + 't/data/openqa-trigger-from-obs/Proj2::appliances/empty.txt', + 't/data/openqa-trigger-from-obs/Proj3::standard/empty.txt', + ]; +diff --git a/t/40-script_load_templates.t b/t/40-script_load_templates.t +index 8be3e0c53..51f67e135 100644 +--- a/t/40-script_load_templates.t ++++ b/t/40-script_load_templates.t +@@ -46,9 +46,10 @@ sub decode { Cpanel::JSON::XS->new->relaxed->decode(path(shift)->slurp); } + test_once '--help', qr/Usage:/, 'help text shown', 1, 'load_templates with no arguments shows usage'; + test_once '--host', qr/Option host requires an argument/, 'host argument error shown', 1, 'required arguments missing'; + +-my $host = 'testhost:1234'; +-my $filename = 't/data/40-templates.pl'; +-my $args = "--host $host $filename"; ++my $host = 'testhost:1234'; ++my $filename = 't/data/40-templates.pl'; ++my $morefilename = 't/data/40-templates-more.pl'; ++my $args = "--host $host $filename"; + test_once $args, qr/unknown error code - host $host unreachable?/, 'invalid host error', 22, 'error on invalid host'; + + $ENV{MOJO_LOG_LEVEL} = 'fatal'; +@@ -71,19 +72,36 @@ test_once $args, $expected, 'Admin may load templates', 0, 'successfully loaded + test_once $args, qr/group with existing name/, 'Duplicate job group', 255, 'failed on duplicate job group'; + + my $fh; +-($fh, $filename) = tempfile(UNLINK => 1, SUFFIX => '.json'); +-$args = "--host $host --apikey $apikey --apisecret $apisecret --json > $filename"; ++my $tempfilename; ++($fh, $tempfilename) = tempfile(UNLINK => 1, SUFFIX => '.json'); ++$args = "--host $host --apikey $apikey --apisecret $apisecret --json > $tempfilename"; + $expected = qr/^$/; + dump_templates $args, $expected, 'dumped fixtures'; + # Clear the data in relevant tables + $schema->resultset($_)->delete for qw(Machines TestSuites Products JobTemplates JobGroups); +-$args = "--host $host --apikey $apikey --apisecret $apisecret $filename"; ++$args = "--host $host --apikey $apikey --apisecret $apisecret $tempfilename"; + $expected = qr/JobGroups.+=> \{ added => 3, of => 3 \}/; + test_once $args, $expected, 're-imported fixtures'; + my ($rh, $reference) = tempfile(UNLINK => 1, SUFFIX => '.json'); + $args = "--host $host --apikey $apikey --apisecret $apisecret --json > $reference"; + $expected = qr/^$/; + dump_templates $args, $expected, 're-dumped fixtures'; +-is_deeply decode($filename), decode($reference), 'both dumps match'; ++is_deeply decode($tempfilename), decode($reference), 'both dumps match'; ++ ++# Clear the data in relevant tables again ++$schema->resultset($_)->delete for qw(Machines TestSuites Products JobTemplates JobGroups); ++# load the templates file with 2 machines ++$args = "--host $host --apikey $apikey --apisecret $apisecret $morefilename"; ++$expected = qr/Machines.+=> \{ added => 2, of => 2 \}/; ++test_once $args, $expected, 'imported MOAR fixtures'; ++# wipe jobgroups manually as --clean explicitly skips it ++$schema->resultset("JobGroups")->delete; ++# now load the templates file with only 1 machine, with --clean ++$args = "--host $host --apikey $apikey --apisecret $apisecret --clean $filename"; ++$expected = qr/Machines.+=> \{ added => 1, of => 1 \}/; ++test_once $args, $expected, 'imported original fixtures'; ++is $schema->resultset('Machines')->count, 1, "only one machine is loaded"; ++my $machine = $schema->resultset('Machines')->first; ++is $machine->name, "32bit", "correct machine is loaded"; + + done_testing; +diff --git a/t/data/40-templates-more.pl b/t/data/40-templates-more.pl +new file mode 100644 +index 000000000..79f5fbb1e +--- /dev/null ++++ b/t/data/40-templates-more.pl +@@ -0,0 +1,37 @@ ++{ ++ JobGroups => [ ++ { ++ group_name => "openSUSE Leap 42.3 Updates", ++ template => "scenarios: {}\nproducts: {}\n", ++ }, ++ ], ++ JobTemplates => [], ++ Machines => [ ++ { ++ backend => "qemu", ++ name => "32bit", ++ settings => [], ++ }, ++ { ++ backend => "qemu", ++ name => "64bit", ++ settings => [], ++ }, ++ ], ++ Products => [ ++ { ++ arch => "x86_64", ++ distri => "opensuse", ++ flavor => "DVD", ++ settings => [], ++ version => 42.2, ++ }, ++ ], ++ TestSuites => [ ++ { ++ name => "uefi", ++ settings => ++ [{key => "DESKTOP", value => "kde"}, {key => "INSTALLONLY", value => 1}, {key => "UEFI", value => 1},], ++ }, ++ ], ++} +-- +2.26.1 + diff --git a/openqa.spec b/openqa.spec index ac9852f..fc38091 100644 --- a/openqa.spec +++ b/openqa.spec @@ -66,7 +66,7 @@ Name: openqa Version: %{github_version} -Release: 47%{?github_date:.%{github_date}git%{shortcommit}}%{?dist} +Release: 48%{?github_date:.%{github_date}git%{shortcommit}}%{?dist} Summary: OS-level automated testing framework License: GPLv2+ Url: http://os-autoinst.github.io/openQA/ @@ -88,6 +88,10 @@ Source3: FedoraMessaging.pm # tests for the fedora-messaging publishing plugin Source4: 23-fedora-messaging.t +# https://github.com/os-autoinst/openQA/pull/2955 +# Fix a logic error in `load_templates --clean` that breaks it +Patch0: 0001-Fix-load_templates-clean-to-wipe-more-than-one-entry.patch + BuildRequires: %{python_scripts_requires} BuildRequires: perl-generators # Standard for packages that have systemd services @@ -575,6 +579,9 @@ fi %{_datadir}/openqa/lib/OpenQA/WebAPI/Plugin/FedoraUpdateRestart.pm %changelog +* Sat Apr 18 2020 Adam Williamson - 4.6-48.20200415git7160d88 +- Backport PR #2955 to fix broken load_templates --clean + * Wed Apr 15 2020 Adam Williamson - 4.6-47.20200415git7160d88 - Bump to latest git - Drop merged patch