Skip to content
Open
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
10 changes: 7 additions & 3 deletions cmd/cli/commands/install-runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"strings"
"time"

"github.com/docker/docker/api/types/container"
"github.com/docker/model-runner/cmd/cli/commands/completion"
"github.com/docker/model-runner/cmd/cli/desktop"
gpupkg "github.com/docker/model-runner/cmd/cli/pkg/gpu"
Expand All @@ -18,6 +17,7 @@ import (
"github.com/docker/model-runner/pkg/inference/backends/llamacpp"
"github.com/docker/model-runner/pkg/inference/backends/vllm"
"github.com/docker/model-runner/pkg/inference/backends/vllmmetal"
"github.com/moby/moby/api/types/container"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -55,6 +55,8 @@ type standaloneRunner struct {
// hostPort is the port that the runner is listening to on the host.
hostPort uint16
// gatewayIP is the gateway IP address that the runner is listening on.
//
// TODO(thaJeztah): consider changing this to a netip.Addr
gatewayIP string
// gatewayPort is the gateway port that the runner is listening on.
gatewayPort uint16
Expand All @@ -65,13 +67,15 @@ type standaloneRunner struct {
func inspectStandaloneRunner(container container.Summary) *standaloneRunner {
result := &standaloneRunner{}
for _, port := range container.Ports {
if port.IP == "127.0.0.1" {
if port.IP.IsLoopback() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using port.IP.IsLoopback() is a more robust and idiomatic way to check for loopback addresses compared to a string comparison with "127.0.0.1". This change correctly leverages the netip package's functionality.

result.hostPort = port.PublicPort
} else {
// We don't really have a good way of knowing what the gateway IP
// address is, but in the standard standalone configuration we only
// bind to two interfaces: 127.0.0.1 and the gateway interface.
result.gatewayIP = port.IP
if port.IP.IsValid() {
result.gatewayIP = port.IP.String()
}
result.gatewayPort = port.PublicPort
}
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/cli/commands/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ import (
"runtime"
"time"

"github.com/docker/docker/api/types/container"
"github.com/docker/docker/pkg/stdcopy"
"github.com/docker/model-runner/cmd/cli/commands/completion"
"github.com/docker/model-runner/cmd/cli/desktop"
"github.com/docker/model-runner/cmd/cli/pkg/standalone"
"github.com/docker/model-runner/cmd/cli/pkg/types"
"github.com/moby/moby/api/pkg/stdcopy"
"github.com/moby/moby/client"
"github.com/nxadm/tail"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -51,7 +51,7 @@ func newLogsCmd() *cobra.Command {
} else if ctrID == "" {
return errors.New("unable to identify Model Runner container")
}
log, err := dockerClient.ContainerLogs(cmd.Context(), ctrID, container.LogsOptions{
log, err := dockerClient.ContainerLogs(cmd.Context(), ctrID, client.ContainerLogsOptions{
ShowStdout: true,
ShowStderr: true,
Follow: follow,
Expand Down
62 changes: 36 additions & 26 deletions cmd/cli/commands/nim.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@ import (
"fmt"
"io"
"net/http"
"net/netip"
"os"
"path/filepath"
"strconv"
"strings"
"time"

"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/image"
"github.com/docker/docker/api/types/mount"
"github.com/docker/docker/client"
"github.com/docker/go-connections/nat"
gpupkg "github.com/docker/model-runner/cmd/cli/pkg/gpu"
"github.com/moby/moby/api/types/container"
"github.com/moby/moby/api/types/mount"
"github.com/moby/moby/api/types/network"
"github.com/moby/moby/client"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -137,7 +137,7 @@ func pullNIMImage(ctx context.Context, dockerClient *client.Client, model string
}
}

pullOptions := image.PullOptions{}
pullOptions := client.ImagePullOptions{}

// Set authentication if available
if authStr != "" {
Expand Down Expand Up @@ -186,7 +186,9 @@ func pullNIMImage(ctx context.Context, dockerClient *client.Client, model string
defer reader.Close()

// Stream pull progress
io.Copy(cmd.OutOrStdout(), reader)
//
// TODO(thaJeztah): format output / progress?
_, _ = io.Copy(cmd.OutOrStdout(), reader)

return nil
}
Expand All @@ -195,15 +197,16 @@ func pullNIMImage(ctx context.Context, dockerClient *client.Client, model string
func findNIMContainer(ctx context.Context, dockerClient *client.Client, model string) (string, error) {
containerName := nimContainerName(model)

containers, err := dockerClient.ContainerList(ctx, container.ListOptions{
res, err := dockerClient.ContainerList(ctx, client.ContainerListOptions{
All: true,
})
if err != nil {
return "", fmt.Errorf("failed to list containers: %w", err)
}

for _, c := range containers {
for _, c := range res.Items {
for _, name := range c.Names {
// TODO(thaJeztah): replace this with a filter, or use "inspect"
if strings.TrimPrefix(name, "/") == containerName {
return c.ID, nil
}
Expand Down Expand Up @@ -246,17 +249,18 @@ func createNIMContainer(ctx context.Context, dockerClient *client.Client, model
}

// Container configuration
env := []string{}
var env []string
if ngcAPIKey != "" {
env = append(env, "NGC_API_KEY="+ngcAPIKey)
}

portStr := strconv.Itoa(nimDefaultPort)
hostPort, _ := network.PortFrom(nimDefaultPort, network.TCP)

config := &container.Config{
Image: model,
Env: env,
ExposedPorts: nat.PortSet{
nat.Port(portStr + "/tcp"): struct{}{},
ExposedPorts: network.PortSet{
hostPort: struct{}{},
},
}

Expand All @@ -269,11 +273,11 @@ func createNIMContainer(ctx context.Context, dockerClient *client.Client, model
Target: "/opt/nim/.cache",
},
},
PortBindings: nat.PortMap{
nat.Port(portStr + "/tcp"): []nat.PortBinding{
PortBindings: network.PortMap{
hostPort: []network.PortBinding{
{
HostIP: "127.0.0.1",
HostPort: portStr,
HostIP: netip.MustParseAddr("127.0.0.1"),
HostPort: strconv.Itoa(nimDefaultPort),
},
},
},
Expand All @@ -291,13 +295,19 @@ func createNIMContainer(ctx context.Context, dockerClient *client.Client, model
}

// Create the container
resp, err := dockerClient.ContainerCreate(ctx, config, hostConfig, nil, nil, containerName)
resp, err := dockerClient.ContainerCreate(ctx, client.ContainerCreateOptions{
Config: config,
HostConfig: hostConfig,
NetworkingConfig: nil,
Platform: nil,
Name: containerName,
})
if err != nil {
return "", fmt.Errorf("failed to create NIM container: %w", err)
}

// Start the container
if err := dockerClient.ContainerStart(ctx, resp.ID, container.StartOptions{}); err != nil {
if _, err := dockerClient.ContainerStart(ctx, resp.ID, client.ContainerStartOptions{}); err != nil {
return "", fmt.Errorf("failed to start NIM container: %w", err)
}

Expand All @@ -315,13 +325,13 @@ func createNIMContainer(ctx context.Context, dockerClient *client.Client, model
func waitForNIMReady(ctx context.Context, cmd *cobra.Command) error {
cmd.Println("Waiting for NIM to be ready (this may take several minutes)...")

client := &http.Client{
httpClient := &http.Client{
Timeout: 5 * time.Second,
}

maxRetries := 120 // 10 minutes with 5 second intervals
for i := 0; i < maxRetries; i++ {
resp, err := client.Get(fmt.Sprintf("http://127.0.0.1:%d/v1/models", nimDefaultPort))
resp, err := httpClient.Get(fmt.Sprintf("http://127.0.0.1:%d/v1/models", nimDefaultPort))
if err == nil {
resp.Body.Close()
if resp.StatusCode == http.StatusOK {
Expand Down Expand Up @@ -356,14 +366,14 @@ func runNIMModel(ctx context.Context, dockerClient *client.Client, model string,

if containerID != "" {
// Container exists, check if it's running
inspect, err := dockerClient.ContainerInspect(ctx, containerID)
inspect, err := dockerClient.ContainerInspect(ctx, containerID, client.ContainerInspectOptions{})
if err != nil {
return fmt.Errorf("failed to inspect NIM container: %w", err)
}

if !inspect.State.Running {
if !inspect.Container.State.Running {
// Container exists but is not running, start it
if err := dockerClient.ContainerStart(ctx, containerID, container.StartOptions{}); err != nil {
if _, err := dockerClient.ContainerStart(ctx, containerID, client.ContainerStartOptions{}); err != nil {
return fmt.Errorf("failed to start existing NIM container: %w", err)
}
cmd.Printf("Started existing NIM container %s\n", nimContainerName(model))
Expand Down Expand Up @@ -397,7 +407,7 @@ func chatWithNIM(cmd *cobra.Command, model, prompt string) error {
// The NIM container runs on localhost:8000 and provides an OpenAI-compatible API

// Create a simple HTTP client to talk to the NIM
client := &http.Client{
httpClient := &http.Client{
Timeout: 300 * time.Second,
}

Expand All @@ -422,7 +432,7 @@ func chatWithNIM(cmd *cobra.Command, model, prompt string) error {

req.Header.Set("Content-Type", "application/json")

resp, err := client.Do(req)
resp, err := httpClient.Do(req)
if err != nil {
return fmt.Errorf("failed to send request to NIM: %w", err)
}
Expand Down
48 changes: 26 additions & 22 deletions cmd/cli/desktop/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@ import (
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/connhelper"
"github.com/docker/cli/cli/context/docker"
"github.com/docker/docker/api/types/container"
clientpkg "github.com/docker/docker/client"
"github.com/docker/model-runner/cmd/cli/pkg/standalone"
"github.com/docker/model-runner/cmd/cli/pkg/types"
"github.com/docker/model-runner/pkg/inference"
modeltls "github.com/docker/model-runner/pkg/tls"
"github.com/moby/moby/api/types/container"
"github.com/moby/moby/client"
)

Expand Down Expand Up @@ -75,7 +74,7 @@ func isCloudContext(cli *command.DockerCli) bool {
}

// DockerClientForContext creates a Docker client for the specified context.
func DockerClientForContext(cli *command.DockerCli, name string) (*clientpkg.Client, error) {
func DockerClientForContext(cli *command.DockerCli, name string) (*client.Client, error) {
c, err := cli.ContextStore().GetMetadata(name)
if err != nil {
return nil, fmt.Errorf("unable to load context metadata: %w", err)
Expand All @@ -85,10 +84,9 @@ func DockerClientForContext(cli *command.DockerCli, name string) (*clientpkg.Cli
return nil, fmt.Errorf("unable to determine context endpoint: %w", err)
}

opts := []clientpkg.Opt{
clientpkg.FromEnv,
clientpkg.WithAPIVersionNegotiation(),
clientpkg.WithHost(endpoint.Host),
opts := []client.Opt{
client.FromEnv,
client.WithHost(endpoint.Host),
}

helper, err := connhelper.GetConnectionHelper(endpoint.Host)
Expand All @@ -97,12 +95,12 @@ func DockerClientForContext(cli *command.DockerCli, name string) (*clientpkg.Cli
}
if helper != nil {
opts = append(opts,
clientpkg.WithHost(helper.Host),
clientpkg.WithDialContext(helper.Dialer),
client.WithHost(helper.Host),
client.WithDialContext(helper.Dialer),
)
}

return clientpkg.NewClientWithOpts(opts...)
return client.New(opts...)
}

// ModelRunnerContext encodes the operational context of a Model CLI command and
Expand Down Expand Up @@ -205,11 +203,14 @@ func wakeUpCloudIfIdle(ctx context.Context, cli *command.DockerCli) error {
if err != nil {
return fmt.Errorf("failed to create Docker client: %w", err)
}
defer dockerClient.Close()

// The call is expected to fail with a client error due to nil arguments, but it triggers
// Docker Cloud to wake up from idle. Only return unexpected failures (network issues,
// server errors) so they're logged as warnings.
_, err = dockerClient.ContainerCreate(ctx, &container.Config{}, nil, nil, nil, "")
_, err = dockerClient.ContainerCreate(ctx, client.ContainerCreateOptions{
Config: &container.Config{},
})
if err != nil && !errdefs.IsInvalidArgument(err) {
return fmt.Errorf("failed to wake up Docker Cloud: %w", err)
}
Expand Down Expand Up @@ -337,11 +338,21 @@ func DetectContext(ctx context.Context, cli *command.DockerCli, printer standalo
// Construct the HTTP client.
var httpClient DockerHttpClient
if kind == types.ModelRunnerEngineKindDesktop {
dockerClient, err := DockerClientForContext(cli, cli.CurrentContext())
if err != nil {
return nil, fmt.Errorf("unable to create model runner client: %w", err)
if useTLS {
// For Desktop context, if TLS is enabled, we should either fully support it or fail fast
// Since Desktop context uses Docker client, we need to handle TLS differently
// For now, we'll fail fast to make the behavior clear
return nil, fmt.Errorf("TLS is not supported for Desktop contexts")
}
httpClient = dockerClient.HTTPClient()

// FIXME(thaJeztah): can we get the user-agent in some other way? (or just use a default, specific to DMR)?
// dockerClient, err := DockerClientForContext(cli, cli.CurrentContext())
// if err != nil {
// return nil, fmt.Errorf("unable to create model runner client: %w", err)
// }
// httpClient = dockerClient.HTTPClient()
// dockerClient.Close()
Comment on lines +348 to +354
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part should probably be looked into; what user-agent do we want to use here? Could model-runner use its own?

httpClient = http.DefaultClient
} else {
httpClient = http.DefaultClient
}
Expand All @@ -353,13 +364,6 @@ func DetectContext(ctx context.Context, cli *command.DockerCli, printer standalo
// Construct TLS client if TLS is enabled
var tlsClient DockerHttpClient
if useTLS {
if kind == types.ModelRunnerEngineKindDesktop {
// For Desktop context, if TLS is enabled, we should either fully support it or fail fast
// Since Desktop context uses Docker client, we need to handle TLS differently
// For now, we'll fail fast to make the behavior clear
return nil, fmt.Errorf("TLS is not supported for Desktop contexts")
}

tlsConfig, err := modeltls.LoadClientTLSConfig(tlsCACert, tlsSkipVerify)
if err != nil {
return nil, fmt.Errorf("unable to load TLS configuration: %w", err)
Expand Down
Loading
Loading