Skip to content

Commit 085e666

Browse files
committed
fix(supervisor): report isolation backend Attach refusals to the gateway
When an isolation backend refused at Attach, the supervisor exited without telling the gateway. The configuration it had reported Accepted before Attach stayed accepted, so the sandbox showed ConfigurationReady: True, and the revision it wrote at startup stayed Pending with no error. ReportSandboxConfigurationRequest gains refused_before_start. On a Denied or Attach refusal the supervisor reports the generation it accepted as Rejected with the flag set, and the gateway records the configuration as rejected with gateway-authored text. Unflagged reports keep today's behavior, so a runtime that fails to apply the generation it is running stays accepted. The supervisor also marks the revision this start wrote as Failed with the backend's message. A revision the user wrote and transient errors are left alone. Closes #4739 Signed-off-by: Shiju <shiju@nvidia.com>
1 parent 7417f75 commit 085e666

10 files changed

Lines changed: 517 additions & 22 deletions

File tree

‎crates/openshell-core/src/grpc_client.rs‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,11 +1074,6 @@ pub async fn sync_policy_and_fetch_snapshot(
10741074

10751075
/// Report an exact runtime configuration generation. Pending registration uses
10761076
/// the snapshot's instance fence; retain that snapshot across registration retries.
1077-
#[tracing::instrument(
1078-
name = "supervisor.gateway.report_sandbox_configuration",
1079-
skip_all,
1080-
fields(otel.kind = "client", otel.status_code = tracing::field::Empty)
1081-
)]
10821077
pub async fn report_sandbox_configuration(
10831078
endpoint: &str,
10841079
sandbox_id: &str,
@@ -1087,10 +1082,9 @@ pub async fn report_sandbox_configuration(
10871082
state: crate::proto::ConfigurationAdmissionState,
10881083
error: &str,
10891084
) -> Result<()> {
1090-
let status = ClientSpanStatus::current();
1091-
let mut client = connect(endpoint).await?;
1092-
client
1093-
.report_sandbox_configuration(crate::proto::ReportSandboxConfigurationRequest {
1085+
report_sandbox_configuration_request(
1086+
endpoint,
1087+
crate::proto::ReportSandboxConfigurationRequest {
10941088
sandbox_id: sandbox_id.to_string(),
10951089
expected_instance_id: snapshot.map_or_else(String::new, |snapshot| {
10961090
snapshot.configuration_instance_id.clone()
@@ -1106,7 +1100,27 @@ pub async fn report_sandbox_configuration(
11061100
.map_or(0, |snapshot| snapshot.provider_env_revision),
11071101
error: error.to_string(),
11081102
}),
1109-
})
1103+
refused_before_start: false,
1104+
},
1105+
)
1106+
.await
1107+
}
1108+
1109+
/// Send a configuration report the caller built itself, such as a refusal of
1110+
/// the generation this instance already accepted.
1111+
#[tracing::instrument(
1112+
name = "supervisor.gateway.report_sandbox_configuration",
1113+
skip_all,
1114+
fields(otel.kind = "client", otel.status_code = tracing::field::Empty)
1115+
)]
1116+
pub async fn report_sandbox_configuration_request(
1117+
endpoint: &str,
1118+
request: crate::proto::ReportSandboxConfigurationRequest,
1119+
) -> Result<()> {
1120+
let status = ClientSpanStatus::current();
1121+
let mut client = connect(endpoint).await?;
1122+
client
1123+
.report_sandbox_configuration(request)
11101124
.await
11111125
.map_err(grpc_status_error)?;
11121126
status.finish(Ok(()))

‎crates/openshell-server/src/grpc/policy.rs‎

Lines changed: 166 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4990,6 +4990,26 @@ fn configuration_failure_diagnostic(error: &Status) -> &'static str {
49904990
}
49914991
}
49924992

4993+
/// Both admissions name the same instance and configuration generation.
4994+
fn same_generation(
4995+
current: &openshell_core::proto::SandboxConfigurationAdmission,
4996+
reported: &openshell_core::proto::SandboxConfigurationAdmission,
4997+
) -> bool {
4998+
(
4999+
&current.instance_id,
5000+
current.policy_version,
5001+
&current.policy_hash,
5002+
current.config_revision,
5003+
current.provider_env_revision,
5004+
) == (
5005+
&reported.instance_id,
5006+
reported.policy_version,
5007+
&reported.policy_hash,
5008+
reported.config_revision,
5009+
reported.provider_env_revision,
5010+
)
5011+
}
5012+
49935013
fn configuration_generation_matches(
49945014
admission: &openshell_core::proto::SandboxConfigurationAdmission,
49955015
config: &GetSandboxConfigResponse,
@@ -5027,6 +5047,12 @@ pub(super) async fn handle_report_sandbox_configuration(
50275047
if reported == ConfigurationAdmissionState::Unspecified {
50285048
return Err(Status::invalid_argument("admission state is required"));
50295049
}
5050+
let refused_before_start = request.get_ref().refused_before_start;
5051+
if refused_before_start && reported != ConfigurationAdmissionState::Rejected {
5052+
return Err(Status::invalid_argument(
5053+
"refused_before_start requires a rejected admission",
5054+
));
5055+
}
50305056
let Some(_mutation_guard) = state
50315057
.compute
50325058
.sandbox_mutation_guard_by_id(&sandbox_id)
@@ -5111,10 +5137,16 @@ pub(super) async fn handle_report_sandbox_configuration(
51115137
if let Some(current) = current
51125138
&& current.state == i32::from(ConfigurationAdmissionState::Accepted)
51135139
{
5114-
// A rejected desired update does not invalidate an accepted runtime.
5115-
let error = admission.error;
5116-
admission = current.clone();
5117-
admission.error = error;
5140+
if refused_before_start && same_generation(current, &admission) {
5141+
// The runtime refused what it accepted, so the workload never
5142+
// started on it. Runtime text stays out of public status.
5143+
admission.error = "The isolation backend refused this configuration before the workload started; create the sandbox again with a policy the backend accepts".to_string();
5144+
} else {
5145+
// A rejected desired update does not invalidate an accepted runtime.
5146+
let error = admission.error;
5147+
admission = current.clone();
5148+
admission.error = error;
5149+
}
51185150
}
51195151
} else {
51205152
admission.error.clear();
@@ -8651,6 +8683,7 @@ mod tests {
86518683
sandbox_id: sandbox_id.to_string(),
86528684
admission: Some(admission),
86538685
expected_instance_id: String::new(),
8686+
refused_before_start: false,
86548687
}),
86558688
sandbox_id,
86568689
)
@@ -8788,6 +8821,134 @@ mod tests {
87888821
}
87898822
}
87908823

8824+
#[tokio::test]
8825+
async fn configuration_admission_records_refusal_of_accepted_generation() {
8826+
use openshell_core::proto::{
8827+
ConfigurationAdmissionState as Admission, ReportSandboxConfigurationRequest,
8828+
SandboxConfigurationAdmission,
8829+
};
8830+
let state = test_server_state().await;
8831+
let sandbox_id = "sb-refused";
8832+
state
8833+
.store
8834+
.put_message(&test_sandbox(
8835+
sandbox_id,
8836+
"refused",
8837+
openshell_policy::restrictive_default_policy(),
8838+
Vec::new(),
8839+
))
8840+
.await
8841+
.unwrap();
8842+
let instance_id = uuid::Uuid::new_v4().to_string();
8843+
let report = |admission, refused_before_start| {
8844+
with_sandbox(
8845+
Request::new(ReportSandboxConfigurationRequest {
8846+
sandbox_id: sandbox_id.to_string(),
8847+
admission: Some(admission),
8848+
expected_instance_id: String::new(),
8849+
refused_before_start,
8850+
}),
8851+
sandbox_id,
8852+
)
8853+
};
8854+
handle_report_sandbox_configuration(
8855+
&state,
8856+
report(
8857+
SandboxConfigurationAdmission {
8858+
instance_id: instance_id.clone(),
8859+
state: Admission::Pending.into(),
8860+
..Default::default()
8861+
},
8862+
false,
8863+
),
8864+
)
8865+
.await
8866+
.unwrap();
8867+
let config = handle_get_sandbox_config(
8868+
&state,
8869+
with_sandbox(
8870+
Request::new(GetSandboxConfigRequest {
8871+
name: "refused".to_string(),
8872+
workspace_scope: Some(openshell_core::proto::workspace_selector(
8873+
"default".to_string(),
8874+
)),
8875+
}),
8876+
sandbox_id,
8877+
),
8878+
)
8879+
.await
8880+
.unwrap()
8881+
.into_inner();
8882+
let accepted = SandboxConfigurationAdmission {
8883+
instance_id,
8884+
state: Admission::Accepted.into(),
8885+
policy_version: config.version,
8886+
policy_hash: config.policy_hash,
8887+
config_revision: config.config_revision,
8888+
provider_env_revision: config.provider_env_revision,
8889+
error: String::new(),
8890+
};
8891+
handle_report_sandbox_configuration(&state, report(accepted.clone(), false))
8892+
.await
8893+
.unwrap();
8894+
let readiness = || async {
8895+
let persisted = state
8896+
.store
8897+
.get_message::<Sandbox>(sandbox_id)
8898+
.await
8899+
.unwrap()
8900+
.unwrap();
8901+
let status = persisted.status.unwrap();
8902+
let ready = status
8903+
.conditions
8904+
.into_iter()
8905+
.find(|condition| condition.r#type == "ConfigurationReady")
8906+
.unwrap();
8907+
(status.configuration_admission.unwrap().state, ready)
8908+
};
8909+
8910+
// A runtime that fails to apply the generation it runs, such as a
8911+
// reconcile of unreachable middleware, keeps the accepted state.
8912+
let mut refused = accepted;
8913+
refused.state = Admission::Rejected.into();
8914+
refused.error = "backend text is not shown".to_string();
8915+
handle_report_sandbox_configuration(&state, report(refused.clone(), false))
8916+
.await
8917+
.unwrap();
8918+
let (admission_state, ready) = readiness().await;
8919+
assert_eq!(admission_state, i32::from(Admission::Accepted));
8920+
assert_eq!(ready.status, "True");
8921+
8922+
let mut flagged_acceptance = refused.clone();
8923+
flagged_acceptance.state = Admission::Accepted.into();
8924+
assert_eq!(
8925+
handle_report_sandbox_configuration(&state, report(flagged_acceptance, true))
8926+
.await
8927+
.unwrap_err()
8928+
.code(),
8929+
Code::InvalidArgument
8930+
);
8931+
let mut other_generation = refused.clone();
8932+
other_generation.policy_hash.push('x');
8933+
assert!(same_generation(&refused, &refused));
8934+
assert!(!same_generation(&refused, &other_generation));
8935+
8936+
handle_report_sandbox_configuration(&state, report(refused, true))
8937+
.await
8938+
.unwrap();
8939+
let (admission_state, ready) = readiness().await;
8940+
assert_eq!(admission_state, i32::from(Admission::Rejected));
8941+
assert_eq!(
8942+
(ready.status.as_str(), ready.reason.as_str()),
8943+
("False", "ConfigurationInvalid")
8944+
);
8945+
assert!(
8946+
ready.message.contains("isolation backend refused"),
8947+
"{ready:?}"
8948+
);
8949+
assert!(!ready.message.contains("backend text"), "{ready:?}");
8950+
}
8951+
87918952
#[test]
87928953
fn configuration_diagnostic_is_bounded_and_removes_control_characters() {
87938954
let diagnostic = bounded_configuration_diagnostic(&format!("rule\n{}", "é".repeat(1000)));
@@ -8829,6 +8990,7 @@ mod tests {
88298990
state: Admission::Pending.into(),
88308991
..Default::default()
88318992
}),
8993+
refused_before_start: false,
88328994
}),
88338995
sandbox_id,
88348996
),

