refactor(server): normalize compute driver config acquisition#1974
refactor(server): normalize compute driver config acquisition#1974elezar wants to merge 3 commits into
Conversation
elezar
requested review from
a team,
derekwaynecarr,
maxamillion and
mrunalp
as code owners
elezar
added
test:e2e
PR Review StatusValidation: this maintainer-authored PR is project-valid because it is concentrated server compute-driver configuration work linked to #1949. It narrows startup validation to the selected driver while preserving local driver runtime defaults. Review findings:
Docs: no Fern docs update is required; this is an internal startup/config acquisition refactor with no direct user-facing UX change. Next state: |
|
Label |
Maintainer Approval NeededGator validation and PR monitoring are complete. Validation: maintainer-authored, project-valid server compute-driver configuration refactor linked to #1949. Human maintainer approval or merge decision is now required. |
elezar
added
gator:approval-needed
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
elezar
force-pushed
the
refactor/1949-compute-driver-acquisition/elezar
branch
from
e3f5996 to
5dc871c
Compare
Signed-off-by: Evan Lezar <elezar@nvidia.com>
elezar
force-pushed
the
refactor/1949-compute-driver-acquisition/elezar
branch
from
5dc871c to
f3f3da3
Compare
| /// Build the selected Podman config from TOML plus runtime defaults. | ||
| pub fn podman_config_from_context( | ||
| context: DriverStartupContext<'_>, | ||
| ) -> Result<PodmanComputeConfig> { | ||
| let mut podman = driver_config_from_context(context, ComputeDriverKind::Podman, "podman")?; | ||
| apply_podman_runtime_defaults(&mut podman, context); | ||
| Ok(podman) | ||
| } | ||
|
|
||
| /// Build the selected Docker config from TOML plus runtime defaults. | ||
| pub fn docker_config_from_context( | ||
| context: DriverStartupContext<'_>, | ||
| ) -> Result<DockerComputeConfig> { | ||
| let mut cfg = driver_config_from_context(context, ComputeDriverKind::Docker, "docker")?; | ||
| apply_docker_runtime_defaults(&mut cfg, context); | ||
| Ok(cfg) | ||
| } | ||
|
|
||
| /// Build the selected VM config from TOML plus runtime defaults. | ||
| pub fn vm_config_from_context(context: DriverStartupContext<'_>) -> Result<VmComputeConfig> { | ||
| let mut cfg = driver_config_from_context(context, ComputeDriverKind::Vm, "vm")?; | ||
| apply_vm_runtime_defaults(&mut cfg, context); | ||
| Ok(cfg) | ||
| } | ||
|
|
There was a problem hiding this comment.
Is there a way that we can start to move all this driver specific config out of the openshell-server crate? Ideally the core libraries wouldn't be aware of these drivers. We initially hardcoded things in just to get a proof of life, but it'd be nice to start to better isolate driver concerns in the driver themselves.
There was a problem hiding this comment.
Yes, I think that would make sense. Do you think this should be done for this PR, or as a follow-up?
I'll do a local investigation to check what the changes will look like and then make a call based on that.
Update: I have created #1999 to track this. I don't think it belongs in this PR.
Re-check After Reviewer UpdateI re-evaluated latest head Disposition: resolved for this PR. The bounded independent re-review found no blocking issues; #1999 is an appropriate follow-up for the broader crate-boundary cleanup and should not block this scoped #1949 refactor. Remaining items:
Checks: Next state: |
3 participants
Summary
Normalize compute driver config acquisition so the CLI prepares gateway startup state while the server constructs only the selected driver config at runtime.
Related Issue
Closes #1949
Changes
Testing
mise run pre-commitpassesChecklist