Skip to content

add host-gateway only when docker server is >= v20#459

Open
jafern14 wants to merge 5 commits into
oblador:masterfrom
jafern14:docker-backwards-compat
Open

add host-gateway only when docker server is >= v20#459
jafern14 wants to merge 5 commits into
oblador:masterfrom
jafern14:docker-backwards-compat

Conversation

@jafern14

@jafern14 jafern14 commented May 26, 2023

Copy link
Copy Markdown

This line appears to have been a breaking change and made docker v20+ a hard requirement when using loki. We can check for the version before applying it so as to allow for the docker engine to still run properly for previous versions.

Thanks for taking a look and providing any feedback. Your work here is very much appreciated!

@jafern14

Copy link
Copy Markdown
Author

I still need to add a test to mock out the docker cli version cmd to prove that it works. I wanted to get your thoughts on this before doing so though. Thanks!

@oblador

oblador commented Jun 5, 2023

Copy link
Copy Markdown
Owner

I'm cool with the change! Make sure to update the yarn.lock by running yarn in the root.

# Conflicts:
#	packages/target-chrome-docker/package.json
#	packages/target-chrome-docker/src/create-chrome-docker-target.js
@oblador

oblador commented Oct 31, 2023

Copy link
Copy Markdown
Owner

It seems this causes issues in GH Actions: Error: listen EACCES: permission denied 0.0.0.0:491

@jafern14

Copy link
Copy Markdown
Author

I'm not so sure this PR is to blame. I see a similar error happening on a build on the upstream master branch

https://github.com/oblador/loki/actions/runs/6707668160/job/18226746902

thoughts?

@oblador

oblador commented Oct 31, 2023

Copy link
Copy Markdown
Owner

Merging with master again and we can see if this is just flake :-)

@oblador

oblador commented Oct 31, 2023

Copy link
Copy Markdown
Owner

Different error now that seems related: 1 request failed to load; http://host.docker.internal:4567/1000/https://loki.js.org/img/favicon.png

@jafern14

Copy link
Copy Markdown
Author

Sounds good to me and thanks for following up on this! I appreciate it 👍.

I don't have time to look into it right now, but I'm wondering if there's some issue with the CI env and the port that docker is using behind the special host-gateway.

Do you know what version of docker is being used in the CI env?

@oblador

oblador commented Oct 31, 2023

Copy link
Copy Markdown
Owner

@oblador

oblador commented Oct 31, 2023

Copy link
Copy Markdown
Owner

Btw, regarding testing, I think it might be better to just have a test matrix for the integration tests rather than mocking stuff.

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.

2 participants