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

Parallels driver is getting called as a built-in library instead of RPC Server #9493

Closed
legal90 opened this issue Oct 19, 2020 · 6 comments · Fixed by #9494 or #9517
Closed

Parallels driver is getting called as a built-in library instead of RPC Server #9493

legal90 opened this issue Oct 19, 2020 · 6 comments · Fixed by #9494 or #9517
Labels
co/parallels-driver Parallels driver issues kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@legal90
Copy link
Contributor

legal90 commented Oct 19, 2020

The issue is well-described in this thread (starting from the linked comment): Parallels/docker-machine-parallels#93 (comment)

We realized that the output of parallels driver does not depend on the actual binary of docker-machine-driver-parallels available in PATH. It appeared that minikube was calling driver's functions as a usual library, without launching the binary as the RPC Server.

I found the root cause of this and will submit a fixing PR very soon

@afbjorklund
Copy link
Collaborator

afbjorklund commented Oct 20, 2020

Interesting, this might actually be the same issue as with all other drivers in the registry: #5549

i.e. we see two calls to init, the real one and then a bogus extra call without any information

@afbjorklund afbjorklund added co/parallels-driver Parallels driver issues kind/bug Categorizes issue or PR as related to a bug. labels Oct 20, 2020
@tstromberg tstromberg added the long-term-support Long-term support issues that can't be fixed in code label Oct 21, 2020
@tstromberg
Copy link
Contributor

This is definitely not by design -- the docs mention that the driver needs to be downloaded first.

@tstromberg tstromberg added priority/backlog Higher priority than priority/awaiting-more-evidence. and removed long-term-support Long-term support issues that can't be fixed in code labels Oct 21, 2020
@tstromberg
Copy link
Contributor

@legal90 - if you have a PR to address this, feel free to send the review my way.

@legal90
Copy link
Contributor Author

legal90 commented Oct 21, 2020

@tstromberg I sent a PR which "fixes" this issue by making minikube <-> parallels driver communication trough RPC: #9494

However, based on #9494 (comment), it seems that you guys prefer to have drivers built-in.
If so - I'll send a new PR soon, which will update the docs and remove the check of driver binary.

@afbjorklund
Copy link
Collaborator

Some drivers are built-in (like virtualbox), some have an external binary (like docker-machine-driver-kvm2).

Our driver "registry" looks to be rather confused, though. So it might very well be that it tries to do both...

I think the main reason for not building everything in, would be that it would then also link to extra libraries.

For instance, the kvm2 driver requires libvirt to be installed on the host. But virtualbox just uses VBoxManage

@legal90
Copy link
Contributor Author

legal90 commented Oct 21, 2020

@afbjorklund Thank you!
Here is the alternative fix which keeps the parallels driver built-in (as it is since a while) and removes the redundant check of docker-machine-driver-parallels binary: #9517

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
co/parallels-driver Parallels driver issues kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
3 participants