flowey: Use 2411 release branch to download artifacts for tests #941
Conversation
|
|
||
| let latest_commit = xshell::cmd!( | ||
| sh, | ||
| "{gh_cli} api repos/microsoft/openvmm/commits/{branch} --jq .sha" |
There was a problem hiding this comment.
Do we really need an API call here, rather than just looking at git history?
There was a problem hiding this comment.
I know it looks/is silly to use the gh cli for this, but this node will run locally (forked repo) and then that would force us to make some assumptions:
- do we want to assume that the user has an "upstream" remote and use that?
- should we add an "upstream" remote if it doesn't exist?
- are we ok messing with people's remotes for this to work?
Using the cli for this sidesteps all those questions, but I'm happy to do it the other way if we have a consensus on what to do.
| aarch64_bin, | ||
| } = request; | ||
|
|
||
| // Download 2411 release IGVM files for running servicing tests |
There was a problem hiding this comment.
Once you have this working, you'll certainly want to generalize this so that we don't need a new node every time we fork another release branch.
There was a problem hiding this comment.
Yeah, there are def some modularization improvements that we can do to make our lives easier in the future.
That said - one important property we'll want to maintain is having some way to work around potential future "drift" between how different release branches package various artifacts.
That is to say - the generalization can't just be "add a new branch: String parameter".
There was a problem hiding this comment.
So you want an enum, so that it's easy to add special casing for different artifact formats in the future?
There was a problem hiding this comment.
Assuming the typed representation of the artifacts is consistent between versions, then yeah - an enum would suffice.
| let downloaded = | ||
| ctx.reqv(|v| flowey_lib_common::download_gh_artifact::Request { | ||
| repo_owner: "microsoft".into(), | ||
| repo_name: "openvmm".into(), | ||
| file_name: format!("{arch_str}-openhcl-igvm"), | ||
| path: v, | ||
| run_id: run_id.clone(), | ||
| }); |
There was a problem hiding this comment.
There's no logic here to traverse commits and look for a finished build. Do we expect some churn on release branches where we should handle this case and find a successful build or is it ok for builds to temporarily fail while we wait on the release branch to finish?
There was a problem hiding this comment.
There's churn. We should probably find the last completed run.
There was a problem hiding this comment.
I wonder, do we really need to look at commits at all? Or can we just look at the run start time?
There was a problem hiding this comment.
You're right, we can use the gh cli to just get the latest completed run. I added a flowey node for this functionality.
justus-camp-microsoft
changed the title
| let sh = xshell::Shell::new()?; | ||
| let gh_cli = rt.read(gh_cli); | ||
|
|
||
| let id = xshell::cmd!(sh, "{gh_cli} run list -R {repo} -b {branch} -w {pipeline_name} -s completed --limit 1 --json databaseId -q .[0].databaseId").read()?; |
There was a problem hiding this comment.
Can you split this up, roughly like this:
| let id = xshell::cmd!(sh, "{gh_cli} run list -R {repo} -b {branch} -w {pipeline_name} -s completed --limit 1 --json databaseId -q .[0].databaseId").read()?; | |
| let id = xshell::cmd!(sh, "{gh_cli} run list \ | |
| -R {repo} \ | |
| -b {branch} \ | |
| -w {pipeline_name} \ | |
| -s completed \ | |
| --limit 1 \ | |
| --json databaseId \ | |
| -q .[0].databaseId").read()?; |
There was a problem hiding this comment.
Looks like the xshell::cmd! macro panics if I split the string up. I can't find any other examples of us using a multiline string anywhere so not sure if there's a workaround.
| ctx.import::<crate::download_release_igvm_files::resolve::Node>(); | ||
| ctx.import::<flowey_lib_common::download_gh_artifact::Node>(); | ||
| ctx.import::<flowey_lib_common::gh_workflow_id::Node>(); | ||
| ctx.import::<flowey_lib_common::git_latest_commit::Node>(); |
There was a problem hiding this comment.
Seems like we don't need this anymore.
There was a problem hiding this comment.
Good call, it'd be cool if flowey could warn about unused imports as I bet a lot of nodes end up with unused imports during iteration that don't get cleaned up.
This introduces a node for downloading release artifacts and using them in
vmm_tests. This change adds logic to download the 2411 release artifacts uses them for a servicing test to go from 2411 to what's currently on main.