Skip to content

Commit 29e2539

Browse files
authored
refactor: deduplicate shared utilities across driver crates (#1660)
Move three duplicated definitions into openshell-core so every consumer has a single canonical source: - format_bytes: identical 14-line function existed in docker, kubernetes, and vm drivers. Moved to openshell-core::progress where all three already imported from. - DEFAULT_SANDBOX_PIDS_LIMIT: i64 constant (2048) duplicated in docker driver and podman config. Moved to openshell-core::config alongside other shared defaults. Podman re-exports it for internal call-site compatibility. - current_time_ms: secrets.rs in openshell-sandbox reimplemented the same logic as openshell-core::time::now_ms(). Remove the local copy and call now_ms() directly via the existing dep.
1 parent 019a986 commit 29e2539

7 files changed

Lines changed: 37 additions & 68 deletions

File tree

crates/openshell-core/src/config.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ pub const DEFAULT_SUPERVISOR_IMAGE: &str = "ghcr.io/nvidia/openshell/supervisor:
3939
/// CDI device identifier for requesting all NVIDIA GPUs.
4040
pub const CDI_GPU_DEVICE_ALL: &str = "nvidia.com/gpu=all";
4141

42+
/// Default maximum number of processes (PIDs) allowed inside a sandbox container.
43+
///
44+
/// Shared by the Docker and Podman drivers; override via driver config.
45+
pub const DEFAULT_SANDBOX_PIDS_LIMIT: i64 = 2048;
46+
4247
/// Compute backends the gateway can orchestrate sandboxes through.
4348
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
4449
#[serde(rename_all = "snake_case")]

crates/openshell-core/src/progress.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,24 @@ pub fn mark_progress_detail<S: BuildHasher>(
3737
) {
3838
metadata.insert(PROGRESS_ACTIVE_DETAIL_KEY.to_string(), detail.into());
3939
}
40+
41+
/// Format a byte count as a human-readable string (B / KB / MB / GB).
42+
///
43+
/// Used by compute drivers when reporting image pull progress.
44+
pub fn format_bytes(bytes: u64) -> String {
45+
const KB: u64 = 1024;
46+
const MB: u64 = 1024 * KB;
47+
const GB: u64 = 1024 * MB;
48+
49+
if bytes >= GB {
50+
#[allow(clippy::cast_precision_loss)]
51+
let gb = bytes as f64 / GB as f64;
52+
format!("{gb:.1} GB")
53+
} else if bytes >= MB {
54+
format!("{} MB", bytes / MB)
55+
} else if bytes >= KB {
56+
format!("{} KB", bytes / KB)
57+
} else {
58+
format!("{bytes} B")
59+
}
60+
}

crates/openshell-driver-docker/src/lib.rs

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,17 @@ use bollard::query_parameters::{
1818
};
1919
use bytes::Bytes;
2020
use futures::{Stream, StreamExt};
21-
use openshell_core::config::{DEFAULT_DOCKER_NETWORK_NAME, DEFAULT_STOP_TIMEOUT_SECS};
21+
use openshell_core::config::{
22+
DEFAULT_DOCKER_NETWORK_NAME, DEFAULT_SANDBOX_PIDS_LIMIT, DEFAULT_STOP_TIMEOUT_SECS,
23+
};
2224
use openshell_core::driver_utils::{
2325
LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME,
2426
LABEL_SANDBOX_NAMESPACE, SUPERVISOR_IMAGE_BINARY_PATH,
2527
};
2628
use openshell_core::gpu::cdi_gpu_device_ids;
2729
use openshell_core::progress::{
2830
PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX,
29-
mark_progress_active, mark_progress_complete, mark_progress_detail,
31+
format_bytes, mark_progress_active, mark_progress_complete, mark_progress_detail,
3032
};
3133
use openshell_core::proto::compute::v1::{
3234
CreateSandboxRequest, CreateSandboxResponse, DeleteSandboxRequest, DeleteSandboxResponse,
@@ -67,7 +69,6 @@ const SUPERVISOR_PATH: &str = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin
6769
const HOST_OPENSHELL_INTERNAL: &str = "host.openshell.internal";
6870
const HOST_DOCKER_INTERNAL: &str = "host.docker.internal";
6971
const DOCKER_NETWORK_DRIVER: &str = "bridge";
70-
const DEFAULT_SANDBOX_PIDS_LIMIT: i64 = 2048;
7172

7273
/// Default image holding the Linux `openshell-sandbox` binary. The gateway
7374
/// pulls this image and extracts the binary to a host-side cache when no
@@ -1446,24 +1447,6 @@ fn format_progress_detail(progress: &ProgressDetail) -> Option<String> {
14461447
}
14471448
}
14481449

1449-
fn format_bytes(bytes: u64) -> String {
1450-
const KB: u64 = 1024;
1451-
const MB: u64 = 1024 * KB;
1452-
const GB: u64 = 1024 * MB;
1453-
1454-
if bytes >= GB {
1455-
#[allow(clippy::cast_precision_loss)]
1456-
let gb = bytes as f64 / GB as f64;
1457-
format!("{gb:.1} GB")
1458-
} else if bytes >= MB {
1459-
format!("{} MB", bytes / MB)
1460-
} else if bytes >= KB {
1461-
format!("{} KB", bytes / KB)
1462-
} else {
1463-
format!("{bytes} B")
1464-
}
1465-
}
1466-
14671450
fn attach_docker_progress_metadata(
14681451
metadata: &mut HashMap<String, String>,
14691452
reason: &str,

crates/openshell-driver-kubernetes/src/driver.rs

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use openshell_core::driver_utils::{
1919
};
2020
use openshell_core::progress::{
2121
PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX,
22-
mark_progress_active, mark_progress_complete, mark_progress_detail,
22+
format_bytes, mark_progress_active, mark_progress_complete, mark_progress_detail,
2323
};
2424
use openshell_core::proto::compute::v1::{
2525
DriverCondition as SandboxCondition, DriverPlatformEvent as PlatformEvent,
@@ -724,24 +724,6 @@ fn extract_image_size(message: &str) -> Option<u64> {
724724
rest[..end].parse().ok()
725725
}
726726

727-
fn format_bytes(bytes: u64) -> String {
728-
const KB: u64 = 1024;
729-
const MB: u64 = 1024 * KB;
730-
const GB: u64 = 1024 * MB;
731-
732-
if bytes >= GB {
733-
#[allow(clippy::cast_precision_loss)]
734-
let gb = bytes as f64 / GB as f64;
735-
format!("{gb:.1} GB")
736-
} else if bytes >= MB {
737-
format!("{} MB", bytes / MB)
738-
} else if bytes >= KB {
739-
format!("{} KB", bytes / KB)
740-
} else {
741-
format!("{bytes} B")
742-
}
743-
}
744-
745727
/// Path where the supervisor binary is mounted inside the agent container.
746728
const SUPERVISOR_MOUNT_PATH: &str = "/opt/openshell/bin";
747729

crates/openshell-driver-podman/src/config.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ use std::str::FromStr;
88

99
/// Default Podman bridge network name.
1010
pub const DEFAULT_NETWORK_NAME: &str = "openshell";
11-
pub const DEFAULT_SANDBOX_PIDS_LIMIT: i64 = 2048;
1211
pub const MACOS_PODMAN_MACHINE_HOST_GATEWAY_IP: &str = "192.168.127.254";
1312

13+
// Re-export the shared default so existing imports inside this crate keep working.
14+
pub use openshell_core::config::DEFAULT_SANDBOX_PIDS_LIMIT;
15+
1416
/// Image pull policy for sandbox and supervisor images.
1517
///
1618
/// Controls when the Podman driver fetches a newer copy of an OCI image

crates/openshell-driver-vm/src/driver.rs

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use oci_client::secrets::RegistryAuth;
2626
use oci_client::{Reference, RegistryOperation};
2727
use openshell_core::progress::{
2828
PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX,
29-
mark_progress_active, mark_progress_complete, mark_progress_detail,
29+
format_bytes, mark_progress_active, mark_progress_complete, mark_progress_detail,
3030
};
3131
use openshell_core::proto::compute::v1::{
3232
CreateSandboxRequest, CreateSandboxResponse, DeleteSandboxRequest, DeleteSandboxResponse,
@@ -4410,24 +4410,6 @@ fn pulling_layer_detail(metadata: &HashMap<String, String>) -> Option<String> {
44104410
))
44114411
}
44124412

4413-
fn format_bytes(bytes: u64) -> String {
4414-
const KB: u64 = 1024;
4415-
const MB: u64 = 1024 * KB;
4416-
const GB: u64 = 1024 * MB;
4417-
4418-
if bytes >= GB {
4419-
#[allow(clippy::cast_precision_loss)]
4420-
let gb = bytes as f64 / GB as f64;
4421-
format!("{gb:.1} GB")
4422-
} else if bytes >= MB {
4423-
format!("{} MB", bytes / MB)
4424-
} else if bytes >= KB {
4425-
format!("{} KB", bytes / KB)
4426-
} else {
4427-
format!("{bytes} B")
4428-
}
4429-
}
4430-
44314413
#[cfg(test)]
44324414
mod tests {
44334415
use super::*;

crates/openshell-sandbox/src/secrets.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
use base64::Engine as _;
5+
use openshell_core::time::now_ms;
56
use std::collections::HashMap;
67
use std::fmt;
78

@@ -26,13 +27,6 @@ fn contains_raw_reserved_marker(value: &str) -> bool {
2627
value.contains(PLACEHOLDER_PREFIX) || value.contains(PROVIDER_ALIAS_MARKER)
2728
}
2829

29-
fn current_time_ms() -> i64 {
30-
std::time::SystemTime::now()
31-
.duration_since(std::time::UNIX_EPOCH)
32-
.map(|duration| i64::try_from(duration.as_millis()).unwrap_or(i64::MAX))
33-
.unwrap_or_default()
34-
}
35-
3630
pub fn contains_reserved_credential_marker(value: &str) -> bool {
3731
if contains_raw_reserved_marker(value) {
3832
return true;
@@ -225,7 +219,7 @@ impl SecretResolver {
225219
let canonical = placeholder_for_env_key(key);
226220
self.by_placeholder.get(&canonical)?
227221
};
228-
if secret.expires_at_ms > 0 && secret.expires_at_ms <= current_time_ms() {
222+
if secret.expires_at_ms > 0 && secret.expires_at_ms <= now_ms() {
229223
tracing::warn!(
230224
location = "resolve_placeholder",
231225
"credential resolution rejected: credential is expired"

0 commit comments

Comments
 (0)