Trust our object pointers

If we fail to parse configuration properly, we need to know that
by getting a coredump, not by ignoring to send out a notification
or some other stupid crap, so checking if assignment turned out
well just has to go. It's a legacy from when we used to do
object lookups based on names anyway.

Signed-off-by: Andreas Ericsson <ae@op5.se>
Use current host state as route_result

Once upon a time we used to run route checks (ie, a check of a host
and its parents) synchronously, and back then we actually set the
route_result variable to something sensible at the end of a failed
service check. However, with The Big Performance Rewrite, route
checks are now asynchronous, so we'll never get a proper result
from it in time to determine if the host is down or not before
we move on to notifying about the service.

A 100% correct solution would be to add triggered notifications,
where one notification can't be sent unless we know the absolute
root cause of the problem (or at least some time has passed since
the problem was detected), but with this fix we just use the current
host state as the route result, meaning we'll never send any
service notifications for hosts that are DOWN or UNREACHABLE
provided:
a) The host check is executed after the host has gone down but
   before the service check is executed
b) The triggered host check comes back with a correct result
   before the service's max_check_attempts has been reached.

Ignoring pathological cases where the host is basically flapping
up and down, this solution will be Good Enough(tm) and took all
of 30 seconds to write.

Signed-off-by: Andreas Ericsson <ae@op5.se>
.gitignore t-tap/test_commands

Buildproduct, so it shouldn't pretend it wants to be tracked.

Signed-off-by: Andreas Ericsson <ae@op5.se>
Get rid of check delay/spread

Everything about it is just absolutely, ridonculously daft - just run
the bloody checks normally, and stop dicking about with whatever that
was!

This is a rework of part of a patch originally submitted by Robin
Sonefors.

Signed-off-by: Andreas Ericsson <ae@op5.com>
Update test config to not use deprecated parameters anymore

Getting a bunch of warnings when reading configuration is not so
very nice.

Signed-off-by: Andreas Ericsson <ae@op5.se>
Stop being so damn stupid when initially scheduling checks

The previous code had a number of problems with it. Not least of
which was the fact that scheduling would take forever if a lot of
checks use a checkperiod that excludes a timeperiod with exceptions
far in the future.

There's also the fact that we had to do special exception handling
for when checks were already scheduled but the check_interval had
changed between restarts.

With this change, we randomize checks into anywhere between now
and now+check_window(obj), which should allow for a pretty decent
distribution.

This fixes the slow-to-start issue reported as issue 82 by
julianbrost on github.

This is a rework of part of a patch originally submitted by
Robin Sonefors.

Reported-by: Julian Brost
Signed-off-by: Andreas Ericsson <ae@op5.se>
Add macro for calculating next check time

The signature is next_check_time(o), where o is either a host or a
service. It yields the next check time (surprise!) by choosing the
correct interval based on the state of the object and the last_check
variable of the object, so those two have to be set.

The interesting part about this is that we can now get scheduling
retention exceptionally cheaply, by simply using the next_check logic
we can (and should; Patch to follow) the whole swamp of fiddling
with checks that are already scheduled but have had their intervals
changed since restart when initially scheduling checks.

Assuming we take into account the fact that checks can be scheduled
to run in the past if they were supposed to run while we were shut
down, we now have an excellent oneline tool to handle scheduling
data retention with zero-exception code.

Signed-off-by: Andreas Ericsson <ae@op5.se>
Use next_check_time(o) when scheduling checks after commands

If one doesn't take the last_check parameter into account, it's
possible to have the checks run way too close together, as could
happen when one disables and then quickly re-enables checks for
a host or a service (as could happen with a misclick, for example).

Since next_check_time() already takes the last_check into account,
this is a prime spot for us to use it.

It's ok if the check gets scheduled at a time where we can't run
it, as we'll just push it forward when we encounter it if that
happens.

Signed-off-by: Andreas Ericsson <ae@op5.se>
Use next_check_time() when rescheduling checks

We used to do the equivalent of next_check_time() in case we
failed to find a suitable time for the check to run, so this
was always the fallback. Since the only downside of using it
as the primary scheduling mechanism is that "next_check" will
sometimes point to a time where the check won't actually be
run, but rather when we'll inspect it again (and that already
happened for checks where we couldn't determine a good next
time to run the check), we gladly take that tradeoff for the
accuracy in checking this provides.

This also means that checks scheduled to run, say, between
08:00-17:00 every weekday won't all be scheduled to run at
08:00 exactly, and we'll keep the checkspread. With a 5 min
interval, a check that last ran at 16:57 will end up running
at 08:02 instead of 08:00, which makes for smoother load on
the monitoring system.

It also lets us avoid the buggy get_next_valid_time() and
its counterpart get_next_invalid_time(), which have some
buggy tests that hide their sometimes inaccurate behaviour.

Signed-off-by: Andreas Ericsson <ae@op5.se>
Make timeperiod timeranges end when they say they do

Previously we would include the end-minute-mark for the timerange
as being a "valid" time, which it really isn't. A timerange
specified as "00:00:24:00" doesn't last the first minute of the next
day. It ends (and starts) at midnight. In the same way, 00:00-00:15
includes all the seconds from midnight up to 00:14:59, but not the
00:15:00 second (or anyone thereafter).