‎crates/openshell-server/src/storage_proto.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,10 @@ mod tests {
156156
// Sandbox-wide grant reporting adds TokenGrantFailureObservation to both
157157
// closures and TokenGrantStatus to public reports. Older rows have no grant
158158
// record; absent report extensions retain compatibility without a rewrite.
159+
// ReportSandboxConfigurationRequest field 4 adds a public-only refusal
160+
// flag. Older supervisors omit it; durable schemas remain unchanged.
159161
const PUBLIC_RPC_SCHEMA_SHA256: &str =
160-
"a91e2244a2c1a8e369efe85bd0e4f2769f0f29f6263137280fe38e8a8da3cc6e";
162+
"dab6deeb6c4ef051efea068dd74d6807fe2b1212d9c21c40bd0dc8708b16ba28";
161163
const DURABLE_SCHEMA_SHA256: &str =
162164
"d4dc7abdb91b919db1f7be0d4652e30cbfda86f3b2ed9b59d5de1f76a2bf691d";
163165
const PUBLIC_DURABLE_OVERLAP_SHA256: &str =

‎crates/openshell-supervisor/src/backend_setup.rs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ pub trait PreparedBackend: Send + Sync {
8181
) -> std::result::Result<BuiltBackend, BackendError>;
8282
}
8383

84+
/// Diagnostic code of an Attach error that is a backend refusal, as opposed to
85+
/// a transient failure the next start can retry.
86+
pub(crate) const ATTACH_REFUSED: &str = "openshell::backend::attach_refused";
87+
8488
/// Created only after name and launch identity checks. Consuming attachment
8589
/// prevents reusing one prepared startup to build or attach a second client.
8690
pub struct SelectedBackend {
@@ -196,7 +200,12 @@ impl SelectedBackend {
196200
},
197201
)
198202
.await
199-
.map_err(|error| miette::miette!(error.to_string()))?;
203+
.map_err(|error| match error {
204+
BackendError::Denied(_) | BackendError::Attach(_) => {
205+
miette::miette!(code = ATTACH_REFUSED, "{error}")
206+
}
207+
error => miette::miette!("{error}"),
208+
})?;
200209
Ok((bound, built.proxy_listener))
201210
}
202211
}

‎crates/openshell-supervisor/src/backend_setup/tests.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,23 @@ async fn attachment_receives_live_supervisor_services() {
521521
assert!(setup.observed.services.lock().unwrap().upgrade().is_none());
522522
}
523523

524+
#[tokio::test]
525+
async fn attach_refusal_is_marked_as_a_refusal() {
526+
let setup = TestSetup::new();
527+
setup.observed.active.store(true, Ordering::SeqCst);
528+
let error = setup
529+
.select()
530+
.attach(setup.services(), policy(), agent())
531+
.await
532+
.err()
533+
.expect("an already attached boundary refuses attachment");
534+
assert_eq!(
535+
error.code().map(|code| code.to_string()).as_deref(),
536+
Some(ATTACH_REFUSED)
537+
);
538+
assert!(error.to_string().contains("already attached"));
539+
}
540+
524541
#[tokio::test]
525542
async fn constructed_backend_must_keep_selected_name() {
526543
let mut setup = TestSetup::new();

0 commit comments

Comments
 (0)