search

harness / gitness

Gitness is an Open Source developer platform with Source Control management, Continuous Integration and Continuous Delivery.
https://gitness.com
Apache License 2.0
32.09k stars 2.8k forks source link

podman host is different #3408

Closed ppphp closed 9 months ago

CLAassistant commented 11 months ago

CLA assistant check
All committers have signed the CLA.

vistaarjuneja commented 11 months ago

hi @ppphp, thank you for your contribution!

There are three places in the code where we've hardcoded host.docker.internal today. In app/pipeline/runner/runner.go, we add in host.docker.internal to extra hosts - this is for linux specifically and not needed for Mac/Windows: https://medium.com/@TimvanBaarsen/how-to-connect-to-the-docker-host-from-inside-a-docker-container-112b4c71bc66.

Another place where we are assuming it to be the default is for the variable GITNESS_URL_CONTAINER in types/config.go. If this env variable is not set, we set it to host.docker.internal by default in cli/server/config.go. This URL is used for example for a pipeline containing a clone step to be able to talk to the container running gitness for the repository. We could probably make the logic to set the Container URL smarter by doing the same detection there in case the value is not set.

Would be great to know if you are testing on linux, and also if you had to change the env variable above to get clone steps working with podman. Thank you for contributing!

johannesHarness commented 11 months ago

@vistaarjuneja I think we should be driving the value from GITNESS_URL_CONTAINER and push that down.

As it is right now, the host used by containers is hard-coded in multiple places. In case the user has some custom networking setup, neither might work, and they can't change it.

Thus, I think we should set the GITNESS_URL_CONTAINER based on the container runtime (if it's not explicitly provided by the user) - and then all other values we should derive from that. In other words, we shouldn't set the host / protocol explicitly (as it is right now)

vistaarjuneja commented 10 months ago

hi @ppphp this looks good to me, there are just some minor lint issues. Would be great if you could fix those and update the PR.

ppphp commented 10 months ago

hi @ppphp this looks good to me, there are just some minor lint issues. Would be great if you could fix those and update the PR.

merge main now, sorry for my late