Run optipng on the worker while uploading images (#858)

the workers just have more CPUs, so doing it remotely just scales better
os-autoinst · Sep 9, 2016 · 7ea7b0d · 7ea7b0d
1 parent 951dded
commit 7ea7b0d
Show file tree

Hide file tree

Showing 7 changed files with 23 additions and 38 deletions.
diff --git a/lib/OpenQA/Schema/Result/Jobs.pm b/lib/OpenQA/Schema/Result/Jobs.pm
@@ -26,7 +26,6 @@ use db_helpers;
 use OpenQA::Utils qw/log_debug log_warning parse_assets_from_settings/;
 use File::Basename qw/basename dirname/;
 use File::Path ();
-use File::Which qw(which);
 use DBIx::Class::Timestamps qw/now/;
 
 # The state and results constants are duplicated in the Python client:
@@ -885,14 +884,6 @@ sub running_modinfo() {
     };
 }
 
-sub optipng {
-    my ($app, $path) = @_;
-    if (which('optipng')) {
-        log_debug("optipng $path");
-        system('optipng', '-quiet', '-preserve', '-o2', $path);
-    }
-}
-
 sub store_image {
     my ($self, $asset, $md5, $thumb) = @_;
 
@@ -901,9 +892,6 @@ sub store_image {
     my $prefixdir = dirname($storepath);
     mkdir($prefixdir) unless (-d $prefixdir);
     $asset->move_to($storepath);
-
-    $OpenQA::Utils::app->gru->enqueue(optipng => $storepath);
-
     log_debug("store_image: $storepath");
 }
 

diff --git a/lib/OpenQA/WebAPI.pm b/lib/OpenQA/WebAPI.pm
@@ -491,7 +491,6 @@ sub startup {
     ## JSON API ends here
     #
 
-    $self->gru->add_task(optipng        => \&OpenQA::Schema::Result::Jobs::optipng);
     $self->gru->add_task(reduce_result  => \&OpenQA::Schema::Result::Jobs::reduce_result);
     $self->gru->add_task(limit_assets   => \&OpenQA::Schema::Result::Assets::limit_assets);
     $self->gru->add_task(download_asset => \&OpenQA::Schema::Result::Assets::download_asset);

diff --git a/lib/OpenQA/WebAPI/Controller/Step.pm b/lib/OpenQA/WebAPI/Controller/Step.pm
@@ -500,9 +500,6 @@ sub save_needle_ajax {
         }
     }
     if ($success) {
-        if (which('optipng')) {
-            system("optipng", "-quiet", "$baseneedle.png");
-        }
         open(my $J, ">", "$baseneedle.json") or $success = 0;
         if ($success) {
             print $J $json;

diff --git a/lib/OpenQA/Worker/Jobs.pm b/lib/OpenQA/Worker/Jobs.pm
@@ -29,6 +29,7 @@ use JSON qw/decode_json/;
 use Fcntl;
 use MIME::Base64;
 use File::Basename qw/basename/;
+use File::Which qw(which);
 
 use POSIX ':sys_wait_h';
 
@@ -511,6 +512,18 @@ sub upload_status(;$) {
     }
 }
 
+sub optimize_image {
+    my ($image) = @_;
+
+    if (which('optipng')) {
+        print("optipng $image\n") if $verbose;
+        # be careful not to be too eager optimizing, this needs to be quick
+        # or we will be considered a dead worker
+        system('optipng', '-quiet', '-o2', $image);
+    }
+    return;
+}
+
 sub upload_images {
     my ($known_images) = @_;
 
@@ -520,12 +533,14 @@ sub upload_images {
     my $ua_url = $OpenQA::Worker::Common::url->clone;
     $ua_url->path("jobs/" . $job->{id} . "/artefact");
 
+    my $fileprefix = "$pooldir/testresults";
     while (my ($md5, $file) = each %$tosend_images) {
         print "upload $file as $md5\n" if ($verbose);
 
+        optimize_image("$fileprefix/$file");
         my $form = {
             file => {
-                file     => "$pooldir/testresults/$file",
+                file     => "$fileprefix/$file",
                 filename => $file
             },
             image => 1,
@@ -535,8 +550,11 @@ sub upload_images {
         # don't use api_call as it retries and does not allow form data
         # (refactor at some point)
         $OpenQA::Worker::Common::ua->post($ua_url => form => $form);
-        if (-f "$pooldir/testresults/.thumbs/$file") {
-            $form->{file}->{file} = "$pooldir/testresults/.thumbs/$file";
+
+        $file = "$fileprefix/.thumbs/$file";
+        if (-f $file) {
+            optimize_image($file);
+            $form->{file}->{file} = $file;
             $form->{thumb} = 1;
             $OpenQA::Worker::Common::ua->post($ua_url => form => $form);
         }

diff --git a/profiles/apparmor.d/usr.share.openqa.script.openqa b/profiles/apparmor.d/usr.share.openqa.script.openqa
@@ -20,7 +20,6 @@
   /etc/openqa/openqa.ini r,
   /proc/meminfo r,
   /tmp/** rw,
-  /usr/bin/optipng rix,
   /usr/bin/perl ix,
   /usr/bin/ssh rcx,
   /usr/lib/git/git rix,

diff --git a/profiles/apparmor.d/usr.share.openqa.script.worker b/profiles/apparmor.d/usr.share.openqa.script.worker
@@ -68,6 +68,7 @@
   /usr/bin/snd2png rix,
   /usr/bin/nice rix,
   /usr/bin/ionice rix,
+  /usr/bin/optipng rix,
   /usr/bin/qemu-img rix,
   /usr/bin/qemu-kvm rix,
   /usr/bin/qemu-system-* rix,

diff --git a/t/14-grutasks.t b/t/14-grutasks.t
@@ -74,11 +74,8 @@ my $schema = OpenQA::Test::Database->new->create();
 
 my $t = Test::Mojo->new('OpenQA::WebAPI');
 
-my $file = 't/data/7da661d0c3faf37d49d33b6fc308f2.png';
-copy("t/images/34/.thumbs/7da661d0c3faf37d49d33b6fc308f2.png", $file);
-is((stat($file))[7], 287, 'original file size');
-$t->app->gru->enqueue(optipng => $file);
 
+# now to something completely different: testing limit_assets
 my $c = OpenQA::WebAPI::Plugin::Gru::Command::gru->new();
 $c->app($t->app);
 
@@ -89,20 +86,6 @@ sub run_gru {
     $c->run('run', '-o');
 }
 
-open(FD, ">", \my $output);
-select FD;
-$c->run('list');
-close(FD);
-select STDOUT;
-like($output, qr,optipng .*'$file';,, 'optipng queued');
-
-$c->run('run', '-o');
-if (which('optipng')) {
-    is((stat($file))[7], 286, 'optimized file size');
-}
-
-# now to something completely different: testing limit_assets
-
 # default asset size limit is 100GiB. In our fixtures, we wind up with
 # four JobsAssets, but one is the only one in its JobGroup and so will
 # always be set to 'keep', so effectively we have three that may get