Split all testing from publish_draft_release workflow and filter execution by labels on the PRs

[Garandor]

Signed-off-by: Adam Reif Garandor@manta.network

Description

Refactors the publish_draft_release workflow so it does only what it's meant to do - publish a draft release.

Changes in functionality:

1. Only run dolphin integration test if the PR touches the dolphin RT ( A-dolphin on PR )
2. Only run calamari with its long integration test if the PR touches calamari ( A-calamari on PR )
3. Only run docker_image_test with calamari_integration_test
4. only run runtime_upgrade_test on release branches and merge to manta
5. we run everything and the kitchen sink when it's merged to manta

All testing happens in a runtime-specific file and is triggered only when the PR touches the corresponding runtime as defined by the PR label.

I think this saves

• developer sanity maintaining the thing
• aws compute usage
• calamari and dolphin test workflows can be easily diffed against each other

Please discuss and contribute!

Also removes the manta runtime docker build from releases as these images are not currently used anywhere

closes #608

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
• Added one label out of the L- group to this PR
• Added one or more labels out of A- and C- groups to this PR
• Re-reviewed Files changed in the Github PR explorer.

Situational Notes:

• If adding functionality, write unit tests!
• If importing a new pallet, choose a proper module index for it, and allow it in BaseFilter. Ensure every extrinsic works from front-end. If there's corresponding tool, ensure both work for each other.
• If needed, update our Javascript/Typescript APIs. These APIs are officially used by exchanges or community developers.
• If modifying existing runtime storage items, make sure to implement storage migrations for the runtime and test them with try-runtime. This includes migrations inherited from upstream changes, and you can search the diffs for modifications of #[pallet::storage] items to check for any.
• If runtime changes, need to update the version numbers properly:
  • authoring_version: The version of the authorship interface. An authoring node will not attempt to author blocks unless this is equal to its native runtime.
  • spec_version: The version of the runtime specification. A full node will not attempt to use its native runtime in substitute for the on-chain Wasm runtime unless all of spec_name, spec_version, and authoring_version are the same between Wasm and native.
  • impl_version: The version of the implementation of the specification. Nodes are free to ignore this; it serves only as an indication that the code is different; as long as the other two versions are the same then while the actual code may be different, it is nonetheless required to do the same thing. Non-consensus-breaking optimizations are about the only changes that could be made which would result in only the impl_version changing.
  • transaction_version: The version of the extrinsics interface. This number must be updated in the following circumstances: extrinsic parameters (number, order, or types) have been changed; extrinsics or pallets have been removed; or the pallet order in the construct_runtime! macro or extrinsic order in a pallet has been changed. You can run the metadata_diff.yml workflow for help. If this number is updated, then the spec_version must also be updated
• Verify benchmarks & weights have been updated for any modified runtime logics

[Garandor]

It looks like having if: on each job is spammy in the list, anyone know if if: is allowed above the job: level in gh actions?

[Garandor]

There are likely duplicated tests in this PR. I'm opening this as a first attempt for discussion. please feel free to pick this up and push more commits

Most likely the runtime_upgrade_test can be refactored out of these as well

[Garandor]

One more optimization could be to use https://github.com/rust-lang/docker-rust-nightly

as in runs_on: rustlang/rust:nightly instead of running rustup at our compute cost on every run

[ghzlatarev]

lgtm. the only reason it was like this was to not dupe the code, but we've ended up doing that in a lot of other places and we can take care of that problem separately. Probably less painful than maintaining this monstrosity.

[Apokalip]

lgmt! I don't know what part of this will be moved to the type script tests, but as seeing how the signer is now on my plate too, that will take some time.

[ghzlatarev]

Not sure if this does something for the build times, it's always building the same binary but not it needs to build it twice sometimes.
Overall 50% - 50% on the core idea of doing less CI and addon with labels, instead of doing all CI and limiting based on some conditions. I guess it's more compute efficient to do the former, but then we also have more human overhead of making sure labeling is correct. And i'm really not sure this is a bottleneck.
I think optimally you want a workflow that just builds the binary and then the split (for easier maintenance) test-workflows can wait for the first workflow and download the binary and then do their thing.

