Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add VOD deployment job to GitHub workflows #597

Merged
merged 3 commits into from
May 4, 2023

Conversation

gbryant-arm
Copy link
Contributor

@gbryant-arm gbryant-arm commented Mar 22, 2023

Our CI doesn't include any tests for the freestanding execution engine (FEE), nor any deployments of relatively complex examples.
In this PR we add a job to GitHub workflows that deploys the video object detection example as a standalone binary, in wasmtime, in the FEE and end-to-end.

The changes add about 5min to the linux job (need to build Veracruz-Linux in addition to running the tests) and the new vod-full-deployment job takes about 4min and is executed after linux. This doesn't affect the total execution time since the icecap job takes more than 30min, which is more than linux and vod-full-deployment combined.

Some ideas to optimise this workflow further (won't be implemented in this PR):

  • Build VOD (native and WASM binaries) in parallel to other jobs then share binaries with deployment job
    • Pro: Saves ~1min
    • Con: Adds another concurrent job to the workflow
  • Add deployment steps to Veracruz build jobs as opposed to having a separate job for deployment
    • Pro: Saves ~2min (container and artifact sharing overhead)
    • Con: Need to rerun the entire job if deployment fails

@gbryant-arm gbryant-arm marked this pull request as ready for review March 22, 2023 17:18
@gbryant-arm
Copy link
Contributor Author

Switching to "ready for review" just to trigger the CI and see what happens

@gbryant-arm gbryant-arm force-pushed the vod-ci branch 28 times, most recently from d8ac13b to 4ad81c7 Compare March 23, 2023 15:01
@gbryant-arm gbryant-arm force-pushed the vod-ci branch 3 times, most recently from 35a6e04 to 53d05a6 Compare April 12, 2023 15:16
@gbryant-arm gbryant-arm changed the title WIP: Add VOD deployment job to GitHub workflows Add VOD deployment job to GitHub workflows Apr 12, 2023
@gbryant-arm
Copy link
Contributor Author

RFR

@egrimley-arm
Copy link
Contributor

Someone who wants to test the CI on their own machine has to copy-paste quite a lot of shell code from main.yml into a terminal. I wonder if it would be better to move the shell code into a script that one could more easily run manually.

Arguably that's a new feature that's distinct from what you're adding here so could be a separate PR, though. And there are various ways one could do it: for example, a single script that runs a different CI job depending on the argument supplied to it?

@gbryant-arm
Copy link
Contributor Author

@egrimley-arm The way things are implemented, the VOD readme is parsed by the CI script while still being palatable to someone who doesn't care about the CI and just wants to build and run VOD. Finding the right balance between sharing code in the README and having CI-specific code is hard to find, and I reckon the balance we've reached here is not bad.
Moving CI code to a separate script is possible but would intricate things further.
Out of curiosity, why and how would we run the CI outside of github?

@egrimley-arm
Copy link
Contributor

Out of curiosity, why and how would we run the CI outside of github?

I tend to do that when investigating why CI failed, and I also like to run an abbreviated version of the CI tests, rather than follow entirely separate instructions, to check whether a change I've made breaks anything.

.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
@gbryant-arm gbryant-arm merged commit 8c2eb9f into veracruz-project:main May 4, 2023
@mathias-arm mathias-arm added the testing Issues related to the Veracruz tests label Jul 27, 2023
@mathias-arm mathias-arm linked an issue Jul 28, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Issues related to the Veracruz tests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add VOD example to CI
4 participants