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
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
}
44 changes: 28 additions & 16 deletions cmd/cli/commands/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,27 +72,39 @@ 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)
// If filter doesn't contain '/', prepend default namespace 'ai/'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the model filter logic to match the same logic as docker cli you can filter by repo + tag but not only by tag

if !strings.Contains(modelFilter, "/") {
modelFilter = "ai/" + modelFilter
}

var filteredModels []dmrm.Model

// Check if filter has a colon (i.e., includes a tag)
hasColon := strings.Contains(modelFilter, ":")

for _, m := range models {
hasMatchingTag := false
var matchingTags []string
for _, tag := range m.Tags {
// Tags are stored in normalized format by the backend
if tag == normalizedFilter {
hasMatchingTag = true
break
}
// Also check without the tag part
modelName, _, _ := strings.Cut(tag, ":")
filterName, _, _ := strings.Cut(normalizedFilter, ":")
if modelName == filterName {
hasMatchingTag = true
break
if hasColon {
// Filter includes a tag part - do exact match
// Tags are stored in normalized format by the backend
if tag == modelFilter {
matchingTags = append(matchingTags, tag)
}
} else {
// Filter has no colon - match repository name only (part before ':')
repository, _, _ := strings.Cut(tag, ":")
if repository == modelFilter {
matchingTags = append(matchingTags, tag)
}
}
}
if hasMatchingTag {
filteredModels = append(filteredModels, m)
// Only include the model if at least one tag matched, and only include matching tags
if len(matchingTags) > 0 {
// Create a copy of the model with only the matching tags
filteredModel := m
filteredModel.Tags = matchingTags
filteredModels = append(filteredModels, filteredModel)
}
}
models = filteredModels
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
Loading
Loading