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

ENH: enable capability from command line to bind host volumes to containers #1451

Closed
wants to merge 5 commits into from

Conversation

rraymondgh
Copy link

See also question I raised on SO https://stackoverflow.com/questions/74516196/nektos-act-docker-containers-how-to-mount-a-volume

There are quite significant overheads with conda and act. Sharing the downloaded packages between host and containers by binding host directory of downloaded packages with act containers significantly improves situation.

This is the first time I've coded in go. I'm not really aware of things like PEP8/black/flake8 and pytest that are standard in python

@rraymondgh rraymondgh requested a review from a team as a code owner November 22, 2022 16:52
@KnisterPeter
Copy link
Member

Hmm, I think I don't really understand the use case.
How would that work on GitHub?
And aren't GitHub Caches good for that?

@rraymondgh
Copy link
Author

rraymondgh commented Nov 22, 2022

@KnisterPeter

Hmm, I think I don't really understand the use case.
How would that work on GitHub?
And aren't GitHub Caches good for that?

conda can be a very big overhead on resolving dependencies and downloading packages. Try cloning geopandas and run act on it. Many hours just to build envs when using act then a few minutes to run tests. I've looked through all the options of https://github.com/marketplace/actions/setup-miniconda which is why I have moved to lock files for running locally. I'm trying to avoid completely restructuring actions that work fine on GitHub servers to run locally. Possibly the conda use case with complex package dependencies is too specific a use case for act. I've extended catthehacker/ubuntu:act-latest to optimise conda and then using a local docker repo to serve it up to act

@KnisterPeter
Copy link
Member

I don't know conda and I still struggle to understand why it works fine (fast) on GitHub while it's slow with act.
Maybe one if the other maintainers knows conda or understands your case.

@rraymondgh
Copy link
Author

I don't know conda and I still struggle to understand why it works fine (fast) on GitHub while it's slow with act.
Maybe one if the other maintainers knows conda or understands your case.

It is quite difficult to get working... I haven't got it working in a satisfactory way yet. One thing I needed was a bit more flexibility to be able to mount volumes in act containers hence decided to contribute this PR back. When I have it working I can add more commentary to this PR. For now the change sitting in my fork is fine.

@ChristopherHX
Copy link
Contributor

ChristopherHX commented Nov 23, 2022

I haven't got it working in a satisfactory way yet.

Do you really need / want container isolation?

  • If yes
    • act --reuse makes it possible to persist the package installation, conda would probably detect that your packages are already installed.
    • start the container yourself via docker cli, then execute act(master/v0.2.34) via -P ...=-self-hosted inside the container.
  • If no
    • execute act(master/v0.2.34) via -P ...=-self-hosted on your linux system, no bind mounts needed, no container

Wouldn't it make sense to add one --container-options flag and get rid of the extra flags, to clean up the act cli? We already have a docker option parser merged to the codebase

@KnisterPeter
Copy link
Member

Wouldn't it make sense to add one --container-options flag and get rid of the extra flags, to clean up the act cli? We already have a docker option parser merged to the codebase

That sounds like a good idea.

@rraymondgh
Copy link
Author

@ChristopherHX

  • If yes

    • act --reuse makes it possible to persist the package installation, conda would probably detect that your packages are already installed.
    • start the container yourself via docker cli, then execute act(master/v0.2.34) via -P ...=-selfhosted inside the container.
  • If no

    • execute act(master/v0.2.34) via -P ...=-selfhosted on your linux system, no bind mounts needed, no container

I have tried --reuse unfortunately https://github.com/marketplace/actions/setup-miniconda isn't smart enough to realise conda env and packages are already in place and then just gets errors from it... I have now pretty much got it working with the fork associated with this PR, a vagrant box and a bit of dark magic with dangerous conda options. Are you referring to this #1015 / your other project wrt -P ...=selfhosted?

Wouldn't it make sense to add one --container-options flag and get rid of the extra flags, to clean up the act cli? We already have a docker option parser merged to the codebase

I spotted that but honestly my go knowledge is really just that I'm a computer scientist / coder, so can read code on any language. I don't have time to invest in really getting to a level in go to make this level of change.

I guess the conclusion is close this PR, too specific use case to be valid....

@ChristopherHX
Copy link
Contributor

I'm reffering with -P ubuntu-latest=-self-hosted to a merged enhancement, which will be released with act v0.2.34. See also #1450 (comment)

Seams like I forget a - in my previous comment, fixed it

Yeah I reused the syntax from some other project of mine

@ChristopherHX
Copy link
Contributor

@rraymondgh Would #1462 work for you?
This adds the --container-options flag.

e.g.

act --container-options "-v $pwd/host_environment:/host_environment" -W test.yml -P ubuntu-latest=ubuntu:latest

This avoids adding every docker cli option one by one, it's a mess.

@rraymondgh
Copy link
Author

@ChristopherHX

act --container-options "-v $pwd/host_environment:/host_environment" -W test.yml -P ubuntu-latest=ubuntu:latest

This avoids adding every docker cli option one by one, it's a mess.

This is excellent. There is a piece of code still looking for docker. Following doesn't stop running, but more of an FYI.

ERRO[0000] failed to obtain container engine info: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?

@rraymondgh
Copy link
Author

self hosted works much better. no need for this

@rraymondgh rraymondgh closed this Nov 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants