Skip to content

Commit 423b3ef

Browse files
sigiliososigilioso
authored
fix: remove support for None backoff policy (#1642)
* fix: remove support for None backoff policy * chore: update docs
1 parent edeee39 commit 423b3ef

File tree

4 files changed

+16
-86
lines changed

4 files changed

+16
-86
lines changed

‎agent-control/src/agent_type/README.md‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,7 @@ These diverse options offer extensive customization for your agent's deployment.
172172

173173
In the `backoff_strategy` we have:
174174

175-
* `type`: This field can take several forms - _none_, _fixed_, _linear_, or _exponential_. It determines the delay timing strategy between retries.
176-
* _none_: Means no delay between retries.
175+
* `type`: This field can take several forms - _fixed_, _linear_, or _exponential_. It determines the delay timing strategy between retries.
177176
* _fixed_: Constant delay interval between retries. This is the default type.
178177
* _linear_: Delay interval increases linearly after each retry.
179178
* _exponential_: Delay interval doubles after each retry.
@@ -400,7 +399,7 @@ variables:
400399
type: string
401400
required: false
402401
default: 20s
403-
402+
404403
deployment:
405404
on_host:
406405
executables:

‎agent-control/src/agent_type/runtime_config/restart_policy.rs‎

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use crate::agent_type::templates::Templateable;
44
use duration_str::deserialize_duration;
55
use serde::Deserialize;
66
use std::{str::FromStr, time::Duration};
7-
use tracing::warn;
87
use wrapper_with_default::WrapperWithDefault;
98

109
use super::templateable_value::TemplateableValue;
@@ -72,19 +71,6 @@ pub struct BackoffStrategyConfig {
7271
pub last_retry_interval: TemplateableValue<BackoffLastRetryInterval>,
7372
}
7473

75-
impl BackoffStrategyConfig {
76-
pub(crate) fn are_values_in_sync_with_type(&self) -> bool {
77-
match self.backoff_type.clone().get() {
78-
BackoffStrategyType::None => {
79-
self.backoff_delay.is_template_empty()
80-
&& self.max_retries.is_template_empty()
81-
&& self.last_retry_interval.is_template_empty()
82-
}
83-
_ => true,
84-
}
85-
}
86-
}
87-
8874
impl Templateable for BackoffStrategyConfig {
8975
fn template_with(self, variables: &Variables) -> Result<Self, AgentTypeError> {
9076
let backoff_type = self.backoff_type.template_with(variables)?;
@@ -98,13 +84,6 @@ impl Templateable for BackoffStrategyConfig {
9884
max_retries,
9985
last_retry_interval,
10086
};
101-
102-
if !result.are_values_in_sync_with_type() {
103-
warn!(
104-
"Backoff strategy type is set to `none`, but some of the backoff strategy fields are set. They will be ignored"
105-
);
106-
}
107-
10887
Ok(result)
10988
}
11089
}
@@ -113,7 +92,6 @@ impl Templateable for BackoffStrategyConfig {
11392
#[serde(rename_all = "lowercase", tag = "type")]
11493
pub enum BackoffStrategyType {
11594
#[default]
116-
None,
11795
Fixed,
11896
Linear,
11997
Exponential,
@@ -124,7 +102,6 @@ impl FromStr for BackoffStrategyType {
124102

125103
fn from_str(s: &str) -> Result<Self, Self::Err> {
126104
match s {
127-
"none" => Ok(Self::None),
128105
"fixed" => Ok(Self::Fixed),
129106
"linear" => Ok(Self::Linear),
130107
"exponential" => Ok(Self::Exponential),
@@ -143,40 +120,3 @@ impl Default for BackoffStrategyConfig {
143120
}
144121
}
145122
}
146-
147-
#[cfg(test)]
148-
mod tests {
149-
use crate::agent_type::runtime_config::templateable_value::TemplateableValue;
150-
151-
use super::{BackoffStrategyConfig, BackoffStrategyType};
152-
153-
#[test]
154-
fn values_in_sync_with_type() {
155-
let strategy = BackoffStrategyConfig {
156-
backoff_type: TemplateableValue::new(BackoffStrategyType::None),
157-
backoff_delay: TemplateableValue::from_template("".to_string()),
158-
max_retries: TemplateableValue::from_template("".to_string()),
159-
last_retry_interval: TemplateableValue::from_template("".to_string()),
160-
};
161-
162-
assert!(strategy.are_values_in_sync_with_type());
163-
164-
let strategy = BackoffStrategyConfig {
165-
backoff_type: TemplateableValue::new(BackoffStrategyType::None),
166-
backoff_delay: TemplateableValue::from_template("".to_string()),
167-
max_retries: TemplateableValue::from_template("".to_string()),
168-
last_retry_interval: TemplateableValue::from_template("${something}".to_string()),
169-
};
170-
171-
assert!(!strategy.are_values_in_sync_with_type());
172-
173-
let strategy = BackoffStrategyConfig {
174-
backoff_type: TemplateableValue::new(BackoffStrategyType::Fixed),
175-
backoff_delay: TemplateableValue::from_template("".to_string()),
176-
max_retries: TemplateableValue::from_template("".to_string()),
177-
last_retry_interval: TemplateableValue::from_template("${something}".to_string()),
178-
};
179-
180-
assert!(strategy.are_values_in_sync_with_type());
181-
}
182-
}

‎agent-control/src/sub_agent/on_host/command/restart_policy.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ impl RestartPolicy {
4141

4242
impl Default for RestartPolicy {
4343
fn default() -> Self {
44-
RestartPolicy::new(BackoffStrategy::None, Vec::new())
44+
RestartPolicy::new(BackoffStrategy::Fixed(Backoff::default()), Vec::new())
4545
}
4646
}
4747

@@ -56,7 +56,6 @@ pub enum BackoffStrategy {
5656
Fixed(Backoff),
5757
Linear(Backoff),
5858
Exponential(Backoff),
59-
None,
6059
}
6160

6261
/// Time Duration interval since last retry to consider a service malfunctioning.
@@ -71,7 +70,6 @@ impl BackoffStrategy {
7170
BackoffStrategy::Fixed(b)
7271
| BackoffStrategy::Linear(b)
7372
| BackoffStrategy::Exponential(b) => b.should_backoff(),
74-
BackoffStrategy::None => true,
7573
}
7674
}
7775

@@ -83,7 +81,6 @@ impl BackoffStrategy {
8381
BackoffStrategy::Fixed(b) => b.backoff(fixed, sleep_func),
8482
BackoffStrategy::Linear(b) => b.backoff(linear, sleep_func),
8583
BackoffStrategy::Exponential(b) => b.backoff(exponential, sleep_func),
86-
BackoffStrategy::None => {}
8784
}
8885
}
8986
}
@@ -157,7 +154,6 @@ impl From<&BackoffStrategyConfig> for BackoffStrategy {
157154
BackoffStrategyType::Exponential => {
158155
BackoffStrategy::Exponential(realize_backoff_config(value))
159156
}
160-
BackoffStrategyType::None => BackoffStrategy::None,
161157
}
162158
}
163159
}
@@ -203,7 +199,7 @@ mod tests {
203199

204200
#[test]
205201
fn test_restart_policy_should_retry() {
206-
let mut rb = RestartPolicy::new(BackoffStrategy::None, vec![1, 3]);
202+
let mut rb = RestartPolicy::new(BackoffStrategy::Fixed(Backoff::default()), vec![1, 3]);
207203
let results = vec![false, true, false, true];
208204

209205
results

‎agent-control/src/sub_agent/on_host/supervisor.rs‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use crate::sub_agent::identity::{AgentIdentity, ID_ATTRIBUTE_NAME};
1414
use crate::sub_agent::on_host::command::command_os::CommandOSNotStarted;
1515
use crate::sub_agent::on_host::command::error::CommandError;
1616
use crate::sub_agent::on_host::command::executable_data::ExecutableData;
17-
use crate::sub_agent::on_host::command::restart_policy::BackoffStrategy;
1817
use crate::sub_agent::on_host::command::shutdown::{
1918
ProcessTerminator, wait_exit_timeout, wait_exit_timeout_default,
2019
};
@@ -300,24 +299,19 @@ impl NotStartedSupervisorOnHost {
300299
// we just unwrap or take the default value (0)
301300
if !restart_policy.should_retry(exit_code.unwrap_or_default()) {
302301
// Log if we are not restarting anymore due to the restart policy being broken
303-
if restart_policy.backoff != BackoffStrategy::None {
304-
warn!(
305-
"supervisor won't restart anymore due to having exceeded its restart policy"
306-
);
302+
warn!("supervisor won't restart anymore due to having exceeded its restart policy");
307303

308-
let unhealthy = Unhealthy::new(
309-
"supervisor exceeded its defined restart policy".to_string(),
310-
);
304+
let unhealthy =
305+
Unhealthy::new("supervisor exceeded its defined restart policy".to_string());
311306

312-
if let Err(err) = health_publisher.publish((
313-
executable_data_clone.id.to_string(),
314-
HealthWithStartTime::new(unhealthy.into(), supervisor_start_time),
315-
)) {
316-
error!(
317-
"Error publishing health status for {}: {}",
318-
executable_data_clone.id, err
319-
);
320-
}
307+
if let Err(err) = health_publisher.publish((
308+
executable_data_clone.id.to_string(),
309+
HealthWithStartTime::new(unhealthy.into(), supervisor_start_time),
310+
)) {
311+
error!(
312+
"Error publishing health status for {}: {}",
313+
executable_data_clone.id, err
314+
);
321315
}
322316
break;
323317
}
@@ -439,6 +433,7 @@ pub mod tests {
439433
use crate::event::channel::pub_sub;
440434
use crate::health::health_checker::HEALTH_CHECKER_THREAD_NAME;
441435
use crate::sub_agent::on_host::command::executable_data::ExecutableData;
436+
use crate::sub_agent::on_host::command::restart_policy::BackoffStrategy;
442437
use crate::sub_agent::on_host::command::restart_policy::{Backoff, RestartPolicy};
443438
use rstest::*;
444439
use std::thread;

0 commit comments

Comments
 (0)