Skip to content

Commit

Permalink
flowey/gh: ensure the "all good" job does what its supposed to do (mi…
Browse files Browse the repository at this point in the history
…crosoft#176)

It seems that on github, this job never actually did what it was
supposed to do, which was:

- pass if all previous jobs passed
- fail if any previous job failed

Instead, it seems that in the case of any previous job failing, GitHub
Actions would simply skip executing the "all good" job entirely, and
treat the skip as a "pass" of the (required) check. An interesting
default indeed...

We never noticed this as we would previously run the PrivateVsoBuild
job, which was a superset of all open-source checks, and would therefore
fail if any open-source check failed, causing the PR to be blocked even
through the "all good" check was ostensibly "passed".

This PR should fix things up such that our "all good" job does what its
supposed to do, by relying on a workaround suggested in
actions/runner#2566 (albeit, a slightly
tweaked variant of it).

* * * 

I also switched the job to run on a Linux machine, which I believe is
marginally faster to provision and execute on.
  • Loading branch information
daprilik authored Oct 22, 2024
1 parent 8fc59b9 commit 865374e
Show file tree
Hide file tree
Showing 14 changed files with 186 additions and 157 deletions.
72 changes: 0 additions & 72 deletions .github/workflows/openvmm-ci.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 0 additions & 51 deletions .github/workflows/openvmm-ci.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions .github/workflows/openvmm-pr.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 20 additions & 14 deletions .github/workflows/openvmm-pr.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions flowey/flowey_cli/src/pipeline_resolver/ado_yaml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ pub fn ado_yaml(
arch,
cond_param_idx,
ref ado_pool,
gh_override_if: _,
gh_global_env: _,
gh_pool: _,
gh_permissions: _,
ref external_read_vars,
Expand Down
2 changes: 2 additions & 0 deletions flowey/flowey_cli/src/pipeline_resolver/direct_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ fn direct_run_do_work(
cond_param_idx,
ado_pool: _,
ado_variables: _,
gh_override_if: _,
gh_global_env: _,
gh_pool: _,
gh_permissions: _,
ref external_read_vars,
Expand Down
6 changes: 6 additions & 0 deletions flowey/flowey_cli/src/pipeline_resolver/generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ pub struct ResolvedPipelineJob {
pub arch: FlowArch,
pub ado_pool: Option<AdoPool>,
pub ado_variables: BTreeMap<String, String>,
pub gh_override_if: Option<String>,
pub gh_global_env: BTreeMap<String, String>,
pub gh_pool: Option<GhRunner>,
pub gh_permissions: BTreeMap<NodeHandle, BTreeMap<GhPermission, GhPermissionValue>>,
pub external_read_vars: BTreeSet<String>,
Expand Down Expand Up @@ -166,6 +168,8 @@ pub fn resolve_pipeline(pipeline: Pipeline) -> anyhow::Result<ResolvedPipeline>
cond_param_idx,
ado_pool,
ado_variables,
gh_override_if,
gh_global_env,
gh_pool,
gh_permissions,
},
Expand Down Expand Up @@ -217,6 +221,8 @@ pub fn resolve_pipeline(pipeline: Pipeline) -> anyhow::Result<ResolvedPipeline>
label,
ado_pool,
ado_variables,
gh_override_if,
gh_global_env,
gh_pool,
gh_permissions,
platform,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,10 @@ pub struct Job {
pub permissions: BTreeMap<Permissions, PermissionValue>,
#[serde(skip_serializing_if = "Vec::is_empty")]
pub needs: Vec<String>,
#[serde(rename = "if", skip_serializing_if = "Option::is_none")]
pub r#if: Option<String>,
#[serde(skip_serializing_if = "BTreeMap::is_empty")]
pub env: BTreeMap<String, String>,
pub steps: Vec<serde_yaml::Value>,
}

Expand Down
4 changes: 4 additions & 0 deletions flowey/flowey_cli/src/pipeline_resolver/github_yaml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ pub fn github_yaml(
arch,
ref external_read_vars,
ado_pool: _,
ref gh_override_if,
ref gh_global_env,
ref gh_pool,
ref gh_permissions,
cond_param_idx: _,
Expand Down Expand Up @@ -565,6 +567,8 @@ EOF
})
.collect()
},
r#if: gh_override_if.clone(),
env: gh_global_env.clone(),
steps: gh_steps,
},
);
Expand Down
4 changes: 4 additions & 0 deletions flowey/flowey_cli/src/pipeline_resolver/viz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ fn viz_pipeline_generic(
cond_param_idx: _,
ref ado_pool,
ado_variables: _,
gh_override_if: _,
gh_global_env: _,
ref gh_pool,
gh_permissions: _,
ref external_read_vars,
Expand Down Expand Up @@ -253,6 +255,8 @@ pub fn viz_pipeline_dot(pipeline: ResolvedPipeline, _backend: FlowBackend) -> an
cond_param_idx: _,
ado_pool,
ado_variables: _,
gh_override_if: _,
gh_global_env: _,
gh_pool,
gh_permissions: _,
external_read_vars: _,
Expand Down
Loading

0 comments on commit 865374e

Please sign in to comment.