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

make docker-ci improvements #163

Open
1 of 4 tasks
andreineculau opened this issue Mar 10, 2020 · 1 comment
Open
1 of 4 tasks

make docker-ci improvements #163

andreineculau opened this issue Mar 10, 2020 · 1 comment

Comments

@andreineculau
Copy link
Contributor

andreineculau commented Mar 10, 2020

  • 1. performance of mount folders on a macos host https://docs.docker.com/docker-for-mac/osxfs-caching/ --- use :cached
  • 2. at times, there will be platform-dependent dependencies, and then it doesn't make sense to mount the whole repo (you'll need to make nuke deps build inside docker, and then once again outside docker, when finished). You might be good to go mounting the repo/.git folder instead of running git checkout . inside docker. This way, still have access to the whole git history, but you have a separate checkout instance
  • 3. proxy variables likes AWS_* or GH_TOKEN from the host to the docker container
  • 4. make docker-ci needs to be followed by a ./.ci.sh before_install to install required system dependencies, plus it can take a long time to create GIDs and do recursive chown, etc so ideally there would be a make docker-ci/scratch and make docker-ci where the latter would actually make use of an existing local image based on SF_DOCKER_CI_IMAGE + GIDs/etc + ./.ci.sh before_install

cc @spschlegel
cc @weetmuts no 4 would be the intermediary step for creating repo-based docker images

@tobiiasl
Copy link
Contributor

Not sure if it is related to this PR but I noted that we explicitly check against TRAVIS env vars in the sf_run_docker_ci_image() function (see https://github.com/tobiipro/support-firecloud/blob/master/repo/ci.sh/before-install.pre.inc.sh#L21-L22).

I guess that should be abstracted in some way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants