feat: upgrade to Go 1.26, add Docker targets, and improve build/test scripts by leinad-sch · Pull Request #147 · perk11/large-model-proxy

leinad-sch · 2026-07-31T16:04:00Z

This PR introduces Docker-based build targets, upgrades the project to Go 1.26, and refactors shell scripts to ensure POSIX compliance and testing robustness.

Key Changes

Build & CI

Go Upgrade: Updated go.mod and CI workflows to use Go 1.26.
Docker Support: Added Docker make targets for building and testing, including Go module cache volume mapping.
Makefile: Added executable-linux target and updated .gitignore to exclude generated binaries.

Shell Script Refactoring

POSIX Compatibility: Updated build.sh and output-test.sh to use /usr/bin/env sh.
Syntax Cleanup: Replaced bashisms (e.g., [[, echo -e) with POSIX equivalents ([, printf) and added proper variable quoting to prevent globbing errors.

Testing Improvements

Dynamic Logging: Refactored testLogOutput to accept a testName parameter for generating dynamic log file paths.
Reliability Fixes:
- Replaced localhost with 127.0.0.1 in procinfo requests to ensure reliable connectivity.
- Updated healthcheck commands to fallback to wget if curl is unavailable.

# make test
go build -o large-model-proxy
go build -o test-server/test-server test-server/main.go
go test -v -parallel 500 -timeout=1m # Tests have a lot of sleeps in them, not CPU bound
=== RUN   TestValidConfigMinimal
...
PASS
ok      large-model-proxy       22.267s
# make test-docker
docker run -e GOOS=linux --rm -v "./.mod:/go/pkg/mod" -v .:/app -w /app golang:1.26-alpine go build -o large-model-proxy
docker run -e GOOS=linux --rm -v "./.mod:/go/pkg/mod" -v .:/app -w /app golang:1.26-alpine go build -o test-server/test-server test-server/main.go
docker run --rm -v "./.mod:/go/pkg/mod" -v .:/app -w /app golang:1.26-alpine go test -v -parallel 500 -timeout=1m # Tests have a lot of sleeps in them, not CPU bound
=== RUN   TestValidConfigMinimal
...
PASS
ok      large-model-proxy       22.563s

perk11 · 2026-08-01T00:41:06Z

 func testVerifyArgsAndEnv(test *testing.T, procPort string, mustHaveEnv bool) {
 	client := &http.Client{}
-	req, err := http.NewRequest("GET", fmt.Sprintf("http://localhost:%s/procinfo", procPort), nil)
+	req, err := http.NewRequest("GET", fmt.Sprintf("http://127.0.0.1:%s/procinfo", procPort), nil)


Can you please explain, why does this specific check need 127.0.0.1?

I had issues with localhost, and in sone systems localhost might be missing (as it is just an entry in /etc/hosts).
I think without this the tests will faill.

I guess I'm not ready to update this to 127.0.0.1 in just a single spot unless we know which "some" systems are and if we want to support them

perk11 · 2026-08-01T00:41:25Z

 							Command:                         "./test-server/test-server",
 							Args:                            "-p 12001 -healthcheck-port 2011 -sleep-before-listening 10s -sleep-before-listening-for-healthcheck 3s -startup-duration 5s",
-							HealthcheckCommand:              "curl --fail http://localhost:2011",
+							HealthcheckCommand:              "if command -v wget >/dev/null 2>&1; then wget -q -S --spider -T 2 \"http://localhost:2011/\"; else curl --fail -s \"http://localhost:2011/\"; fi",


I'm not sure why switching to wget makes sense here?

The image golang:1.26-alpine doe not have curl, only wget.
My machine does not have wget (and I do not want wget), only curl.

This command covers both.
I could change the order, to try curl first and fall back to wget, if you want, from my view there is no difference.

perk11 · 2026-08-01T00:45:01Z

 	go build -o large-model-proxy
+
+executable-docker:
+	docker run -e GOOS=linux --rm -v "./.mod:/go/pkg/mod" -v .:/app -w /app golang:1.26-alpine go build -o large-model-proxy


I'm not sure what's GOOs=linux doing here?

Probably redundant, but it should not hurt.
If there are other OS builds, imo it would make it a little bit more readable.

I don't want to maintain redundant. Let's remove everything redundant

perk11 · 2026-08-01T00:50:34Z

Thank you for the contribution!
I cherry-picked the 4 commits that I understood (although I don't understand how the change from localhost to 127.0.0.1 helps) and left a few questions about the rest

Replace hardcoded curl healthcheck commands with a shell snippet that attempts to use wget first, falling back to curl if wget is unavailable. This improves robustness across different test environments.

perk11 · 2026-08-02T04:10:33Z

 							ProxyTargetPort:                 "12001",
 							Command:                         "./test-server/test-server",
 							Args:                            "-p 12001 -healthcheck-port 2011 -sleep-before-listening 10s -sleep-before-listening-for-healthcheck 3s -startup-duration 5s",
-							HealthcheckCommand:              "curl --fail http://localhost:2011",


Let's add a Dockerfile over supporting wget here

A Dockerfile would not solve the issue for systems without curl, and add a lot of extra time, complexity and maintenance overhead to the tests.
With this fix you could clone the repo and run make test or make test-docker, and you do not need anything else just Golang or Docker and curl or wget.

@leinad-sch I don't want to support systems without curl. There is a maintenance cost to decisions like this one and making everyone install curl is an easier path.

We can have a script that builds docker container from Dockerfile, the same way it works with debian-package target, and then it will still be a single command.

Update Go version to 1.26 in go.mod and CI workflows. Add Docker-based make targets for building and testing. Update GitHub Actions workflow to use Go 1.26. Ensure compatibility with Alpine-based Docker images.

perk11 · 2026-08-04T04:04:20Z

@leinad-sch If you still like to move this forward, let's switch to a Dockerfile. It can still stay a single make command to both build (if not already built) and run. and then I think we can get it cleaned up and merged, but it's probably going to take a few iterations.

perk11 reviewed Aug 1, 2026

View reviewed changes

test: fallback to wget for healthcheck commands

2ee38d3

Replace hardcoded curl healthcheck commands with a shell snippet that attempts to use wget first, falling back to curl if wget is unavailable. This improves robustness across different test environments.

leinad-sch force-pushed the docker-tests branch 2 times, most recently from 542135e to 895be1d Compare August 1, 2026 20:43

perk11 reviewed Aug 2, 2026

View reviewed changes

leinad-sch force-pushed the docker-tests branch from 895be1d to 8384e10 Compare August 2, 2026 09:01

feat: update Go version to 1.26 and add Docker build targets

ba78ac8

Update Go version to 1.26 in go.mod and CI workflows. Add Docker-based make targets for building and testing. Update GitHub Actions workflow to use Go 1.26. Ensure compatibility with Alpine-based Docker images.

leinad-sch force-pushed the docker-tests branch from 8384e10 to ba78ac8 Compare August 2, 2026 09:05

leinad-sch marked this pull request as draft August 2, 2026 10:31

Conversation

leinad-sch commented Jul 31, 2026

Key Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leinad-sch Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perk11 commented Aug 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perk11 commented Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

leinad-sch Aug 1, 2026 •

edited

Loading