Conversation
Reviewer's GuideAdds a health check wait loop to the cluster start flow so that Sequence diagram for updated cluster start with HAProxy health waitsequenceDiagram
actor User
participant CLI as runStart
participant HAProxyManager as Manager
participant HAProxy
participant KubeAPI as KubernetesAPI
User->>CLI: cluster start
CLI->>HAProxyManager: EnsureHAProxy(ctx, 0)
HAProxyManager->>HAProxy: start or ensure container
HAProxy-->>HAProxyManager: ready to accept traffic
CLI->>HAProxyManager: WaitForHealthy(ctx)
HAProxyManager->>HAProxyManager: GetPublishedPort(ctx)
HAProxyManager-->>CLI: [port]
loop every 2s until 2m
CLI->>HAProxy: GET /healthz via https://localhost:port
HAProxy->>KubeAPI: proxy GET /healthz
KubeAPI-->>HAProxy: HTTP 200 or non-200/error
HAProxy-->>CLI: HTTP response
alt HTTP 200
CLI->>User: cluster start returns successfully and loop ends
else non-200 or error
CLI->>CLI: continue waiting
end
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The health check HTTP client ignores the provided context when issuing requests; consider using
http.NewRequestWithContextso cancellation and timeouts are driven byctxrather than only the fixed 5s client timeout. InsecureSkipVerify: truein the TLS config is a security footgun; if possible, either document why this is required or tighten it (e.g., using a proper CA or making the behavior configurable).- Using
time.After(2 * time.Minute)inside the loop setup will create a timer that can’t be stopped; prefertime.NewTimerwith adefer timer.Stop()to avoid potential leaks in long‑running processes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The health check HTTP client ignores the provided context when issuing requests; consider using `http.NewRequestWithContext` so cancellation and timeouts are driven by `ctx` rather than only the fixed 5s client timeout.
- `InsecureSkipVerify: true` in the TLS config is a security footgun; if possible, either document why this is required or tighten it (e.g., using a proper CA or making the behavior configurable).
- Using `time.After(2 * time.Minute)` inside the loop setup will create a timer that can’t be stopped; prefer `time.NewTimer` with a `defer timer.Stop()` to avoid potential leaks in long‑running processes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Poll the Kubernetes API /healthz endpoint through HAProxy until it returns HTTP 200 before completing cluster start. This prevents callers from getting EOF errors when immediately using kubectl. Fixes: #31 Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Alice Frosi <afrosi@redhat.com>
Route the kube client through the HAProxy load balancer instead of the control plane container's direct published port. The direct path goes through passt port forwarding which is unreliable under CI load, causing label application to silently fail. Also make labeling errors fatal so users get a clear failure when --label flags are not applied. Fixes #31 Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Poll the Kubernetes API /healthz endpoint through HAProxy until it returns HTTP 200 before completing cluster start. This prevents callers from getting EOF errors when immediately using kubectl.
Fixes: #31
Summary by Sourcery
Bug Fixes: