Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 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
151 changes: 125 additions & 26 deletions cmd/cli/commands/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -587,32 +587,6 @@ func TestIntegration_TagModel(t *testing.T) {
})
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this verification because it's redundant. If a tag fails the individual test will fail, so this test is not adding any additional coverage and it becomes a false positive in some cases (for example when tagging a model using same reference as tag, its a no op, but this test will fail because the expected number of tagged models will not match with the actual)

// Final verification: List the model and verify all tags are present
t.Run("verify all tags in model inspect", func(t *testing.T) {
inspectedModel, err := env.client.Inspect(modelID, false)
require.NoError(t, err, "Failed to inspect model by ID")

t.Logf("Model has %d tags: %v", len(inspectedModel.Tags), inspectedModel.Tags)

// The model should have at least the original tag plus all created tags
require.GreaterOrEqual(t, len(inspectedModel.Tags), len(createdTags)+1,
"Model should have at least %d tags (original + created)", len(createdTags)+1)

// Verify each created tag is in the model's tag list
for expectedTag := range createdTags {
found := false
for _, actualTag := range inspectedModel.Tags {
if actualTag == expectedTag || actualTag == fmt.Sprintf("%s:latest", expectedTag) { // Handle implicit latest tag
found = true
break
}
}
require.True(t, found, "Expected tag %s not found in model's tag list", expectedTag)
}

t.Logf("✓ All %d created tags verified in model's tag list", len(createdTags))
})

// Test error case: tagging non-existent model
t.Run("error on non-existent model", func(t *testing.T) {
err := tagModel(newTagCmd(), env.client, "non-existent-model:v1", "ai/should-fail:latest")
Expand Down Expand Up @@ -1016,6 +990,131 @@ func TestIntegration_RemoveModel(t *testing.T) {
})
}

// TestIntegration_PackageModel tests packaging a GGUF model file
// to ensure the model is properly loaded and tagged in the model store.
// This test reproduces issue #461 where packaging fails with "model not found" during tagging.
func TestIntegration_PackageModel(t *testing.T) {
env := setupTestEnv(t)

// Ensure no models exist initially
models, err := listModels(false, env.client, true, false, "")
require.NoError(t, err)
if len(models) != 0 {
t.Fatal("Expected no initial models, but found some")
}

// Use the dummy GGUF file from assets
dummyGGUFPath := filepath.Join("../../../assets/dummy.gguf")
absPath, err := filepath.Abs(dummyGGUFPath)
require.NoError(t, err)

// Check if the file exists
_, err = os.Stat(absPath)
require.NoError(t, err, "dummy.gguf not found at %s", absPath)

// Test case 1: Package a GGUF file with a simple tag
t.Run("package GGUF with simple tag", func(t *testing.T) {
targetTag := "ai/packaged-test:latest"

// Create package options
opts := packageOptions{
ggufPath: absPath,
tag: targetTag,
}

// Execute the package command using the helper function with test client
t.Logf("Packaging GGUF file %s as %s", absPath, targetTag)
err := packageModel(env.ctx, newPackagedCmd(), env.client, opts)
require.NoError(t, err, "Failed to package GGUF model")

// Verify the model was loaded and tagged
t.Logf("Verifying model was loaded and tagged")
models, err := listModels(false, env.client, false, false, "")
require.NoError(t, err)
require.NotEmpty(t, models, "No models found after packaging")

// Verify we can inspect the model by tag
model, err := env.client.Inspect(targetTag, false)
require.NoError(t, err, "Failed to inspect packaged model by tag: %s", targetTag)
require.NotEmpty(t, model.ID, "Model ID should not be empty")
require.Contains(t, model.Tags, targetTag, "Model should have the expected tag")

t.Logf("✓ Successfully packaged and tagged model: %s (ID: %s)", targetTag, model.ID[7:19])

// Cleanup
err = removeModel(env.client, model.ID, true)
require.NoError(t, err, "Failed to remove model")
})

// Test case 2: Package with context size override
t.Run("package GGUF with context size", func(t *testing.T) {
targetTag := "ai/packaged-ctx:latest"

// Create package options with context size
opts := packageOptions{
ggufPath: absPath,
tag: targetTag,
contextSize: 4096,
}

// Create a command for context
cmd := newPackagedCmd()
// Set the flag as changed for context size
cmd.Flags().Set("context-size", "4096")

// Execute the package command using the helper function with test client
t.Logf("Packaging GGUF file with context size 4096 as %s", targetTag)
err := packageModel(env.ctx, cmd, env.client, opts)
require.NoError(t, err, "Failed to package GGUF model with context size")

// Verify the model was loaded and tagged
model, err := env.client.Inspect(targetTag, false)
require.NoError(t, err, "Failed to inspect packaged model")
require.Contains(t, model.Tags, targetTag, "Model should have the expected tag")

t.Logf("✓ Successfully packaged model with context size: %s", targetTag)

// Cleanup
err = removeModel(env.client, model.ID, true)
require.NoError(t, err, "Failed to remove model")
})

// Test case 3: Package with different org
t.Run("package GGUF with custom org", func(t *testing.T) {
targetTag := "myorg/packaged-test:v1"

// Create package options
opts := packageOptions{
ggufPath: absPath,
tag: targetTag,
}

// Create a command for context
cmd := newPackagedCmd()

// Execute the package command using the helper function with test client
t.Logf("Packaging GGUF file as %s", targetTag)
err := packageModel(env.ctx, cmd, env.client, opts)
require.NoError(t, err, "Failed to package GGUF model with custom org")

// Verify the model was loaded and tagged
model, err := env.client.Inspect(targetTag, false)
require.NoError(t, err, "Failed to inspect packaged model")
require.Contains(t, model.Tags, targetTag, "Model should have the expected tag")

t.Logf("✓ Successfully packaged model with custom org: %s", targetTag)

// Cleanup
err = removeModel(env.client, model.ID, true)
require.NoError(t, err, "Failed to remove model")
})

// Verify all models are cleaned up
models, err = listModels(false, env.client, true, false, "")
require.NoError(t, err)
require.Empty(t, strings.TrimSpace(models), "All models should be removed after cleanup")
}

