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

podman host is different #3408

Closed
wants to merge 7 commits into from
Closed

podman host is different #3408

wants to merge 7 commits into from

Conversation

ppphp
Copy link

@ppphp ppphp commented Oct 7, 2023

No description provided.

@CLAassistant
Copy link

CLAassistant commented Oct 7, 2023

CLA assistant check
All committers have signed the CLA.

@vistaarjuneja
Copy link
Collaborator

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
Copy link
Collaborator

johannesHarness commented Oct 9, 2023

@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
Copy link
Collaborator

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
Copy link
Author

ppphp commented Nov 7, 2023

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

@ppphp ppphp closed this Nov 14, 2023
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

Successfully merging this pull request may close these issues.

None yet

4 participants