diff --git a/plane/src/drone/runtime/docker/commands.rs b/plane/src/drone/runtime/docker/commands.rs index 5cb4f7e5b..665d1c26a 100644 --- a/plane/src/drone/runtime/docker/commands.rs +++ b/plane/src/drone/runtime/docker/commands.rs @@ -1,4 +1,4 @@ -use super::{backend_id_to_container_id, types::ContainerId, DockerRuntime}; +use super::{types::ContainerId, DockerRuntime}; use crate::{ names::BackendName, protocol::AcquiredKey, @@ -52,13 +52,8 @@ pub async fn pull_image( Ok(()) } -pub fn create_labels(backend_id: &BackendName) -> HashMap { - vec![( - super::PLANE_DOCKER_LABEL.to_string(), - backend_id.to_string(), - )] - .into_iter() - .collect() +fn create_labels() -> HashMap { + HashMap::from([(super::PLANE_DOCKER_LABEL.to_string(), "true".to_string())]) } fn create_port_bindings() -> HashMap>> { @@ -208,7 +203,7 @@ pub fn get_container_config_from_executor_config( Ok(bollard::container::Config { image: Some(exec_config.image.clone()), - labels: backend_id.map(create_labels), + labels: Some(create_labels()), env: Some(env), exposed_ports: Some( vec![(format!("{}/tcp", CONTAINER_PORT), HashMap::new())] @@ -266,7 +261,7 @@ pub async fn run_container( acquired_key: Option<&AcquiredKey>, static_token: Option<&BearerToken>, ) -> Result { - let container_id = backend_id_to_container_id(backend_id); + let container_id: ContainerId = backend_id.into(); let options = bollard::container::CreateContainerOptions { name: container_id.to_string(), diff --git a/plane/src/drone/runtime/docker/metrics.rs b/plane/src/drone/runtime/docker/metrics.rs index 5d0e5d611..f870f6e37 100644 --- a/plane/src/drone/runtime/docker/metrics.rs +++ b/plane/src/drone/runtime/docker/metrics.rs @@ -1,4 +1,4 @@ -use super::{backend_id_to_container_id, types::ContainerId, MetricsCallback}; +use super::{types::ContainerId, MetricsCallback}; use crate::{database::backend::BackendMetricsMessage, names::BackendName}; use bollard::{container::StatsOptions, Docker}; use futures_util::Stream; @@ -22,7 +22,7 @@ pub async fn metrics_loop( docker: Docker, callback: Arc>>, ) { - let container_id = backend_id_to_container_id(&backend_id); + let container_id = ContainerId::from(&backend_id); let mut stream = stream_metrics(&docker, &container_id); diff --git a/plane/src/drone/runtime/docker/mod.rs b/plane/src/drone/runtime/docker/mod.rs index d1173aede..b4e72cf38 100644 --- a/plane/src/drone/runtime/docker/mod.rs +++ b/plane/src/drone/runtime/docker/mod.rs @@ -44,10 +44,6 @@ mod wait_backend; /// The existence of this label is used to determine whether a container is managed by Plane. const PLANE_DOCKER_LABEL: &str = "dev.plane.backend"; -pub fn backend_id_to_container_id(backend_id: &BackendName) -> ContainerId { - ContainerId::from(format!("plane-{}", backend_id)) -} - #[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct DockerRuntimeConfig { pub runtime: Option, @@ -109,18 +105,22 @@ async fn events_loop( tracing::warn!("Received event without attributes."); continue; }; - let Some(backend_id) = attributes.get(PLANE_DOCKER_LABEL) else { + if !attributes.contains_key(PLANE_DOCKER_LABEL) { tracing::warn!(?e.actor, "Ignoring event without Plane backend ID label."); continue; }; + let Some(container_id) = attributes.get("name").cloned().map(ContainerId::from) else { + tracing::warn!(?e.actor, "Ignoring event without name attribute."); + continue; + }; + let Ok(backend_id) = BackendName::try_from(container_id.clone()) else { + tracing::warn!(?e.actor, ?container_id, "Ignoring event with invalid backend ID."); + continue; + }; if e.action.as_deref() == Some("start") { - tracing::info!(backend_id = backend_id.as_value(), "Received start event."); + tracing::info!(?backend_id, "Received start event."); - let Ok(backend_id) = BackendName::try_from(backend_id.to_string()) else { - tracing::warn!(?e.actor, "Ignoring start event with invalid backend ID."); - continue; - }; let docker = docker.clone(); let metrics_callback = metrics_callback.clone(); tracing::info!(%backend_id, "Spawning metrics loop."); @@ -135,17 +135,6 @@ async fn events_loop( let exit_code = attributes.get("exitCode"); let exit_code = exit_code.and_then(|s| s.parse::().ok()); - let backend_id = match BackendName::try_from(backend_id.to_string()) { - Ok(backend_id) => backend_id, - Err(err) => { - tracing::warn!( - ?err, - backend_id = backend_id.as_value(), - "Ignoring event with invalid backend ID." - ); - continue; - } - }; tracing::info!( exit_code, @@ -203,7 +192,7 @@ impl Runtime for DockerRuntime { } async fn terminate(&self, backend_id: &BackendName, hard: bool) -> Result { - let container_id = backend_id_to_container_id(backend_id); + let container_id: ContainerId = backend_id.into(); let result = if hard { self.docker diff --git a/plane/src/drone/runtime/docker/types.rs b/plane/src/drone/runtime/docker/types.rs index 5096eaad9..eb2a563ad 100644 --- a/plane/src/drone/runtime/docker/types.rs +++ b/plane/src/drone/runtime/docker/types.rs @@ -1,3 +1,4 @@ +use crate::names::BackendName; use serde::{Deserialize, Serialize}; use std::fmt::Display; @@ -10,6 +11,12 @@ impl From for ContainerId { } } +impl From<&BackendName> for ContainerId { + fn from(backend_id: &BackendName) -> Self { + Self(format!("plane-{}", backend_id)) + } +} + impl ContainerId { pub fn as_str(&self) -> &str { &self.0 @@ -21,3 +28,16 @@ impl Display for ContainerId { write!(f, "{}", &self.0) } } +#[cfg(test)] +mod tests { + use super::*; + use crate::names::{BackendName, Name}; + + #[test] + fn test_backend_name_container_id_roundtrip() { + let original_name = BackendName::new_random(); + let container_id: ContainerId = ContainerId::from(&original_name); + let roundtrip_name = BackendName::try_from(container_id).expect("Should convert back"); + assert_eq!(original_name, roundtrip_name); + } +} diff --git a/plane/src/names.rs b/plane/src/names.rs index b71eb13aa..ae842adfd 100644 --- a/plane/src/names.rs +++ b/plane/src/names.rs @@ -1,4 +1,4 @@ -use crate::types::NodeKind; +use crate::{drone::runtime::docker::types::ContainerId, types::NodeKind}; use clap::error::ErrorKind; use serde::{Deserialize, Serialize}; use std::fmt::{Debug, Display}; @@ -163,6 +163,19 @@ entity_name!(DroneName, Some("dr")); entity_name!(AcmeDnsServerName, Some("ns")); entity_name!(BackendActionName, Some("ak")); +impl TryFrom for BackendName { + type Error = NameError; + + fn try_from(value: ContainerId) -> Result { + value + .as_str() + .strip_prefix("plane-") + .ok_or_else(|| NameError::InvalidPrefix(value.to_string(), "plane-".to_string()))? + .to_string() + .try_into() + } +} + pub trait NodeName: Name { fn kind(&self) -> NodeKind; } @@ -278,4 +291,16 @@ mod tests { let name = "co-".to_string() + &"a".repeat(100 - 3); assert_eq!(Err(NameError::TooLong(100)), ControllerName::try_from(name)); } + + #[test] + fn test_backend_name_from_invalid_container_id() { + let container_id = ContainerId::from("invalid-123".to_string()); + assert_eq!( + Err(NameError::InvalidPrefix( + "invalid-123".to_string(), + "plane-".to_string() + )), + BackendName::try_from(container_id) + ); + } }