func int32ptr(n int32) *int32 {
return &n
}
6 changes: 2 additions & 4 deletions cmd/cli/commands/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,20 +72,18 @@ func listModels(openai bool, desktopClient *desktop.Client, quiet bool, jsonForm
}

if modelFilter != "" {
// Normalize the filter to match stored model names (backend normalizes when storing)
normalizedFilter := dmrm.NormalizeModelName(modelFilter)
var filteredModels []dmrm.Model
for _, m := range models {
hasMatchingTag := false
for _, tag := range m.Tags {
// Tags are stored in normalized format by the backend
if tag == normalizedFilter {
if tag == modelFilter {
hasMatchingTag = true
break
}
// Also check without the tag part
modelName, _, _ := strings.Cut(tag, ":")
filterName, _, _ := strings.Cut(normalizedFilter, ":")
filterName, _, _ := strings.Cut(modelFilter, ":")
if modelName == filterName {
hasMatchingTag = true
break
Expand Down
10 changes: 5 additions & 5 deletions cmd/cli/commands/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func newPackagedCmd() *cobra.Command {
},
RunE: func(cmd *cobra.Command, args []string) error {
opts.tag = args[0]
if err := packageModel(cmd, opts); err != nil {
if err := packageModel(cmd.Context(), cmd, desktopClient, opts); err != nil {
cmd.PrintErrln("Failed to package model")
return fmt.Errorf("package model: %w", err)
}
Expand Down Expand Up @@ -254,7 +254,7 @@ func initializeBuilder(cmd *cobra.Command, opts packageOptions) (*builderInitRes
return result, nil
}

func packageModel(cmd *cobra.Command, opts packageOptions) error {
func packageModel(ctx context.Context, cmd *cobra.Command, client *desktop.Client, opts packageOptions) error {
var (
target builder.Target
err error
Expand All @@ -264,7 +264,7 @@ func packageModel(cmd *cobra.Command, opts packageOptions) error {
registry.WithUserAgent("docker-model-cli/" + desktop.Version),
).NewTarget(opts.tag)
} else {
target, err = newModelRunnerTarget(desktopClient, opts.tag)
target, err = newModelRunnerTarget(client, opts.tag)
}
if err != nil {
return err
Expand Down Expand Up @@ -357,7 +357,7 @@ func packageModel(cmd *cobra.Command, opts packageOptions) error {
done := make(chan error, 1)
go func() {
defer pw.Close()
done <- pkg.Build(cmd.Context(), target, pw)
done <- pkg.Build(ctx, target, pw)
}()

scanner := bufio.NewScanner(pr)
Expand Down Expand Up @@ -443,7 +443,7 @@ func (t *modelRunnerTarget) Write(ctx context.Context, mdl types.ModelArtifact,
return fmt.Errorf("get model ID: %w", err)
}
if t.tag.String() != "" {
if err := desktopClient.Tag(id, parseRepo(t.tag), t.tag.TagStr()); err != nil {
if err := t.client.Tag(id, parseRepo(t.tag), t.tag.TagStr()); err != nil {
return fmt.Errorf("tag model: %w", err)
}
}
Expand Down
128 changes: 0 additions & 128 deletions cmd/cli/commands/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,88 +4,8 @@ import (
"errors"
"fmt"
"testing"

"github.com/docker/model-runner/pkg/inference/models"
)

func TestNormalizeModelName(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
name: "simple model name",
input: "gemma3",
expected: "ai/gemma3:latest",
},
{
name: "model name with tag",
input: "gemma3:v1",
expected: "ai/gemma3:v1",
},
{
name: "model name with org",
input: "myorg/gemma3",
expected: "myorg/gemma3:latest",
},
{
name: "model name with org and tag",
input: "myorg/gemma3:v1",
expected: "myorg/gemma3:v1",
},
{
name: "fully qualified model name",
input: "ai/gemma3:latest",
expected: "ai/gemma3:latest",
},
{
name: "huggingface model",
input: "hf.co/bartowski/model",
expected: "huggingface.co/bartowski/model:latest",
},
{
name: "huggingface model with tag",
input: "hf.co/bartowski/model:Q4_K_S",
expected: "huggingface.co/bartowski/model:q4_k_s",
},
{
name: "registry with model",
input: "docker.io/library/model",
expected: "docker.io/library/model:latest",
},
{
name: "registry with model and tag",
input: "docker.io/library/model:v1",
expected: "docker.io/library/model:v1",
},
{
name: "empty string",
input: "",
expected: "",
},
{
name: "ai prefix already present",
input: "ai/gemma3",
expected: "ai/gemma3:latest",
},
{
name: "model name with latest tag already",
input: "gemma3:latest",
expected: "ai/gemma3:latest",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := models.NormalizeModelName(tt.input)
if result != tt.expected {
t.Errorf("NormalizeModelName(%q) = %q, want %q", tt.input, result, tt.expected)
}
})
}
}

func TestStripDefaultsFromModelName(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -174,54 +94,6 @@ func TestStripDefaultsFromModelName(t *testing.T) {
}
}

// TestNormalizeModelNameConsistency verifies that locally packaged models
// (without namespace) get normalized the same way as other operations.
// This test documents the fix for the bug where `docker model package my-model`
// would create a model that couldn't be run with `docker model run my-model`.
func TestNormalizeModelNameConsistency(t *testing.T) {
tests := []struct {
name string
userProvidedName string
expectedNormalizedName string
description string
}{
{
name: "locally packaged model without namespace",
userProvidedName: "my-model",
expectedNormalizedName: "ai/my-model:latest",
description: "When a user packages a local model as 'my-model', it should be normalized to 'ai/my-model:latest'",
},
{
name: "locally packaged model without namespace but with tag",
userProvidedName: "my-model:v1.0",
expectedNormalizedName: "ai/my-model:v1.0",
description: "When a user packages a local model as 'my-model:v1.0', it should be normalized to 'ai/my-model:v1.0'",
},
{
name: "model with explicit namespace",
userProvidedName: "myorg/my-model",
expectedNormalizedName: "myorg/my-model:latest",
description: "When a user packages a model with explicit org 'myorg/my-model', it should keep the org",
},
{
name: "model with ai namespace explicitly set",
userProvidedName: "ai/my-model",
expectedNormalizedName: "ai/my-model:latest",
description: "When a user explicitly sets 'ai/' namespace, it should remain the same",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := models.NormalizeModelName(tt.userProvidedName)
if result != tt.expectedNormalizedName {
t.Errorf("%s: NormalizeModelName(%q) = %q, want %q",
tt.description, tt.userProvidedName, result, tt.expectedNormalizedName)
}
})
}
}

// TestHandleClientErrorFormat verifies that the error format follows the expected pattern.
func TestHandleClientErrorFormat(t *testing.T) {
t.Run("error format is message: original error", func(t *testing.T) {
Expand Down
Loading
Loading