Skip to content

Commit 6c4fbc7

Browse files
refi64sjoerdsimons
refi64
authored and
sjoerdsimons
committed
Allow globally setting the default monitor job timeout
PR #17 added support for job timeouts to the runner. This means that, if your build jobs take a long time to complete, you must now override the timeout via `--job-timeout`. However, because the given option did not exist in previous releases, deploying the updated version becomes more difficult: the new option is not supported by the old version, but the moment you deploy the new version, your builds might time out until the option is passed. To circumvent this, this PR adds support for setting a global timeout for all monitor jobs the runner generates, avoiding the need to urgently make modifications to the pipelines. Signed-off-by: Ryan Gonzalez <[email protected]>
1 parent 6594c4e commit 6c4fbc7

File tree

4 files changed

+155
-24
lines changed

4 files changed

+155
-24
lines changed

‎chart/templates/deployment.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ spec:
4848
value: {{ .Values.log_level | quote }}
4949
- name: OBS_RUNNER_LOG_FORMAT
5050
value: {{ .Values.log_format | quote }}
51+
{{- if .Values.default_monitor_job_timeout }}
52+
- name: OBS_RUNNER_DEFAULT_MONITOR_JOB_TIMEOUT
53+
value: {{ .Values.default_monitor_job_timeout | quote }}
54+
{{- end }}
5155
resources:
5256
{{- toYaml .Values.resources | nindent 12 }}
5357
volumes:

‎chart/values.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ replicaCount: 1
66

77
log_level: info
88
log_format: json
9+
default_monitor_job_timeout: ''
910
gitlab:
1011
url: https://your.gitlab.server.org
1112
#Set the runner token for a first deployment. It's inherited on later

‎src/handler.rs‎

Lines changed: 132 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,10 @@ fn get_job_variable<'job>(job: &'job Job, key: &str) -> Result<Variable<'job>> {
207207
#[derive(Debug, Derivative)]
208208
#[derivative(Default)]
209209
pub struct HandlerOptions {
210-
monitor: PackageMonitoringOptions,
210+
pub monitor: PackageMonitoringOptions,
211+
pub default_monitor_job_timeout: Option<String>,
211212
#[derivative(Default(value = "LOG_TAIL_2MB"))]
212-
log_tail: u64,
213+
pub log_tail: u64,
213214
}
214215

215216
pub struct ObsJobHandler {
@@ -412,7 +413,9 @@ impl ObsJobHandler {
412413
tags: vec![args.tag],
413414
artifact_expiration: args.artifact_expiration,
414415
prefix: args.job_prefix,
415-
timeout: args.job_timeout,
416+
timeout: args
417+
.job_timeout
418+
.or_else(|| self.options.default_monitor_job_timeout.clone()),
416419
rules: args.rules,
417420
download_binaries: if let Some(build_results_dir) = args.build_results_dir {
418421
PipelineDownloadBinaries::OnSuccess {
@@ -740,9 +743,20 @@ mod tests {
740743

741744
const JOB_TIMEOUT: u64 = 3600;
742745
const TEST_LOG_TAIL: u64 = 50;
743-
744746
const OLD_STATUS_SLEEP_DURATION: Duration = Duration::from_millis(100);
745747

748+
const DEFAULT_HANDLER_OPTIONS: HandlerOptions = HandlerOptions {
749+
default_monitor_job_timeout: None,
750+
log_tail: TEST_LOG_TAIL,
751+
monitor: PackageMonitoringOptions {
752+
sleep_on_building: Duration::ZERO,
753+
sleep_on_old_status: OLD_STATUS_SLEEP_DURATION,
754+
// High limit, since we don't really test that
755+
// functionality in the handler tests.
756+
max_old_status_retries: 99,
757+
},
758+
};
759+
746760
static COLOR_EYRE_INSTALL: Once = Once::new();
747761

748762
struct TestContext {
@@ -933,25 +947,17 @@ mod tests {
933947
.collect()
934948
}
935949

936-
async fn run_obs_handler(context: &mut TestContext) {
950+
async fn run_obs_handler_with_options(context: &mut TestContext, options: HandlerOptions) {
937951
run_handler(context, move |job| {
938-
assert_ok!(ObsJobHandler::from_obs_config_in_job(
939-
job,
940-
HandlerOptions {
941-
log_tail: TEST_LOG_TAIL,
942-
monitor: PackageMonitoringOptions {
943-
sleep_on_building: Duration::ZERO,
944-
sleep_on_old_status: OLD_STATUS_SLEEP_DURATION,
945-
// High limit, since we don't really test that
946-
// functionality in the handler tests.
947-
max_old_status_retries: 99,
948-
},
949-
},
950-
))
952+
assert_ok!(ObsJobHandler::from_obs_config_in_job(job, options))
951953
})
952954
.await;
953955
}
954956

957+
async fn run_obs_handler(context: &mut TestContext) {
958+
run_obs_handler_with_options(context, DEFAULT_HANDLER_OPTIONS).await;
959+
}
960+
955961
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
956962
enum DputTest {
957963
Basic,
@@ -1827,4 +1833,112 @@ mod tests {
18271833
})
18281834
.await;
18291835
}
1836+
1837+
#[derive(Debug, PartialEq, Eq)]
1838+
enum GenerateMonitorTimeoutLocation {
1839+
HandlerOption,
1840+
Argument,
1841+
}
1842+
1843+
#[rstest]
1844+
#[tokio::test]
1845+
async fn test_generate_monitor_timeouts(
1846+
#[future] test_context: (TestContext, GitlabLayer),
1847+
#[values(
1848+
None,
1849+
Some(GenerateMonitorTimeoutLocation::HandlerOption),
1850+
Some(GenerateMonitorTimeoutLocation::Argument)
1851+
)]
1852+
test: Option<GenerateMonitorTimeoutLocation>,
1853+
) {
1854+
const TEST_MONITOR_TIMEOUT: &str = "10 minutes";
1855+
1856+
let (mut context, layer) = test_context.await;
1857+
with_tracing(layer, async {
1858+
let build_info = ObsBuildInfo {
1859+
project: TEST_PROJECT.to_owned(),
1860+
package: TEST_PACKAGE_1.to_owned(),
1861+
rev: Some("1".to_owned()),
1862+
srcmd5: Some("abc".to_owned()),
1863+
is_branched: false,
1864+
enabled_repos: [(
1865+
RepoArch {
1866+
repo: TEST_REPO.to_owned(),
1867+
arch: TEST_ARCH_1.to_owned(),
1868+
},
1869+
CommitBuildInfo {
1870+
prev_endtime_for_commit: None,
1871+
},
1872+
)]
1873+
.into(),
1874+
};
1875+
1876+
let build_info = put_artifacts(
1877+
&mut context,
1878+
[(
1879+
DEFAULT_BUILD_INFO.to_owned(),
1880+
serde_yaml::to_vec(&build_info).unwrap(),
1881+
)]
1882+
.into(),
1883+
)
1884+
.await;
1885+
1886+
let mut generate_spec = JobSpec {
1887+
name: "generate".to_owned(),
1888+
script: vec!["generate-monitor tag".to_owned()],
1889+
dependencies: vec![build_info],
1890+
..Default::default()
1891+
};
1892+
1893+
if test == Some(GenerateMonitorTimeoutLocation::Argument) {
1894+
use std::fmt::Write;
1895+
write!(
1896+
&mut generate_spec.script[0],
1897+
" --job-timeout '{TEST_MONITOR_TIMEOUT}'"
1898+
)
1899+
.unwrap();
1900+
}
1901+
1902+
let generate = enqueue_job(&context, generate_spec);
1903+
1904+
if test == Some(GenerateMonitorTimeoutLocation::HandlerOption) {
1905+
run_obs_handler_with_options(
1906+
&mut context,
1907+
HandlerOptions {
1908+
default_monitor_job_timeout: Some(TEST_MONITOR_TIMEOUT.to_owned()),
1909+
..DEFAULT_HANDLER_OPTIONS
1910+
},
1911+
)
1912+
.await;
1913+
} else {
1914+
run_obs_handler(&mut context).await;
1915+
}
1916+
assert_eq!(generate.state(), MockJobState::Success);
1917+
1918+
let results = get_job_artifacts(&generate);
1919+
let pipeline_yaml: serde_yaml::Value = assert_ok!(serde_yaml::from_slice(
1920+
results.get(DEFAULT_MONITOR_PIPELINE).unwrap()
1921+
));
1922+
let pipeline_map = pipeline_yaml.as_mapping().unwrap();
1923+
1924+
let monitor_map = pipeline_map
1925+
.into_iter()
1926+
.next()
1927+
.unwrap()
1928+
.1
1929+
.as_mapping()
1930+
.unwrap();
1931+
1932+
let timeout_yaml = monitor_map.get(&"timeout".into());
1933+
if test.is_some() {
1934+
assert_eq!(
1935+
timeout_yaml.unwrap().as_str().unwrap(),
1936+
TEST_MONITOR_TIMEOUT
1937+
);
1938+
} else {
1939+
assert_none!(timeout_yaml);
1940+
}
1941+
})
1942+
.await;
1943+
}
18301944
}

‎src/main.rs‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::{fmt, str::FromStr};
1+
use std::{fmt, str::FromStr, sync::Arc};
22

33
use clap::Parser;
44
use color_eyre::eyre::Result;
@@ -88,6 +88,8 @@ struct Args {
8888
log_format: LogFormat,
8989
#[clap(long, env = "OBS_RUNNER_MAX_JOBS", default_value_t = 64, parse(try_from_str=parse_max_jobs))]
9090
max_jobs: usize,
91+
#[clap(long, env = "OBS_RUNNER_DEFAULT_MONITOR_JOB_TIMEOUT")]
92+
default_monitor_job_timeout: Option<String>,
9193
}
9294

9395
fn formatter_layer<E, S>(format: E, targets: Targets) -> impl Layer<S>
@@ -134,15 +136,25 @@ async fn main() {
134136

135137
color_eyre::install().unwrap();
136138

139+
let default_monitor_job_timeout = Arc::new(args.default_monitor_job_timeout);
140+
137141
info!("Starting runner...");
138142
runner
139143
.run(
140-
|job| async {
141-
ObsJobHandler::from_obs_config_in_job(job, HandlerOptions::default()).map_err(
142-
|err| {
144+
move |job| {
145+
let default_monitor_job_timeout = (*default_monitor_job_timeout).clone();
146+
async {
147+
ObsJobHandler::from_obs_config_in_job(
148+
job,
149+
HandlerOptions {
150+
default_monitor_job_timeout,
151+
..Default::default()
152+
},
153+
)
154+
.map_err(|err| {
143155
error!("Failed to create new client: {:?}", err);
144-
},
145-
)
156+
})
157+
}
146158
},
147159
args.max_jobs,
148160
)

0 commit comments

Comments
 (0)