From fa6d7abc13d00455109c1d722dc72ff0bca772c3 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Thu, 12 Aug 2021 12:03:25 -0400 Subject: [PATCH] hydra-notify: move BuildFinished processing to an Event --- src/lib/Hydra/Event/BuildFinished.pm | 44 ++++++++++++- src/script/hydra-notify | 57 ++++------------- t/Event.t | 36 ----------- t/Event/BuildFinished.t | 92 ++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 84 deletions(-) create mode 100644 t/Event/BuildFinished.t diff --git a/src/lib/Hydra/Event/BuildFinished.pm b/src/lib/Hydra/Event/BuildFinished.pm index 50ac2054..dc94b5ee 100644 --- a/src/lib/Hydra/Event/BuildFinished.pm +++ b/src/lib/Hydra/Event/BuildFinished.pm @@ -18,8 +18,48 @@ sub parse :prototype(@) { } sub new { - my ($self, $build_id, $dependencies) = @_; - return bless { "build_id" => $build_id, "dependencies" => $dependencies }, $self; + my ($self, $build_id, $dependent_ids) = @_; + return bless { + "build_id" => $build_id, + "dependent_ids" => $dependent_ids, + "build" => undef, + "dependents" => [], + }, $self; +} + +sub load { + my ($self, $db) = @_; + + if (!defined($self->{"build"})) { + $self->{"build"} = $db->resultset('Builds')->find($self->{"build_id"}) + or die "build $self->{'build_id'} does not exist\n"; + + foreach my $id (@{$self->{"dependent_ids"}}) { + my $dep = $db->resultset('Builds')->find($id) + or die "dependent build $id does not exist\n"; + push @{$self->{"dependents"}}, $dep; + } + } +} + +sub execute { + my ($self, $db, $plugin) = @_; + + $self->load($db); + + $plugin->buildFinished($self->{"build"}, $self->{"dependents"}); + + # Mark the build and all dependents as having their notifications "finished". + # + # Otherwise, the dependent builds will remain with notificationpendingsince set + # until hydra-notify is started, as buildFinished is never emitted for them. + foreach my $b ($self->{"build"}, @{$self->{"dependents"}}) { + if ($b->finished && defined($b->notificationpendingsince)) { + $b->update({ notificationpendingsince => undef }) + } + } + + return 1; } 1; diff --git a/src/script/hydra-notify b/src/script/hydra-notify index 7ecb7ddf..2c323b7f 100755 --- a/src/script/hydra-notify +++ b/src/script/hydra-notify @@ -2,10 +2,11 @@ use strict; use utf8; +use Hydra::Event::BuildFinished; +use Hydra::Helper::AddBuilds; +use Hydra::Helper::Nix; use Hydra::Plugin; use Hydra::PostgresListener; -use Hydra::Helper::Nix; -use Hydra::Helper::AddBuilds; use Getopt::Long; STDERR->autoflush(1); @@ -44,44 +45,15 @@ sub runPluginsForEvent { } } -sub buildFinished { - my ($buildId, @deps) = @_; - - my $build = $db->resultset('Builds')->find($buildId) - or die "build $buildId does not exist\n"; - - my @dependents; - foreach my $id (@deps) { - my $dep = $db->resultset('Builds')->find($id) - or die "build $id does not exist\n"; - push @dependents, $dep; - } - - foreach my $plugin (@plugins) { - eval { - $plugin->buildFinished($build, [@dependents]); - 1; - } or do { - print STDERR "error with $plugin->buildFinished: $@\n"; - } - } - - # We have to iterate through all dependents as well, and if they are finished - # to mark their notificationpendingsince. - # Otherwise, the dependent builds will remain with notificationpendingsince set - # until hydra-notify is started, as buildFinished is never emitted for them. - foreach my $b ($build, @dependents) { - $b->update({ notificationpendingsince => undef }) if $b->finished; - } -} - # Process builds that finished while hydra-notify wasn't running. for my $build ($db->resultset('Builds')->search( { notificationpendingsince => { '!=', undef } })) { - my $buildId = $build->id; - print STDERR "sending notifications for build ${\$buildId}...\n"; - buildFinished($build->id); + print STDERR "sending notifications for build $build->id...\n"; + + + my $event = Hydra::Event::BuildFinished->new($build->id); + runPluginsForEvent($event); } @@ -93,18 +65,11 @@ while (!$queued_only) { my $channelName = $message->{"channel"}; my $pid = $message->{"pid"}; my $payload = $message->{"payload"}; - #print STDERR "got '$channelName' from $pid: $payload\n"; - - my @payload = split /\t/, $payload; eval { - if ($channelName eq "build_started" || $channelName eq "step_finished" ) { - my $event = Hydra::Event::new_event($channelName, $message->{"payload"}); - runPluginsForEvent($event); - } elsif ($channelName eq "build_finished") { - my $buildId = int($payload[0]); - buildFinished($buildId, @payload[1..$#payload]); - } + my $event = Hydra::Event::new_event($channelName, $message->{"payload"}); + runPluginsForEvent($event); + 1; } or do { print STDERR "error processing message '$payload' on channel '$channelName': $@\n"; diff --git a/t/Event.t b/t/Event.t index d70c661e..a15b95ea 100644 --- a/t/Event.t +++ b/t/Event.t @@ -1,6 +1,5 @@ use strict; use Hydra::Event; -use Hydra::Event::BuildFinished; use Test2::V0; use Test2::Tools::Exception; @@ -12,41 +11,6 @@ subtest "Event: new event" => sub { is($event->{'event'}->{'build_id'}, 19); }; - -subtest "Payload type: build_finished" => sub { - like( - dies { Hydra::Event::parse_payload("build_finished", "") }, - qr/at least one argument/, - "empty payload" - ); - like( - dies { Hydra::Event::parse_payload("build_finished", "abc123") }, - qr/should be integers/, - "build ID should be an integer" - ); - like( - dies { Hydra::Event::parse_payload("build_finished", "123\tabc123") }, - qr/should be integers/, - "dependent ID should be an integer" - ); - is( - Hydra::Event::parse_payload("build_finished", "123"), - Hydra::Event::BuildFinished->new(123, []), - "no dependent builds" - ); - is( - Hydra::Event::parse_payload("build_finished", "123\t456"), - Hydra::Event::BuildFinished->new(123, [456]), - "one dependent build" - ); - is( - Hydra::Event::parse_payload("build_finished", "123\t456\t789\t012\t345"), - Hydra::Event::BuildFinished->new(123, [456, 789, 12, 345]), - "four dependent builds" - ); -}; - - subtest "Payload type: bogus" => sub { like( dies { Hydra::Event::parse_payload("bogus", "") }, diff --git a/t/Event/BuildFinished.t b/t/Event/BuildFinished.t new file mode 100644 index 00000000..8650f95c --- /dev/null +++ b/t/Event/BuildFinished.t @@ -0,0 +1,92 @@ +use strict; +use Setup; + +my %ctx = test_init(); + +require Hydra::Schema; +require Hydra::Model::DB; +use Hydra::Event; +use Hydra::Event::BuildFinished; + +use Test2::V0; +use Test2::Tools::Exception; +use Test2::Tools::Mock qw(mock_obj); + +my $db = Hydra::Model::DB->new; +hydra_setup($db); + +subtest "Parsing" => sub { + like( + dies { Hydra::Event::parse_payload("build_finished", "") }, + qr/at least one argument/, + "empty payload" + ); + like( + dies { Hydra::Event::parse_payload("build_finished", "abc123") }, + qr/should be integers/, + "build ID should be an integer" + ); + like( + dies { Hydra::Event::parse_payload("build_finished", "123\tabc123") }, + qr/should be integers/, + "dependent ID should be an integer" + ); + is( + Hydra::Event::parse_payload("build_finished", "123"), + Hydra::Event::BuildFinished->new(123, []), + "no dependent builds" + ); + is( + Hydra::Event::parse_payload("build_finished", "123\t456"), + Hydra::Event::BuildFinished->new(123, [456]), + "one dependent build" + ); + is( + Hydra::Event::parse_payload("build_finished", "123\t456\t789\t012\t345"), + Hydra::Event::BuildFinished->new(123, [456, 789, 12, 345]), + "four dependent builds" + ); +}; + +my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"}); +my $jobset = createBaseJobset("basic", "basic.nix", $ctx{jobsdir}); +ok(evalSucceeds($jobset), "Evaluating jobs/basic.nix should exit with return code 0"); +is(nrQueuedBuildsForJobset($jobset), 3, "Evaluating jobs/basic.nix should result in 3 builds"); + +subtest "load" => sub { + my ($build, $dependent_a, $dependent_b) = $db->resultset('Builds')->search( + { }, + { limit => 3 } + )->all; + + my $event = Hydra::Event::BuildFinished->new($build->id, [$dependent_a->id, $dependent_b->id]); + + $event->load($db); + + is($event->{"build"}->id, $build->id, "The build record matches."); + is($event->{"dependents"}[0]->id, $dependent_a->id, "The dependent_a record matches."); + is($event->{"dependents"}[1]->id, $dependent_b->id, "The dependent_b record matches."); + + # Create a fake "plugin" with a buildFinished sub, the sub sets this + # global passedBuild and passedDependents variables for verifying. + my $passedBuild; + my $passedDependents; + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "buildFinished" => sub { + my ($self, $build, $dependents) = @_; + $passedBuild = $build; + $passedDependents = $dependents; + } + ] + ); + + $event->execute($db, $plugin); + + is($passedBuild->id, $build->id, "The plugin's buildFinished hook is called with a matching build"); + is($passedDependents->[0]->id, $dependent_a->id, "The plugin's buildFinished hook is called with a matching dependent_a"); + is($passedDependents->[1]->id, $dependent_b->id, "The plugin's buildFinished hook is called with a matching dependent_b"); +}; + +done_testing;