From c30f084f328092770f932fbaf68a1a8a8f69d530 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Mon, 7 Feb 2022 16:12:23 -0500 Subject: [PATCH 1/5] eval_started event: change interface to traceID\tjobsetID I was not going to break the interface until I noticed the current implementation uses the string literal \t. --- doc/manual/src/notifications.md | 2 +- src/lib/Hydra/Event.pm | 2 + src/lib/Hydra/Event/EvalStarted.pm | 53 ++++++++++++ src/lib/Hydra/Plugin.pm | 6 ++ src/script/hydra-eval-jobset | 2 +- src/script/hydra-notify | 1 + t/Hydra/Event/EvalStarted.t | 94 +++++++++++++++++++++ t/scripts/hydra-eval-jobset/notifications.t | 11 ++- 8 files changed, 168 insertions(+), 3 deletions(-) create mode 100644 src/lib/Hydra/Event/EvalStarted.pm create mode 100644 t/Hydra/Event/EvalStarted.t diff --git a/doc/manual/src/notifications.md b/doc/manual/src/notifications.md index 0c666559..00436507 100644 --- a/doc/manual/src/notifications.md +++ b/doc/manual/src/notifications.md @@ -57,7 +57,7 @@ It is possible for subsequent deliveries of the same `build_finished` data to im ### `eval_started` -* **Payload:** Exactly three values, separated by the two-character string `\t` (ie: not a tab): an opaque trace ID representing this evaluation, the name of the project, and the name of the jobset. +* **Payload:** Exactly two values, tab separated: an opaque trace ID representing this evaluation, and the ID of the jobset. * **When:** At the beginning of the evaluation phase for the jobset, before any work is done. * **Delivery Semantics:** Ephemeral. `hydra-notify` must be running to react to this event. No record of this event is stored. diff --git a/src/lib/Hydra/Event.pm b/src/lib/Hydra/Event.pm index 86cf9055..b75e06be 100644 --- a/src/lib/Hydra/Event.pm +++ b/src/lib/Hydra/Event.pm @@ -7,6 +7,7 @@ use Hydra::Event::BuildQueued; use Hydra::Event::BuildStarted; use Hydra::Event::CachedBuildFinished; use Hydra::Event::CachedBuildQueued; +use Hydra::Event::EvalStarted; use Hydra::Event::StepFinished; my %channels_to_events = ( @@ -15,6 +16,7 @@ my %channels_to_events = ( build_started => \&Hydra::Event::BuildStarted::parse, cached_build_finished => \&Hydra::Event::CachedBuildFinished::parse, cached_build_queued => \&Hydra::Event::CachedBuildQueued::parse, + eval_started => \&Hydra::Event::EvalStarted::parse, step_finished => \&Hydra::Event::StepFinished::parse, ); diff --git a/src/lib/Hydra/Event/EvalStarted.pm b/src/lib/Hydra/Event/EvalStarted.pm new file mode 100644 index 00000000..cd7015d9 --- /dev/null +++ b/src/lib/Hydra/Event/EvalStarted.pm @@ -0,0 +1,53 @@ +package Hydra::Event::EvalStarted; + +use strict; +use warnings; + +sub parse :prototype(@) { + unless (@_ == 2) { + die "eval_started: payload takes two arguments, but ", scalar(@_), " were given"; + } + + my ($trace_id, $jobset_id) = @_; + + unless ($jobset_id =~ /^\d+$/) { + die "eval_started: payload argument should be an integer, but '", $jobset_id, "' was given" + } + + return Hydra::Event::EvalStarted->new($trace_id, int($jobset_id)); +} + +sub new { + my ($self, $trace_id, $jobset_id) = @_; + return bless { + "trace_id" => $trace_id, + "jobset_id" => $jobset_id, + "jobset" => undef + }, $self; +} + +sub interestedIn { + my ($self, $plugin) = @_; + return int(defined($plugin->can('evalStarted'))); +} + +sub load { + my ($self, $db) = @_; + + if (!defined($self->{"jobset"})) { + $self->{"jobset"} = $db->resultset('Jobsets')->find({ id => $self->{"jobset_id"}}) + or die "Jobset $self->{'jobset_id'} does not exist\n"; + } +} + +sub execute { + my ($self, $db, $plugin) = @_; + + $self->load($db); + + $plugin->evalStarted($self->{"trace_id"}, $self->{"jobset"}); + + return 1; +} + +1; diff --git a/src/lib/Hydra/Plugin.pm b/src/lib/Hydra/Plugin.pm index b42e4b4c..a5c84b27 100644 --- a/src/lib/Hydra/Plugin.pm +++ b/src/lib/Hydra/Plugin.pm @@ -31,6 +31,12 @@ sub instantiate { # See the tests in t/Event/*.t for arguments, and the documentation for # notify events for semantics. # + +# # Called when an evaluation of $jobset has begun. +# sub evalStarted { +# my ($self, $traceID, $jobset) = @_; +# } + # # Called when build $build has been queued. # sub buildQueued { # my ($self, $build) = @_; diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset index de437ecd..cec7b4c2 100755 --- a/src/script/hydra-eval-jobset +++ b/src/script/hydra-eval-jobset @@ -861,7 +861,7 @@ sub checkJobset { my $tmpId = "${startTime}.$$"; $db->storage->dbh->do("notify eval_started, ?", undef, - join('\t', $tmpId, $jobset->get_column('project'), $jobset->name)); + join("\t", $tmpId, $jobset->get_column('id'))); eval { checkJobsetWrapped($jobset, $tmpId); diff --git a/src/script/hydra-notify b/src/script/hydra-notify index 04130f4f..48717790 100755 --- a/src/script/hydra-notify +++ b/src/script/hydra-notify @@ -98,6 +98,7 @@ $listener->subscribe("build_queued"); $listener->subscribe("build_started"); $listener->subscribe("cached_build_finished"); $listener->subscribe("cached_build_queued"); +$listener->subscribe("eval_started"); $listener->subscribe("hydra_notify_dump_metrics"); $listener->subscribe("step_finished"); diff --git a/t/Hydra/Event/EvalStarted.t b/t/Hydra/Event/EvalStarted.t new file mode 100644 index 00000000..9a8d70e4 --- /dev/null +++ b/t/Hydra/Event/EvalStarted.t @@ -0,0 +1,94 @@ +use strict; +use warnings; +use Setup; +use Hydra::Event; +use Hydra::Event::EvalStarted; +use Test2::V0; +use Test2::Tools::Exception; +use Test2::Tools::Mock qw(mock_obj); + +my $ctx = test_context(); + +my $builds = $ctx->makeAndEvaluateJobset( + expression => "basic.nix", + build => 1 +); + +subtest "Parsing eval_started" => sub { + like( + dies { Hydra::Event::parse_payload("eval_started", "") }, + qr/two arguments/, + "empty payload" + ); + like( + dies { Hydra::Event::parse_payload("eval_started", "abc123") }, + qr/two arguments/, + "one argument" + ); + like( + dies { Hydra::Event::parse_payload("eval_started", "abc123\tabc123\tabc123") }, + qr/two arguments/, + "three arguments" + ); + like( + dies { Hydra::Event::parse_payload("eval_started", "abc123\tabc123") }, + qr/should be an integer/, + "not an integer: second argument" + ); + is( + Hydra::Event::parse_payload("eval_started", "abc123\t456"), + Hydra::Event::EvalStarted->new("abc123", 456) + ); +}; + +subtest "interested" => sub { + my $event = Hydra::Event::EvalStarted->new(123, []); + + subtest "A plugin which does not implement the API" => sub { + my $plugin = {}; + my $mock = mock_obj $plugin => (); + + is($event->interestedIn($plugin), 0, "The plugin is not interesting."); + }; + + subtest "A plugin which does implement the API" => sub { + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "evalStarted" => sub {} + ] + ); + + is($event->interestedIn($plugin), 1, "The plugin is interesting."); + }; +}; + +subtest "load" => sub { + my $jobset = $builds->{"empty_dir"}->jobset; + + my $event = Hydra::Event::EvalStarted->new("traceID", $jobset->id); + + $event->load($ctx->db()); + is($event->{"jobset"}->get_column("id"), $jobset->id, "The jobset record matches."); + + # Create a fake "plugin" with a evalStarted sub, the sub sets this + # "global" passedTraceID, passedJobset + my $passedTraceID; + my $passedJobset; + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "evalStarted" => sub { + my ($self, $traceID, $jobset) = @_; + $passedTraceID = $traceID; + $passedJobset = $jobset; + } + ] + ); + + $event->execute($ctx->db(), $plugin); + is($passedTraceID, "traceID", "The plugin is told what the trace ID was"); + is($passedJobset->get_column("id"), $jobset->id, "The plugin's evalStarted hook is called with the right jobset"); +}; + +done_testing; diff --git a/t/scripts/hydra-eval-jobset/notifications.t b/t/scripts/hydra-eval-jobset/notifications.t index 8fe5d0f4..3e914bb0 100644 --- a/t/scripts/hydra-eval-jobset/notifications.t +++ b/t/scripts/hydra-eval-jobset/notifications.t @@ -36,9 +36,18 @@ my $builds = $ctx->makeAndEvaluateJobset( jobsdir => $jobsetdir, build => 0 ); +my $jobset = $builds->{"stable-job-queued"}->jobset; + +my $traceID; subtest "on the initial evaluation" => sub { - is($listener->block_for_messages(0)->()->{"channel"}, "eval_started", "every eval starts with a notification"); + my $startedMsg = $listener->block_for_messages(0)->(); + is($startedMsg->{"channel"}, "eval_started", "every eval starts with a notification"); + + my ($traceID, $jobsetID) = split("\t", $startedMsg->{"payload"}); + isnt($traceID, "", "we got a trace id"); + is($jobsetID, $jobset->get_column('id'), "the jobset ID matches"); + is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 1/4 builds being queued"); is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 2/4 builds being queued"); is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 3/4 builds being queued"); From be531c6c5719c0ac4c130bd40eaf4c3af93e71ee Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Mon, 7 Feb 2022 14:59:19 -0500 Subject: [PATCH 2/5] notifications.t: create a helper to parse the actual messages from the evaluator using the Event code --- t/scripts/hydra-eval-jobset/notifications.t | 30 +++++++++++++++------ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/t/scripts/hydra-eval-jobset/notifications.t b/t/scripts/hydra-eval-jobset/notifications.t index 3e914bb0..77cec71b 100644 --- a/t/scripts/hydra-eval-jobset/notifications.t +++ b/t/scripts/hydra-eval-jobset/notifications.t @@ -5,6 +5,24 @@ use Setup; use Test2::V0; use File::Copy; use Hydra::PostgresListener; +use Hydra::Event; + +# expectEvent(Hydra::PostgresLister, name of the channel to expect, a sub which gets the parsed event) +sub expectEvent { + my ($listener, $expectedChannel, $then) = @_; + my $message = $listener->block_for_messages(0)->(); + + my $channel = $message->{"channel"}; + + if ($channel eq $expectedChannel) { + my $event = Hydra::Event->new_event($message->{"channel"}, $message->{"payload"}); + local $_ = $event->{event}; + $then->(); + } else { + is($expectedChannel, $channel, "Expecting a message on channel $channel"); + } +} + my $ctx = test_context( hydra_config => q| @@ -38,15 +56,11 @@ my $builds = $ctx->makeAndEvaluateJobset( ); my $jobset = $builds->{"stable-job-queued"}->jobset; -my $traceID; - subtest "on the initial evaluation" => sub { - my $startedMsg = $listener->block_for_messages(0)->(); - is($startedMsg->{"channel"}, "eval_started", "every eval starts with a notification"); - - my ($traceID, $jobsetID) = split("\t", $startedMsg->{"payload"}); - isnt($traceID, "", "we got a trace id"); - is($jobsetID, $jobset->get_column('id'), "the jobset ID matches"); + expectEvent($listener, "eval_started", sub { + isnt($_->{"trace_id"}, "", "We got a trace ID"); + is($_->{"jobset_id"}, $jobset->get_column('id'), "the jobset ID matches"); + }); is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 1/4 builds being queued"); is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 2/4 builds being queued"); From 2597fa8c110ec64343bb864523880a93dbc701d9 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Mon, 7 Feb 2022 16:13:26 -0500 Subject: [PATCH 3/5] eval_cached event: change interface to traceID\tjobsetID\tevaluationID I was not going to break the interface until I noticed the current implementation uses the string literal \t. --- doc/manual/src/notifications.md | 2 +- src/lib/Hydra/Event.pm | 2 + src/lib/Hydra/Event/EvalCached.pm | 63 +++++++++++ src/lib/Hydra/Plugin.pm | 5 + src/script/hydra-eval-jobset | 6 +- src/script/hydra-notify | 2 + t/Hydra/Event/EvalCached.t | 112 ++++++++++++++++++++ t/scripts/hydra-eval-jobset/notifications.t | 19 +++- 8 files changed, 206 insertions(+), 5 deletions(-) create mode 100644 src/lib/Hydra/Event/EvalCached.pm create mode 100644 t/Hydra/Event/EvalCached.t diff --git a/doc/manual/src/notifications.md b/doc/manual/src/notifications.md index 00436507..be8e0c94 100644 --- a/doc/manual/src/notifications.md +++ b/doc/manual/src/notifications.md @@ -69,7 +69,7 @@ It is possible for subsequent deliveries of the same `build_finished` data to im ### `eval_cached` -* **Payload:** Exactly one value: an opaque trace ID representing this evaluation. +* **Payload:** Exactly three values: an opaque trace ID representing this evaluation, the ID of the jobset, and the ID of the previous identical evaluation. * **When:** After the evaluator fetches inputs, if none of the inputs changed. * **Delivery Semantics:** Ephemeral. `hydra-notify` must be running to react to this event. No record of this event is stored. diff --git a/src/lib/Hydra/Event.pm b/src/lib/Hydra/Event.pm index b75e06be..caaf5a5f 100644 --- a/src/lib/Hydra/Event.pm +++ b/src/lib/Hydra/Event.pm @@ -7,6 +7,7 @@ use Hydra::Event::BuildQueued; use Hydra::Event::BuildStarted; use Hydra::Event::CachedBuildFinished; use Hydra::Event::CachedBuildQueued; +use Hydra::Event::EvalCached; use Hydra::Event::EvalStarted; use Hydra::Event::StepFinished; @@ -16,6 +17,7 @@ my %channels_to_events = ( build_started => \&Hydra::Event::BuildStarted::parse, cached_build_finished => \&Hydra::Event::CachedBuildFinished::parse, cached_build_queued => \&Hydra::Event::CachedBuildQueued::parse, + eval_cached => \&Hydra::Event::EvalCached::parse, eval_started => \&Hydra::Event::EvalStarted::parse, step_finished => \&Hydra::Event::StepFinished::parse, ); diff --git a/src/lib/Hydra/Event/EvalCached.pm b/src/lib/Hydra/Event/EvalCached.pm new file mode 100644 index 00000000..1c47fcb3 --- /dev/null +++ b/src/lib/Hydra/Event/EvalCached.pm @@ -0,0 +1,63 @@ +package Hydra::Event::EvalCached; + +use strict; +use warnings; + +sub parse :prototype(@) { + unless (@_ == 3) { + die "eval_cached: payload takes exactly three arguments, but ", scalar(@_), " were given"; + } + + my ($trace_id, $jobset_id, $evaluation_id) = @_; + + unless ($jobset_id =~ /^\d+$/) { + die "eval_cached: payload argument jobset_id should be an integer, but '", $jobset_id, "' was given" + } + unless ($evaluation_id =~ /^\d+$/) { + die "eval_cached: payload argument evaluation_id should be an integer, but '", $evaluation_id, "' was given" + } + + return Hydra::Event::EvalCached->new($trace_id, int($jobset_id), int($evaluation_id)); +} + +sub new { + my ($self, $trace_id, $jobset_id, $evaluation_id) = @_; + return bless { + "trace_id" => $trace_id, + "jobset_id" => $jobset_id, + "evaluation_id" => $evaluation_id, + "jobset" => undef, + "evaluation" => undef + }, $self; +} + +sub interestedIn { + my ($self, $plugin) = @_; + return int(defined($plugin->can('evalCached'))); +} + +sub load { + my ($self, $db) = @_; + + if (!defined($self->{"jobset"})) { + $self->{"jobset"} = $db->resultset('Jobsets')->find({ id => $self->{"jobset_id"}}) + or die "Jobset $self->{'jobset_id'} does not exist\n"; + } + + if (!defined($self->{"evaluation"})) { + $self->{"evaluation"} = $db->resultset('JobsetEvals')->find({ id => $self->{"evaluation_id"}}) + or die "Jobset $self->{'jobset_id'} does not exist\n"; + } +} + +sub execute { + my ($self, $db, $plugin) = @_; + + $self->load($db); + + $plugin->evalCached($self->{"trace_id"}, $self->{"jobset"}, $self->{"evaluation"}); + + return 1; +} + +1; diff --git a/src/lib/Hydra/Plugin.pm b/src/lib/Hydra/Plugin.pm index a5c84b27..562a0296 100644 --- a/src/lib/Hydra/Plugin.pm +++ b/src/lib/Hydra/Plugin.pm @@ -37,6 +37,11 @@ sub instantiate { # my ($self, $traceID, $jobset) = @_; # } +# # Called when an evaluation of $jobset determined the inputs had not changed. +# sub evalCached { +# my ($self, $traceID, $jobset, $evaluation) = @_; +# } + # # Called when build $build has been queued. # sub buildQueued { # my ($self, $build) = @_; diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset index cec7b4c2..f6594515 100755 --- a/src/script/hydra-eval-jobset +++ b/src/script/hydra-eval-jobset @@ -670,7 +670,11 @@ sub checkJobsetWrapped { Net::Statsd::increment("hydra.evaluator.unchanged_checkouts"); $db->txn_do(sub { $jobset->update({ lastcheckedtime => time, fetcherrormsg => undef }); - $db->storage->dbh->do("notify eval_cached, ?", undef, join('\t', $tmpId)); + $db->storage->dbh->do("notify eval_cached, ?", undef, join("\t", + $tmpId, + $jobset->get_column('id'), + $prevEval->get_column('id')) + ); }); return; } diff --git a/src/script/hydra-notify b/src/script/hydra-notify index 48717790..031e2460 100755 --- a/src/script/hydra-notify +++ b/src/script/hydra-notify @@ -98,10 +98,12 @@ $listener->subscribe("build_queued"); $listener->subscribe("build_started"); $listener->subscribe("cached_build_finished"); $listener->subscribe("cached_build_queued"); +$listener->subscribe("eval_cached"); $listener->subscribe("eval_started"); $listener->subscribe("hydra_notify_dump_metrics"); $listener->subscribe("step_finished"); + # Process builds that finished while hydra-notify wasn't running. for my $build ($db->resultset('Builds')->search( { notificationpendingsince => { '!=', undef } })) diff --git a/t/Hydra/Event/EvalCached.t b/t/Hydra/Event/EvalCached.t new file mode 100644 index 00000000..a6af091b --- /dev/null +++ b/t/Hydra/Event/EvalCached.t @@ -0,0 +1,112 @@ +use strict; +use warnings; +use Setup; +use Hydra::Event; +use Hydra::Event::EvalCached; +use Test2::V0; +use Test2::Tools::Exception; +use Test2::Tools::Mock qw(mock_obj); + + +my $ctx = test_context(); +my $builds = $ctx->makeAndEvaluateJobset( + expression => "basic.nix", + build => 1 +); + + +subtest "Parsing eval_cached" => sub { + like( + dies { Hydra::Event::parse_payload("eval_cached", "") }, + qr/three arguments/, + "empty payload" + ); + like( + dies { Hydra::Event::parse_payload("eval_cached", "abc123") }, + qr/three arguments/, + "one argument" + ); + like( + dies { Hydra::Event::parse_payload("eval_cached", "abc123\tabc123") }, + qr/three arguments/, + "two arguments" + ); + like( + dies { Hydra::Event::parse_payload("eval_cached", "abc123\tabc123\tabc123\tabc123") }, + qr/three arguments/, + "four arguments" + ); + like( + dies { Hydra::Event::parse_payload("eval_cached", "abc123\tabc123\t123") }, + qr/should be an integer/, + "not an integer: second position" + ); + like( + dies { Hydra::Event::parse_payload("eval_cached", "abc123\t123\tabc123") }, + qr/should be an integer/, + "not an integer: third position" + ); + is( + Hydra::Event::parse_payload("eval_cached", "abc123\t123\t456"), + Hydra::Event::EvalCached->new("abc123", 123, 456) + ); +}; + +subtest "interested" => sub { + my $event = Hydra::Event::EvalCached->new("abc123", 123, 456); + + subtest "A plugin which does not implement the API" => sub { + my $plugin = {}; + my $mock = mock_obj $plugin => (); + + is($event->interestedIn($plugin), 0, "The plugin is not interesting."); + }; + + subtest "A plugin which does implement the API" => sub { + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "evalCached" => sub {} + ] + ); + + is($event->interestedIn($plugin), 1, "The plugin is interesting."); + }; +}; + +subtest "load" => sub { + my $jobset = $builds->{"empty_dir"}->jobset; + my $evaluation = $builds->{"empty_dir"}->jobsetevals->first(); + + my $event = Hydra::Event::EvalCached->new("traceID", $jobset->id, $evaluation->id); + + $event->load($ctx->db()); + is($event->{"trace_id"}, "traceID", "The Trace ID matches"); + is($event->{"jobset_id"}, $jobset->id, "The Jobset ID matches"); + is($event->{"evaluation_id"}, $evaluation->id, "The Evaluation ID matches"); + + + # Create a fake "plugin" with a evalCached sub, the sub sets these + # "globals" + my $passedTraceID; + my $passedJobset; + my $passedEvaluation; + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "evalCached" => sub { + my ($self, $traceID, $jobset, $evaluation) = @_; + $passedTraceID = $traceID; + $passedJobset = $jobset; + $passedEvaluation = $evaluation; + } + ] + ); + + $event->execute($ctx->db(), $plugin); + is($passedTraceID, "traceID", "We get the expected trace ID"); + is($passedJobset->id, $jobset->id, "The correct jobset is passed"); + is($passedEvaluation->id, $evaluation->id, "The correct evaluation is passed"); +}; + +done_testing; diff --git a/t/scripts/hydra-eval-jobset/notifications.t b/t/scripts/hydra-eval-jobset/notifications.t index 77cec71b..6fdd64fc 100644 --- a/t/scripts/hydra-eval-jobset/notifications.t +++ b/t/scripts/hydra-eval-jobset/notifications.t @@ -55,6 +55,7 @@ my $builds = $ctx->makeAndEvaluateJobset( build => 0 ); my $jobset = $builds->{"stable-job-queued"}->jobset; +my $evaluation = $builds->{"stable-job-queued"}->jobsetevals->first(); subtest "on the initial evaluation" => sub { expectEvent($listener, "eval_started", sub { @@ -72,9 +73,21 @@ subtest "on the initial evaluation" => sub { }; subtest "on a subsequent, totally cached / unchanged evaluation" => sub { - ok(evalSucceeds($builds->{"variable-job"}->jobset), "evaluating for the second time"); - is($listener->block_for_messages(0)->()->{"channel"}, "eval_started", "an evaluation has started"); - is($listener->block_for_messages(0)->()->{"channel"}, "eval_cached", "the evaluation finished and nothing changed"); + ok(evalSucceeds($jobset), "evaluating for the second time"); + + my $traceID; + expectEvent($listener, "eval_started", sub { + isnt($_->{"trace_id"}, "", "We got a trace ID"); + $traceID = $_->{"trace_id"}; + is($_->{"jobset_id"}, $jobset->get_column('id'), "the jobset ID matches"); + }); + + expectEvent($listener, "eval_cached", sub { + is($_->{"trace_id"}, $traceID, "Trace ID matches"); + is($_->{"jobset_id"}, $jobset->get_column('id'), "the jobset ID matches"); + is($_->{"evaluation_id"}, $evaluation->get_column('id'), "the evaluation ID matches"); + }); + is($listener->block_for_messages(0)->(), undef, "there are no more messages from the evaluator"); }; From d512e6220f1790fae30b3cc72e0cb1269cac1c18 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Mon, 7 Feb 2022 16:14:18 -0500 Subject: [PATCH 4/5] eval_failed event: change interface to traceID\tjobsetID I was not going to break the interface until I noticed the other eval_* events used literal \ts --- doc/manual/src/notifications.md | 2 +- src/lib/Hydra/Event.pm | 2 + src/lib/Hydra/Event/EvalFailed.pm | 53 ++++++++++++ src/lib/Hydra/Plugin.pm | 5 ++ src/script/hydra-eval-jobset | 4 +- src/script/hydra-notify | 1 + t/Hydra/Event/EvalFailed.t | 94 +++++++++++++++++++++ t/scripts/hydra-eval-jobset/notifications.t | 15 +++- 8 files changed, 171 insertions(+), 5 deletions(-) create mode 100644 src/lib/Hydra/Event/EvalFailed.pm create mode 100644 t/Hydra/Event/EvalFailed.t diff --git a/doc/manual/src/notifications.md b/doc/manual/src/notifications.md index be8e0c94..a2569172 100644 --- a/doc/manual/src/notifications.md +++ b/doc/manual/src/notifications.md @@ -75,7 +75,7 @@ It is possible for subsequent deliveries of the same `build_finished` data to im ### `eval_failed` -* **Payload:** Exactly one value: an opaque trace ID representing this evaluation. +* **Payload:** Exactly two values: an opaque trace ID representing this evaluation, and the ID of the jobset. * **When:** After any fetching any input fails, or any other evaluation error occurs. * **Delivery Semantics:** Ephemeral. `hydra-notify` must be running to react to this event. No record of this event is stored. diff --git a/src/lib/Hydra/Event.pm b/src/lib/Hydra/Event.pm index caaf5a5f..0d50235e 100644 --- a/src/lib/Hydra/Event.pm +++ b/src/lib/Hydra/Event.pm @@ -8,6 +8,7 @@ use Hydra::Event::BuildStarted; use Hydra::Event::CachedBuildFinished; use Hydra::Event::CachedBuildQueued; use Hydra::Event::EvalCached; +use Hydra::Event::EvalFailed; use Hydra::Event::EvalStarted; use Hydra::Event::StepFinished; @@ -18,6 +19,7 @@ my %channels_to_events = ( cached_build_finished => \&Hydra::Event::CachedBuildFinished::parse, cached_build_queued => \&Hydra::Event::CachedBuildQueued::parse, eval_cached => \&Hydra::Event::EvalCached::parse, + eval_failed => \&Hydra::Event::EvalFailed::parse, eval_started => \&Hydra::Event::EvalStarted::parse, step_finished => \&Hydra::Event::StepFinished::parse, ); diff --git a/src/lib/Hydra/Event/EvalFailed.pm b/src/lib/Hydra/Event/EvalFailed.pm new file mode 100644 index 00000000..7c1be356 --- /dev/null +++ b/src/lib/Hydra/Event/EvalFailed.pm @@ -0,0 +1,53 @@ +package Hydra::Event::EvalFailed; + +use strict; +use warnings; + +sub parse :prototype(@) { + unless (@_ == 2) { + die "eval_failed: payload takes two arguments, but ", scalar(@_), " were given"; + } + + my ($trace_id, $jobset_id) = @_; + + unless ($jobset_id =~ /^\d+$/) { + die "eval_failed: payload argument should be an integer, but '", $jobset_id, "' was given" + } + + return Hydra::Event::EvalFailed->new($trace_id, int($jobset_id)); +} + +sub new { + my ($self, $trace_id, $jobset_id) = @_; + return bless { + "trace_id" => $trace_id, + "jobset_id" => $jobset_id, + "jobset" => undef + }, $self; +} + +sub interestedIn { + my ($self, $plugin) = @_; + return int(defined($plugin->can('evalFailed'))); +} + +sub load { + my ($self, $db) = @_; + + if (!defined($self->{"jobset"})) { + $self->{"jobset"} = $db->resultset('Jobsets')->find({ id => $self->{"jobset_id"}}) + or die "Jobset $self->{'jobset_id'} does not exist\n"; + } +} + +sub execute { + my ($self, $db, $plugin) = @_; + + $self->load($db); + + $plugin->evalFailed($self->{"trace_id"}, $self->{"jobset"}); + + return 1; +} + +1; diff --git a/src/lib/Hydra/Plugin.pm b/src/lib/Hydra/Plugin.pm index 562a0296..ead65b9f 100644 --- a/src/lib/Hydra/Plugin.pm +++ b/src/lib/Hydra/Plugin.pm @@ -42,6 +42,11 @@ sub instantiate { # my ($self, $traceID, $jobset, $evaluation) = @_; # } +# # Called when an evaluation of $jobset failed. +# sub evalFailed { +# my ($self, $traceID, $jobset) = @_; +# } + # # Called when build $build has been queued. # sub buildQueued { # my ($self, $build) = @_; diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset index f6594515..12ccfd5d 100755 --- a/src/script/hydra-eval-jobset +++ b/src/script/hydra-eval-jobset @@ -654,7 +654,7 @@ sub checkJobsetWrapped { print STDERR $fetchError; $db->txn_do(sub { $jobset->update({ lastcheckedtime => time, fetcherrormsg => $fetchError }) if !$dryRun; - $db->storage->dbh->do("notify eval_failed, ?", undef, join('\t', $tmpId)); + $db->storage->dbh->do("notify eval_failed, ?", undef, join("\t", $tmpId, $jobset->get_column('id'))); }); return; } @@ -882,7 +882,7 @@ sub checkJobset { $db->txn_do(sub { $jobset->update({lastcheckedtime => $eventTime}); setJobsetError($jobset, $checkError, $eventTime); - $db->storage->dbh->do("notify eval_failed, ?", undef, join('\t', $tmpId)); + $db->storage->dbh->do("notify eval_failed, ?", undef, join("\t", $tmpId, $jobset->get_column('id'))); }) if !$dryRun; $failed = 1; } diff --git a/src/script/hydra-notify b/src/script/hydra-notify index 031e2460..40f87ffd 100755 --- a/src/script/hydra-notify +++ b/src/script/hydra-notify @@ -99,6 +99,7 @@ $listener->subscribe("build_started"); $listener->subscribe("cached_build_finished"); $listener->subscribe("cached_build_queued"); $listener->subscribe("eval_cached"); +$listener->subscribe("eval_failed"); $listener->subscribe("eval_started"); $listener->subscribe("hydra_notify_dump_metrics"); $listener->subscribe("step_finished"); diff --git a/t/Hydra/Event/EvalFailed.t b/t/Hydra/Event/EvalFailed.t new file mode 100644 index 00000000..a48e8e4a --- /dev/null +++ b/t/Hydra/Event/EvalFailed.t @@ -0,0 +1,94 @@ +use strict; +use warnings; +use Setup; +use Hydra::Event; +use Hydra::Event::EvalFailed; +use Test2::V0; +use Test2::Tools::Exception; +use Test2::Tools::Mock qw(mock_obj); + +my $ctx = test_context(); + +my $builds = $ctx->makeAndEvaluateJobset( + expression => "basic.nix", + build => 1 +); + +subtest "Parsing eval_failed" => sub { + like( + dies { Hydra::Event::parse_payload("eval_failed", "") }, + qr/two arguments/, + "empty payload" + ); + like( + dies { Hydra::Event::parse_payload("eval_failed", "abc123") }, + qr/two arguments/, + "one argument" + ); + like( + dies { Hydra::Event::parse_payload("eval_failed", "abc123\tabc123\tabc123") }, + qr/two arguments/, + "three arguments" + ); + like( + dies { Hydra::Event::parse_payload("eval_failed", "abc123\tabc123") }, + qr/should be an integer/, + "not an integer: second argument" + ); + is( + Hydra::Event::parse_payload("eval_failed", "abc123\t456"), + Hydra::Event::EvalFailed->new("abc123", 456) + ); +}; + +subtest "interested" => sub { + my $event = Hydra::Event::EvalFailed->new(123, []); + + subtest "A plugin which does not implement the API" => sub { + my $plugin = {}; + my $mock = mock_obj $plugin => (); + + is($event->interestedIn($plugin), 0, "The plugin is not interesting."); + }; + + subtest "A plugin which does implement the API" => sub { + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "evalFailed" => sub {} + ] + ); + + is($event->interestedIn($plugin), 1, "The plugin is interesting."); + }; +}; + +subtest "load" => sub { + my $jobset = $builds->{"empty_dir"}->jobset; + + my $event = Hydra::Event::EvalFailed->new("traceID", $jobset->id); + + $event->load($ctx->db()); + is($event->{"jobset"}->get_column("id"), $jobset->id, "The jobset record matches."); + + # Create a fake "plugin" with a evalFailed sub, the sub sets this + # "global" passedTraceID, passedJobset + my $passedTraceID; + my $passedJobset; + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "evalFailed" => sub { + my ($self, $traceID, $jobset) = @_; + $passedTraceID = $traceID; + $passedJobset = $jobset; + } + ] + ); + + $event->execute($ctx->db(), $plugin); + is($passedTraceID, "traceID", "The plugin is told what the trace ID was"); + is($passedJobset->get_column("id"), $jobset->id, "The plugin's evalFailed hook is called with the right jobset"); +}; + +done_testing; diff --git a/t/scripts/hydra-eval-jobset/notifications.t b/t/scripts/hydra-eval-jobset/notifications.t index 6fdd64fc..e8c3281f 100644 --- a/t/scripts/hydra-eval-jobset/notifications.t +++ b/t/scripts/hydra-eval-jobset/notifications.t @@ -142,8 +142,19 @@ subtest "on a fresh evaluation with corrupted sources" => sub { close $fh; ok(evalFails($builds->{"variable-job"}->jobset), "evaluating the corrupted job"); - is($listener->block_for_messages(0)->()->{"channel"}, "eval_started", "the evaluation started"); - is($listener->block_for_messages(0)->()->{"channel"}, "eval_failed", "the evaluation failed"); + + my $traceID; + expectEvent($listener, "eval_started", sub { + isnt($_->{"trace_id"}, "", "We got a trace ID"); + $traceID = $_->{"trace_id"}; + is($_->{"jobset_id"}, $jobset->get_column('id'), "the jobset ID matches"); + }); + + expectEvent($listener, "eval_failed", sub { + is($_->{"trace_id"}, $traceID, "Trace ID matches"); + is($_->{"jobset_id"}, $jobset->get_column('id'), "the jobset ID matches"); + }); + is($listener->block_for_messages(0)->(), undef, "there are no more messages from the evaluator"); }; From 517dce285a851efd732affc084c7083aed2e98cd Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Mon, 7 Feb 2022 15:40:17 -0500 Subject: [PATCH 5/5] eval_added event: change interface to traceID\tjobsetID\tevaluationID I was not going to break the interface until I noticed the current implementation uses the string literal \t. --- doc/manual/src/notifications.md | 2 +- src/lib/Hydra/Event.pm | 2 + src/lib/Hydra/Event/EvalAdded.pm | 63 +++++++++++ src/lib/Hydra/Plugin.pm | 5 + src/script/hydra-eval-jobset | 2 +- src/script/hydra-notify | 1 + t/Hydra/Event/EvalAdded.t | 112 ++++++++++++++++++++ t/scripts/hydra-eval-jobset/notifications.t | 11 +- 8 files changed, 194 insertions(+), 4 deletions(-) create mode 100644 src/lib/Hydra/Event/EvalAdded.pm create mode 100644 t/Hydra/Event/EvalAdded.t diff --git a/doc/manual/src/notifications.md b/doc/manual/src/notifications.md index a2569172..f1203af8 100644 --- a/doc/manual/src/notifications.md +++ b/doc/manual/src/notifications.md @@ -63,7 +63,7 @@ It is possible for subsequent deliveries of the same `build_finished` data to im ### `eval_added` -* **Payload:** Exactly two values, separated by the two-character string `\t` (ie: not a tab): an opaque trace ID representing this evaluation, and the ID of the JobsetEval record. +* **Payload:** Exactly three values, tab separated: an opaque trace ID representing this evaluation, the ID of the jobset, and the ID of the JobsetEval record. * **When:** After the evaluator fetches inputs and completes the evaluation successfully. * **Delivery Semantics:** Ephemeral. `hydra-notify` must be running to react to this event. No record of this event is stored. diff --git a/src/lib/Hydra/Event.pm b/src/lib/Hydra/Event.pm index 0d50235e..9faf02e9 100644 --- a/src/lib/Hydra/Event.pm +++ b/src/lib/Hydra/Event.pm @@ -7,6 +7,7 @@ use Hydra::Event::BuildQueued; use Hydra::Event::BuildStarted; use Hydra::Event::CachedBuildFinished; use Hydra::Event::CachedBuildQueued; +use Hydra::Event::EvalAdded; use Hydra::Event::EvalCached; use Hydra::Event::EvalFailed; use Hydra::Event::EvalStarted; @@ -18,6 +19,7 @@ my %channels_to_events = ( build_started => \&Hydra::Event::BuildStarted::parse, cached_build_finished => \&Hydra::Event::CachedBuildFinished::parse, cached_build_queued => \&Hydra::Event::CachedBuildQueued::parse, + eval_added => \&Hydra::Event::EvalAdded::parse, eval_cached => \&Hydra::Event::EvalCached::parse, eval_failed => \&Hydra::Event::EvalFailed::parse, eval_started => \&Hydra::Event::EvalStarted::parse, diff --git a/src/lib/Hydra/Event/EvalAdded.pm b/src/lib/Hydra/Event/EvalAdded.pm new file mode 100644 index 00000000..fb3a4a8f --- /dev/null +++ b/src/lib/Hydra/Event/EvalAdded.pm @@ -0,0 +1,63 @@ +package Hydra::Event::EvalAdded; + +use strict; +use warnings; + +sub parse :prototype(@) { + unless (@_ == 3) { + die "eval_added: payload takes exactly three arguments, but ", scalar(@_), " were given"; + } + + my ($trace_id, $jobset_id, $evaluation_id) = @_; + + unless ($jobset_id =~ /^\d+$/) { + die "eval_added: payload argument jobset_id should be an integer, but '", $jobset_id, "' was given" + } + unless ($evaluation_id =~ /^\d+$/) { + die "eval_added: payload argument evaluation_id should be an integer, but '", $evaluation_id, "' was given" + } + + return Hydra::Event::EvalAdded->new($trace_id, int($jobset_id), int($evaluation_id)); +} + +sub new { + my ($self, $trace_id, $jobset_id, $evaluation_id) = @_; + return bless { + "trace_id" => $trace_id, + "jobset_id" => $jobset_id, + "evaluation_id" => $evaluation_id, + "jobset" => undef, + "evaluation" => undef + }, $self; +} + +sub interestedIn { + my ($self, $plugin) = @_; + return int(defined($plugin->can('evalAdded'))); +} + +sub load { + my ($self, $db) = @_; + + if (!defined($self->{"jobset"})) { + $self->{"jobset"} = $db->resultset('Jobsets')->find({ id => $self->{"jobset_id"}}) + or die "Jobset $self->{'jobset_id'} does not exist\n"; + } + + if (!defined($self->{"evaluation"})) { + $self->{"evaluation"} = $db->resultset('JobsetEvals')->find({ id => $self->{"evaluation_id"}}) + or die "Jobset $self->{'jobset_id'} does not exist\n"; + } +} + +sub execute { + my ($self, $db, $plugin) = @_; + + $self->load($db); + + $plugin->evalAdded($self->{"trace_id"}, $self->{"jobset"}, $self->{"evaluation"}); + + return 1; +} + +1; diff --git a/src/lib/Hydra/Plugin.pm b/src/lib/Hydra/Plugin.pm index ead65b9f..6cd46521 100644 --- a/src/lib/Hydra/Plugin.pm +++ b/src/lib/Hydra/Plugin.pm @@ -47,6 +47,11 @@ sub instantiate { # my ($self, $traceID, $jobset) = @_; # } +# # Called when $evaluation of $jobset has completed successfully. +# sub evalAdded { +# my ($self, $traceID, $jobset, $evaluation) = @_; +# } + # # Called when build $build has been queued. # sub buildQueued { # my ($self, $build) = @_; diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset index 12ccfd5d..a68fe858 100755 --- a/src/script/hydra-eval-jobset +++ b/src/script/hydra-eval-jobset @@ -771,7 +771,7 @@ sub checkJobsetWrapped { }); $db->storage->dbh->do("notify eval_added, ?", undef, - join('\t', $tmpId, $ev->id)); + join("\t", $tmpId, $jobset->get_column('id'), $ev->id)); if ($jobsetChanged) { # Create JobsetEvalMembers mappings. diff --git a/src/script/hydra-notify b/src/script/hydra-notify index 40f87ffd..3b8ffe6d 100755 --- a/src/script/hydra-notify +++ b/src/script/hydra-notify @@ -98,6 +98,7 @@ $listener->subscribe("build_queued"); $listener->subscribe("build_started"); $listener->subscribe("cached_build_finished"); $listener->subscribe("cached_build_queued"); +$listener->subscribe("eval_added"); $listener->subscribe("eval_cached"); $listener->subscribe("eval_failed"); $listener->subscribe("eval_started"); diff --git a/t/Hydra/Event/EvalAdded.t b/t/Hydra/Event/EvalAdded.t new file mode 100644 index 00000000..140fa462 --- /dev/null +++ b/t/Hydra/Event/EvalAdded.t @@ -0,0 +1,112 @@ +use strict; +use warnings; +use Setup; +use Hydra::Event; +use Hydra::Event::EvalAdded; +use Test2::V0; +use Test2::Tools::Exception; +use Test2::Tools::Mock qw(mock_obj); + + +my $ctx = test_context(); +my $builds = $ctx->makeAndEvaluateJobset( + expression => "basic.nix", + build => 1 +); + + +subtest "Parsing eval_added" => sub { + like( + dies { Hydra::Event::parse_payload("eval_added", "") }, + qr/three arguments/, + "empty payload" + ); + like( + dies { Hydra::Event::parse_payload("eval_added", "abc123") }, + qr/three arguments/, + "one argument" + ); + like( + dies { Hydra::Event::parse_payload("eval_added", "abc123\tabc123") }, + qr/three arguments/, + "two arguments" + ); + like( + dies { Hydra::Event::parse_payload("eval_added", "abc123\tabc123\tabc123\tabc123") }, + qr/three arguments/, + "four arguments" + ); + like( + dies { Hydra::Event::parse_payload("eval_added", "abc123\tabc123\t123") }, + qr/should be an integer/, + "not an integer: second position" + ); + like( + dies { Hydra::Event::parse_payload("eval_added", "abc123\t123\tabc123") }, + qr/should be an integer/, + "not an integer: third position" + ); + is( + Hydra::Event::parse_payload("eval_added", "abc123\t123\t456"), + Hydra::Event::EvalAdded->new("abc123", 123, 456) + ); +}; + +subtest "interested" => sub { + my $event = Hydra::Event::EvalAdded->new("abc123", 123, 456); + + subtest "A plugin which does not implement the API" => sub { + my $plugin = {}; + my $mock = mock_obj $plugin => (); + + is($event->interestedIn($plugin), 0, "The plugin is not interesting."); + }; + + subtest "A plugin which does implement the API" => sub { + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "evalAdded" => sub {} + ] + ); + + is($event->interestedIn($plugin), 1, "The plugin is interesting."); + }; +}; + +subtest "load" => sub { + my $jobset = $builds->{"empty_dir"}->jobset; + my $evaluation = $builds->{"empty_dir"}->jobsetevals->first(); + + my $event = Hydra::Event::EvalAdded->new("traceID", $jobset->id, $evaluation->id); + + $event->load($ctx->db()); + is($event->{"trace_id"}, "traceID", "The Trace ID matches"); + is($event->{"jobset_id"}, $jobset->id, "The Jobset ID matches"); + is($event->{"evaluation_id"}, $evaluation->id, "The Evaluation ID matches"); + + + # Create a fake "plugin" with a evalAdded sub, the sub sets these + # "globals" + my $passedTraceID; + my $passedJobset; + my $passedEvaluation; + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "evalAdded" => sub { + my ($self, $traceID, $jobset, $evaluation) = @_; + $passedTraceID = $traceID; + $passedJobset = $jobset; + $passedEvaluation = $evaluation; + } + ] + ); + + $event->execute($ctx->db(), $plugin); + is($passedTraceID, "traceID", "We get the expected trace ID"); + is($passedJobset->id, $jobset->id, "The correct jobset is passed"); + is($passedEvaluation->id, $evaluation->id, "The correct evaluation is passed"); +}; + +done_testing; diff --git a/t/scripts/hydra-eval-jobset/notifications.t b/t/scripts/hydra-eval-jobset/notifications.t index e8c3281f..d2412432 100644 --- a/t/scripts/hydra-eval-jobset/notifications.t +++ b/t/scripts/hydra-eval-jobset/notifications.t @@ -67,13 +67,18 @@ subtest "on the initial evaluation" => sub { is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 2/4 builds being queued"); is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 3/4 builds being queued"); is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 4/4 builds being queued"); - is($listener->block_for_messages(0)->()->{"channel"}, "eval_added", "the evaluation has completed"); + + expectEvent($listener, "eval_added", sub { + is($_->{"jobset_id"}, $jobset->get_column('id'), "the jobset ID matches"); + is($_->{"evaluation_id"}, $evaluation->get_column('id'), "the evaluation ID matches"); + }); is($listener->block_for_messages(0)->()->{"channel"}, "builds_added", "new builds have been scheduled"); is($listener->block_for_messages(0)->(), undef, "there are no more messages from the evaluator"); }; subtest "on a subsequent, totally cached / unchanged evaluation" => sub { ok(evalSucceeds($jobset), "evaluating for the second time"); + my $evaluation = $builds->{"stable-job-queued"}->jobsetevals->first(); my $traceID; expectEvent($listener, "eval_started", sub { @@ -103,7 +108,9 @@ subtest "on a fresh evaluation with changed sources" => sub { $builds->{"stable-job-failing"}->discard_changes(); ok(evalSucceeds($builds->{"variable-job"}->jobset), "evaluating for the third time"); - is($listener->block_for_messages(0)->()->{"channel"}, "eval_started", "the evaluation started"); + expectEvent($listener, "eval_started", sub { + is($_->{"jobset_id"}, $jobset->get_column('id'), "the jobset ID matches"); + }); # The order of builds is randomized when writing to the database, # so we can't expect the list in any specific order here.