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(k8s): skip setMinikubeDockerEnv when vm-driver=None #910

Merged
merged 2 commits into from
Jul 5, 2019

Conversation

dev-kpyc
Copy link
Contributor

@dev-kpyc dev-kpyc force-pushed the kpyc/fix-minikube-driver-none branch from e16be89 to a655dd2 Compare June 29, 2019 18:37
@edvald
Copy link
Collaborator

edvald commented Jun 30, 2019

Thanks @dev-kpyc! Looks good. integ tests are failing for unrelated reasons btw, seems to be for all external contributors.

edvald
edvald previously approved these changes Jun 30, 2019
@edvald edvald dismissed their stale review June 30, 2019 16:48

Found an issue

@edvald
Copy link
Collaborator

edvald commented Jun 30, 2019

Oops, spoke too soon. This will actually fail if you start minikube without a --vm-driver flag: minikube config get vm-driver exits with code 1 and a Error: specified key could not be found in config message. We'd need to explicitly catch that error, then it should be all good.

@dev-kpyc dev-kpyc force-pushed the kpyc/fix-minikube-driver-none branch 2 times, most recently from 8ffbe81 to 336fa6e Compare July 4, 2019 02:19
@dev-kpyc dev-kpyc force-pushed the kpyc/fix-minikube-driver-none branch from 336fa6e to 0825c5a Compare July 4, 2019 02:36
@edvald
Copy link
Collaborator

edvald commented Jul 4, 2019

Looks good @dev-kpyc. The integ test failures are unrelated to this. I'll do a quick manual check and then merge.

@edvald edvald merged commit ff58f9f into garden-io:master Jul 5, 2019
@dev-kpyc dev-kpyc deleted the kpyc/fix-minikube-driver-none branch July 6, 2019 18:00
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