Skip to content

feat: migrate evstack health checks to HTTP /health/ready endpoint#149

Closed
randygrok wants to merge 1 commit intomainfrom
evstack-http-health
Closed

feat: migrate evstack health checks to HTTP /health/ready endpoint#149
randygrok wants to merge 1 commit intomainfrom
evstack-http-health

Conversation

@randygrok
Copy link
Collaborator

@randygrok randygrok commented Nov 6, 2025

Update evstack health checks to use the new HTTP /health/ready endpoint
instead of the gRPC /evnode.v1.HealthService/Livez endpoint.

Changes:

  • Replace gRPC endpoint with /health/ready HTTP endpoint
  • Update health check method from POST to GET
  • Continue using RPC port (7331) where HTTP endpoints are served
  • Remove unused bytes import

Overview

Related to evstack/ev-node#2800

Summary by CodeRabbit

  • Chores
    • Updated health check mechanism to use standard HTTP readiness endpoint instead of custom RPC endpoint.

Update evstack health checks to use the new HTTP /health/ready endpoint
instead of the gRPC /evnode.v1.HealthService/Livez endpoint.

Changes:
- Replace gRPC endpoint with /health/ready HTTP endpoint
- Update health check method from POST to GET
- Continue using RPC port (7331) where HTTP endpoints are served
- Remove unused bytes import
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Walkthrough

Standardized health check implementation across Docker framework components by replacing internal RPC-based POST health checks (/evnode.v1.HealthService/Livez) with external HTTP GET requests to /health/ready endpoint in three related node configuration files.

Changes

Cohort / File(s) Summary
Health check endpoint updates
framework/docker/evmsingle_with_reth_test.go, framework/docker/evstack/evmsingle/node.go, framework/docker/evstack/node.go
Replaced /evnode.v1.HealthService/Livez POST endpoint with /health/ready GET endpoint. Removed bytes import and JSON Content-Type header generation. Switched from POST with JSON body to GET with no body. Added context cancellation path in waitForSelfReady for evmsingle/node.go.

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant Poller as Health Check Poller
    participant Old as Old Endpoint<br/>(/evnode.v1.HealthService/Livez)
    participant New as New Endpoint<br/>(/health/ready)
    
    rect rgb(230, 245, 230)
    Note over Old: Previous Flow
    App->>Poller: Start health check
    Poller->>Old: POST + JSON body<br/>(Content-Type header)
    Old-->>Poller: Response
    Poller-->>App: Health status
    end
    
    rect rgb(245, 230, 230)
    Note over New: New Flow
    App->>Poller: Start health check
    Poller->>New: GET + no body
    New-->>Poller: Response
    Poller-->>App: Health status
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Repetitive pattern: All three files follow the same health check migration (endpoint + HTTP method swap)
  • Focus areas:
    • Verify /health/ready endpoint is consistently available across all affected services
    • Confirm context cancellation logic in evmsingle/node.go properly handles cancellation scenarios
    • Ensure TCP port liveness check still functions correctly before HTTP request

Suggested reviewers

  • chatton
  • tty47
  • tac0turtle

Poem

🐰 Hop, hop, huzzah! The health checks now flow so clean,
No more JSON bodies in requests, if you know what I mean!
A simple GET to /health/ready, light and bright,
From gRPC endpoints to HTTP—what a delight!
thump thump 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: migrating evstack health checks to use the HTTP /health/ready endpoint instead of the previous gRPC endpoint.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch evstack-http-health

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
framework/docker/evstack/node.go (1)

282-303: Health check correctly migrated to GET /health/ready.

The implementation properly uses GET with no body and checks for 200 OK status.

Consider adding a TCP port liveness check before the HTTP request (similar to framework/docker/evstack/evmsingle/node.go lines 215-220) to reduce debug log noise from connection errors:

 func (n *Node) isNodeHealthy(client *http.Client, healthURL string) bool {
+	// Quick TCP check first to avoid HTTP errors when port isn't open yet
+	if c, err := net.DialTimeout("tcp", fmt.Sprintf("0.0.0.0:%s", n.externalPorts.RPC), 2*time.Second); err == nil {
+		_ = c.Close()
+	} else {
+		return false
+	}
+
 	req, err := http.NewRequest("GET", healthURL, nil)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2bea587 and 3ec2da2.

📒 Files selected for processing (3)
  • framework/docker/evmsingle_with_reth_test.go (1 hunks)
  • framework/docker/evstack/evmsingle/node.go (2 hunks)
  • framework/docker/evstack/node.go (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T08:24:51.731Z
Learnt from: chatton
Repo: celestiaorg/tastora PR: 106
File: framework/docker/dataavailability/config.go:3-7
Timestamp: 2025-08-27T08:24:51.731Z
Learning: The tastora codebase consistently uses `github.com/moby/moby/client` for Docker client imports throughout all framework/docker packages. Do not suggest changing this to `github.com/docker/docker/client` as it would break type compatibility.

Applied to files:

  • framework/docker/evmsingle_with_reth_test.go
🧬 Code graph analysis (3)
framework/docker/evmsingle_with_reth_test.go (1)
framework/types/network.go (1)
  • Ports (50-63)
framework/docker/evstack/node.go (2)
framework/docker/dataavailability/node.go (1)
  • Node (61-77)
framework/docker/container/node.go (1)
  • Node (20-31)
framework/docker/evstack/evmsingle/node.go (2)
framework/docker/evstack/node.go (1)
  • Node (40-55)
framework/docker/evstack/reth/node.go (1)
  • Node (23-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: test
  • GitHub Check: Analyze (go)
  • GitHub Check: Run golangci-lint
🔇 Additional comments (3)
framework/docker/evmsingle_with_reth_test.go (1)

191-199: LGTM! Health check migration implemented correctly.

The migration from the gRPC endpoint to the HTTP /health/ready endpoint is properly implemented. The GET request with no body and 200 OK status check follows HTTP health check conventions.

framework/docker/evstack/node.go (1)

258-280: LGTM! Proper health check polling with context cancellation.

The waitForNodeReady function correctly migrates to the /health/ready endpoint and includes appropriate context cancellation handling.

framework/docker/evstack/evmsingle/node.go (1)

201-231: LGTM! Well-implemented health check with best practices.

The migration to the /health/ready endpoint is correctly implemented with:

  • Proper context cancellation handling (lines 210-211)
  • TCP port liveness check before HTTP request (lines 215-220) to reduce error noise
  • GET method with no body (line 221)
  • Appropriate status code validation (line 225)

The 120-second timeout is longer than the 60 seconds used in framework/docker/evstack/node.go, which appears intentional given this is the evm-single startup path.

@randygrok randygrok marked this pull request as draft November 6, 2025 09:36
github-merge-queue bot pushed a commit to evstack/ev-node that referenced this pull request Nov 10, 2025
This commit fixes issue #2643 where the health endpoint still reports OK
when a node has stopped producing blocks.

Closes #2643

<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.

NOTE: PR titles should follow semantic commits:
https://www.conventionalcommits.org/en/v1.0.0/
-->

## Overview

<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 

Ex: Closes #<issue number>
-->


Needs merge and tag of tastora PR
celestiaorg/tastora#149

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@chatton chatton closed this in #150 Nov 11, 2025
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.

1 participant