This patch makes it so, and also updates relevant tests to reflect
the change.

Signed-off-by: Andreas Ericsson <ae@op5.se>
timeperiod tests: Make 24x7 tests sort-of-optional

When debugging or refactoring the timeperiod code, it's pretty damn
annoying to have almost 6000 tests that clutter up the screen with
their output. Adding printf() debugging makes the whole mess
completely unbearable. With this patch, it's straightforward to
skip those tests, although that will result in a tap warning about
us not running the number of tests we planned.

While at it; fix the loops. Unconditionally incrementing a counter
inside a while()-loop and unconditionally initializing it just
before it is just bad style.

Signed-off-by: Andreas Ericsson <ae@op5.se>
Fix timeperiod handling

Unfortunately, a lot of the more complex timeperiod tests were
downright wrong, leading us to use buggy code in the timeperiod
handling.

This patch fixes them. Future commits will fix the broken tests
in a series of patches reworking them completely.

Manually going through the ~400 relevant testcases was *not* fun,
so please be really careful when adding more tests and make sure
the expected output is actually correct.

Signed-off-by: Andreas Ericsson <ae@op5.se>
test-config: Add more timeperiods for testing

Signed-off-by: Andreas Ericsson <ae@op5.se>
test_timeperiods: Revamp and fix timeperiod testing

Earlier timeperiod tests tested only the high level functions
and had very little debug output for when things went wrong.

With this patch, we add some convenient macros that explain what
we got and what we expected.

We also add tests for get_matching_timerange, which is at the
heart of all other timeperiod stuff.

Since a lot of tests failed, I've also gone through all of them
to make sure all tests are actually sane, and the ones that were
previously broken have been fixed so the input actually matches
the expected output.

Since there are a *lot* of tests, I've only updated the broken
ones to use the new macros, since I needed the debug output from
them while fixing them.

Signed-off-by: Andreas Ericsson <ae@op5.se>
Avoid updating our checked-in retention data file when testing

This unfortunately removes one test from the commands, but since
it only tests if Naemon receives the commands and not how well
it actually saves the state information, it's quite a useless
test anyway.

This removes the constant annoyance of not being able to run
"git bisect run", since the tests no longer modify checked in
files.

Signed-off-by: Andreas Ericsson <ae@op5.se>
.gitignore naemon.log

Tests create it but we don't want it.

Signed-off-by: Andreas Ericsson <ae@op5.se>
checks: Consider long output when stalking

This patch fixes a bug where Nagios would not consider if the long
plugin output had changed when deciding on whether to generate an alert
for hosts and services for which stalking was enabled.

To achieve this, the API for process_host_check_result has changed in a
non-backwards-compatible way, by means of no longer exposing it since it
never should have been exposed to begin with (it's a very internal
helper function).

The patch includes test cases to reproduce the bug in the
test_neb_callbacks suite, since those already had some tests dealing
with stalking in particular.

This fixes op5#8102.

Signed-off-by: Anton Lofgren <alofgren@op5.com>
Signed-off-by: Robin Sonefors <robin.sonefors@op5.com>
downtime: Don't crash when host downtime_depth < 0

When a host's downtime_depth was going to go below zero, instead of
ignoring it like a good little monitoring system (not one that's as good
as one that would actually log it, but still), Naemon would dereference
svc (which pointed to NULL) and then try decrement a value only a couple
of dozen bytes into the memory address space, still firmly inside kernel
space, leading to instant crashes and sadness all around.

Bad Naemon! You're not supposed to look at kernel space memory! Now go
look at the downtime type before doing anything else!

Yes, just like that! Who's a good boy? You're a good boy! Now you've
resolved bug op5#8440!

Signed-off-by: Robin Sonefors <robin.sonefors@op5.com>
checks: Add broker attribute for first check

Merlin insists on not using a check's initial state, but instead assumes
that every user of the merlin log database pretends that there's a state
called "pending" that all checks are in until their first check run -
despite the fact that Naemon actually has a state for them, configurable
via the initial_state variable. Therefore, it needs Naemon to tell it
that "This is the first check result in the history of everything", so
it can log it to database, so the pending will end already.

This adds that hook for merlin, so merlin patches can fix bug op5#8798.

Signed-off-by: Robin Sonefors <robin.sonefors@op5.com>
notifications: Increment counter after _START hook

The current_notification_number is supposed to be increased when, and
only when, an notification is sent - that is, it is initially
incremented, but if there was nobody to notify, it is decremented back
again.

There are two global neb hooks per notification - one at _START and one
at _END. You might think that _START runs before, and _END runs after.
You'd be wrong then, though: _START runs after
current_notification_number has been incremented, and if it cancels the
notification, current_notification_number will - incorrectly - stay
incremented.

The result, for merlin, is that it will end up distributing blocked
notifications, so that the current notification number will grow on
other nodes, but stay at whatever it is at on the current node.

This makes _START run at the start.

Change-Id: I50701a929201a040ac572f7be98cec6f2c7f60ae
Signed-off-by: Robin Sonefors <robin.sonefors@op5.com>