Skip to content

all: add rules for docker-model-plugin packages#1190

Merged
thaJeztah merged 1 commit intodocker:masterfrom
xenoscopic:docker-model-packaging
May 15, 2025
Merged

all: add rules for docker-model-plugin packages#1190
thaJeztah merged 1 commit intodocker:masterfrom
xenoscopic:docker-model-packaging

Conversation

@xenoscopic
Copy link
Contributor

- What I did

I added a docker-model-plugin package in support of Docker Model Runner's upcoming support for Docker CE.

- How I did it

I tried to emulate the structure of the docker-buildx-plugin and docker-compose-plugin builds / specs (primarily compose). One small ugly bit is that the source repo is called model-cli, and Go requires it retain that name, so I've had to use a src/github.com/docker/model-cli path for the code, but for all the variables and specs I've omitted the -cli suffix and just used model/MODEL nomenclature.

- How to verify it

The package should build successfully on all supported target distros.

- Description for the changelog

Add `docker-model-plugin` package to enable Docker Model Runner support in Docker CE

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah requested a review from neersighted May 7, 2025 22:24
Copy link

@p1-0tr p1-0tr left a comment

Choose a reason for hiding this comment

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

FWIW, LGTM :)

Copy link
Contributor

@doringeman doringeman left a comment

Choose a reason for hiding this comment

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

LGTM (FWIW)!

neersighted
neersighted previously approved these changes May 13, 2025
@xenoscopic
Copy link
Contributor Author

Hey @neersighted can we wait a few more hours before merging? We're going to bump the version tag in just a bit. Then we should be at feature-complete.

Recommends: docker-buildx-plugin,
docker-compose-plugin
docker-compose-plugin,
docker-model-plugin
Copy link
Member

Choose a reason for hiding this comment

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

One thing I was considering, but perhaps I'm over-thinking; as the model CLI is still beta, should we already add it to "recommends" here, or should we leave it (for now) handle this through the get.docker.com script (adding it as "recommends" once it's out of beta / nearing non-beta?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm okay with leaving it out of recommends, though I think we'll probably try to get it added to the Docker CE install instructions. CC @francesco-corti or @kiview in case they have any thoughts.

Copy link
Member

Choose a reason for hiding this comment

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

We could make it Suggest (weaker than Recommends)

@neersighted @tianon any thoughts?

@neersighted
Copy link
Member

Sounds good; I was going to wait for @thaJeztah to take a look as well if possible.

@xenoscopic
Copy link
Contributor Author

xenoscopic commented May 13, 2025

I've bumped the Model CLI version to v0.1.23 and downgraded the Deb/RPM recommendations to suggestions. I think this is okay to merge once the reviewers feel it's ready.

@xenoscopic xenoscopic force-pushed the docker-model-packaging branch from 3218864 to 908e995 Compare May 13, 2025 23:40
Docker Model Runner's support for Docker CE is going to require
availability of the docker model command, so this commit adds the new
package by following the mechanisms used by buildx and compose.

Signed-off-by: Jacob Howard <jacob.howard@docker.com>
@xenoscopic xenoscopic force-pushed the docker-model-packaging branch from 908e995 to cefea11 Compare May 14, 2025 16:54
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

once packaged are up (as part of the docker-ce release), we should als update https://github.com/docker/docker-install to include the package by default

@thaJeztah
Copy link
Member

@neersighted gave me an "LGTM" on slack; let me bring this one in

@thaJeztah thaJeztah merged commit 8840a50 into docker:master May 15, 2025
14 checks passed
@xenoscopic xenoscopic deleted the docker-model-packaging branch May 15, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants