From 43ca25e55b5fdcf57ca47560bfd060eba49efacf Mon Sep 17 00:00:00 2001 From: mbecker20 Date: Sun, 5 Jul 2026 12:18:40 -0700 Subject: [PATCH] allow all builders build to cancel during build --- bin/core/src/api/execute/build.rs | 30 +++---- bin/periphery/src/api/build/mod.rs | 121 +++++++++++++++++++------- bin/periphery/src/api/mod.rs | 1 + bin/periphery/src/state.rs | 8 ++ client/periphery/rs/src/api/build.rs | 13 ++- ui/src/resources/build/executions.tsx | 55 +++++------- 6 files changed, 144 insertions(+), 84 deletions(-) diff --git a/bin/core/src/api/execute/build.rs b/bin/core/src/api/execute/build.rs index be539d39f..bde3bee55 100644 --- a/bin/core/src/api/execute/build.rs +++ b/bin/core/src/api/execute/build.rs @@ -28,7 +28,7 @@ use komodo_client::{ alert::{Alert, AlertData, SeverityLevel}, all_logs_success, build::{Build, BuildConfig}, - builder::{Builder, BuilderConfig}, + builder::Builder, deployment::DeploymentState, komodo_timestamp, optional_string, permission::PermissionLevel, @@ -175,9 +175,6 @@ impl Resolve for RunBuild { let builder = resource::get::(&build.config.builder_id).await?; - let is_server_builder = - matches!(&builder.config, BuilderConfig::Server(_)); - tokio::spawn(async move { let poll = async { loop { @@ -186,19 +183,13 @@ impl Resolve for RunBuild { id = cancel_recv.recv() => id? }; if incoming_build_id == build_id { - if is_server_builder { - update.push_error_log("Cancel acknowledged", "Build cancellation is not possible on server builders at this time. Use an AWS builder to enable this feature."); - } else { - update.push_simple_log("Cancel acknowledged", "The build cancellation has been queued, it may still take some time."); - } + update.push_simple_log("Cancel acknowledged", "The build cancellation has been queued, it may still take some time."); update.finalize(); let id = update.id.clone(); if let Err(e) = update_update(update).await { warn!("Failed to modify Update {id} on db | {e:#}"); } - if !is_server_builder { - cancel_clone.cancel(); - } + cancel_clone.cancel(); return Ok(()); } } @@ -273,11 +264,11 @@ impl Resolve for RunBuild { replacers: Default::default(), }) => res, _ = cancel.cancelled() => { - debug!("Build cancelled during clone, cleaning up builder"); - update.push_error_log("Build cancelled", String::from("user cancelled build during repo clone")); + debug!("Build cancelled during repo clone, cleaning up builder"); + update.push_error_log("Build cancelled", String::from("Build cancelled during repo clone")); cleanup_builder_instance(periphery, cleanup_data, &mut update) .await; - info!("Builder cleaned up"); + debug!("Builder cleaned up"); return handle_early_return(update, build.id, build.name, true).await }, }; @@ -323,7 +314,14 @@ impl Resolve for RunBuild { }) => res.context("Failed at call to Periphery to build"), _ = cancel.cancelled() => { info!("Build cancelled during build, cleaning up builder"); - update.push_error_log("Build cancelled", String::from("User cancelled build during docker build")); + if let Err(e) = periphery.request(api::build::CancelBuild { + id: build.id.clone() + }) + .await + .context("Failed to cancel build execution on Server") { + update.push_error_log("Cancel Build", format_serror(&e.into())); + } + update.push_error_log("Build Cancelled", String::from("User cancelled build during image build step")); cleanup_builder_instance(periphery, cleanup_data, &mut update) .await; return handle_early_return(update, build.id, build.name, true).await diff --git a/bin/periphery/src/api/build/mod.rs b/bin/periphery/src/api/build/mod.rs index 21368818d..6bba92b61 100644 --- a/bin/periphery/src/api/build/mod.rs +++ b/bin/periphery/src/api/build/mod.rs @@ -11,7 +11,7 @@ use command::{ use formatting::format_serror; use interpolate::Interpolator; use komodo_client::entities::{ - EnvironmentVar, all_logs_success, + EnvironmentVar, NoData, all_logs_success, build::{Build, BuildConfig}, environment_vars_from_str, optional_string, to_path_compatible_name, @@ -19,17 +19,19 @@ use komodo_client::entities::{ }; use mogh_resolver::Resolve; use periphery_client::api::build::{ - self, GetDockerfileContentsOnHost, + self, CancelBuild, GetDockerfileContentsOnHost, GetDockerfileContentsOnHostResponse, PruneBuilders, PruneBuildx, WriteDockerfileContentsToHost, }; use tokio::fs; +use tokio_util::sync::CancellationToken; use tracing::Instrument; use crate::{ config::periphery_config, docker::docker_login, helpers::{format_extra_args, format_labels}, + state::build_cancel_cache, }; mod helpers; @@ -133,7 +135,8 @@ impl Resolve for build::Build { fields( id = args.id.to_string(), core = args.core, - build = self.build.name, + build_id = self.build.id, + build_name = self.build.name, repo = self.repo.as_ref().map(|repo| &repo.name), ) )] @@ -263,6 +266,11 @@ impl Resolve for build::Build { } }; + let cancel = CancellationToken::new(); + build_cancel_cache() + .insert(build.id.clone(), cancel.clone()) + .await; + // Pre Build if !pre_build.is_none() { let pre_build_path = build_path.join(&pre_build.path); @@ -270,7 +278,9 @@ impl Resolve for build::Build { if let Some(log) = run_komodo_command_with_sanitization( "Pre Build", &pre_build.command, - CommandOptions::default().path(pre_build_path.as_path()), + CommandOptions::default() + .path(pre_build_path.as_path()) + .cancel(cancel.clone()), if pre_build.shell_mode { KomodoCommandMode::Shell } else { @@ -284,53 +294,73 @@ impl Resolve for build::Build { let success = log.success; logs.push(log); if !success { + build_cancel_cache().remove(&build.id).await; return Ok(logs); } } } // Get command parts + // Do this in fallible block to ensure + // cancel token removed from cache on failure. + let command = async { + // Add VERSION to build args (if not already there) + let mut build_args = environment_vars_from_str(build_args) + .context("Invalid build_args")?; + if !build_args.iter().any(|a| a.variable == "VERSION") { + build_args.push(EnvironmentVar { + variable: String::from("VERSION"), + value: build.config.version.to_string(), + }); + } + let build_args = parse_build_args(&build_args); - // Add VERSION to build args (if not already there) - let mut build_args = environment_vars_from_str(build_args) - .context("Invalid build_args")?; - if !build_args.iter().any(|a| a.variable == "VERSION") { - build_args.push(EnvironmentVar { - variable: String::from("VERSION"), - value: build.config.version.to_string(), - }); - } - let build_args = parse_build_args(&build_args); + let secret_args = environment_vars_from_str(secret_args) + .context("Invalid secret_args")?; + let command_secret_args = + parse_secret_args(&secret_args, &build_path).await?; - let secret_args = environment_vars_from_str(secret_args) - .context("Invalid secret_args")?; - let command_secret_args = - parse_secret_args(&secret_args, &build_path).await?; + let labels = format_labels( + &environment_vars_from_str(labels) + .context("Invalid labels")?, + ); - let labels = format_labels( - &environment_vars_from_str(labels).context("Invalid labels")?, - ); + let extra_args = format_extra_args(extra_args); - let extra_args = format_extra_args(extra_args); + let buildx = if *use_buildx { " buildx" } else { "" }; - let buildx = if *use_buildx { " buildx" } else { "" }; + let image_tags = build + .get_image_tags_as_arg( + commit_hash.as_deref(), + &additional_tags, + ) + .context("Failed to parse image tags into command")?; - let image_tags = build - .get_image_tags_as_arg(commit_hash.as_deref(), &additional_tags) - .context("Failed to parse image tags into command")?; + let maybe_push = if should_push { " --push" } else { "" }; - let maybe_push = if should_push { " --push" } else { "" }; + // Construct command + let command = format!( + "docker{buildx} build{build_args}{command_secret_args}{extra_args}{labels}{image_tags}{maybe_push} -f {dockerfile_path} .", + ); - // Construct command - let command = format!( - "docker{buildx} build{build_args}{command_secret_args}{extra_args}{labels}{image_tags}{maybe_push} -f {dockerfile_path} .", - ); + anyhow::Ok(command) + }.await; + + let command = match command { + Ok(command) => command, + Err(e) => { + build_cancel_cache().remove(&build.id).await; + return Err(e); + } + }; let span = info_span!("RunDockerBuild"); if let Some(build_log) = run_komodo_command_with_sanitization( "Docker Build", command, - CommandOptions::default().path(build_path.as_path()), + CommandOptions::default() + .path(build_path.as_path()) + .cancel(cancel), KomodoCommandMode::Shell, &replacers, ) @@ -340,12 +370,39 @@ impl Resolve for build::Build { logs.push(build_log); }; + build_cancel_cache().remove(&build.id).await; + Ok(logs) } } // +impl Resolve for CancelBuild { + #[instrument( + "CancelBuild", + skip_all, + fields( + build_id = self.id, + id = args.id.to_string(), + core = args.core, + ) + )] + async fn resolve( + self, + args: &crate::api::Args, + ) -> anyhow::Result { + build_cancel_cache() + .get(&self.id) + .await + .context("No running build found")? + .cancel(); + Ok(NoData {}) + } +} + +// + impl Resolve for PruneBuilders { #[instrument( "PruneBuilders", diff --git a/bin/periphery/src/api/mod.rs b/bin/periphery/src/api/mod.rs index 8aa54c4d4..bf4382aab 100644 --- a/bin/periphery/src/api/mod.rs +++ b/bin/periphery/src/api/mod.rs @@ -68,6 +68,7 @@ pub enum PeripheryRequest { GetDockerfileContentsOnHost(GetDockerfileContentsOnHost), WriteDockerfileContentsToHost(WriteDockerfileContentsToHost), Build(Build), + CancelBuild(CancelBuild), PruneBuilders(PruneBuilders), PruneBuildx(PruneBuildx), diff --git a/bin/periphery/src/state.rs b/bin/periphery/src/state.rs index 08da16458..c2457a900 100644 --- a/bin/periphery/src/state.rs +++ b/bin/periphery/src/state.rs @@ -341,3 +341,11 @@ pub async fn host_public_ip() -> Option<&'static String> { .await .as_ref() } + +type CancelCache = CloneCache; + +/// Maps build id => CancellationToken +pub fn build_cancel_cache() -> &'static CancelCache { + static BUILD_CANCEL_CACHE: OnceLock = OnceLock::new(); + BUILD_CANCEL_CACHE.get_or_init(Default::default) +} diff --git a/client/periphery/rs/src/api/build.rs b/client/periphery/rs/src/api/build.rs index d4e8fb55e..f42257d8e 100644 --- a/client/periphery/rs/src/api/build.rs +++ b/client/periphery/rs/src/api/build.rs @@ -1,9 +1,11 @@ use komodo_client::entities::{ - FileContents, repo::Repo, update::Log, + FileContents, NoData, repo::Repo, update::Log, }; use mogh_resolver::Resolve; use serde::{Deserialize, Serialize}; +// + #[derive(Serialize, Deserialize, Debug, Clone, Resolve)] #[response(BuildResponse)] #[error(anyhow::Error)] @@ -29,6 +31,15 @@ pub type BuildResponse = Vec; // +#[derive(Serialize, Deserialize, Debug, Clone, Resolve)] +#[response(NoData)] +#[error(anyhow::Error)] +pub struct CancelBuild { + pub id: String, +} + +// + /// Get the dockerfile contents on the host, for builds using /// `files_on_host`. #[derive(Debug, Clone, Serialize, Deserialize, Resolve)] diff --git a/ui/src/resources/build/executions.tsx b/ui/src/resources/build/executions.tsx index 6894b0c71..ae6251393 100644 --- a/ui/src/resources/build/executions.tsx +++ b/ui/src/resources/build/executions.tsx @@ -1,7 +1,10 @@ import { Types } from "komodo_client"; -import { useExecute, usePermissions, useRead } from "@/lib/hooks"; -import { useBuilder } from "../builder"; -import { useBuild } from "."; +import { + useExecute, + useIsCancelling, + usePermissions, + useRead, +} from "@/lib/hooks"; import { ConfirmButton } from "mogh_ui"; import { ICONS } from "@/lib/icons"; @@ -12,45 +15,28 @@ export function RunBuild({ id }: { id: string }) { { build: id }, { refetchInterval: 5_000 }, ).data?.building; - const updates = useRead("ListUpdates", { - query: { - "target.type": "Build", - "target.id": id, - }, - }).data; - const { mutate: runBuild, isPending: runPending } = useExecute("RunBuild"); - const { mutate: cancelBuild, isPending: cancelPending } = + const { mutate: run, isPending: runPending } = useExecute("RunBuild"); + const { mutate: cancel, isPending: cancelPending } = useExecute("CancelBuild"); - const build = useBuild(id); - const builder = useBuilder(build?.info.builder_id); - const canCancel = builder?.info.builder_type === "Aws"; + const cancelling = useIsCancelling( + { type: "Build", id }, + Types.Operation.RunBuild, + Types.Operation.CancelBuild, + ); // make sure hidden without perms. - // not usually necessary, but this button also used in deployment actions. + // not usually necessary as execution area hidden without execute + // on the resource, but this button is also used in deployment executions. if (!canExecute) return null; - // updates come in in descending order, so 'find' will find latest update matching operation - const latestBuild = updates?.updates.find( - (u) => u.operation === Types.Operation.RunBuild, - ); - const latestCancel = updates?.updates.find( - (u) => u.operation === Types.Operation.CancelBuild, - ); - const cancelDisabled = - !canCancel || - cancelPending || - (latestCancel && latestBuild - ? latestCancel!.start_ts > latestBuild!.start_ts - : false); - - if (building && canCancel) { + if (building) { return ( } - onClick={() => cancelBuild({ build: id })} - disabled={cancelDisabled} + onClick={() => cancel({ build: id })} + loading={cancelPending || cancelling} > Cancel Build @@ -59,9 +45,8 @@ export function RunBuild({ id }: { id: string }) { return ( } - loading={runPending || building} - onClick={() => runBuild({ build: id })} - disabled={runPending || building} + onClick={() => run({ build: id })} + loading={runPending} > Build