fix(#3124): adapt hive-c0re's converge loop to a Copy AgentState
Making `AgentState` `Copy` in the preceding commit is a change to every consumer of the type, not to the crate that declares it. `hive-c0re` grew its own consumer while this branch was in review, and under `-D pedantic` a one-byte enum taken by reference is `trivially_copy_pass_by_ref` and a `.clone()` on it is `clone_on_copy`. Neither crate is wrong alone; the merge is. `decide` now takes the state by value and the call sites drop the `&`. No behaviour change — the function only matches on the value.
This commit is contained in:
parent
76d5871d20
commit
8dbccd5578
1 changed files with 14 additions and 21 deletions
|
|
@ -171,7 +171,7 @@ fn plan(
|
||||||
let Some(intent) = intents.get(agent) else {
|
let Some(intent) = intents.get(agent) else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
match decide(&decl.state, present.contains(agent), *intent) {
|
match decide(decl.state, present.contains(agent), *intent) {
|
||||||
Converge::Deploy => plan.deploy.push(agent.clone()),
|
Converge::Deploy => plan.deploy.push(agent.clone()),
|
||||||
Converge::Start => plan.start.push(agent.clone()),
|
Converge::Start => plan.start.push(agent.clone()),
|
||||||
Converge::Stop => plan.stop.push(agent.clone()),
|
Converge::Stop => plan.stop.push(agent.clone()),
|
||||||
|
|
@ -196,7 +196,7 @@ enum Converge {
|
||||||
/// The whole decision, pure: no queue, no store, no container. The three
|
/// The whole decision, pure: no queue, no store, no container. The three
|
||||||
/// inputs are exactly what [`converge`] reads per agent, so the table below is
|
/// inputs are exactly what [`converge`] reads per agent, so the table below is
|
||||||
/// the behaviour rather than a model of it.
|
/// the behaviour rather than a model of it.
|
||||||
fn decide(state: &AgentState, present: bool, intent: Option<Wanted>) -> Converge {
|
fn decide(state: AgentState, present: bool, intent: Option<Wanted>) -> Converge {
|
||||||
match state {
|
match state {
|
||||||
AgentState::Up if !present => Converge::Deploy,
|
AgentState::Up if !present => Converge::Deploy,
|
||||||
AgentState::Up => {
|
AgentState::Up => {
|
||||||
|
|
@ -231,14 +231,7 @@ mod tests {
|
||||||
HiveWanted {
|
HiveWanted {
|
||||||
agents: agents
|
agents: agents
|
||||||
.iter()
|
.iter()
|
||||||
.map(|(name, state)| {
|
.map(|(name, state)| ((*name).to_owned(), AgentWanted { state: *state }))
|
||||||
(
|
|
||||||
(*name).to_owned(),
|
|
||||||
AgentWanted {
|
|
||||||
state: state.clone(),
|
|
||||||
},
|
|
||||||
)
|
|
||||||
})
|
|
||||||
.collect(),
|
.collect(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -292,10 +285,10 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_declared_agent_this_hive_does_not_have_is_deployed() {
|
fn a_declared_agent_this_hive_does_not_have_is_deployed() {
|
||||||
assert_eq!(decide(&AgentState::Up, false, None), Converge::Deploy);
|
assert_eq!(decide(AgentState::Up, false, None), Converge::Deploy);
|
||||||
// Even holding an intent: a row can outlive its container.
|
// Even holding an intent: a row can outlive its container.
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
decide(&AgentState::Up, false, Some(Wanted::Up)),
|
decide(AgentState::Up, false, Some(Wanted::Up)),
|
||||||
Converge::Deploy
|
Converge::Deploy
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -303,11 +296,11 @@ mod tests {
|
||||||
#[test]
|
#[test]
|
||||||
fn a_hive_that_already_agrees_queues_nothing() {
|
fn a_hive_that_already_agrees_queues_nothing() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
decide(&AgentState::Up, true, Some(Wanted::Up)),
|
decide(AgentState::Up, true, Some(Wanted::Up)),
|
||||||
Converge::Nothing
|
Converge::Nothing
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
decide(&AgentState::Offline, true, Some(Wanted::Offline)),
|
decide(AgentState::Offline, true, Some(Wanted::Offline)),
|
||||||
Converge::Nothing
|
Converge::Nothing
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -315,11 +308,11 @@ mod tests {
|
||||||
#[test]
|
#[test]
|
||||||
fn a_disagreeing_intent_is_converged_both_ways() {
|
fn a_disagreeing_intent_is_converged_both_ways() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
decide(&AgentState::Up, true, Some(Wanted::Offline)),
|
decide(AgentState::Up, true, Some(Wanted::Offline)),
|
||||||
Converge::Start
|
Converge::Start
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
decide(&AgentState::Offline, true, Some(Wanted::Up)),
|
decide(AgentState::Offline, true, Some(Wanted::Up)),
|
||||||
Converge::Stop
|
Converge::Stop
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -329,15 +322,15 @@ mod tests {
|
||||||
/// to be doing.
|
/// to be doing.
|
||||||
#[test]
|
#[test]
|
||||||
fn an_agent_with_no_intent_row_is_converged() {
|
fn an_agent_with_no_intent_row_is_converged() {
|
||||||
assert_eq!(decide(&AgentState::Up, true, None), Converge::Start);
|
assert_eq!(decide(AgentState::Up, true, None), Converge::Start);
|
||||||
assert_eq!(decide(&AgentState::Offline, true, None), Converge::Stop);
|
assert_eq!(decide(AgentState::Offline, true, None), Converge::Stop);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Declared offline, no container: the one absent case that must not
|
/// Declared offline, no container: the one absent case that must not
|
||||||
/// deploy. Its control is the `Up` case above, which must.
|
/// deploy. Its control is the `Up` case above, which must.
|
||||||
#[test]
|
#[test]
|
||||||
fn an_absent_agent_declared_offline_is_left_absent() {
|
fn an_absent_agent_declared_offline_is_left_absent() {
|
||||||
assert_eq!(decide(&AgentState::Offline, false, None), Converge::Nothing);
|
assert_eq!(decide(AgentState::Offline, false, None), Converge::Nothing);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Version skew is handled one layer up, at the decode: `AgentState` is
|
/// Version skew is handled one layer up, at the decode: `AgentState` is
|
||||||
|
|
@ -350,8 +343,8 @@ mod tests {
|
||||||
fn every_state_this_build_knows_is_covered_above() {
|
fn every_state_this_build_knows_is_covered_above() {
|
||||||
for state in [AgentState::Up, AgentState::Offline] {
|
for state in [AgentState::Up, AgentState::Offline] {
|
||||||
let seen = [true, false].iter().any(|present| {
|
let seen = [true, false].iter().any(|present| {
|
||||||
decide(&state, *present, None) != Converge::Nothing
|
decide(state, *present, None) != Converge::Nothing
|
||||||
|| decide(&state, *present, Some(Wanted::Up)) != Converge::Nothing
|
|| decide(state, *present, Some(Wanted::Up)) != Converge::Nothing
|
||||||
});
|
});
|
||||||
assert!(seen, "{state:?} produces no action in any combination");
|
assert!(seen, "{state:?} produces no action in any combination");
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue