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

Install dev dependencies in separate environment on self-hosted runner #432

Closed
danielhollas opened this issue Feb 27, 2024 · 9 comments
Closed
Assignees

Comments

@danielhollas
Copy link
Contributor

I saw a very strange test failure in #431

https://github.com/aiidalab/aiidalab-docker-stack/actions/runs/8054045469/job/22002214446?pr=431

Pytest fails in a very peculiar way inside inside pytest-selenium plugin. But we do not use this plugin in the repo!!!

Looking at the .github/actions/create-dev-env/action.yml, it seems that we're installing via pip into some global folder? This seems very bad. We should probably create a venv and install things locally. Something like python -m venv && source .venv/bin/activate && pip install -r requirements-dev.txt.

@unkcpz
Copy link
Member

unkcpz commented Feb 27, 2024

The environment is set when the runner user started, so every time the action is run in with the environment activate.
I do it by having runsvc.sh (this is the script start the self-host runner) as:

➜  aiidalab-actions-runner cat runsvc.sh 
#!/bin/bash

# convert SIGTERM signal to SIGINT
# for more info on how to propagate SIGTERM to a child process see: http://veithen.github.io/2014/11/16/sigterm-propagation.html
trap 'kill -INT $PID' TERM INT

if [ -f ".path" ]; then
    # configure
    export PATH=`cat .path`
    echo ".path=${PATH}"
    source .venv/aiidalab-runner/bin/activate
    export PATH=$HOME/.venv/aiidalab-runner/bin:$PATH
    export DOCKER_HOST="unix://$HOME/.colima/aiidalab/docker.sock"
fi

nodever=${GITHUB_ACTIONS_RUNNER_FORCED_NODE_VERSION:-node16}

# insert anything to setup env when running as a service
# run the host process which keep the listener alive
./externals/$nodever/bin/node ./bin/RunnerService.js &
PID=$!
wait $PID
trap - TERM INT
wait $PID

But I think to make it more clear, this source activate better to move in the action. Because we use this environment for both the aiidalab-docker-stack and aiidalab-qe. The python environments are the same for both at the moment, but should be independent.

@danielhollas
Copy link
Contributor Author

Because we use this environment for both the aiidalab-docker-stack and aiidalab-qe. The python environments are the same for both at the moment, but should be independent.

Oh, that's definitely not good. And I think even in the same repo, each individual jobs should actually have their own environment, otherwise it's very easy for different PRs to step on each other.

I don't thing this logic should live in the actions. Surely, it must be possible to provide some setup script that runs each time when a runner picks up a job?

@danielhollas
Copy link
Contributor Author

The environment is set when the runner user started, so every time the action is run in with the environment activate.
I do it by having runsvc.sh (this is the script start the self-host runner) as:

It would be great to put all the runner logic into some repo, otherwise it's completely invisible.

@danielhollas
Copy link
Contributor Author

I am quite confused about the overall logic in the self-hosted runners. Can I ask, how many runners can run concurrently?

Thing I am worried about is specifically this part of build-test-upload

- name: Reset docker state and cleanup artifacts 🗑️

The comment says:

Self-hosted runners share a state (whole VM) between runs
Also, they might have running or stopped containers,
which are not cleaned up by docker system prun

In this step all the containers are stopped and deleted. Given the above comment, how is it ensured that this does not interfere with a running job?

@unkcpz
Copy link
Member

unkcpz commented Mar 20, 2024

Can I ask, how many runners can run concurrently?

Only one can run concurrently. The aiida-core runner and the aiidalab runner are seprated so they can run at the same time.
But the aiidalab-qe test and the aiidalab-docker-stack test will run in sequence.

@danielhollas danielhollas changed the title Install dev dependencies in in separate environment on self-hosted runner Install dev dependencies in separate environment on self-hosted runner Apr 28, 2024
@danielhollas
Copy link
Contributor Author

@unkcpz if aiida-core and aiidalab runners can run concurrently, do they also have separate docker daemons? Because in the current workflow there is a logic that stops docker and cleans or images, containers. So I am just wondering if they are fully separate in this regard. I am not sure how docker works in multiuser environments.

@unkcpz
Copy link
Member

unkcpz commented Apr 29, 2024

@unkcpz if aiida-core and aiidalab runners can run concurrently, do they also have separate docker daemons?

Yes, they are separated docker instance managed by colima. The DOCKER_HOST is set in the runsvc.sh for each runner point to aiidalab and aiida-core runner. For each runner, the colima (docker) has:

PROFILE       STATUS     ARCH       CPUS    MEMORY    DISK     RUNTIME    ADDRESS
aiida-core    Running    aarch64    2       2GiB      60GiB    docker     
aiidalab      Running    aarch64    2       2GiB      60GiB    docker   

@unkcpz
Copy link
Member

unkcpz commented Apr 29, 2024

For the python environment, what you mentioned is correct, there is a buggy set that aiidalab-docker-stack and aiidalab-qe use the same python environment. The environment setup should also be independent for these two, otherwise every rerun will override the docker build packages installed by the other. Currently it causes no issue since they have the same dependencies (set in requirements-dev.txt).

@danielhollas
Copy link
Contributor Author

Closing for now, but this will need to be fixed if we ever go back to the self-hosted runner.

@danielhollas danielhollas closed this as not planned Won't fix, can't repro, duplicate, stale May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants