diff --git a/crates/trident/src/engine/manual_rollback/mod.rs b/crates/trident/src/engine/manual_rollback/mod.rs index d9715b72eb..0922676212 100644 --- a/crates/trident/src/engine/manual_rollback/mod.rs +++ b/crates/trident/src/engine/manual_rollback/mod.rs @@ -85,11 +85,25 @@ pub fn check_rollback( } /// Handle manual rollback operations. +/// +/// Mirrors `engine::update::update()`'s `(ExitKind, ServicingType)` return +/// shape: a no-op ("nothing to roll back") reports +/// `(ExitKind::Done, ServicingType::NoActiveServicing)` rather than a bare +/// `ExitKind::Done` that's indistinguishable from a real rollback at the +/// gRPC layer (see `ServicingResponse.servicing_kind` in servicing.proto). pub fn execute_rollback( datastore: &mut DataStore, requested_rollback_kind: ManualRollbackRequestKind, allowed_operations: &Operations, -) -> Result { +) -> Result<(ExitKind, ServicingType), TridentError> { + // Tracks the rollback kind actually staged this call, so the trailing + // "stage completed, finalize not requested this call" return below can + // report it instead of a generic NoActiveServicing. Stays None when + // has_stage() didn't run this call (finalize-only), or when it exited + // early via the no-rollback-available branch below (which returns its + // own explicit NoActiveServicing directly). + let mut staged_rollback_type = None; + // Perform staging if operation is allowed if allowed_operations.has_stage() { match datastore.host_status().servicing_state { @@ -125,7 +139,7 @@ pub fn execute_rollback( Some(rollback_item) => rollback_item, None => { info!("No available rollbacks to perform"); - return Ok(ExitKind::Done); + return Ok((ExitKind::Done, ServicingType::NoActiveServicing)); } }; @@ -133,6 +147,7 @@ pub fn execute_rollback( ManualRollbackKind::Ab => ServicingType::ManualRollbackAb, ManualRollbackKind::Runtime => ServicingType::ManualRollbackRuntime, }; + staged_rollback_type = Some(rollback_type); let engine_context = EngineContext::new(EngineContextParams { spec: requested_rollback.spec.clone(), @@ -203,9 +218,12 @@ pub fn execute_rollback( datastore.host_status().servicing_state, ); - return finalize_result; + return finalize_result.map(|exit_kind| (exit_kind, current_servicing_type)); } - Ok(ExitKind::Done) + Ok(( + ExitKind::Done, + staged_rollback_type.unwrap_or(ServicingType::NoActiveServicing), + )) } /// Stage manual rollback. diff --git a/crates/trident/src/lib.rs b/crates/trident/src/lib.rs index d947ebb807..21b7f1f750 100644 --- a/crates/trident/src/lib.rs +++ b/crates/trident/src/lib.rs @@ -840,7 +840,7 @@ impl Trident { invoke_if_next_is_runtime: bool, invoke_available_ab: bool, allowed_operations: Operations, - ) -> Result { + ) -> Result<(ExitKind, ServicingType), TridentError> { // If host's servicing state is not in Provisioned or ManualRollback*, cannot // execute a rollback. if !matches!( @@ -853,7 +853,7 @@ impl Trident { "Cannot trigger rollback from current state ({:?})", datastore.host_status().servicing_state ); - return Ok(ExitKind::Done); + return Ok((ExitKind::Done, ServicingType::NoActiveServicing)); } let rollback_result = self.execute_and_record_error(datastore, |datastore| { diff --git a/crates/trident/src/main.rs b/crates/trident/src/main.rs index 239cc235e6..07f1339873 100644 --- a/crates/trident/src/main.rs +++ b/crates/trident/src/main.rs @@ -195,12 +195,14 @@ fn run_trident( ab, ref allowed_operations, .. - } => trident.rollback( - &mut datastore, - runtime, - ab, - cli::to_operations(allowed_operations), - ), + } => trident + .rollback( + &mut datastore, + runtime, + ab, + cli::to_operations(allowed_operations), + ) + .map(|(exit_kind, _servicing_type)| exit_kind), Commands::RebuildRaid { .. } => trident .rebuild_raid(&mut datastore) .map(|()| ExitKind::Done), diff --git a/crates/trident/src/server/mod.rs b/crates/trident/src/server/mod.rs index 086f5d81b9..fb8ce91ecd 100644 --- a/crates/trident/src/server/mod.rs +++ b/crates/trident/src/server/mod.rs @@ -18,8 +18,9 @@ use tonic::transport::Server; use tonic_middleware::MiddlewareFor; use trident_proto::v1::{ - commit_service_server::CommitServiceServer, streaming_service_server::StreamingServiceServer, - update_service_server::UpdateServiceServer, version_service_server::VersionServiceServer, + commit_service_server::CommitServiceServer, rollback_service_server::RollbackServiceServer, + streaming_service_server::StreamingServiceServer, update_service_server::UpdateServiceServer, + version_service_server::VersionServiceServer, }; #[cfg(feature = "grpc-preview")] @@ -27,8 +28,8 @@ use trident_proto::v1preview::{ commit_service_server::CommitServiceServer as CommitServiceServerPreview, install_service_server::InstallServiceServer, rebuild_raid_service_server::RebuildRaidServiceServer, - rollback_service_server::RollbackServiceServer, status_service_server::StatusServiceServer, - validation_service_server::ValidationServiceServer, + rollback_service_server::RollbackServiceServer as RollbackServiceServerPreview, + status_service_server::StatusServiceServer, validation_service_server::ValidationServiceServer, }; use crate::{ @@ -223,6 +224,10 @@ async fn server_main_inner( .add_service(MiddlewareFor::new( CommitServiceServer::from_arc(trident_server.clone()), activity_tracker.middleware(), + )) + .add_service(MiddlewareFor::new( + RollbackServiceServer::from_arc(trident_server.clone()), + activity_tracker.middleware(), )); #[cfg(feature = "grpc-preview")] @@ -237,7 +242,7 @@ async fn server_main_inner( activity_tracker.middleware(), )) .add_service(MiddlewareFor::new( - RollbackServiceServer::from_arc(trident_server.clone()), + RollbackServiceServerPreview::from_arc(trident_server.clone()), activity_tracker.middleware(), )) .add_service(MiddlewareFor::new( diff --git a/crates/trident/src/server/tridentserver/services/mod.rs b/crates/trident/src/server/tridentserver/services/mod.rs index 4031cb269d..e58bd12c6c 100644 --- a/crates/trident/src/server/tridentserver/services/mod.rs +++ b/crates/trident/src/server/tridentserver/services/mod.rs @@ -3,6 +3,7 @@ use trident_proto::v1::{RebootHandling, RebootManagement}; use crate::server::tridentserver::RebootDecision; mod commit; +mod rollback; mod streaming; mod update; mod version; @@ -12,8 +13,6 @@ mod install; #[cfg(feature = "grpc-preview")] mod rebuild_raid; #[cfg(feature = "grpc-preview")] -mod rollback; -#[cfg(feature = "grpc-preview")] mod status; #[cfg(feature = "grpc-preview")] mod validation; diff --git a/crates/trident/src/server/tridentserver/services/rollback.rs b/crates/trident/src/server/tridentserver/services/rollback.rs index 4ce1fb9910..8c711e09b4 100644 --- a/crates/trident/src/server/tridentserver/services/rollback.rs +++ b/crates/trident/src/server/tridentserver/services/rollback.rs @@ -1,65 +1,217 @@ use tonic::{async_trait, Request, Response, Status}; -use trident_api::error::{InternalError, TridentError}; +use trident_api::{ + config::{Operation, Operations}, + error::{TridentError, TridentResultExt}, +}; +use trident_proto::v1::{ + rollback_service_server::RollbackService, ManualRollbackKind, RollbackFinalizeRequest, + RollbackRequest, RollbackStageRequest, +}; + +#[cfg(feature = "grpc-preview")] +use trident_api::error::InternalError; +#[cfg(feature = "grpc-preview")] use trident_proto::v1preview::{ - rollback_service_server::RollbackService, CheckRollbackRequest, CheckRollbackResponse, - GetRollbackChainRequest, GetRollbackChainResponse, GetRollbackTargetRequest, - GetRollbackTargetResponse, RollbackFinalizeRequest, RollbackRequest, RollbackStageRequest, + rollback_service_server::RollbackService as RollbackServicePreview, CheckRollbackKind, + CheckRollbackRequest, CheckRollbackResponse, GetRollbackChainRequest, GetRollbackChainResponse, + GetRollbackTargetRequest, GetRollbackTargetResponse, }; -use crate::server::{ - tridentserver::{RebootDecision, ServicingResponseStream}, - TridentServer, +#[cfg(feature = "grpc-preview")] +use crate::engine::manual_rollback::utils::{ + ManualRollbackContext, ManualRollbackKind as InternalManualRollbackKind, + ManualRollbackRequestKind, +}; +use crate::{ + server::{ + tridentserver::{RebootDecision, ServicingResponseStream}, + TridentServer, + }, + DataStore, Trident, }; -#[async_trait] -impl RollbackService for TridentServer { - async fn check_rollback( - &self, - _request: Request, - ) -> Result, Status> { - self.reading_request("check_rollback", || { - Err(TridentError::new(InternalError::Internal( - "Not implemented: check_rollback", - ))) - }) - .await +/// Converts a wire-level `ManualRollbackKind` into the internal +/// `ManualRollbackRequestKind`. Only used by the preview-only +/// `check_rollback` query below. +#[cfg(feature = "grpc-preview")] +fn manual_rollback_request_kind( + kind: ManualRollbackKind, +) -> Result { + match kind { + ManualRollbackKind::Unspecified | ManualRollbackKind::AnyRollbackRequested => { + Ok(ManualRollbackRequestKind::RollbackNext) + } + ManualRollbackKind::AbRollbackRequested => { + Ok(ManualRollbackRequestKind::RollbackAvailableAbUpdate) + } + ManualRollbackKind::RuntimeRollbackRequested => { + Ok(ManualRollbackRequestKind::RollbackOnlyIfNextIsRuntimeUpdate) + } } +} +#[async_trait] +impl RollbackService for TridentServer { type RollbackStream = ServicingResponseStream; async fn rollback( &self, - _request: Request, + request: Request, ) -> Result, Status> { - self.servicing_request("rollback", RebootDecision::Error, || { - Err(TridentError::new(InternalError::Internal( - "Not implemented: rollback", - ))) - }) + let req = request.into_inner(); + let Some(finalize) = req.finalize else { + return Err(Status::invalid_argument("Missing finalize configuration")); + }; + + let data_store_path = self.agent_config.datastore_path().to_owned(); + let logstream = self.logstream.clone(); + let tracestream = self.tracestream.clone(); + + self.servicing_request( + "rollback", + super::reboot_allowed(&finalize.reboot), + move || { + let mut trident: Trident = + Trident::new(None, &data_store_path, logstream, tracestream) + .message("Failed to initialize Trident")?; + + let mut datastore = DataStore::open_or_create(&data_store_path) + .message("Failed to open datastore")?; + + let (invoke_if_next_is_runtime, invoke_available_ab) = + manual_rollback_flags(req.kind())?; + + trident + .rollback( + &mut datastore, + invoke_if_next_is_runtime, + invoke_available_ab, + Operations::all(), + ) + .map(|(exit_kind, servicing_type)| { + (exit_kind, None, Some(servicing_type.into())) + }) + }, + ) } type RollbackStageStream = ServicingResponseStream; async fn rollback_stage( &self, - _request: Request, + request: Request, ) -> Result, Status> { - self.servicing_request("rollback_stage", RebootDecision::Error, || { - Err(TridentError::new(InternalError::Internal( - "Not implemented: rollback_stage", - ))) + let req = request.into_inner(); + + let data_store_path = self.agent_config.datastore_path().to_owned(); + let logstream = self.logstream.clone(); + let tracestream = self.tracestream.clone(); + + self.servicing_request("rollback_stage", RebootDecision::Error, move || { + let mut trident: Trident = Trident::new(None, &data_store_path, logstream, tracestream) + .message("Failed to initialize Trident")?; + + let mut datastore = + DataStore::open_or_create(&data_store_path).message("Failed to open datastore")?; + + let (invoke_if_next_is_runtime, invoke_available_ab) = + manual_rollback_flags(req.kind())?; + + trident + .rollback( + &mut datastore, + invoke_if_next_is_runtime, + invoke_available_ab, + Operation::Stage.into(), + ) + .map(|(exit_kind, servicing_type)| (exit_kind, None, Some(servicing_type.into()))) }) } type RollbackFinalizeStream = ServicingResponseStream; async fn rollback_finalize( &self, - _request: Request, + request: Request, ) -> Result, Status> { - self.servicing_request("rollback_finalize", RebootDecision::Error, || { - Err(TridentError::new(InternalError::Internal( - "Not implemented: rollback_finalize", - ))) + let finalize = request.into_inner(); + + let data_store_path = self.agent_config.datastore_path().to_owned(); + let logstream = self.logstream.clone(); + let tracestream = self.tracestream.clone(); + + self.servicing_request( + "rollback_finalize", + super::reboot_allowed(&finalize.reboot), + move || { + let mut trident: Trident = + Trident::new(None, &data_store_path, logstream, tracestream) + .message("Failed to initialize Trident")?; + + let mut datastore = DataStore::open_or_create(&data_store_path) + .message("Failed to open datastore")?; + + // The rollback kind was already resolved and staged by + // RollbackStage/Rollback, so finalize just needs to proceed + // with whatever is currently staged. RollbackNext resolves to + // the currently-staged rollback because staging already + // narrowed the chain. + trident + .rollback(&mut datastore, false, false, Operation::Finalize.into()) + .map(|(exit_kind, servicing_type)| { + (exit_kind, None, Some(servicing_type.into())) + }) + }, + ) + } +} + +/// Converts a wire-level `ManualRollbackKind` into the +/// `(invoke_if_next_is_runtime, invoke_available_ab)` flag pair expected by +/// `Trident::rollback`. +fn manual_rollback_flags(kind: ManualRollbackKind) -> Result<(bool, bool), TridentError> { + match kind { + ManualRollbackKind::Unspecified | ManualRollbackKind::AnyRollbackRequested => { + Ok((false, false)) + } + ManualRollbackKind::AbRollbackRequested => Ok((false, true)), + ManualRollbackKind::RuntimeRollbackRequested => Ok((true, false)), + } +} + +#[cfg(feature = "grpc-preview")] +#[async_trait] +impl RollbackServicePreview for TridentServer { + async fn check_rollback( + &self, + request: Request, + ) -> Result, Status> { + let req = request.into_inner(); + let data_store_path = self.agent_config.datastore_path().to_owned(); + + self.reading_request("check_rollback", move || { + let requested_kind = manual_rollback_request_kind(req.kind())?; + + let datastore = + DataStore::open_or_create(&data_store_path).message("Failed to open datastore")?; + + let host_statuses = datastore + .get_host_statuses() + .message("Failed to get datastore HostStatus entries")?; + let rollback_context = ManualRollbackContext::new(&host_statuses) + .message("Failed to create manual rollback context")?; + let kind = rollback_context.get_requested_rollback(requested_kind)?; + + Ok(CheckRollbackResponse { + kind: match kind.map(|item| item.kind) { + None => CheckRollbackKind::NoRollbackAvailable, + Some(InternalManualRollbackKind::Ab) => CheckRollbackKind::AbRollbackExpected, + Some(InternalManualRollbackKind::Runtime) => { + CheckRollbackKind::RuntimeRollbackExpected + } + } + .into(), + }) }) + .await } async fn get_rollback_chain( diff --git a/proto/trident/v1/rollback_service.proto b/proto/trident/v1/rollback_service.proto new file mode 100644 index 0000000000..17e653fe60 --- /dev/null +++ b/proto/trident/v1/rollback_service.proto @@ -0,0 +1,57 @@ +// Proto file defining the stable RollbackService and related messages. +// +// This is the stable (v1) subset of the rollback contract: Rollback, +// RollbackStage, and RollbackFinalize, the operations trident-acl-agent +// depends on for AB-update rollback. CheckRollback, GetRollbackChain, and +// GetRollbackTarget remain in trident.v1preview (see rollback_service.proto +// there). CheckRollback was briefly promoted here but was demoted back to +// preview once RollbackStage/RollbackFinalize/Rollback started reporting +// ServicingKind::NoneRequired for a no-op (same as every other servicing +// RPC), which removed trident-acl-agent's only reason to call it - see +// ServicingResponse.servicing_kind in servicing.proto. + +syntax = "proto3"; + +package trident.v1; + +import "trident/v1/servicing.proto"; + +// RollbackService provides methods for performing OS rollbacks. +service RollbackService { + // Rollback performs a rollback. + rpc Rollback(RollbackRequest) returns (stream ServicingResponse); + + // RollbackStage performs the stage operation of a rollback. + rpc RollbackStage(RollbackStageRequest) returns (stream ServicingResponse); + + // RollbackFinalize performs the finalize operation of a rollback. + rpc RollbackFinalize(RollbackFinalizeRequest) returns (stream ServicingResponse); +} + +enum ManualRollbackKind { + MANUAL_ROLLBACK_KIND_UNSPECIFIED = 0; + // No specific rollback kind was requested + ANY_ROLLBACK_REQUESTED = 1; + // Rollback of an A/B update was requested + AB_ROLLBACK_REQUESTED = 2; + // Rollback of a runtime update was requested + RUNTIME_ROLLBACK_REQUESTED = 3; +} + +message RollbackRequest { + // The rollback kind parameter for the servicing operation. + ManualRollbackKind kind = 1; + + // Reboot management configuration for the rollback operation. + RollbackFinalizeRequest finalize = 2; +} + +message RollbackStageRequest { + // The rollback kind parameter for the servicing operation. + ManualRollbackKind kind = 1; +} + +message RollbackFinalizeRequest { + // Reboot management configuration for the rollback operation. + RebootManagement reboot = 1; +} diff --git a/proto/trident/v1preview/rollback_service.proto b/proto/trident/v1preview/rollback_service.proto index bf049b565c..33ababc78f 100644 --- a/proto/trident/v1preview/rollback_service.proto +++ b/proto/trident/v1preview/rollback_service.proto @@ -1,25 +1,28 @@ -// Proto file defining the RollbackService and related messages. +// Proto file defining the preview-only extensions to RollbackService. +// +// Rollback, RollbackStage, and RollbackFinalize have been promoted to the +// stable trident.v1.RollbackService (see trident/v1/rollback_service.proto). +// CheckRollback was promoted alongside them but has been demoted back here: +// once RollbackStage/RollbackFinalize/Rollback started reporting +// ServicingKind::NoneRequired for a no-op the same way every other +// servicing RPC does (see CompletedResponse.servicing_kind on +// trident.v1.ServicingResponse), trident-acl-agent no longer needs a +// separate precondition query to detect "nothing to roll back" - it reads +// servicing_kind off the RollbackStage response it already makes. This +// file now retains all three still-preview query operations: +// CheckRollback, GetRollbackChain, and GetRollbackTarget. syntax = "proto3"; package trident.v1preview; -import "trident/v1/servicing.proto"; +import "trident/v1/rollback_service.proto"; -// RollbackService provides methods for performing OS rollbacks. +// RollbackService provides preview-only query methods for OS rollbacks. service RollbackService { // CheckRollback checks what type of rollback would be performed. rpc CheckRollback(CheckRollbackRequest) returns (CheckRollbackResponse); - // Rollback performs a rollback. - rpc Rollback(RollbackRequest) returns (stream trident.v1.ServicingResponse); - - // RollbackStage performs the stage operation of a rollback. - rpc RollbackStage(RollbackStageRequest) returns (stream trident.v1.ServicingResponse); - - // RollbackFinalize performs the finalize operation of a rollback. - rpc RollbackFinalize(RollbackFinalizeRequest) returns (stream trident.v1.ServicingResponse); - // GetRollbackChain returns list of available rollbacks. rpc GetRollbackChain(GetRollbackChainRequest) returns (GetRollbackChainResponse); @@ -27,21 +30,6 @@ service RollbackService { rpc GetRollbackTarget(GetRollbackTargetRequest) returns (GetRollbackTargetResponse); } -enum ManualRollbackKind { - MANUAL_ROLLBACK_KIND_UNSPECIFIED = 0; - // No specific rollback kind was requested - ANY_ROLLBACK_REQUESTED = 1; - // Rollback of an A/B update was requested - AB_ROLLBACK_REQUESTED = 2; - // Rollback of a runtime update was requested - RUNTIME_ROLLBACK_REQUESTED = 3; -} - -message CheckRollbackRequest { - // The rollback kind parameter for the servicing operation. - ManualRollbackKind kind = 1; -} - enum CheckRollbackKind { CHECK_ROLLBACK_KIND_UNSPECIFIED = 0; // No rollbacks are available @@ -52,27 +40,14 @@ enum CheckRollbackKind { RUNTIME_ROLLBACK_EXPECTED = 3; } -message CheckRollbackResponse { - // The rollback kind as a string. - CheckRollbackKind kind = 1; -} - -message RollbackRequest { - // The rollback kind parameter for the servicing operation. - ManualRollbackKind kind = 1; - - // Reboot management configuration for the rollback operation. - RollbackFinalizeRequest finalize = 2; -} - -message RollbackStageRequest { +message CheckRollbackRequest { // The rollback kind parameter for the servicing operation. - ManualRollbackKind kind = 1; + trident.v1.ManualRollbackKind kind = 1; } -message RollbackFinalizeRequest { - // Reboot management configuration for the rollback operation. - trident.v1.RebootManagement reboot = 1; +message CheckRollbackResponse { + // The rollback kind as a string. + CheckRollbackKind kind = 1; } message GetRollbackChainRequest {}