Add reinstall-runner command implementation #232
Conversation
Reviewer's GuideThis PR adds a new "reinstall-runner" CLI command for Docker Model Runner by registering it in the root command, implementing its full reinstallation workflow with context-aware flag defaults and Docker operations, and complements it with unit tests and updated documentation. Class diagram for the new reinstall-runner command implementationclassDiagram
class reinstall_runner_command {
+port: uint16
+host: string
+gpuMode: string
+doNotTrack: bool
+RunE(cmd, args): error
}
reinstall_runner_command --|> cobra_Command
class standalone {
+PruneControllerContainers(ctx, dockerClient, bool, cmd): error
+EnsureControllerImage(ctx, dockerClient, gpu, cmd): error
+EnsureModelStorageVolume(ctx, dockerClient, cmd): (volume, error)
+CreateControllerContainer(ctx, dockerClient, port, host, environment, doNotTrack, gpu, modelStorageVolume, cmd, engineKind): error
+DefaultControllerPortMoby: uint16
+DefaultControllerPortCloud: uint16
}
class gpupkg {
+ProbeGPUSupport(ctx, dockerClient): (GPUSupport, error)
+GPUSupportCUDA
}
class desktop {
+DockerClientForContext(dockerCLI, context): (dockerClient, error)
}
reinstall_runner_command --> standalone : uses
reinstall_runner_command --> gpupkg : uses
reinstall_runner_command --> desktop : uses
class types {
+ModelRunnerEngineKindDesktop
+ModelRunnerEngineKindMobyManual
+ModelRunnerEngineKindCloud
}
reinstall_runner_command --> types : checks engine kind
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello @ericcurtin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new reinstall-runner command that allows users to reinstall the Docker Model Runner in a single operation. The command removes the existing runner container and reinstalls it with specified configuration while preserving models and images.
Key Changes:
- Implements the
reinstall-runnercommand with support for configuring port, host, GPU settings, and tracking preferences - Adds comprehensive test coverage for command flags and properties
- Includes documentation for the new command with detailed option descriptions
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/cli/commands/reinstall-runner.go | Core implementation of the reinstall-runner command with flag handling and reinstallation logic |
| cmd/cli/commands/root.go | Registers the new reinstall-runner command in the CLI |
| cmd/cli/commands/reinstall-runner_test.go | Test suite covering command flags, properties, and validation |
| cmd/cli/docs/reference/model_reinstall-runner.md | User-facing documentation for the reinstall-runner command |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `cmd/cli/commands/reinstall-runner_test.go:50` </location>
<code_context>
+ }
+}
+
+func TestReinstallRunnerCommandType(t *testing.T) {
+ cmd := newReinstallRunner()
+
</code_context>
<issue_to_address>
**suggestion (testing):** No test coverage for command execution or error handling in RunE.
Please add tests that exercise RunE's execution logic, including scenarios with different contexts and flags, and cases that should trigger errors such as unsupported engine kinds or GPU mode issues.
Suggested implementation:
```golang
func TestReinstallRunnerCommandType(t *testing.T) {
cmd := newReinstallRunner()
}
func TestReinstallRunnerRunE_Success(t *testing.T) {
cmd := newReinstallRunner()
// Set up flags for a successful run
cmd.Flags().Set("host", "127.0.0.1")
cmd.Flags().Set("port", "8080")
cmd.Flags().Set("gpu", "false")
cmd.Flags().Set("do-not-track", "false")
// Simulate a supported engine kind
cmd.Flags().Set("engine-kind", "docker")
err := cmd.RunE(cmd, []string{})
if err != nil {
t.Errorf("RunE should succeed, got error: %v", err)
}
}
func TestReinstallRunnerRunE_UnsupportedEngineKind(t *testing.T) {
cmd := newReinstallRunner()
cmd.Flags().Set("engine-kind", "unsupported-engine")
err := cmd.RunE(cmd, []string{})
if err == nil {
t.Errorf("RunE should fail for unsupported engine kind")
}
}
func TestReinstallRunnerRunE_GPUError(t *testing.T) {
cmd := newReinstallRunner()
cmd.Flags().Set("gpu", "true")
// Simulate a context where GPU mode is not supported
cmd.Flags().Set("engine-kind", "docker")
// You may need to mock or simulate the GPU check logic here
err := cmd.RunE(cmd, []string{})
if err == nil {
t.Errorf("RunE should fail when GPU mode is not supported")
}
}
func TestReinstallRunnerRunE_FlagError(t *testing.T) {
cmd := newReinstallRunner()
// Do not set required flags to simulate error
err := cmd.RunE(cmd, []string{})
if err == nil {
t.Errorf("RunE should fail when required flags are missing")
}
}
```
- You may need to adjust the flag names and error conditions to match the actual implementation of `RunE`.
- If `RunE` depends on external systems or context, consider using mocks or stubs for those dependencies.
- If `engine-kind` is not a flag, replace it with the correct flag or argument that controls engine selection.
- If GPU error logic is more complex, you may need to mock the relevant checks or system calls.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new reinstall-runner command, which provides a convenient one-step way to reinstall the Docker Model Runner. The implementation correctly handles various engine contexts and reuses existing standalone package functions for pruning, image pulling, and container creation. The changes also include unit tests for the new command and corresponding documentation.
My review focuses on improving code maintainability and test quality. I've identified significant code duplication between the new reinstall-runner command and the existing install-runner command, and I recommend refactoring the shared logic into helper functions. Additionally, I've suggested a small improvement in the new tests to make them more robust.
ericcurtin
force-pushed
the
implement-docker-model-reinstall-runner
branch
from
dece7d2 to
1956beb
Compare
ericcurtin
force-pushed
the
implement-docker-model-reinstall-runner
branch
from
1956beb to
2105f16
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
So one can reinstall in one command Signed-off-by: Eric Curtin <[email protected]>
ericcurtin
force-pushed
the
implement-docker-model-reinstall-runner
branch
from
2105f16 to
6073d73
Compare
|
@p1-0tr @ilopezluna PTAL |
So one can reinstall in one command
Summary by Sourcery
Add a new CLI command to perform a one-step reinstallation of Docker Model Runner in standalone Docker Engine contexts.
New Features:
Documentation:
Tests: