Skip to content

Commit e5c175e

Browse files
refi64sjoerdsimons
refi64
authored and
sjoerdsimons
committed
Fix including excluded repositories due to broken status
There's a short period of time after a package is uploaded that OBS might report it as `broken`, with the details set to `empty`. This means that, when the build metadata is gathered, excluded architectures end up being *included*, because the status is `broken` instead of `excluded`. In order to work around this, wait for a short period of time if the package's status is `broken` and details are `empty`, that way we can actually check if the architecture is excluded. Signed-off-by: Ryan Gonzalez <[email protected]>
1 parent 75d3bf5 commit e5c175e

File tree

3 files changed

+212
-31
lines changed

3 files changed

+212
-31
lines changed

‎Cargo.lock‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/build_meta.rs‎

Lines changed: 201 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
use std::collections::HashMap;
1+
use std::{collections::HashMap, time::Duration};
22

33
use color_eyre::eyre::{Result, WrapErr};
4+
use gitlab_runner::outputln;
45
use open_build_service_api as obs;
56
use serde::{Deserialize, Serialize};
67
use tracing::{debug, info_span, instrument, Instrument};
@@ -29,15 +30,92 @@ pub enum BuildHistoryRetrieval {
2930
None,
3031
}
3132

33+
#[derive(Debug)]
34+
pub struct BuildMetaWaitOptions {
35+
pub sleep_on_empty: Duration,
36+
}
37+
38+
impl Default for BuildMetaWaitOptions {
39+
fn default() -> Self {
40+
Self {
41+
sleep_on_empty: Duration::from_secs(15),
42+
}
43+
}
44+
}
45+
46+
#[derive(Debug)]
47+
pub struct BuildMetaOptions {
48+
pub history_retrieval: BuildHistoryRetrieval,
49+
pub wait_options: BuildMetaWaitOptions,
50+
}
51+
52+
#[instrument(skip(client))]
53+
async fn get_status(
54+
client: &obs::Client,
55+
project: &str,
56+
package: &str,
57+
repo: &str,
58+
arch: &str,
59+
) -> Result<obs::BuildStatus> {
60+
let status = retry_request(|| async {
61+
client
62+
.project(project.to_owned())
63+
.package(package.to_owned())
64+
.status(repo, arch)
65+
.await
66+
})
67+
.await
68+
.wrap_err_with(|| {
69+
format!(
70+
"Failed to get status of {}/{}/{}/{}",
71+
project, repo, arch, package
72+
)
73+
})?;
74+
debug!(?status);
75+
76+
Ok(status)
77+
}
78+
79+
fn is_status_empty(status: &obs::BuildStatus) -> bool {
80+
status.code == obs::PackageCode::Broken && matches!(status.details.as_deref(), Some("empty"))
81+
}
82+
83+
#[instrument(skip(client))]
84+
async fn get_status_if_not_broken(
85+
client: &obs::Client,
86+
project: &str,
87+
package: &str,
88+
repo: &str,
89+
arch: &str,
90+
options: &BuildMetaWaitOptions,
91+
) -> Result<obs::BuildStatus> {
92+
let mut status = get_status(client, project, package, repo, arch).await?;
93+
94+
if is_status_empty(&status) {
95+
outputln!("Waiting for package contents to appear...");
96+
for attempt in 0.. {
97+
debug!(?attempt);
98+
tokio::time::sleep(options.sleep_on_empty).await;
99+
100+
status = get_status(client, project, package, repo, arch).await?;
101+
if !is_status_empty(&status) {
102+
break;
103+
}
104+
}
105+
}
106+
107+
Ok(status)
108+
}
109+
32110
impl BuildMeta {
33111
#[instrument(skip(client))]
34112
pub async fn get_if_package_exists(
35113
client: &obs::Client,
36114
project: &str,
37115
package: &str,
38-
history_retrieval: BuildHistoryRetrieval,
116+
options: &BuildMetaOptions,
39117
) -> Result<Option<BuildMeta>> {
40-
match Self::get(client, project, package, history_retrieval).await {
118+
match Self::get(client, project, package, options).await {
41119
Ok(result) => Ok(Some(result)),
42120
Err(e) => {
43121
for cause in e.chain() {
@@ -60,7 +138,7 @@ impl BuildMeta {
60138
client: &obs::Client,
61139
project: &str,
62140
package: &str,
63-
history_retrieval: BuildHistoryRetrieval,
141+
options: &BuildMetaOptions,
64142
) -> Result<BuildMeta> {
65143
let project_meta =
66144
retry_request(|| async { client.project(project.to_owned()).meta().await })
@@ -73,22 +151,15 @@ impl BuildMeta {
73151

74152
for repo_meta in project_meta.repositories {
75153
for arch in repo_meta.arches {
76-
let status = retry_request(|| async {
77-
client
78-
.project(project.to_owned())
79-
.package(package.to_owned())
80-
.status(&repo_meta.name, &arch)
81-
.await
82-
})
83-
.await
84-
.wrap_err_with(|| {
85-
format!(
86-
"Failed to get status of {}/{}/{}/{}",
87-
project, repo_meta.name, arch, package
88-
)
89-
})?;
90-
debug!(?status);
91-
154+
let status = get_status_if_not_broken(
155+
client,
156+
project,
157+
package,
158+
&repo_meta.name,
159+
&arch,
160+
&options.wait_options,
161+
)
162+
.await?;
92163
if matches!(
93164
status.code,
94165
obs::PackageCode::Disabled | obs::PackageCode::Excluded
@@ -97,7 +168,7 @@ impl BuildMeta {
97168
continue;
98169
}
99170

100-
let jobhist = match history_retrieval {
171+
let jobhist = match options.history_retrieval {
101172
BuildHistoryRetrieval::Full => {
102173
retry_request(|| async {
103174
client
@@ -244,7 +315,10 @@ mod tests {
244315
&client,
245316
TEST_PROJECT,
246317
TEST_PACKAGE_1,
247-
BuildHistoryRetrieval::Full
318+
&BuildMetaOptions {
319+
history_retrieval: BuildHistoryRetrieval::Full,
320+
wait_options: Default::default(),
321+
}
248322
)
249323
.await
250324
);
@@ -265,7 +339,10 @@ mod tests {
265339
&client,
266340
TEST_PROJECT,
267341
TEST_PACKAGE_1,
268-
BuildHistoryRetrieval::None
342+
&BuildMetaOptions {
343+
history_retrieval: BuildHistoryRetrieval::None,
344+
wait_options: Default::default(),
345+
}
269346
)
270347
.await
271348
);
@@ -307,7 +384,10 @@ mod tests {
307384
&client,
308385
TEST_PROJECT,
309386
TEST_PACKAGE_1,
310-
BuildHistoryRetrieval::Full
387+
&BuildMetaOptions {
388+
history_retrieval: BuildHistoryRetrieval::Full,
389+
wait_options: Default::default(),
390+
}
311391
)
312392
.await
313393
);
@@ -336,7 +416,10 @@ mod tests {
336416
&client,
337417
TEST_PROJECT,
338418
TEST_PACKAGE_1,
339-
BuildHistoryRetrieval::Full
419+
&BuildMetaOptions {
420+
history_retrieval: BuildHistoryRetrieval::Full,
421+
wait_options: Default::default(),
422+
}
340423
)
341424
.await
342425
);
@@ -366,7 +449,99 @@ mod tests {
366449
&client,
367450
TEST_PROJECT,
368451
TEST_PACKAGE_1,
369-
BuildHistoryRetrieval::Full
452+
&BuildMetaOptions {
453+
history_retrieval: BuildHistoryRetrieval::Full,
454+
wait_options: Default::default()
455+
}
456+
)
457+
.await
458+
);
459+
assert_eq!(meta.enabled_repos.len(), 0);
460+
}
461+
462+
#[tokio::test]
463+
async fn test_build_meta_ignores_empty() {
464+
const EMPTY_SLEEP_DURATION: Duration = Duration::from_millis(100);
465+
466+
let repo_arch_1 = RepoArch {
467+
repo: TEST_REPO.to_owned(),
468+
arch: TEST_ARCH_1.to_owned(),
469+
};
470+
471+
let mock = create_default_mock().await;
472+
mock.add_project(TEST_PROJECT.to_owned());
473+
mock.add_new_package(
474+
TEST_PROJECT,
475+
TEST_PACKAGE_1.to_owned(),
476+
MockPackageOptions::default(),
477+
);
478+
479+
mock.add_or_update_repository(
480+
TEST_PROJECT,
481+
TEST_REPO.to_owned(),
482+
TEST_ARCH_1.to_owned(),
483+
MockRepositoryCode::Finished,
484+
);
485+
486+
mock.set_package_build_status(
487+
TEST_PROJECT,
488+
TEST_REPO,
489+
TEST_ARCH_1,
490+
TEST_PACKAGE_1.to_owned(),
491+
MockBuildStatus::new(MockPackageCode::Broken),
492+
);
493+
494+
let client = create_default_client(&mock);
495+
496+
let meta = assert_ok!(
497+
BuildMeta::get(
498+
&client,
499+
TEST_PROJECT,
500+
TEST_PACKAGE_1,
501+
&BuildMetaOptions {
502+
history_retrieval: BuildHistoryRetrieval::Full,
503+
wait_options: Default::default(),
504+
}
505+
)
506+
.await
507+
);
508+
assert_eq!(meta.enabled_repos.len(), 1);
509+
assert!(meta.enabled_repos.contains_key(&repo_arch_1));
510+
511+
mock.set_package_build_status(
512+
TEST_PROJECT,
513+
TEST_REPO,
514+
TEST_ARCH_1,
515+
TEST_PACKAGE_1.to_owned(),
516+
MockBuildStatus {
517+
code: MockPackageCode::Broken,
518+
details: "empty".to_owned(),
519+
..Default::default()
520+
},
521+
);
522+
523+
tokio::spawn(async move {
524+
tokio::time::sleep(EMPTY_SLEEP_DURATION * 10).await;
525+
mock.set_package_build_status(
526+
TEST_PROJECT,
527+
TEST_REPO,
528+
TEST_ARCH_1,
529+
TEST_PACKAGE_1.to_owned(),
530+
MockBuildStatus::new(MockPackageCode::Excluded),
531+
);
532+
});
533+
534+
let meta = assert_ok!(
535+
BuildMeta::get(
536+
&client,
537+
TEST_PROJECT,
538+
TEST_PACKAGE_1,
539+
&BuildMetaOptions {
540+
history_retrieval: BuildHistoryRetrieval::Full,
541+
wait_options: BuildMetaWaitOptions {
542+
sleep_on_empty: EMPTY_SLEEP_DURATION,
543+
},
544+
}
370545
)
371546
.await
372547
);

‎src/handler.rs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use tracing::{debug, error, instrument};
2626
use crate::{
2727
artifacts::{save_to_tempfile, ArtifactDirectory},
2828
binaries::download_binaries,
29-
build_meta::{BuildHistoryRetrieval, BuildMeta, CommitBuildInfo, RepoArch},
29+
build_meta::{BuildHistoryRetrieval, BuildMeta, BuildMetaOptions, CommitBuildInfo, RepoArch},
3030
monitor::{MonitoredPackage, ObsMonitor, PackageCompletion, PackageMonitoringOptions},
3131
pipeline::{generate_monitor_pipeline, GeneratePipelineOptions, PipelineDownloadBinaries},
3232
prune::prune_branch,
@@ -310,7 +310,10 @@ impl ObsJobHandler {
310310
&self.client,
311311
&build_info.project,
312312
&build_info.package,
313-
BuildHistoryRetrieval::Full,
313+
&BuildMetaOptions {
314+
history_retrieval: BuildHistoryRetrieval::Full,
315+
wait_options: Default::default(),
316+
},
314317
)
315318
.await?;
316319
debug!(?initial_build_meta);
@@ -328,7 +331,10 @@ impl ObsJobHandler {
328331
&self.client,
329332
&build_info.project,
330333
&build_info.package,
331-
BuildHistoryRetrieval::None,
334+
&BuildMetaOptions {
335+
history_retrieval: BuildHistoryRetrieval::None,
336+
wait_options: Default::default(),
337+
},
332338
)
333339
.await?
334340
};

0 commit comments

Comments
 (0)