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

feat: add support for running dokken in a container #281

Merged
merged 2 commits into from
Apr 23, 2023

Conversation

ThatsMrTalbot
Copy link
Contributor

Description

When kitchen-dokken is run inside docker itself with the socket mounted in it fails for a number of reasons:

  • Kitchen dokken will write files within $HOME/.dokken within the container, then attempt to bind bind that into the container being converged. Because bind mounts mount files/folders from the host this will cause failure.
  • If the docker instance is a docker-desktop instance (for example Docker for Mac) then the transport will resolve the incorrect address for the ssh connection

This change causes dokken to detect when it is run inside docker, and if it is it will aways use a data container. It will also connect to the data container using the internal "host.docker.internal" address if it detects the docker instance it is running is is a docker-desktop instance.

Issues Resolved

List any existing issues this PR resolves, or any Discourse or
StackOverflow discussions that are relevant

Type of Change

Our release process assumes you are using Conventional Commit messages.

The most important prefixes you should have in mind are:

  • _fix_: which represents bug fixes, and correlates to a SemVer patch.
  • _feat_: which represents a new feature, and correlates to a SemVer minor.
  • _feat!_:, or fix!:, refactor!:, etc., which represent a breaking change (indicated by the !) and will result in a major version change.

If you have not included a conventional commit message this can be fixed on merge.

Check List

@damacus
Copy link
Contributor

damacus commented Feb 20, 2023

LGTM! Can you rebase on main, please? Looks like we've got some linting errors. But happy to merge when ready

@damacus damacus self-assigned this Feb 20, 2023
@ThatsMrTalbot
Copy link
Contributor Author

ThatsMrTalbot commented Feb 22, 2023

I've rebased off main and fixed the linting issues that "bundle exec rake style" gave me, happy to fix up anything else that the workflow throws up

@ThatsMrTalbot
Copy link
Contributor Author

@damacus Would you be able to give this a look?

@damacus
Copy link
Contributor

damacus commented Mar 3, 2023

Oh you know what. This might fix some of the other new MacOS bugs I've seen with a new docker desktop install (non-root installed)

@ThatsMrTalbot
Copy link
Contributor Author

Potentially, these code changes only really change the behaviour if it dokken is running inside a container, aside from the change to the "docker_for_mac_or_win" function, which I did because all recent versions of docker desktop use the new name so the function was broken.

@tas50 tas50 merged commit bb38aca into test-kitchen:main Apr 23, 2023
@tas50
Copy link
Member

tas50 commented Apr 23, 2023

Thanks @ThatsMrTalbot

@ThatsMrTalbot
Copy link
Contributor Author

Are there plans to cut a release soon?

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

Successfully merging this pull request may close these issues.

4 participants