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.

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

Use monitors instead of custom healthcheck.py script to verify service dependencies' health #22951

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Dec 16, 2024

Fixes: mozilla/addons#15249
Relates to: mozilla/addons#15248

Description

the custom healthcheck.py script was returning false positives. Instead of fixing this scripts, it makes more sense to use the existing infrastructure we have from monitors to ensure services are running correctly.

Context

We can use a management command running a set of monitors on a timeout to verify service dependencies are up. This is useful in the initializion command itself that depdends on the database being up, and after the initialization command where we want to ensure several critical dependencies are up before letting make up return successfully.

Testing

With a running environment you can run

./manage.py monitors
  1. stop the worker container.. the monitor should fail
  2. add an exception to the version view function.. the monitor should fail
  3. stop the mysql container.. the monitor should fail

Any combination of these should result in a failure. You can adjust the attempts parameter or the services parameter to check different services.

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@KevinMind KevinMind changed the title Addons-15248-fix Use monitors instead of custom healthcheck.py script to verify service dependencies' health Dec 16, 2024
@KevinMind KevinMind changed the base branch from addons-15248-revert to master December 16, 2024 12:54
@KevinMind KevinMind requested review from a team and diox and removed request for a team December 16, 2024 12:54
@KevinMind KevinMind force-pushed the addons-15248-fix branch 4 times, most recently from 6bb1a40 to 0c00dff Compare December 17, 2024 15:55
Copy link
Member

@diox diox left a comment

Choose a reason for hiding this comment

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

Still playing with it locally, some comments

docker-compose.yml Outdated Show resolved Hide resolved
src/olympia/amo/monitors.py Outdated Show resolved Hide resolved
src/olympia/amo/monitors.py Outdated Show resolved Hide resolved
@KevinMind KevinMind requested a review from diox December 17, 2024 16:38
@KevinMind KevinMind force-pushed the addons-15248-fix branch 3 times, most recently from f0fe4ec to 825fa3c Compare December 17, 2024 17:10
f'Expected {settings.DB_CHARSET}, '
f'recieved {result[1]}',
id='setup.E005',
try:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you "stop" the mysql service and run

./manage.py monitors --service olympia_database

The command raises here as it is a check. we want to return the error object which means that check should not raise.

In general, this django check should also not raise but return the error object.

Copy link
Member

@diox diox left a comment

Choose a reason for hiding this comment

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

r+ once CI pass

- Updated `docker-compose.yml` to improve health checks for worker and web services, ensuring they are monitored effectively.
- Introduced a new management command `monitors.py` to check the health of various services, including database and web.
- Updated `Makefile-docker` to replace healthcheck with monitors
- Updated `initialize.py` to ensure the database is running before proceeding with initialization.
- Removed the obsolete `healthcheck.py` script in favor of the new monitoring approach.
- Added tests for the new monitoring functionality to ensure reliability.

TMP: better tests
@KevinMind KevinMind merged commit a9aa528 into master Dec 17, 2024
36 checks passed
@KevinMind KevinMind deleted the addons-15248-fix branch December 17, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: mysql check is not strong enough in CI
2 participants