Skip to content

refactor(server): colocate Docker driver config construction with Podman/K8s#2042

Open
r3v5 wants to merge 1 commit into
NVIDIA:mainfrom
r3v5:colocate-local-driver-config-construction-tests
Open

refactor(server): colocate Docker driver config construction with Podman/K8s#2042
r3v5 wants to merge 1 commit into
NVIDIA:mainfrom
r3v5:colocate-local-driver-config-construction-tests

Conversation

@r3v5

@r3v5 r3v5 commented Jun 29, 2026

Copy link
Copy Markdown

Summary

  • Move Docker config construction from cli.rs into lib.rs to match the pattern used by Podman and Kubernetes drivers
  • Docker config is now built inside build_compute_runtime() via docker_config_from_file() and apply_docker_local_tls_defaults(), eliminating the docker_config parameter from run_server() and build_compute_runtime()
  • Colocate docker_config_reads_bind_mount_opt_in_from_driver_table test next to its constructor in lib.rs

Related Issue

Closes #1853

Changes

crates/openshell-server/src/lib.rs

  • Add docker_config_from_file() — mirrors podman_config_from_file() / kubernetes_config_from_file()
  • Add apply_docker_local_tls_defaults() — mirrors apply_podman_local_tls_defaults()
  • Wire Docker branch in build_compute_runtime() to build config inline (matches Podman branch)
  • Remove docker_config parameter from run_server() and build_compute_runtime()
  • Move Docker config test from cli.rs to lib.rs

crates/openshell-server/src/cli.rs

  • Remove build_docker_config() function
  • Remove DockerComputeConfig import
  • Remove Docker config test (moved to lib.rs)
  • Remove docker_config from run_from_args() call site

crates/openshell-server/src/compute/mod.rs

  • Fix pre-existing type mismatch in driver_sandbox_spec_from_public test (separate commit)

Testing

  • cargo test -p openshell-server — 892 passed
  • mise run ci — full green
  • cargo clippy — clean
  • cargo fmt — clean
  • No behavioral change — Docker driver config still inherits from gateway section, merges driver-specific overrides, and applies TLS defaults

Checklist

  • Conventional Commits format
  • Signed-off-by (DCO)
  • No secrets or credentials
  • Scoped to issue at hand

🤖 Generated with Claude Code

@r3v5 r3v5 requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners June 29, 2026 11:58
@copy-pr-bot

copy-pr-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@r3v5

r3v5 commented Jun 29, 2026

Copy link
Copy Markdown
Author

I have read the DCO document and I hereby sign the DCO.

@r3v5

r3v5 commented Jun 29, 2026

Copy link
Copy Markdown
Author

recheck

@r3v5 r3v5 force-pushed the colocate-local-driver-config-construction-tests branch 2 times, most recently from 7565f76 to 13410e5 Compare June 29, 2026 12:06
…man/K8s

Move Docker config construction from cli.rs into lib.rs to match the
pattern used by Podman and Kubernetes drivers. Docker config is now
built inside build_compute_runtime() via docker_config_from_file() and
apply_docker_local_tls_defaults(), eliminating the docker_config
parameter from run_server() and build_compute_runtime().

The colocated test docker_config_reads_bind_mount_opt_in_from_driver_table
moves from cli.rs to lib.rs next to its constructor.

Refs: RHAIENG-5899

Signed-off-by: Ian Miller <milleryan2003@gmail.com>
@r3v5 r3v5 force-pushed the colocate-local-driver-config-construction-tests branch from 13410e5 to b170ae7 Compare June 29, 2026 12:08
@elezar

elezar commented Jun 29, 2026

Copy link
Copy Markdown
Member

There is some overlap between this and #1974 (although I think some of the general cleanup is still applicable).

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.

refactor(server): colocate local driver config construction tests

2 participants