Run E2E shielder tests in the CI. #25
Conversation
| @@ -145,6 +153,15 @@ docker_cargo() { | |||
| -c "cargo ${1}" | |||
| } | |||
|
|
|||
| ci_docker_cargo() { | |||
There was a problem hiding this comment.
We need a different version of the method when running in CI due to issues w/ permissions when running as a docker process - we can't create directories in cargo cache (and we try to do that when compiling contracts). Hence mounting of these dirs is removed.
There was a problem hiding this comment.
I find that hard to believe - since it works just fine for compiling here. Take a good look at mapping users and user groups when running these containers, that's usually what is responsible for no read/write privileges.
There was a problem hiding this comment.
In TheButton you're copying everything and running Makefile inside docker. It makes "some" sense there as there are many contracts to be built and Yapee didn't want to spin up the same docker container multiple times (and couldn't mount cargo dirs directly for the exact same reason I can't here), but it's not the case here.
tl;dr; in TheButton you don't write to underlying file system but the dir owned by the docker.
There was a problem hiding this comment.
Take a good look at mapping users and user groups when running these containers, that's usually what is responsible for no read/write privileges.
I had tried that, asked on Slack last week as well, but it doesn't work. Docker user is probably not a part of the required group on these machines and is not allowed to write to ~/.cargo/* dirs.
There was a problem hiding this comment.
Then I don't understand why is the deployment script set up differently than in TheButton? It runs CI on exactly the same self-hosted instance. It mounts and writes to the contract directories, same.
Either you don't have to mount .cargo, or just ask devops guys to make it writable for the usergroup that the docker is part of. Or launch docker as a user in one of those groups. We control that instance.
Having two methods to launch will come back and bite, and will make any problems on CI irreproducible locally, making it even harder to debug potential problems.
There was a problem hiding this comment.
In TheButton you don't have the self-contained Makefile and CI is set up differently from the local development. Here I'm re-using the same scripts, commands, etc except for these 3 lines in cargo-docker.
Either you don't have to mount .cargo,
Well, you don't have to - it's just speeding up the build time.
Having two methods to launch will come back and bite, and will make any problems on CI irreproducible locally, making it even harder to debug potential problems.
This setup is IMO actually going to help w/ that - the local vs CI setup is basically identical (again, except for the part whether you use cached dependencies to download them every time). It's much better than the situation TheButton in that case.
| uses: actions/checkout@v3 | ||
|
|
||
| - name: Install Rust toolchain | ||
| uses: actions-rs/toolchain@v1 |
There was a problem hiding this comment.
No, wait. This is needed. Just look at one of the recent, failed jobs. All self-hosted workflows require that https://github.com/Cardinal-Cryptography/aleph-node/blob/main/.github/workflows/_build-liminal-node.yml
There was a problem hiding this comment.
See if toolchain.yaml with a version won't solve this (next comment)
| uses: actions-rs/toolchain@v1 | ||
|
|
||
| - name: Install WASM target | ||
| run: rustup target add wasm32-unknown-unknown |
There was a problem hiding this comment.
shouldn't the target be baked into in the docker image already?
There was a problem hiding this comment.
Target backed to docker image can be overriden by toolchain in the contract itself. If they don't match it will fail when building.
There was a problem hiding this comment.
For solution try:
[toolchain]
channel = "nightly-2022-10-30"
targets = [ "wasm32-unknown-unknown" ]
components = [ "rust-src" ]then imho it should pull the required toolchain, target and components
There was a problem hiding this comment.
Yes, this will work. It's just an alternative way of doing the same thing.
| uses: actions/checkout@v3 | ||
|
|
||
| - name: Install Rust toolchain | ||
| uses: actions-rs/toolchain@v1 |
This reverts commit ed043ce.
No description provided.