Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 34 additions & 11 deletions pkg/provision/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func Provision(ctx context.Context, cfg config.DockerConfig, logger *zap.Logger)
}
kubecfg.CleanupStale()

hostConfig, err := buildHostConfig(cfg)
hostConfig, exposedPorts, err := buildHostConfig(cfg)
if err != nil {
_ = cli.Close()
return nil, err
Expand Down Expand Up @@ -171,6 +171,16 @@ func Provision(ctx context.Context, cfg config.DockerConfig, logger *zap.Logger)
// Gateway as the ingress controller; two of them
// would fight over host:80/:443.
Cmd: []string{"server", "--tls-san=127.0.0.1", "--disable=traefik"},
// ExposedPorts must list every guest port carried by
// HostConfig.PortBindings. The Docker CLI auto-fills
// this when you `-p`; the moby SDK does not. Engine
// 28+ silently drops bindings that lack an
// ExposedPorts entry in some request shapes (issue
// #16: NetworkSettings.Ports == {} on ubuntu-latest
// when invoked via the released binary from bash,
// despite the in-process e2e succeeding on the same
// runner).
ExposedPorts: exposedPorts,
},
HostConfig: hostConfig,
})
Expand Down Expand Up @@ -275,33 +285,46 @@ func writeRegistriesToContainer(ctx context.Context, cli *client.Client, contain
}

// buildHostConfig translates the YAML-shaped DockerConfig into
// the moby HostConfig. Encapsulated so the Provision code path
// stays linear.
// the moby HostConfig + the matching Config.ExposedPorts set.
// Both are required for the daemon to publish bindings:
//
// - HostConfig.PortBindings tells the daemon "publish these
// guest ports on these host ports."
// - Config.ExposedPorts declares the same guest ports as the
// container's exposed surface. The Docker CLI's `docker run
// -p ...` auto-fills both; the SDK's ContainerCreate does
// NOT and Engine 28+ silently drops bindings when ExposedPorts
// is missing in some request shapes -- the container starts
// but NetworkSettings.Ports comes back as `{}` and the host
// never sees a forward (issue #16).
//
// PortBindings come straight from cfg.PortForwards, which is
// where the API port (6443) and any ingress ports (80/443/...)
// are declared. Validation in CommonConfig guarantees a 6443
// entry exists, so the host's kubectl can reach the API server.
func buildHostConfig(cfg config.DockerConfig) (*container.HostConfig, error) {
func buildHostConfig(cfg config.DockerConfig) (*container.HostConfig, network.PortSet, error) {
bindings := network.PortMap{}
exposed := network.PortSet{}
for _, pf := range cfg.PortForwards {
guest, err := network.ParsePort(pf.Guest)
if err != nil {
return nil, fmt.Errorf("parse guest port %q: %w", pf.Guest, err)
return nil, nil, fmt.Errorf("parse guest port %q: %w", pf.Guest, err)
}
// HostIP must be set explicitly to a valid netip.Addr.
// The zero value is `invalid IP`, which moby v1.54+
// renders as an empty JSON string and the Docker Engine
// daemon (28.x) silently drops the binding from
// NetworkSettings.Ports -- the container starts but
// nothing is published to the host, so kubectl can't
// reach the apiserver. Mirroring `docker run -p ...`
// NetworkSettings.Ports. Mirroring `docker run -p ...`
// semantics, IPv4Unspecified ("0.0.0.0") binds on every
// interface. HostPort empty lets docker pick a free port.
bindings[guest] = append(bindings[guest], network.PortBinding{
HostIP: netip.IPv4Unspecified(),
HostPort: pf.Host,
})
// Same guest port lands in ExposedPorts; the daemon
// requires the pair for `docker run -p`-equivalent
// publish semantics on Engine 28+.
exposed[guest] = struct{}{}
}
hc := &container.HostConfig{
Privileged: true,
Expand All @@ -314,7 +337,7 @@ func buildHostConfig(cfg config.DockerConfig) (*container.HostConfig, error) {
if cfg.Memory != "" {
mb, err := atoi(cfg.Memory)
if err != nil {
return nil, fmt.Errorf("parse memory %q: %w", cfg.Memory, err)
return nil, nil, fmt.Errorf("parse memory %q: %w", cfg.Memory, err)
}
hc.Memory = int64(mb) * 1024 * 1024
}
Expand All @@ -323,11 +346,11 @@ func buildHostConfig(cfg config.DockerConfig) (*container.HostConfig, error) {
// use-case and would need float parsing.
n, err := atoi(cfg.CPUs)
if err != nil {
return nil, fmt.Errorf("parse cpus %q: %w", cfg.CPUs, err)
return nil, nil, fmt.Errorf("parse cpus %q: %w", cfg.CPUs, err)
}
hc.NanoCPUs = int64(n) * 1_000_000_000
}
return hc, nil
return hc, exposed, nil
}

// Teardown removes the container. keepDisk is ignored -- k3s state
Expand Down
48 changes: 47 additions & 1 deletion pkg/provision/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func TestBuildHostConfig_PortBindingsHaveValidHostIP(t *testing.T) {
},
},
}
hc, err := buildHostConfig(cfg)
hc, _, err := buildHostConfig(cfg)
if err != nil {
t.Fatalf("buildHostConfig: %v", err)
}
Expand All @@ -116,6 +116,52 @@ func TestBuildHostConfig_PortBindingsHaveValidHostIP(t *testing.T) {
}
}

// TestBuildHostConfig_ExposedPortsMirrorBindings is the
// regression guard for issue #16: ExposedPorts must list every
// guest port that PortBindings carries. The Docker CLI's
// `docker run -p ...` auto-fills both; the moby SDK's
// ContainerCreate does not. Engine 28+ silently drops bindings
// when ExposedPorts is missing in some request shapes -- the
// container starts but NetworkSettings.Ports comes back `{}`
// and the host can't reach the apiserver, despite the
// in-process go-test path succeeding on the same runner.
//
// Both fields must agree key-by-key. Drift here = the bug
// reappears, possibly in a way that's only visible when the
// released binary runs under bash on a fresh ubuntu-latest.
func TestBuildHostConfig_ExposedPortsMirrorBindings(t *testing.T) {
cfg := config.DockerConfig{
CommonConfig: config.CommonConfig{
PortForwards: []config.PortForward{
{Host: "6443", Guest: "6443"},
{Host: "80", Guest: "80"},
{Host: "443", Guest: "443"},
{Host: "8944", Guest: "8944"},
},
},
}
hc, exposed, err := buildHostConfig(cfg)
if err != nil {
t.Fatalf("buildHostConfig: %v", err)
}
if got, want := len(exposed), len(hc.PortBindings); got != want {
t.Fatalf("ExposedPorts length %d != PortBindings length %d", got, want)
}
for guest := range hc.PortBindings {
if _, ok := exposed[guest]; !ok {
t.Errorf("ExposedPorts missing guest %s carried by PortBindings; Engine 28+ silently drops bindings without a matching ExposedPorts entry", guest)
}
}
// And the inverse: no extra ExposedPorts entries that
// don't appear in PortBindings (would mean we're declaring
// surface area we don't intend to publish).
for guest := range exposed {
if _, ok := hc.PortBindings[guest]; !ok {
t.Errorf("ExposedPorts has guest %s not carried by PortBindings; declares surface y-cluster doesn't intend to publish", guest)
}
}
}

// Caller-cancelled ctx: the loop returns ctx.Err() rather than the
// readiness deadline message. Guards against a refactor that drops
// the select { <-ctx.Done() } branch and silently makes the wait
Expand Down