RedHatInsights / insights-api-common-rails

Header, Encryption, RBAC, Serialization, Pagination and other common behavior for Insights microservices built with Rails
Apache License 2.0
3 stars 25 forks source link

Add generic StatusController that can be mapped to `/health` for liveness probe across services #143

Closed lindgrenj6 closed 4 years ago

lindgrenj6 commented 4 years ago

This work was initially implemented and reviewed here: https://github.com/RedHatInsights/catalog-api/pull/528

This is basically the extraction of the (albeit tiny) controller that can then be added to any application and then mapped to /health as in the dummy app here. The goal is to use this across the microservices to have a liveness probe watch this URL.

lindgrenj6 commented 4 years ago

cc @carbonin

carbonin commented 4 years ago

@bdunne want to take a look?

lindgrenj6 commented 4 years ago

@bdunne the goal is to pull the fields into kibana or something - that way we can keep track of the state over time etc. It's not set up yet, but will be soon.

bdunne commented 4 years ago

@bdunne the goal is to pull the fields into kibana or something - that way we can keep track of the state over time etc.

bdunne commented 4 years ago

If we split the migration issues out of this and just focus on the liveness probe, maybe we should just return HEAD 200? After reading this blog article it sounds like our liveness probe shouldn't even check the connection to the database. What do you think?

lindgrenj6 commented 4 years ago

@bdunne Yes, I'm ok with removing most (if not all) of that logic in the controller as long as we make the pod impossible to start if the database migrations fail.

miq-bot commented 4 years ago

Checked commit https://github.com/lindgrenj6/insights-api-common-rails/commit/e57bbb1f5a50749f9605d5fd45906915ca999ab4 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 6 files checked, 1 offense detected

spec/dummy/config/routes.rb

lindgrenj6 commented 4 years ago

@bdunne @carbonin I pulled out the db migration stuff - opened a PR on the catalog side to change the behavior of the container to not start up if the database migrations fail. https://github.com/RedHatInsights/catalog-api/pull/563