From f28aa904364f3f990144bac5ab5c3bb9c93de47c Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Tue, 8 Jan 2013 10:55:58 -0700 Subject: [PATCH] Finish writing and testing service scheduling code. --- bin/pt-agent | 31 +++-- t/pt-agent/make_new_crontab.t | 77 +++++++++++-- t/pt-agent/run_agent.t | 32 +++++- t/pt-agent/samples/crontab001.out | 1 - t/pt-agent/samples/crontab002.in | 1 + t/pt-agent/samples/crontab002.out | 2 + t/pt-agent/samples/crontab003.in | 3 + t/pt-agent/samples/crontab003.out | 2 + t/pt-agent/samples/service001 | 2 +- t/pt-agent/schedule_services.t | 182 ++++++++++++++++++++++++++++++ 10 files changed, 310 insertions(+), 23 deletions(-) create mode 100644 t/pt-agent/samples/crontab002.in create mode 100644 t/pt-agent/samples/crontab002.out create mode 100644 t/pt-agent/samples/crontab003.in create mode 100644 t/pt-agent/samples/crontab003.out create mode 100644 t/pt-agent/schedule_services.t diff --git a/bin/pt-agent b/bin/pt-agent index e1cd6ad8..2af48944 100755 --- a/bin/pt-agent +++ b/bin/pt-agent @@ -4421,6 +4421,7 @@ sub main { # TODO: only the main proc needs write access # ######################################################################## my $config_file = get_config_file(); + _info("Config file: $config_file"); if ( -f $config_file ) { die "$config_file is not writable.\n" unless -w $config_file; @@ -4723,6 +4724,7 @@ sub run_agent { ); schedule_services( services => $new_services, + lib_dir => $lib_dir, ); $services = $new_services; } @@ -4838,21 +4840,30 @@ sub schedule_services { have_required_args(\%args, qw( services + lib_dir )) or die; my $services = $args{services}; + my $lib_dir = $args{lib_dir}; _info("Scheduling services"); my $new_crontab = make_new_crontab(%args); - _info("New crontab:\n", $new_crontab); + _info("New crontab:\n" . $new_crontab || ''); - my ($fh, $file) = tempfile(); - print { $fh } $new_crontab; - close $fh; + my $crontab_file = "$lib_dir/crontab"; + open my $fh, '>', $crontab_file + or die "Error opening $crontab_file: $OS_ERROR"; + print { $fh } $new_crontab + or die "Error writing to $crontab_file: $OS_ERROR"; + close $fh + or die "Error closing $crontab_file: $OS_ERROR"; - system("crontab $file"); - - unlink $file; + my $err_file = "$lib_dir/crontab.err"; + system("crontab $crontab_file > $err_file 2>&1"); + if ( $CHILD_ERROR ) { + my $error = `cat $err_file`; + die "Error setting new crontab: $error\n"; + } return; } @@ -4866,7 +4877,8 @@ sub make_new_crontab { my $services = $args{services}; # Optional args - my $crontab_list = $args{crontab_list} || `crontab -l`; + my $crontab_list = defined $args{crontab_list} ? $args{crontab_list} + : `crontab -l 2>/dev/null`; my @other_lines = grep { $_ !~ m/pt-agent --run-service/ } @@ -4878,7 +4890,7 @@ sub make_new_crontab { $service->schedule . " pt-agent --run-service " . $service->name; } - my $new_crontab = join("\n", @other_lines, @pt_agent_lines, "\n"); + my $new_crontab = join("\n", @other_lines, @pt_agent_lines) . "\n"; return $new_crontab; } @@ -4907,7 +4919,6 @@ sub send_data { sub get_config_file { my $home_dir = $ENV{HOME} || $ENV{HOMEPATH} || $ENV{USERPROFILE} || '.'; my $config_file = "$home_dir/.pt-agent.conf"; - _info("Config file: $config_file"); return $config_file; } diff --git a/t/pt-agent/make_new_crontab.t b/t/pt-agent/make_new_crontab.t index 10145ee0..1c031a17 100644 --- a/t/pt-agent/make_new_crontab.t +++ b/t/pt-agent/make_new_crontab.t @@ -11,7 +11,7 @@ use warnings FATAL => 'all'; use English qw(-no_match_vars); use Test::More; use JSON; -use File::Temp qw(tempdir); +use File::Temp qw(tempfile); use Percona::Test; require "$trunk/bin/pt-agent"; @@ -24,14 +24,12 @@ sub test_make_new_crontab { my (%args) = @_; have_required_args(\%args, qw( file - name services )) or die; my $file = $args{file}; - my $name = $args{name}; my $services = $args{services}; - my $crontab_list = slurp_file("$trunk/$sample/$file.in"); + my $crontab_list = slurp_file("$trunk/$sample/$file.in"); my $new_crontab = pt_agent::make_new_crontab( services => $services, @@ -44,14 +42,10 @@ sub test_make_new_crontab { "$sample/$file.out", cmd_output => 1, ), - "$name" + $args{name} || $file, ); } -# ############################################################################# -# Empty crontab, new service. -# ############################################################################# - my $run0 = Percona::WebAPI::Resource::Run->new( number => '0', program => 'pt-query-digest', @@ -66,12 +60,75 @@ my $svc0 = Percona::WebAPI::Resource::Service->new( runs => [ $run0 ], ); +# Empty crontab, add the service. test_make_new_crontab( - name => "crontab001", file => "crontab001", services => [ $svc0 ], ); +# Crontab has another line, add the service to it. +test_make_new_crontab( + file => "crontab002", + services => [ $svc0 ], +); + +# Crontab has another line and an old service, remove the old service +# and add the current service. +test_make_new_crontab( + file => "crontab003", + services => [ $svc0 ], +); + +# ############################################################################# +# Use real crontab. +# ############################################################################# + +# The previous tests pass in a crontab file to make testing easier. +# Now test that make_new_crontab() will run `crontab -l' if not given +# input. To test this, we add a fake line to our crontab. If +# make_new_crontab() really runs `crontab -l', then this fake line +# will be in the new crontab it returns. + +my $crontab = `crontab -l 2>/dev/null`; +SKIP: { + skip 'Crontab is not empty', 3 if $crontab; + + my ($fh, $file) = tempfile(); + print {$fh} "* 0 * * * date > /dev/null"; + close $fh or warn "Cannot close $file: $OS_ERROR"; + my $output = `crontab $file 2>&1`; + + $crontab = `crontab -l 2>&1`; + + is( + $crontab, + "* 0 * * * date > /dev/null", + "Set other crontab line" + ) or diag($output); + + unlink $file or warn "Cannot remove $file: $OS_ERROR"; + + my $new_crontab = pt_agent::make_new_crontab( + services => [ $svc0 ], + ); + + is( + $new_crontab, + "* 0 * * * date > /dev/null +* 8 * * 1,2,3,4,5 pt-agent --run-service query-monitor +", + "Runs crontab -l by default" + ); + + system("crontab -r 2>/dev/null"); + $crontab = `crontab -l 2>/dev/null`; + is( + $crontab, + "", + "Removed crontab" + ); +}; + # ############################################################################# # Done. # ############################################################################# diff --git a/t/pt-agent/run_agent.t b/t/pt-agent/run_agent.t index 1ed36de8..36a5b8d8 100644 --- a/t/pt-agent/run_agent.t +++ b/t/pt-agent/run_agent.t @@ -20,6 +20,15 @@ require "$trunk/bin/pt-agent"; Percona::Toolkit->import(qw(Dumper)); Percona::WebAPI::Representation->import(qw(as_hashref)); +# Running the agent is going to cause it to schedule the services, +# i.e. write a real crontab. The test box/user shouldn't have a +# crontab, so we'll warn and clobber it if there is one. +my $crontab = `crontab -l 2>/dev/null`; +if ( $crontab ) { + warn "Removing crontab: $crontab\n"; + `crontab -r`; +} + # ############################################################################# # Create mock client and Agent # ############################################################################# @@ -136,7 +145,7 @@ my $run0 = Percona::WebAPI::Resource::Run->new( my $svc0 = Percona::WebAPI::Resource::Service->new( name => 'query-monitor', alias => 'Query Monitor', - schedule => '...', + schedule => '* * * * *', runs => [ $run0 ], ); @@ -232,7 +241,16 @@ ok( "query-monitor service file" ); +$crontab = `crontab -l 2>/dev/null`; +like( + $crontab, + qr/pt-agent --run-service query-monitor$/m, + "Scheduled service with crontab" +); + +# ############################################################################# # Run run_agent() again, like the agent had been stopped and restarted. +# ############################################################################# $ua->{responses}->{get} = [ # First check, fail @@ -308,9 +326,21 @@ ok( "No Service diff, no service file changes" ); +my $new_crontab = `crontab -l 2>/dev/null`; +is( + $new_crontab, + $crontab, + "Crontab is the same" +); + # ############################################################################# # Done. # ############################################################################# + +# This shouldn't cause an error, but if it does, let it show up +# in the results as an error. +`crontab -r`; + if ( -f $config_file ) { unlink $config_file or warn "Error removing $config_file: $OS_ERROR"; diff --git a/t/pt-agent/samples/crontab001.out b/t/pt-agent/samples/crontab001.out index 4435ec65..b6747f2c 100644 --- a/t/pt-agent/samples/crontab001.out +++ b/t/pt-agent/samples/crontab001.out @@ -1,2 +1 @@ * 8 * * 1,2,3,4,5 pt-agent --run-service query-monitor - diff --git a/t/pt-agent/samples/crontab002.in b/t/pt-agent/samples/crontab002.in new file mode 100644 index 00000000..072510da --- /dev/null +++ b/t/pt-agent/samples/crontab002.in @@ -0,0 +1 @@ +17 3 * * 1 cmd diff --git a/t/pt-agent/samples/crontab002.out b/t/pt-agent/samples/crontab002.out new file mode 100644 index 00000000..71ff08a9 --- /dev/null +++ b/t/pt-agent/samples/crontab002.out @@ -0,0 +1,2 @@ +17 3 * * 1 cmd +* 8 * * 1,2,3,4,5 pt-agent --run-service query-monitor diff --git a/t/pt-agent/samples/crontab003.in b/t/pt-agent/samples/crontab003.in new file mode 100644 index 00000000..25ac9ae4 --- /dev/null +++ b/t/pt-agent/samples/crontab003.in @@ -0,0 +1,3 @@ +17 3 * * 1 cmd +* * * * 1 pt-agent --run-service old-service + diff --git a/t/pt-agent/samples/crontab003.out b/t/pt-agent/samples/crontab003.out new file mode 100644 index 00000000..71ff08a9 --- /dev/null +++ b/t/pt-agent/samples/crontab003.out @@ -0,0 +1,2 @@ +17 3 * * 1 cmd +* 8 * * 1,2,3,4,5 pt-agent --run-service query-monitor diff --git a/t/pt-agent/samples/service001 b/t/pt-agent/samples/service001 index fc6a6329..69800703 100644 --- a/t/pt-agent/samples/service001 +++ b/t/pt-agent/samples/service001 @@ -1 +1 @@ -{"runs":[{"number":"0","options":"--output json","output":"spool","program":"pt-query-digest"}],"name":"query-monitor","alias":"Query Monitor","schedule":"..."} \ No newline at end of file +{"runs":[{"number":"0","options":"--output json","output":"spool","program":"pt-query-digest"}],"name":"query-monitor","alias":"Query Monitor","schedule":"* * * * *"} \ No newline at end of file diff --git a/t/pt-agent/schedule_services.t b/t/pt-agent/schedule_services.t new file mode 100644 index 00000000..b55a4a52 --- /dev/null +++ b/t/pt-agent/schedule_services.t @@ -0,0 +1,182 @@ +#!/usr/bin/env perl + +BEGIN { + die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n" + unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH}; + unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib"; +}; + +use strict; +use warnings FATAL => 'all'; +use English qw(-no_match_vars); +use Test::More; +use JSON; +use File::Temp qw(tempfile tempdir); + +use Percona::Test; +require "$trunk/bin/pt-agent"; + +my $crontab = `crontab -l 2>/dev/null`; +if ( $crontab ) { + plan skip_all => 'Crontab is not empty'; +} + +Percona::Toolkit->import(qw(have_required_args Dumper)); + +my $sample = "t/pt-agent/samples"; +my $tmpdir = tempdir("/tmp/pt-agent.$PID.XXXXXX", CLEANUP => 1); + +# ############################################################################# +# Schedule a good crontab. +# ############################################################################# + +my $run0 = Percona::WebAPI::Resource::Run->new( + number => '0', + program => 'pt-query-digest', + options => '--output json', + output => 'spool', +); + +my $svc0 = Percona::WebAPI::Resource::Service->new( + name => 'query-monitor', + alias => 'Query Monitor', + schedule => '* 8 * * 1,2,3,4,5', + runs => [ $run0 ], +); + +# First add a fake line so we can know that the real, existing +# crontab is used and not clobbered. +my ($fh, $file) = tempfile(); +print {$fh} "* 0 * * * date > /dev/null"; +close $fh or warn "Cannot close $file: $OS_ERROR"; +my $output = `crontab $file 2>&1`; + +$crontab = `crontab -l 2>&1`; + +is( + $crontab, + "* 0 * * * date > /dev/null", + "Set other crontab line" +) or diag($output); + +unlink $file or warn "Cannot remove $file: $OS_ERROR"; + +eval { + $output = output( + sub { + pt_agent::schedule_services( + services => [ $svc0 ], + lib_dir => $tmpdir, + ) + }, + stderr => 1, + ); +}; + +is( + $EVAL_ERROR, + "", + "No error" +) or diag($output); + +$crontab = `crontab -l 2>/dev/null`; +is( + $crontab, + "* 0 * * * date > /dev/null +* 8 * * 1,2,3,4,5 pt-agent --run-service query-monitor +", + "schedule_services()" +); + +ok( + -f "$tmpdir/crontab", + "Wrote crontab to --lib/crontab" +) or diag(`ls -l $tmpdir`); + +ok( + -f "$tmpdir/crontab.err", + "Write --lib/crontab.err", +) or diag(`ls -l $tmpdir`); + +my $err = -f "$tmpdir/crontab.err" ? `cat $tmpdir/crontab.err` : ''; +is( + $err, + "", + "No crontab error" +); + +system("crontab -r 2>/dev/null"); +$crontab = `crontab -l 2>/dev/null`; +is( + $crontab, + "", + "Removed crontab" +); + +# ############################################################################# +# Handle bad crontab lines. +# ############################################################################# + +$svc0 = Percona::WebAPI::Resource::Service->new( + name => 'query-monitor', + alias => 'Query Monitor', + schedule => '* * * * Foo', # "foo":0: bad day-of-week + runs => [ $run0 ], +); + +eval { + $output = output( + sub { + pt_agent::schedule_services( + services => [ $svc0 ], + lib_dir => $tmpdir, + ), + }, + stderr => 1, + die => 1, + ); +}; + +like( + $EVAL_ERROR, + qr/Error setting new crontab/, + "Throws errors" +) or diag($output); + +$crontab = `crontab -l 2>/dev/null`; +is( + $crontab, + "", + "Bad schedule_services()" +); + +ok( + -f "$tmpdir/crontab", + "Wrote crontab to --lib/crontab" +) or diag(`ls -l $tmpdir`); + +ok( + -f "$tmpdir/crontab.err", + "Write --lib/crontab.err", +) or diag(`ls -l $tmpdir`); + +$err = -f "$tmpdir/crontab.err" ? `cat $tmpdir/crontab.err` : ''; +like( + $err, + qr/bad/, + "Crontab error" +); + +system("crontab -r 2>/dev/null"); +$crontab = `crontab -l 2>/dev/null`; +is( + $crontab, + "", + "Removed crontab" +); + + +# ############################################################################# +# Done. +# ############################################################################# +done_testing;