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

fix: ensure docker in docker can run properly via new env variable #770

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

davemooreuws
Copy link
Member

@davemooreuws davemooreuws commented Aug 13, 2024

Fixes #769

Adds new env variable called NITRIC_DOCKER_HOST , this can be set to an IP like 172.17.0.4 on runners like GitLab and other Docker in Docker environments.

I have tested on:

  • GitLab runners with docker 27

Copy link

codecov bot commented Aug 13, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 1.08%. Comparing base (1723d34) to head (1b4ad81).
Report is 1 commits behind head on main.

Files Patch % Lines
pkg/project/service.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #770   +/-   ##
=====================================
  Coverage   1.08%   1.08%           
=====================================
  Files         80      79    -1     
  Lines       8886    8853   -33     
=====================================
  Hits          96      96           
+ Misses      8780    8747   -33     
  Partials      10      10           
Flag Coverage Δ
unittests 1.08% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tjholm
tjholm previously requested changes Aug 13, 2024
pkg/docker/linuxhost.go Outdated Show resolved Hide resolved
@tjholm
Copy link
Member

tjholm commented Aug 13, 2024

It looks like gitlab exposes a docker hostname locally.

DOCKER_HOST as you've mentioned appears to be a convention for configuring this in CI/CD.

When you tested setting DOCKER_HOST did you use the env variable in the CLI to set host.docker.internal to the same value?

Rather than searching the eth0 which may not apply in all cases we could look at allowing it to be configured? e.g. use DOCKER_HOST or something else like NITRIC_HOST?

Main concern is that a search for eth0 will not cover all cases or may change/break in ways we may not be able to control or predict.

@davemooreuws
Copy link
Member Author

davemooreuws commented Aug 13, 2024

It looks like gitlab exposes a docker hostname locally.

DOCKER_HOST as you've mentioned appears to be a convention for configuring this in CI/CD.

When you tested setting DOCKER_HOST did you use the env variable in the CLI to set host.docker.internal to the same value?

Rather than searching the eth0 which may not apply in all cases we could look at allowing it to be configured? e.g. use DOCKER_HOST or something else like NITRIC_HOST?

Main concern is that a search for eth0 will not cover all cases or may change/break in ways we may not be able to control or predict.

Agreed, we want to avoid it being brittle. Will need to double check setting the same value as DOCKER_HOST. I think a nitric based env var will work more broadly (have a feeling the azure and GCP might need configuring as well), but let's try it tomorrow. We can keep the WSL fix in.

@davemooreuws davemooreuws marked this pull request as draft August 14, 2024 00:56
@davemooreuws davemooreuws force-pushed the fix/linux-dind branch 5 times, most recently from c4a5e18 to 8961e02 Compare August 14, 2024 02:55
@davemooreuws davemooreuws marked this pull request as ready for review August 14, 2024 02:55
@davemooreuws davemooreuws changed the title fix: ensure docker in docker runs use correct host IP fix: ensure docker in docker can run properly via new env variable Aug 14, 2024
@davemooreuws davemooreuws force-pushed the fix/linux-dind branch 2 times, most recently from 36d4f86 to 0e32b5e Compare August 14, 2024 03:03
@davemooreuws davemooreuws marked this pull request as draft August 14, 2024 03:59
@davemooreuws davemooreuws marked this pull request as ready for review August 14, 2024 06:09
@tjholm tjholm merged commit 40df95f into main Aug 14, 2024
5 of 6 checks passed
@tjholm tjholm deleted the fix/linux-dind branch August 14, 2024 06:42
@nitric-bot
Copy link
Contributor

🎉 This PR is included in version 1.50.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

GitLab pipelines issue running nitric up on shared runners
6 participants