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

Have lexbox API container check fw-headless health #1192

Merged
merged 5 commits into from
Nov 7, 2024

Conversation

rmunn
Copy link
Contributor

@rmunn rmunn commented Oct 31, 2024

Fix #1185.

Health checks for fw-headless container now go through lexbox-api's health check endpoint.

@rmunn rmunn self-assigned this Oct 31, 2024
Copy link

github-actions bot commented Oct 31, 2024

C# Unit Tests

75 tests  ±0   75 ✅ ±0   5s ⏱️ ±0s
13 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit ac6c860. ± Comparison against base commit e751594.

♻️ This comment has been updated with latest results.

@rmunn
Copy link
Contributor Author

rmunn commented Oct 31, 2024

Commit 6789760 implements the HealthCheckConfig idea I mentioned in the PR description.

@rmunn rmunn requested a review from hahn-kev October 31, 2024 09:26
@rmunn
Copy link
Contributor Author

rmunn commented Oct 31, 2024

@hahn-kev - I might have done something slightly wrong with registering the health checks in ASP.NET — because when I just tested this in local-dev, the health check returned Unhealthy when I intended for it to return Degraded. I had everything up and running and the health checks were returning Healthy. Then I ran kubectl delete pod fw-headless-6bd44b6d66-tngql and immediately re-ran the health checks on http://localhost:5158/api/healthz.

It should have returned "Degraded" according to how we designed this (and I thought I'd implemented it correctly), but instead it returned "Unhealthy". However, I can't find my error; from what I can see, it should have been returning "Degraded" since it couldn't connect to the fw-headless health endpoint. So I need a second pair of eyes on the code to tell me what mistake I'm making.

Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

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

one minor change, otherwise it looks good

@rmunn rmunn requested a review from hahn-kev November 1, 2024 07:42
hahn-kev
hahn-kev previously approved these changes Nov 5, 2024
@rmunn
Copy link
Contributor Author

rmunn commented Nov 6, 2024

@hahn-kev - Unfortunately, you'll need to re-approve. I had to rebase on top of develop to fix a merge conflict in skaffold.yaml. And since my post-rebase was not the same set of changes that you approved (I removed the skaffold.yaml change since we're going to be deleting it soon anyway), that means GitHub doesn't auto-reapply your approval. (For all it knows, the two lines I removed from skaffold.yaml could have broken the whole application!)

@rmunn rmunn merged commit b41478b into develop Nov 7, 2024
11 checks passed
@rmunn rmunn deleted the feat/fw-headless-health-check branch November 7, 2024 10:09
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.

make lexbox api health check depend on fw-headless health check
2 participants