[Garandor]

Not sure if this does something for the build times, it's always building the same binary but not it needs to build it twice sometimes.

That's why i'm saying the last big optimization is to make a reusable workflow ( #320 ) and that runs once and all others use the common build output from that.
I think it's worth the extra compute for now given that it

• saves time if the integration tests and RT upgrades are not run on every commit
• is significantly more maintainable

[Garandor]

Plus I for one can't reason about the dependencies of the CI jobs in a >1k spaghetti code workflow.
Did you know that calamari-integration-test depends on buiild-runtimes when it doesn't actually use it?

There seems to be a problem with aws scheduling tho
https://github.com/Manta-Network/Manta/runs/8036038006?check_suite_focus=true#step:2:29

Error: aws scheduled instance starting error
Error: VcpuLimitExceeded: You have requested more vCPU capacity than your current vCPU limit of 2064 allows for the instance bucket that the specified instance type belongs to. Please visit http://aws.amazon.com/contact-us/ec2-request to request an adjustment to this limit.

[ghzlatarev]

Not sure if this does something for the build times, it's always building the same binary but not it needs to build it twice sometimes.

That's why i'm saying the last big optimization is to make a reusable workflow and that runs once and all others use the common build output from that. I think it's worth the extra compute for now given that it

• saves time if the integration tests and RT upgrades are not run on every commit
• is significantly more maintainable

I agree it would make things easier to maintain, just saying it won't do much about build times, which you included as a reason in the description.
My only concern here is the extra human effort of doing the labeling correctly, so the right workflows are triggered, and then the reviewers reviewing it correctly. So I'd keep the split of the workflows but no the labeling.

[Garandor]

I agree it would make things easier to maintain, just saying it won't do much about build times, which you included as a reason in the description.

Looking at the build times, I agree. The check_tests workflow that always runs takes ~90mins which is the same or more as all the integration tests which run in parallel, so skipping those won't speed up the build unfortunately :(
It will however save ~1 hour of pointlessly wasted cloud compute per skipped integration test

My only concern here is the extra human effort of doing the labeling correctly, so the right workflows are triggered, and then the reviewers reviewing it correctly. So I'd keep the split of the workflows but no the labeling.

We're labelling these anyway because our changelog depends on them being set correctly and

• laziness is not an excuse ( we need to add it to the PR template tho )
• the workflows will run on merge at the latest ( whether labelled or not )

[Dengjianping]

I saw we're building docker image for manta(actually it's the same image as calamari's). It doesn't make much sense to me.
https://hub.docker.com/r/mantanetwork/manta/tags

We should not push manta image to dockerhub.
We can enable it when manta wins the auction.

[Garandor]

I saw we're building docker image for manta(actually it's the same image as calamari's). It doesn't make much sense to me. https://hub.docker.com/r/mantanetwork/manta/tags

We should not push manta image to dockerhub. We can enable it when manta wins the auction.

Thx for the suggestion, i added it

in the future, pls add new stuff that isn't related with the current PR as a new issue, I'd rather not have a PR that's meant to be about one thing become larger in scope ( and delay being merged )

[ghzlatarev]

I agree it would make things easier to maintain, just saying it won't do much about build times, which you included as a reason in the description.

Looking at the build times, I agree. The check_tests workflow that always runs takes ~90mins which is the same or more as all the integration tests which run in parallel, so skipping those won't speed up the build unfortunately :( It will however save ~1 hour of pointlessly wasted cloud compute per skipped integration test

My only concern here is the extra human effort of doing the labeling correctly, so the right workflows are triggered, and then the reviewers reviewing it correctly. So I'd keep the split of the workflows but no the labeling.

We're labelling these anyway because our changelog depends on them being set correctly and

• laziness is not an excuse ( we need to add it to the PR template tho )
• the workflows will run on merge at the latest ( whether labelled or not )

I'm 50/50 so cool to try it, but as you mention let's add a line to the PR template as a reminder that labeling needs to be done correctly, otherwise CI won't run. As well as a reminder that just because the code diff doesn't include some runtime, it doesn't mean that CI shouldn't be run for it (eg: changing the CI integration test workflows themselves)