-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Conversation
💯 for @zedtux's amazing work. |
Tank you @caske33 !! |
Hell yeah! |
INCREDIBLE to see this during DockerCon! Awesome! Taking a look on my ubuntu machine. Exciting! |
@zedtux Maybe you should also update the readme to say that it also supports linux :) |
It's done @caske33. The only thing is as we still need to work on the packaging, I'm not touching the parts about the download and uninstall. |
@zedtux Maybe a new issue should be created so this PR can be merged already? |
Fine for me @caske33. |
Getting some errors on fresh install of ubuntu 14.04
|
Can you post all the commands you've run ? On my Debian box, I'm running:
And it's working. |
What is missing in order to merge this pull request ? |
@zedtux With DockerCon last week there hasn't been much time to QA your PR - I'll take a look at it today. |
Can you provide your version of NPM and NODEJS - Seem I'm already getting some dependency issues. |
Just to give you an idea of my steps so far from a vanilla install of 14.04 LTS $ sudo apt-add-repository ppa:chris-lea/node.js
$ sudo apt-get update
$ sudo apt-get install build-essential
$ sudo apt-get install nodejs This may be basic stuff, but it will probably need to be added in the docs for it to boot up. Also it seems that your current setup doesn't install docker? or it fails to install because of suid? |
@TeckniX chrislea is an old ppa. You should probably download node from nodesource: https://nodesource.com/blog/chris-lea-joins-forces-with-nodesource
|
Sorry @TeckniX but it's a bite tricky for me to work on Linux and provide features for the OS X / Windows version. I've just moved and my iMac is still in his box (yes .. what a shame !! :D) but I will tell you my versions as soon as possible. Now you should probably discuss more with @caske33 who's using Ubuntu (I'm using Debian). Finally, regarding the second screenshot you've posted, it looks like you didn't used |
@zedtux no need to add features for osx/windows, I just want to get ubuntu in there and working! The error is more likely to be related to the missing docker daemon - When Kitematic starts (on OSX/windows) it creates a VM to allow docker to function properly, as seen in the Setup.js: IMHO the Linux distro should do the same, although it can simply install/setup the docker daemon locally. Usually installed in /usr/local/bin since it won't be a managed package. (unless I missed where this is setup in your PR?) |
Sorry I wasn't clear. I'm talking about me, developing things like the editable volumes... I'm developing on Linux, but I can't include the Linux support in the editable volumes branch, so I have to do some tricking things in order to developer under Linux and provide the feature for the official repository.
The PR is not installing Docker as it has to be done using the package manager. This is quite a big job to implement as it should manage all the different Linux distribution. The purpose of this PR is not to make Kitematic working on a fresh Linux box, it's about to make Kitematic working on Linux, meaning you have a step before which is to make sure Docker is running on your machine. But I fully agree with you that at the end of the day, Kitematic on Linux should take care to install Docker automagically. |
And BTW the Docker installation should be done as a dependency of the Kitematic package. There's no need for a wizard. |
@zedtux in regards to this:
I agree 100% and it's what Kitematic does on Windows/Mac - As mentioned your PR disabled this as seen at the following line: |
@TeckniX what @zedtux is trying to say is that it shouldn't be done through the code but through the .deb, .rpm packages etc, which should have a dependency of the docker executable. The packages is something that still needs work, but we'll need help from someone with experience with .deb and .rpm packages. If you do this through code, you want have to check which package manager is installed, and then use the correct one. That's cubersome and shouldn't be done in code anyway. It should be in the .deb/.rpm package. |
@caske33 I didn't envision the Although the same argument could be made with ps: Obviously this is my opinion only based on how Kitematic currently works and the need to keep things homogeneous. (aka it does/checks the same thing on all platforms) |
The problem with linux is that you have a lot more ways of installing docker. Take a look at http://docs.docker.com/installation/. Only 1 of them is windows and 1 is Mac OS X. But you've got a couple of install guides for linux based distro's. That's why, for the maintainability of the feature, I suggest NOT to implement it in the code itself. However, maybe some sort of dialog box could be displayed if docker isn't installed? (maybe with link to the page I referenced earlier) so people can install it manually? |
I would be ok with that approach, although I believe most users using Kitematic would be on an ubuntu-like system - Basically let the user know that docker needs to be installed and provide a link to that page, if we don't detect an 'ubuntu' distro? Maybe this could show up where the VM error usually shows up? @zedtux what do you think? Edit: |
After merging the PR it works nicely on ubuntu 14.10. |
nice ! thank you for your contribution @zedtux |
Some additional feedback after installing docker from the above 'wget' command:
"Uncaught Exception: |
Almost there! |
Thanks @JeffDM but it's no more working ... 😮 |
Ho ... no ..it's fine :) sorry, I'm pushing ... |
That one last comment and then that's it from me. Amazing work @zedtux! |
Merge upstream master
@JeffDM the change is done. All seems to work still. |
Followed the install here https://github.com/docker/kitematic/wiki/Early-Linux-Support - got the following error. What am I doing wrong? Let me know if I just screwed up somewhere. |
Thanks @ryanleesipes for having reported this. I'll have a look at it. |
@ryanleesipes it's wired. I don't have this error. |
@ryanleesipes I think you found a bug in Kitematic. I don't have the error raised, but I can see the bug. The file |
LGTM |
Woohoo!!! Congrats @zedtux for pushing on this. All merged now 🎉 |
Whhoooo !! I'm so proud of this ! Amwesome ! Thank you so much @JeffDM, @FrenchBen, @mchiang0610 and @TeckniX ! 💪 🤘 |
Thank you @zedtux for being so persistent on this...well appreciated |
Oh, cool! Thanks @zedtux!! |
You're welcome @master-lincoln and @thaJeztah 🙇 |
@zedtux awesome to see this merged! Thank you so much! |
And thank you for having guide me in the last moments :) |
@zedtux I'm happy to see this merged 🎉 ! I think you helped a lot of people. Thank you for keeping up the good work! 💯 |
Thank you @caske33, I'm glade to help people. |
@ryanleesipes the bug you found has been fixed in this commit by @JeffDM |
@zedtux fantastic. Thanks for pointing me at that! :) |
Everything is working great. Anyone aware of any efforts to put this in a PPA? |
@ryanleesipes @FrenchBen is working on it I believe. |
Woohoo! Horray for getting this one!
|
This branch add the missing Linux support.
Basically it avoid the wizard to run and force
dockerode
to use/var/run/docker.sock
as asocketPath
.Regarding the in-container-shell only the GNOME terminal has been implemented (as I'm using Debian) but a warning message asking to open an issue in the project will be shown in other cases so that we can implement them one after the other.
The missing part of this merge request is the packaging. I don't really want to deal with that and let packagers taking care of this task but I'm available to help in case of questions and Grunt task fabrication.