Skip to content

Remove local health port binding, check upstream directly, and fix Docker PR CI#5

Merged
tma merged 7 commits intomainfrom
copilot/fix-tcp-bind-error
Mar 10, 2026
Merged

Remove local health port binding, check upstream directly, and fix Docker PR CI#5
tma merged 7 commits intomainfrom
copilot/fix-tcp-bind-error

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 9, 2026

This PR removes the separate local health listener and makes mbproxy -health verify upstream connectivity directly, then follows up on the resulting CI failure by fixing the Docker workflow used for pull requests.

Changes Made

  • Removed the need for a dedicated local health port binding and switched container health checks to use mbproxy -health against the upstream connection directly.
  • Reduced the Docker healthcheck interval so the direct upstream probe opens backend connections less frequently.
  • Investigated the failing GitHub Actions Docker workflow and confirmed the failure was caused by pull request builds trying to use registry-backed cache access against ghcr.io, which returned 403 Forbidden.
  • Updated /home/runner/work/mbproxy/mbproxy/.github/workflows/docker.yml so the Docker workflow remains a single build step while using PR-safe cache behavior:
    • pull request builds use GitHub Actions cache and do not push images
    • main and tag builds continue using registry-backed cache and publish to GHCR

Validation

  • Verified the updated workflow YAML parses correctly.
  • Verified the Docker image still builds locally.
  • Ran local CI checks in Docker:
    • go fmt ./...
    • go vet ./...
    • go test -v -race ./...
  • Re-ran automated review and security checks with no remaining review comments and no CodeQL alerts.

This keeps the original health-check behavior change intact while ensuring the PR Docker workflow no longer fails due to GHCR cache authorization during pull request builds.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI assigned Copilot and tma Mar 9, 2026
Copilot AI linked an issue Mar 9, 2026 that may be closed by this pull request
Copilot AI and others added 3 commits March 9, 2026 11:46
Co-authored-by: tma <4719+tma@users.noreply.github.com>
Co-authored-by: tma <4719+tma@users.noreply.github.com>
Co-authored-by: tma <4719+tma@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix address already in use error for TCP socket Remove local health port binding and make -health check upstream directly Mar 9, 2026
Copilot finished work on behalf of tma March 9, 2026 11:54
Co-authored-by: tma <4719+tma@users.noreply.github.com>
Co-authored-by: tma <4719+tma@users.noreply.github.com>
Copilot finished work on behalf of tma March 10, 2026 07:48
Co-authored-by: tma <4719+tma@users.noreply.github.com>
Copilot AI changed the title Remove local health port binding and make -health check upstream directly Remove local health port binding, check upstream directly, and fix Docker PR CI Mar 10, 2026
Copilot finished work on behalf of tma March 10, 2026 08:14
@tma tma marked this pull request as ready for review March 10, 2026 08:15
@tma tma merged commit 9b12f3f into main Mar 10, 2026
2 checks passed
@tma tma deleted the copilot/fix-tcp-bind-error branch March 10, 2026 08:18
tma added a commit that referenced this pull request Mar 11, 2026
Revert the direct-upstream health probe introduced in #5, which opened
a second TCP connection to the Modbus device on every check. Many
Modbus devices only accept one concurrent connection, so the probe
interfered with normal proxy operation in production.

Restore the original passive HTTP health server that checks internal
connection state without touching upstream. Make it opt-in via
HEALTH_LISTEN so bare-binary users running multiple instances (the
root cause of #4) no longer hit port conflicts. The Dockerfile sets
HEALTH_LISTEN=:8080 so Docker users get health checks automatically.

Closes #4
tma added a commit that referenced this pull request Mar 11, 2026
* fix: restore HTTP-based health server, make it opt-in

Revert the direct-upstream health probe introduced in #5, which opened
a second TCP connection to the Modbus device on every check. Many
Modbus devices only accept one concurrent connection, so the probe
interfered with normal proxy operation in production.

Restore the original passive HTTP health server that checks internal
connection state without touching upstream. Make it opt-in via
HEALTH_LISTEN so bare-binary users running multiple instances (the
root cause of #4) no longer hit port conflicts. The Dockerfile sets
HEALTH_LISTEN=:8080 so Docker users get health checks automatically.

Closes #4

* fix: normalize wildcard hosts in CheckHealth, isolate config test env

Co-authored-by: tma <4719+tma@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
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.

error="listen tcp :8080: bind: address already in use"

2 participants