Reduce use of project dir to be more flexible regarding custom test runs

* Locate needles relative to the job's needles directory (keeping legacy
  way to locate needles as a fallback for compatibility)
* Don't try to update needles which can not be located
* Don't use OpenQA::Utils::needle_info() to locate needles in file
  API
    * It is not required to parse the needle JSON here
    * The comment stating the needle path comes from the database
      was wrong. It was only deduced via needledir() which is now
      used directly.
* No longer pass the project dir to isotovideo
* See https://progress.opensuse.org/issues/56789
  and https://progress.opensuse.org/issues/58184

lib/OpenQA/Schema/Result/JobModules.pm

@@ -266,19 +266,12 @@ sub store_needle_infos {
         $needle_cache = \%hash;
     }
 
-    my %needles;
-
     for my $detail (@{$details}) {
         if ($detail->{needle}) {
-            my $nfn    = $detail->{json};
-            my $needle = OpenQA::Schema::Result::Needles::update_needle($nfn, $self, 1, $needle_cache);
-            $needles{$needle->id} ||= 1;
+            OpenQA::Schema::Result::Needles::update_needle($detail->{json}, $self, 1, $needle_cache);
         }
         for my $needle (@{$detail->{needles} || []}) {
-            my $nfn    = $needle->{json};
-            my $needle = OpenQA::Schema::Result::Needles::update_needle($nfn, $self, 0, $needle_cache);
-            # failing needles are more interesting than succeeding, so ignore previous values
-            $needles{$needle->id} = -1;
+            OpenQA::Schema::Result::Needles::update_needle($needle->{json}, $self, 0, $needle_cache);
         }
     }
 

lib/OpenQA/Schema/Result/Needles.pm

@@ -1,4 +1,4 @@
-# Copyright (C) 2015-2017 SUSE LLC
+# Copyright (C) 2015-2019 SUSE LLC
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -29,7 +29,7 @@ use File::Spec::Functions 'catdir';
 use OpenQA::Git;
 use OpenQA::Jobs::Constants;
 use OpenQA::Schema::Result::Jobs;
-use OpenQA::Utils qw(log_error);
+use OpenQA::Utils qw(log_error locate_needle);
 
 __PACKAGE__->table('needles');
 __PACKAGE__->load_components(qw(InflateColumn::DateTime Timestamps));
@@ -103,30 +103,30 @@ sub update_needle_cache {
     }
 }
 
-# save the needle informations
+# save the needle information
 # be aware that giving the optional needle_cache hash ref, makes you responsible
 # to call update_needle_cache after a loop
 sub update_needle {
     my ($filename, $module, $matched, $needle_cache) = @_;
 
+    # assume that path of the JSON file is relative to the job's needle dir or (to support legacy versions
+    # of os-autoinst) relative to the "share dir" (the $bmwqemu::vars{PRJDIR} variable in legacy os-autoinst)
+    my $needle_dir;
+    unless (-f $filename) {
+        return undef unless $filename = locate_needle($filename, $needle_dir = $module->job->needle_dir);
+    }
+
     my $schema = OpenQA::Schema->singleton;
     my $guard  = $schema->txn_scope_guard;
 
-    if (!-f $filename) {
-        $filename = catdir($OpenQA::Utils::sharedir, $filename);
-        if (!-f $filename) {
-            log_error(
-                "Needle file $filename not found where expected. Check $OpenQA::Utils::prjdir for distri symlinks");
-        }
-    }
     my $needle;
     if ($needle_cache) {
         $needle = $needle_cache->{$filename};
     }
     if (!$needle) {
         # create a canonical path out of it
         my $realpath       = realpath($filename);
-        my $needledir_path = realpath($module->job->needle_dir());
+        my $needledir_path = realpath($needle_dir // $module->job->needle_dir);
         my $dir;
         my $basename;
         if (index($realpath, $needledir_path) != 0) {    # leave old behaviour as it is

lib/OpenQA/Utils.pm

@@ -41,6 +41,7 @@ our @EXPORT  = qw(
   $prjdir
   $resultdir
   &data_name
+  &locate_needle
   &needle_info
   &needledir
   &productdir
@@ -178,49 +179,43 @@ sub needledir {
 
 sub log_warning;
 
-sub needle_info {
-    my ($name, $distri, $version, $fn) = @_;
-    local $/;
+sub locate_needle {
+    my ($relative_needle_path, $needles_dir) = @_;
 
-    my $needledir = needledir($distri, $version);
+    my $absolute_filename = catdir($needles_dir, $relative_needle_path);
+    my $needle_exists     = -f $absolute_filename;
 
-    if (!$fn) {
-        $fn = "$needledir/$name.json";
-    }
-    elsif (!-f $fn) {
-        $fn        = catfile($sharedir, $fn);
-        $needledir = dirname($fn);
-    }
-    else {
-        $needledir = dirname($fn);
+    if (!$needle_exists) {
+        $absolute_filename = catdir($OpenQA::Utils::sharedir, $relative_needle_path);
+        $needle_exists     = -f $absolute_filename;
     }
+    return $absolute_filename if $needle_exists;
 
-    my $JF;
-    unless (open($JF, '<', $fn)) {
-        log_warning("$fn: $!");
-        return;
-    }
+    log_error("Needle file $relative_needle_path not found within $needles_dir.");
+    return undef;
+}
+
+sub needle_info {
+    my ($name, $distri, $version, $file_name, $needles_dir) = @_;
+
+    $file_name = locate_needle($file_name, $needles_dir) if !-f $file_name;
+    return undef unless defined $file_name;
 
     my $needle;
     try {
-        $needle = decode_json(<$JF>);
+        $needle = decode_json(Mojo::File->new($file_name)->slurp);
     }
     catch {
-        log_warning("failed to parse $fn: $_");
-        # technically not required, $needle should remain undefined. Being superstitious human I add:
-        undef $needle;
-    }
-    finally {
-        close($JF);
+        log_warning("Failed to parse $file_name: $_");
     };
-    return unless $needle;
+    return undef unless defined $needle;
 
-    my $png_fname = basename($fn, '.json') . '.png';
-    my $pngfile   = File::Spec->catpath('', $needledir, $png_fname);
+    my $png_fname = basename($file_name, '.json') . '.png';
+    my $pngfile   = File::Spec->catpath('', $needles_dir, $png_fname);
 
-    $needle->{needledir} = $needledir;
+    $needle->{needledir} = $needles_dir;
     $needle->{image}     = $pngfile;
-    $needle->{json}      = $fn;
+    $needle->{json}      = $file_name;
     $needle->{name}      = $name;
     $needle->{distri}    = $distri;
     $needle->{version}   = $version;

lib/OpenQA/WebAPI/Controller/File.pm

@@ -34,19 +34,16 @@ sub needle {
     my $distri   = $self->param('distri');
     my $version  = $self->param('version') || '';
     my $jsonfile = $self->param('jsonfile') || '';
-    # make sure the directory of the file parameter is a real subdir of
-    # testcasedir before applying it as needledir to prevent access
-    # outside of the zoo
+
+    # make sure the directory of the file parameter is a real subdir of testcasedir before
+    # applying it as needledir to prevent access outside of the zoo
     if ($jsonfile && !is_in_tests($jsonfile)) {
         warn "$jsonfile is not in a subdir of $prjdir/share/tests or $prjdir/tests";
         return $self->render(text => 'Forbidden', status => 403);
     }
-    my $needle = needle_info($name, $distri, $version, $jsonfile);
-    return $self->reply->not_found unless $needle;
 
-    $self->{static} = Mojolicious::Static->new;
-    # needledir is an absolute path from the needle database
-    push @{$self->{static}->paths}, $needle->{needledir};
+    # locate the needle in the needle directory for the given distri and version
+    push(@{($self->{static} = Mojolicious::Static->new)->paths}, needledir($distri, $version));
 
     # name is an URL parameter and can't contain slashes, so it should be safe
     return $self->serve_static_($name . $format);

lib/OpenQA/WebAPI/Controller/Step.pm

@@ -126,6 +126,7 @@ sub edit {
     my $imgname       = $module_detail->{screenshot};
     my $distri        = $job->DISTRI;
     my $dversion      = $job->VERSION || '';
+    my $needle_dir    = $job->needle_dir;
 
     # Each object in @needles will contain the name, both the url and the local path
     # of the image and 2 lists of areas: 'area' and 'matches'.
@@ -151,8 +152,9 @@ sub edit {
         $screenshot = $self->_new_screenshot($tags, $imgname, $module_detail->{area});
 
         # Second position: the only needle (with the same matches)
-        my $needle_info = $self->_extended_needle_info($needle_name, \%basic_needle_data, $module_detail->{json}, 0,
-            \@error_messages);
+        my $needle_info
+          = $self->_extended_needle_info($needle_dir, $needle_name, \%basic_needle_data, $module_detail->{json},
+            0, \@error_messages);
         if ($needle_info) {
             $needle_info->{matches} = $screenshot->{matches};
             push(@needles, $needle_info);
@@ -166,10 +168,10 @@ sub edit {
         # $needle contains information from result, in which 'areas' refers to the best matches.
         # We also use $area for transforming the match information into a real area
         for my $needle (@$module_detail_needles) {
-            my $needle_info
-              = $self->_extended_needle_info($needle->{name}, \%basic_needle_data, $needle->{json},
-                $needle->{error}, \@error_messages)
-              || next;
+            my $needle_info = $self->_extended_needle_info(
+                $needle_dir,     $needle->{name},  \%basic_needle_data,
+                $needle->{json}, $needle->{error}, \@error_messages
+            ) || next;
             my $matches = $needle_info->{matches};
             for my $match (@{$needle->{area}}) {
                 my %area = (
@@ -209,8 +211,8 @@ sub edit {
                 ));
             # get needle info to show the needle also in selection
             my $needle_info
-              = $self->_extended_needle_info($new_needle->name, \%basic_needle_data, $new_needle->path, undef,
-                \@error_messages)
+              = $self->_extended_needle_info($needle_dir, $new_needle->name, \%basic_needle_data, $new_needle->path,
+                undef, \@error_messages)
               || next;
             $needle_info->{title} = 'new: ' . $needle_info->{title};
             push(@needles, $needle_info);
@@ -295,12 +297,12 @@ sub _new_screenshot {
 }
 
 sub _extended_needle_info {
-    my ($self, $needle_name, $basic_needle_data, $file_name, $error, $error_messages) = @_;
+    my ($self, $needle_dir, $needle_name, $basic_needle_data, $file_name, $error, $error_messages) = @_;
 
     my $overall_list_of_tags = $basic_needle_data->{tags};
     my $distri               = $basic_needle_data->{distri};
     my $version              = $basic_needle_data->{version};
-    my $needle_info          = needle_info($needle_name, $distri, $version, $file_name);
+    my $needle_info          = needle_info($needle_name, $distri, $version, $file_name, $needle_dir);
     if (!$needle_info) {
         my $error_message = sprintf('Could not parse needle: %s for %s %s', $needle_name, $distri, $version);
         $self->app->log->error($error_message);
@@ -467,8 +469,9 @@ sub viewimg {
     my $module_detail = $self->stash('module_detail');
     my $job           = $self->stash('job');
     return $self->reply->not_found unless $job;
-    my $distri   = $job->DISTRI;
-    my $dversion = $job->VERSION || '';
+    my $needle_dir = $job->needle_dir;
+    my $distri     = $job->DISTRI;
+    my $dversion   = $job->VERSION || '';
 
     # initialize hash to store needle lists by tags
     my %needles_by_tag;
@@ -500,7 +503,7 @@ sub viewimg {
     # load primary needle match
     my $primary_match;
     if (my $needle = $module_detail->{needle}) {
-        if (my $needleinfo = needle_info($needle, $distri, $dversion, $module_detail->{json})) {
+        if (my $needleinfo = needle_info($needle, $distri, $dversion, $module_detail->{json}, $needle_dir)) {
             my $info = {
                 name          => $needle,
                 image         => $self->needle_url($distri, $needle . '.png', $dversion, $needleinfo->{json}),
@@ -520,7 +523,7 @@ sub viewimg {
     if ($module_detail->{needles}) {
         for my $needle (@{$module_detail->{needles}}) {
             my $needlename = $needle->{name};
-            my $needleinfo = needle_info($needlename, $distri, $dversion, $needle->{json});
+            my $needleinfo = needle_info($needlename, $distri, $dversion, $needle->{json}, $needle_dir);
             next unless $needleinfo;
             my $info = {
                 name    => $needlename,

lib/OpenQA/Worker/Engines/isotovideo.pm

@@ -204,7 +204,6 @@ sub engine_workit {
         OPENQA_URL      => $openqa_url,
         WORKER_INSTANCE => $instance,
         WORKER_ID       => $workerid,
-        PRJDIR          => $OpenQA::Utils::sharedir,
         %$job_settings
     );
     log_debug('Job settings:');
@@ -231,8 +230,6 @@ sub engine_workit {
             );
             my $rsync_request_description = "Rsync cache request from '$rsync_source' to '$shared_cache'";
 
-            $vars{PRJDIR} = $shared_cache;
-
             # enqueue rsync task; retry in some error cases
             for (my $remaining_tries = 3; $remaining_tries > 0; --$remaining_tries) {
                 return {error => "Failed to send $rsync_request_description"} unless $rsync_request->enqueue;

t/21-needles.t

@@ -1,6 +1,7 @@
 #!/usr/bin/env perl -w
 
 # Copyright (C) 2016 Red Hat
+# Copyright (C) 2019 SUSE LLC
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -20,10 +21,12 @@ use warnings;
 
 use FindBin;
 use lib "$FindBin::Bin/lib";
+use Cwd 'abs_path';
 use OpenQA::Schema;
 use OpenQA::Test::Database;
 use OpenQA::Task::Needle::Scan;
 use File::Find;
+use Test::Output 'combined_like';
 use Test::More;
 use Test::Mojo;
 use Test::Warnings;
@@ -168,4 +171,38 @@ is($needles->find({filename => "test-nonexistent.json"})->file_present, 0);
 # this needle exists, so it should have file_present set to 1
 is($needles->find({filename => "installer/test-nestedneedle-1.json"})->file_present, 1);
 
+subtest 'handling relative paths in update_needle' => sub {
+    is($module->job->needle_dir,
+        $needledir_fedora, 'needle dir of job deduced from settings (prerequisite for handling relative paths)');
+
+    subtest 'handle needle path relative to share dir (legacy os-autoinst)' => sub {
+        my $needle
+          = OpenQA::Schema::Result::Needles::update_needle('tests/fedora/needles/test-rootneedle.json', $module, 0);
+        is(
+            $needle->path,
+            abs_path('t/data/openqa/share/tests/fedora/needles/test-rootneedle.json'),
+            'needle path correct'
+        );
+    };
+    subtest 'handle needle path relative to needle dir' => sub {
+        my $needle = OpenQA::Schema::Result::Needles::update_needle('test-rootneedle.json', $module, 0);
+        is(
+            $needle->path,
+            abs_path('t/data/openqa/share/tests/fedora/needles/test-rootneedle.json'),
+            'needle path correct'
+        );
+    };
+    subtest 'handle needle path to non existent needle' => sub {
+        my $needle;
+        combined_like(
+            sub {
+                $needle = OpenQA::Schema::Result::Needles::update_needle('test-does-not-exist.json', $module, 0);
+            },
+            qr/Needle file test-does-not-exist\.json not found within $needledir_fedora/,
+            'error logged',
+        );
+        is($needle, undef, 'no needle created');
+    };
+};
+
 done_testing;