Skip to content

Fix: Handle empty string healthcheck as readiness healthcheck #1402

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

Merged
merged 2 commits into from
Aug 19, 2025
+6 −1
Merged

Fix: Handle empty string healthcheck as readiness healthcheck #1402

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3637bb2
Fix: Handle empty string healthcheck as overall healthcheck
yangligt2 Aug 18, 2025
a58fef4
Apply suggestion from liu-cong
yangligt2 Aug 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion cmd/epp/runner/health.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,12 @@ func (s *healthServer) Check(ctx context.Context, in *healthPb.HealthCheckReques
case ReadinessCheckService:
checkName = "readiness"
isPassing = isLive && s.isLeader.Load()
case LivenessCheckService, "": // Default to liveness check if service is empty
case "": // Handle overall server health for load balancers that use an empty service name.
Copy link
Collaborator

Choose a reason for hiding this comment

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

very very minor, non-blocking nit: move the empty string case to the bottom of the list

checkName = "overall(empty string)"
// The overall health for a load balancer should reflect readiness to accept traffic,
// which is true only for the leader pod that has synced its data.
isPassing = isLive && s.isLeader.Load()
case LivenessCheckService:
checkName = "liveness"
// Any pod that is running and can respond to this gRPC check is considered "live".
// The datastore sync status should not affect liveness, only readiness.
Expand Down