Skip to content

Commit 3c9ebf0

Browse files
igalshilmanclaude
andcommitted
Integrate #139 selector scoping with the in-process hash inputs
The status labelSelector's pod-template-hash must equal the latest ReplicaSet's hash, and in-process tunnel mode added the resolved RestateCloudEnvironment values to that hash — computed without them the selector would scope the HPA to a hash no ReplicaSet has. Thread the same params into latest_version_label_selector (resolution extracted into a shared helper); if they can't be resolved the reconcile has already failed, so the previous selector is kept rather than overwritten with a mismatching one. Pinned by a test asserting the two hash sites agree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 9bcc232 commit 3c9ebf0

1 file changed

Lines changed: 77 additions & 26 deletions

File tree

src/controllers/restatedeployment/controller.rs

Lines changed: 77 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,26 @@ fn error_policy<K, C>(_rs: Arc<K>, _: &Error, _ctx: C) -> Action {
207207
}
208208

209209
impl RestateDeployment {
210+
/// Resolve the RestateCloudEnvironment values a `tunnelMode: in-process`
211+
/// deployment derives its identity from (None for every other mode). They feed
212+
/// the revision hash, the env vars injected into the pods, the registered URL,
213+
/// and the status labelSelector — all of which must agree.
214+
fn in_process_tunnel_params(&self, ctx: &Context) -> Result<Option<InProcessTunnelParams>> {
215+
if !self.spec.restate.is_in_process_tunnel() {
216+
return Ok(None);
217+
}
218+
let Some(cloud) = self.spec.restate.register.cloud.as_deref() else {
219+
return Err(Error::InvalidRestateConfig(
220+
"tunnelMode: in-process requires registering against a RestateCloudEnvironment (spec.restate.register.cloud)"
221+
.into(),
222+
));
223+
};
224+
let Some(rce) = ctx.rce_store.get(&ObjectRef::new(cloud)) else {
225+
return Err(Error::RestateCloudEnvironmentNotFound(cloud.into()));
226+
};
227+
Ok(Some(InProcessTunnelParams::from_rce(rce.as_ref())?))
228+
}
229+
210230
// Reconcile (for non-finalizer related changes)
211231
async fn reconcile(
212232
&self,
@@ -220,20 +240,7 @@ impl RestateDeployment {
220240
// tunnelMode: in-process needs the RestateCloudEnvironment values up front:
221241
// they feed the revision hash, the env vars injected into the pods, and the
222242
// registered URL, all of which must agree within one reconcile.
223-
let in_process_tunnel = if self.spec.restate.is_in_process_tunnel() {
224-
let Some(cloud) = self.spec.restate.register.cloud.as_deref() else {
225-
return Err(Error::InvalidRestateConfig(
226-
"tunnelMode: in-process requires registering against a RestateCloudEnvironment (spec.restate.register.cloud)"
227-
.into(),
228-
));
229-
};
230-
let Some(rce) = ctx.rce_store.get(&ObjectRef::new(cloud)) else {
231-
return Err(Error::RestateCloudEnvironmentNotFound(cloud.into()));
232-
};
233-
Some(InProcessTunnelParams::from_rce(rce.as_ref())?)
234-
} else {
235-
None
236-
};
243+
let in_process_tunnel = self.in_process_tunnel_params(&ctx)?;
237244

238245
let pod_template_annotation = reconcilers::replicaset::pod_template_annotation(self);
239246

@@ -836,7 +843,14 @@ impl RestateDeployment {
836843

837844
// Only set labelSelector for ReplicaSet mode (Knative manages pods directly)
838845
if !is_knative {
839-
rsd_status.label_selector = latest_version_label_selector(self);
846+
// The selector hash must use the same inputs as the latest ReplicaSet's
847+
// hash, including the in-process tunnel params. If those can't be
848+
// resolved the reconcile above already failed; keep the previous
849+
// selector rather than scoping to a hash no ReplicaSet has.
850+
if let Ok(in_process_tunnel) = self.in_process_tunnel_params(&ctx) {
851+
rsd_status.label_selector =
852+
latest_version_label_selector(self, in_process_tunnel.as_ref());
853+
}
840854
}
841855
rsd_status.observed_generation = self.metadata.generation;
842856

@@ -1073,12 +1087,19 @@ impl RestateDeployment {
10731087
/// ReplicaSets still draining pinned invocations, polluting the averaged metric
10741088
/// for the whole (potentially multi-hour) drain window and under-provisioning
10751089
/// the genuinely-busy latest version. The hash is computed the same way as the
1076-
/// latest versioned ReplicaSet, so the selector matches exactly that RS's pods.
1090+
/// latest versioned ReplicaSet, so the selector matches exactly that RS's pods —
1091+
/// which is why it takes the same in-process tunnel params as the hash.
10771092
/// See #139.
1078-
fn latest_version_label_selector(rsd: &RestateDeployment) -> Option<String> {
1093+
fn latest_version_label_selector(
1094+
rsd: &RestateDeployment,
1095+
in_process_tunnel: Option<&InProcessTunnelParams>,
1096+
) -> Option<String> {
10791097
let pod_template_annotation = reconcilers::replicaset::pod_template_annotation(rsd);
1080-
let latest_hash =
1081-
reconcilers::replicaset::generate_pod_template_hash(rsd, &pod_template_annotation);
1098+
let latest_hash = reconcilers::replicaset::generate_pod_template_hash(
1099+
rsd,
1100+
&pod_template_annotation,
1101+
in_process_tunnel,
1102+
);
10821103

10831104
let mut label_selector = rsd.spec.selector.clone().unwrap_or_default();
10841105
label_selector
@@ -1391,13 +1412,13 @@ mod tests {
13911412
/// The hash the latest versioned ReplicaSet would use, recomputed independently.
13921413
fn latest_hash(rsd: &RestateDeployment) -> String {
13931414
let annotation = reconcilers::replicaset::pod_template_annotation(rsd);
1394-
reconcilers::replicaset::generate_pod_template_hash(rsd, &annotation)
1415+
reconcilers::replicaset::generate_pod_template_hash(rsd, &annotation, None)
13951416
}
13961417

13971418
#[test]
13981419
fn label_selector_appends_pod_template_hash_when_no_user_selector() {
13991420
let rsd = make_rsd(None, "greeter:v1");
1400-
let selector = latest_version_label_selector(&rsd).expect("selector is set");
1421+
let selector = latest_version_label_selector(&rsd, None).expect("selector is set");
14011422

14021423
// With no user selector the result is exactly the version scoping.
14031424
assert_eq!(
@@ -1409,7 +1430,7 @@ mod tests {
14091430
#[test]
14101431
fn label_selector_preserves_user_match_labels() {
14111432
let rsd = make_rsd(Some(&[("app", "greeter")]), "greeter:v1");
1412-
let selector = latest_version_label_selector(&rsd).expect("selector is set");
1433+
let selector = latest_version_label_selector(&rsd, None).expect("selector is set");
14131434

14141435
// Both the user's label and the version scoping must be present (order
14151436
// is not guaranteed by Selector::to_string).
@@ -1433,7 +1454,7 @@ mod tests {
14331454
// same one used to name/select the latest versioned ReplicaSet, so the
14341455
// HPA aggregates exactly that RS's pods and not old draining versions'.
14351456
let rsd = make_rsd(Some(&[("app", "greeter")]), "greeter:v1");
1436-
let selector = latest_version_label_selector(&rsd).expect("selector is set");
1457+
let selector = latest_version_label_selector(&rsd, None).expect("selector is set");
14371458

14381459
let versioned_name = format!("{}-{}", rsd.name_any(), latest_hash(&rsd));
14391460
let hash = versioned_name
@@ -1447,20 +1468,50 @@ mod tests {
14471468
);
14481469
}
14491470

1471+
#[test]
1472+
fn label_selector_tracks_in_process_tunnel_params() {
1473+
// The #139 guarantee must hold for tunnelMode: in-process too: the selector
1474+
// hash incorporates the same tunnel params as the latest ReplicaSet's hash —
1475+
// computed without them it would scope the HPA to a hash no RS has.
1476+
use crate::resources::restatecloudenvironments::InProcessTunnelParams;
1477+
1478+
let rsd = make_rsd(Some(&[("app", "greeter")]), "greeter:v1");
1479+
let params = InProcessTunnelParams {
1480+
environment_id: "env_123".into(),
1481+
region: "us".into(),
1482+
signing_public_key: "publickeyv1_abc".into(),
1483+
};
1484+
1485+
let annotation = reconcilers::replicaset::pod_template_annotation(&rsd);
1486+
let rs_hash =
1487+
reconcilers::replicaset::generate_pod_template_hash(&rsd, &annotation, Some(&params));
1488+
1489+
let selector = latest_version_label_selector(&rsd, Some(&params)).expect("selector is set");
1490+
assert!(
1491+
selector.contains(&format!("{}={}", POD_TEMPLATE_HASH_LABEL, rs_hash)),
1492+
"selector {selector:?} does not scope to the in-process RS hash {rs_hash}"
1493+
);
1494+
assert_ne!(
1495+
selector,
1496+
latest_version_label_selector(&rsd, None).expect("selector without params"),
1497+
"params must change the selector, or the two hash sites have diverged"
1498+
);
1499+
}
1500+
14501501
#[test]
14511502
fn label_selector_changes_with_pod_template() {
14521503
// A different pod template => different version => different selector,
14531504
// so the status selector actually tracks the current version.
14541505
let v1 = make_rsd(Some(&[("app", "greeter")]), "greeter:v1");
14551506
let v2 = make_rsd(Some(&[("app", "greeter")]), "greeter:v2");
14561507

1457-
let s1 = latest_version_label_selector(&v1).expect("v1 selector");
1458-
let s2 = latest_version_label_selector(&v2).expect("v2 selector");
1508+
let s1 = latest_version_label_selector(&v1, None).expect("v1 selector");
1509+
let s2 = latest_version_label_selector(&v2, None).expect("v2 selector");
14591510

14601511
assert_ne!(s1, s2, "selector should differ across pod templates");
14611512

14621513
// ...and it is deterministic for the same template.
1463-
let s1_again = latest_version_label_selector(&v1).expect("v1 selector again");
1514+
let s1_again = latest_version_label_selector(&v1, None).expect("v1 selector again");
14641515
assert_eq!(s1, s1_again, "selector should be deterministic");
14651516
}
14661517
}

0 commit comments

Comments
 (0)