Fix route-based port mapping: use hostPort as container port
allocateRoutePorts() was using the route's port field (the mc-proxy listener port, e.g. 443) as the container internal port in the podman port mapping. For L7 routes, apps don't listen on the mc-proxy port — they read $PORT (set to the assigned host port) and listen on that. The mapping host:53204 → container:443 fails because nothing listens on 443 inside the container. Fix: use hostPort as both the host and container port, so $PORT = host port = container port. Broke mcdoc in production (manually fixed, now permanently fixed). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -198,7 +198,10 @@ func (a *Agent) allocateRoutePorts(service, component string, routes []registry.
|
|||||||
return nil, nil, fmt.Errorf("store host port for route %q: %w", r.Name, err)
|
return nil, nil, fmt.Errorf("store host port for route %q: %w", r.Name, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
ports = append(ports, fmt.Sprintf("127.0.0.1:%d:%d", hostPort, r.Port))
|
// The container port must match hostPort (which is also set as $PORT),
|
||||||
|
// so the app's listen address matches the podman port mapping.
|
||||||
|
// r.Port is the mc-proxy listener port, NOT the container port.
|
||||||
|
ports = append(ports, fmt.Sprintf("127.0.0.1:%d:%d", hostPort, hostPort))
|
||||||
|
|
||||||
if len(routes) == 1 {
|
if len(routes) == 1 {
|
||||||
env = append(env, fmt.Sprintf("PORT=%d", hostPort))
|
env = append(env, fmt.Sprintf("PORT=%d", hostPort))
|
||||||
|
|||||||
159
internal/agent/deploy_test.go
Normal file
159
internal/agent/deploy_test.go
Normal file
@@ -0,0 +1,159 @@
|
|||||||
|
package agent
|
||||||
|
|
||||||
|
import (
|
||||||
|
"database/sql"
|
||||||
|
"fmt"
|
||||||
|
"log/slog"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"git.wntrmute.dev/mc/mcp/internal/registry"
|
||||||
|
)
|
||||||
|
|
||||||
|
func openTestDB(t *testing.T) *sql.DB {
|
||||||
|
t.Helper()
|
||||||
|
db, err := registry.Open(filepath.Join(t.TempDir(), "test.db"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("open db: %v", err)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() { _ = db.Close() })
|
||||||
|
return db
|
||||||
|
}
|
||||||
|
|
||||||
|
func testAgent(t *testing.T) *Agent {
|
||||||
|
t.Helper()
|
||||||
|
return &Agent{
|
||||||
|
DB: openTestDB(t),
|
||||||
|
PortAlloc: NewPortAllocator(),
|
||||||
|
Logger: slog.New(slog.NewTextHandler(os.Stderr, nil)),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// seedComponent creates the service and component in the registry so that
|
||||||
|
// allocateRoutePorts can store host ports for it.
|
||||||
|
func seedComponent(t *testing.T, db *sql.DB, service, component string, routes []registry.Route) {
|
||||||
|
t.Helper()
|
||||||
|
if err := registry.CreateService(db, service, true); err != nil {
|
||||||
|
t.Fatalf("create service: %v", err)
|
||||||
|
}
|
||||||
|
if err := registry.CreateComponent(db, ®istry.Component{
|
||||||
|
Name: component,
|
||||||
|
Service: service,
|
||||||
|
Image: "img:latest",
|
||||||
|
DesiredState: "running",
|
||||||
|
ObservedState: "unknown",
|
||||||
|
Routes: routes,
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("create component: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAllocateRoutePorts_SingleRoute(t *testing.T) {
|
||||||
|
a := testAgent(t)
|
||||||
|
routes := []registry.Route{
|
||||||
|
{Name: "default", Port: 443, Mode: "l7"},
|
||||||
|
}
|
||||||
|
seedComponent(t, a.DB, "mcdoc", "mcdoc", routes)
|
||||||
|
|
||||||
|
ports, env, err := a.allocateRoutePorts("mcdoc", "mcdoc", routes)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("allocateRoutePorts: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(ports) != 1 {
|
||||||
|
t.Fatalf("expected 1 port mapping, got %d", len(ports))
|
||||||
|
}
|
||||||
|
if len(env) != 1 {
|
||||||
|
t.Fatalf("expected 1 env var, got %d", len(env))
|
||||||
|
}
|
||||||
|
|
||||||
|
// Parse the port mapping: should be "127.0.0.1:<hostPort>:<hostPort>"
|
||||||
|
// NOT "127.0.0.1:<hostPort>:443"
|
||||||
|
var hostPort, containerPort int
|
||||||
|
n, _ := fmt.Sscanf(ports[0], "127.0.0.1:%d:%d", &hostPort, &containerPort)
|
||||||
|
if n != 2 {
|
||||||
|
t.Fatalf("failed to parse port mapping %q", ports[0])
|
||||||
|
}
|
||||||
|
if hostPort != containerPort {
|
||||||
|
t.Errorf("host port (%d) != container port (%d); container port must match host port for $PORT consistency", hostPort, containerPort)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Env var should be PORT=<hostPort>
|
||||||
|
var envPort int
|
||||||
|
n, _ = fmt.Sscanf(env[0], "PORT=%d", &envPort)
|
||||||
|
if n != 1 {
|
||||||
|
t.Fatalf("failed to parse env var %q", env[0])
|
||||||
|
}
|
||||||
|
if envPort != hostPort {
|
||||||
|
t.Errorf("PORT env (%d) != host port (%d)", envPort, hostPort)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAllocateRoutePorts_MultiRoute(t *testing.T) {
|
||||||
|
a := testAgent(t)
|
||||||
|
routes := []registry.Route{
|
||||||
|
{Name: "rest", Port: 8443, Mode: "l4"},
|
||||||
|
{Name: "grpc", Port: 9443, Mode: "l4"},
|
||||||
|
}
|
||||||
|
seedComponent(t, a.DB, "metacrypt", "api", routes)
|
||||||
|
|
||||||
|
ports, env, err := a.allocateRoutePorts("metacrypt", "api", routes)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("allocateRoutePorts: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(ports) != 2 {
|
||||||
|
t.Fatalf("expected 2 port mappings, got %d", len(ports))
|
||||||
|
}
|
||||||
|
if len(env) != 2 {
|
||||||
|
t.Fatalf("expected 2 env vars, got %d", len(env))
|
||||||
|
}
|
||||||
|
|
||||||
|
// Each port mapping should have host port == container port.
|
||||||
|
for i, p := range ports {
|
||||||
|
var hp, cp int
|
||||||
|
n, _ := fmt.Sscanf(p, "127.0.0.1:%d:%d", &hp, &cp)
|
||||||
|
if n != 2 {
|
||||||
|
t.Fatalf("port[%d]: failed to parse %q", i, p)
|
||||||
|
}
|
||||||
|
if hp != cp {
|
||||||
|
t.Errorf("port[%d]: host port (%d) != container port (%d)", i, hp, cp)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Env vars should be PORT_REST and PORT_GRPC (not bare PORT).
|
||||||
|
if env[0][:10] != "PORT_REST=" {
|
||||||
|
t.Errorf("env[0] = %q, want PORT_REST=...", env[0])
|
||||||
|
}
|
||||||
|
if env[1][:10] != "PORT_GRPC=" {
|
||||||
|
t.Errorf("env[1] = %q, want PORT_GRPC=...", env[1])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAllocateRoutePorts_L7PortNotUsedAsContainerPort(t *testing.T) {
|
||||||
|
a := testAgent(t)
|
||||||
|
routes := []registry.Route{
|
||||||
|
{Name: "default", Port: 443, Mode: "l7"},
|
||||||
|
}
|
||||||
|
seedComponent(t, a.DB, "svc", "web", routes)
|
||||||
|
|
||||||
|
ports, _, err := a.allocateRoutePorts("svc", "web", routes)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("allocateRoutePorts: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The container port must NOT be 443 (the mc-proxy listener port).
|
||||||
|
// It must be the host port (which is in range 10000-60000).
|
||||||
|
var hostPort, containerPort int
|
||||||
|
n, _ := fmt.Sscanf(ports[0], "127.0.0.1:%d:%d", &hostPort, &containerPort)
|
||||||
|
if n != 2 {
|
||||||
|
t.Fatalf("failed to parse port mapping %q", ports[0])
|
||||||
|
}
|
||||||
|
if containerPort == 443 {
|
||||||
|
t.Errorf("container port is 443 (mc-proxy listener); should be %d (host port)", hostPort)
|
||||||
|
}
|
||||||
|
if containerPort < portRangeMin || containerPort >= portRangeMax {
|
||||||
|
t.Errorf("container port %d outside allocation range [%d, %d)", containerPort, portRangeMin, portRangeMax)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -90,18 +90,19 @@ func TestBuildRunArgs(t *testing.T) {
|
|||||||
})
|
})
|
||||||
|
|
||||||
t.Run("full spec with env", func(t *testing.T) {
|
t.Run("full spec with env", func(t *testing.T) {
|
||||||
|
// Route-allocated ports: host port = container port (matches $PORT).
|
||||||
spec := ContainerSpec{
|
spec := ContainerSpec{
|
||||||
Name: "svc-api",
|
Name: "svc-api",
|
||||||
Image: "img:latest",
|
Image: "img:latest",
|
||||||
Network: "net",
|
Network: "net",
|
||||||
Ports: []string{"127.0.0.1:12345:8443"},
|
Ports: []string{"127.0.0.1:12345:12345"},
|
||||||
Volumes: []string{"/srv:/srv"},
|
Volumes: []string{"/srv:/srv"},
|
||||||
Env: []string{"PORT=12345"},
|
Env: []string{"PORT=12345"},
|
||||||
}
|
}
|
||||||
requireEqualArgs(t, p.BuildRunArgs(spec), []string{
|
requireEqualArgs(t, p.BuildRunArgs(spec), []string{
|
||||||
"run", "-d", "--name", "svc-api",
|
"run", "-d", "--name", "svc-api",
|
||||||
"--network", "net",
|
"--network", "net",
|
||||||
"-p", "127.0.0.1:12345:8443",
|
"-p", "127.0.0.1:12345:12345",
|
||||||
"-v", "/srv:/srv",
|
"-v", "/srv:/srv",
|
||||||
"-e", "PORT=12345",
|
"-e", "PORT=12345",
|
||||||
"img:latest",
|
"img:latest",
|
||||||
|
|||||||
@@ -38,7 +38,7 @@ service McpAgentService {
|
|||||||
|
|
||||||
message RouteSpec {
|
message RouteSpec {
|
||||||
string name = 1; // route name (used for $PORT_<NAME>)
|
string name = 1; // route name (used for $PORT_<NAME>)
|
||||||
int32 port = 2; // external port on mc-proxy
|
int32 port = 2; // mc-proxy listener port (e.g. 443, 8443, 9443); NOT the container internal port
|
||||||
string mode = 3; // "l4" or "l7"
|
string mode = 3; // "l4" or "l7"
|
||||||
string hostname = 4; // optional public hostname override
|
string hostname = 4; // optional public hostname override
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user