Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor to ignore a failure for the grace period #21

Merged
merged 12 commits into from
Oct 24, 2023
Merged

Refactor to ignore a failure for the grace period #21

merged 12 commits into from
Oct 24, 2023

Conversation

gregcorbett
Copy link
Member

@gregcorbett gregcorbett commented Aug 30, 2023 •
edited
Loading

Supersedes #18, depends on #23 and #22. Resolves GT-235.

  • currently, an error in the pipeline immediately causes the next icinga check to raise an error.
  • this led to flapping hour to hour, generating emails that were ignored.
  • this should mean we only get alerts if the failover pipeline has not succeeded within the grace period.
  • adding an output on a successful restore in 1_runDbUpdate.sh simplifies the logic needed by this check. We don't have to worry about "if/when the pipeline failed?", we only care about "when did it last succeed?".

This PR will also add:

  • unit tests for the check_db_dump_recent.py script
  • a GitHub workflow to run those tests at PR time
  • a dependabot config to keep workflows/actions up to date

@gregcorbett gregcorbett force-pushed the check_db_dump_recent_rework_rework branch from 9436e96 to b588ab1 Compare August 30, 2023 14:31
@gregcorbett gregcorbett mentioned this pull request Aug 30, 2023
Closed
@gregcorbett gregcorbett force-pushed the check_db_dump_recent_rework_rework branch 2 times, most recently from 9b5e91c to 8870fdb Compare August 30, 2023 15:08
gregcorbett added a commit that referenced this pull request Aug 30, 2023
@gregcorbett
Merge pull request #23 from gregcorbett/check_db_dump_recent_whitespace
402d60f 
Whitespace fixes for #21
@gregcorbett gregcorbett force-pushed the check_db_dump_recent_rework_rework branch from 23c315d to fd93934 Compare August 31, 2023 07:43
@gregcorbett gregcorbett mentioned this pull request Aug 31, 2023
Merged
@gregcorbett gregcorbett force-pushed the check_db_dump_recent_rework_rework branch 2 times, most recently from 1801587 to 3dfcfd4 Compare August 31, 2023 10:49
@gregcorbett gregcorbett marked this pull request as ready for review August 31, 2023 10:54
@gregcorbett
Copy link
Member Author

The new unit tests do pass: https://github.com/gregcorbett/gocdb-failover-scripts/actions/runs/6036355188

@gregcorbett gregcorbett requested review from tofu-rocketry and a team August 31, 2023 15:34
@gregcorbett gregcorbett self-assigned this Aug 31, 2023
rowan04
rowan04 reviewed Sep 14, 2023
View reviewed changes
Copy link
Member

@rowan04 rowan04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some grammatical changes. Rest of the code lgtm

.github/workflows/unit-test.yml Outdated Show resolved Hide resolved
check/check_db_dump_recent.py Outdated Show resolved Hide resolved
check/check_db_dump_recent.py Outdated Show resolved Hide resolved
@tofu-rocketry
Copy link
Member

Could you add some notes to the PR about the GitHub workflow changes? (Assuming they are intentionally part of this PR.)

@gregcorbett
Copy link
Member Author

Done

tofu-rocketry
tofu-rocketry requested changes Sep 28, 2023
View reviewed changes
importDBdmpFile/1_runDbUpdate.sh Show resolved Hide resolved
check/check_db_dump_recent.py Show resolved Hide resolved
check/check_db_dump_recent.py Outdated Show resolved Hide resolved
test/test_check_db_dump_recent.py Outdated Show resolved Hide resolved
test/test_check_db_dump_recent.py Outdated Show resolved Hide resolved
test/test_check_db_dump_recent.py Show resolved Hide resolved
test/test_check_db_dump_recent.py Outdated Show resolved Hide resolved
test/test_check_db_dump_recent.py Outdated Show resolved Hide resolved
test/test_check_db_dump_recent.py Show resolved Hide resolved
tofu-rocketry
tofu-rocketry requested changes Oct 11, 2023
View reviewed changes
importDBdmpFile/1_runDbUpdate.sh Outdated Show resolved Hide resolved
test/test_check_db_dump_recent.py Outdated Show resolved Hide resolved
tofu-rocketry
tofu-rocketry approved these changes Oct 19, 2023
View reviewed changes
Copy link
Member

@tofu-rocketry tofu-rocketry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok, though puzzled by use of help.

test/test_check_db_dump_recent.py Outdated Show resolved Hide resolved
@gregcorbett gregcorbett force-pushed the check_db_dump_recent_rework_rework branch from 529bdf3 to d2d681a Compare October 19, 2023 13:59
gregcorbett and others added 6 commits October 19, 2023 15:12
@gregcorbett
Make output for Oracle and MariaDB the same.
2e0af91 
- Improve comments about final logger/echo commands as well
@gregcorbett @tofu-rocketry
Refactor to ignore a failure for the grace period
45e4584 
- currently, an error in the pipeline immediately causes the next icinga
  check to raise an error.
- this led to flapping hour to hour, generating emails that were ignored.
- this should mean we only get alerts if the failover pipeline has not
  succeeded within the grace period.

Co-authored-by: Adrian Coveney <4836233+tofu-rocketry@users.noreply.github.com>
@gregcorbett @tofu-rocketry
Add a unit test for ORACLE functionality
8d28e42 
Co-authored-by: Adrian Coveney <4836233+tofu-rocketry@users.noreply.github.com>
@gregcorbett
Update to reflect new unit test file
d78d654
@gregcorbett
Add MariaDB test cases
d9231d6 
- these are largely the same as the ORACLE case, except the log
  lines have additional timestamps in them.
@gregcorbett
Add test case for the empty case
bbfae64
gregcorbett and others added 6 commits October 19, 2023 15:27
@gregcorbett
Add test case for non existant log file
a6c289c
@gregcorbett
Rework check_db_dump_recent import and comment
aafddf6
@gregcorbett
Shorten long lines.
8d6c12f
@gregcorbett
Add GitHub Action for unit test and coverage
f0d84fa
@gregcorbett @tofu-rocketry
Add dependabot.yml to keep Actions up to date
16cf6f1 
Co-authored-by: tofu-rocketry <tofu-rocketry@users.noreply.github.com>
@gregcorbett @rowan04 @tofu-rocketry
Apply SPG Fixes found in code review
9610cfe 
Co-authored-by: Rowan <rowanmoss04@gmail.com>
Co-authored-by: Adrian Coveney <4836233+tofu-rocketry@users.noreply.github.com>
@gregcorbett gregcorbett force-pushed the check_db_dump_recent_rework_rework branch from d2d681a to 9610cfe Compare October 19, 2023 14:28
@gregcorbett
Copy link
Member Author

I've squashed the commits down a bit, but GitHub confirms no changes have snuck in.

@tofu-rocketry
Copy link
Member

We're all green then. :shipit:

@gregcorbett gregcorbett merged commit f85c489 into GOCDB:main Oct 24, 2023
4 checks passed
@gregcorbett gregcorbett deleted the check_db_dump_recent_rework_rework branch October 24, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rowan04 rowan04 rowan04 left review comments

@tofu-rocketry tofu-rocketry tofu-rocketry approved these changes

Assignees

@gregcorbett gregcorbett

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gregcorbett @tofu-rocketry @rowan04