machine-drivers / docker-machine-driver-xhyve

docker-machine/minikube/minishift driver plugin for xhyve/hyperkit (native macOS hypervisor.framework)
https://godoc.org/github.com/machine-drivers/docker-machine-driver-xhyve
BSD 3-Clause "New" or "Revised" License
888 stars 74 forks source link

xhyve: Fix included '\tappend\s' in kexec boot option #153

Closed zchee closed 7 years ago

zchee commented 7 years ago

Now kernel option is like

{
    "ConfigVersion": 3,
    "Driver": {
        "BootCmd": "\tappend loglevel=3 user=docker console=ttyS0 console=tty0 noembed nomodeset norestore waitusb=10:LABEL=boot2docker-data base",
    },

xhyve can boot even if including \tappend, but that's wrong kernel option. Fix it. ref: https://regex101.com/r/LghmGX/4

/cc @praveenkumar Could you test it? If no problem, I'll release new version after merge it.

zchee commented 7 years ago

@praveenkumar If I have time when re-fix regexp, also adding *_test.go unit testing. I have avoided it until now (because it was troublesome :P), It's inevitable.


BTW, if you no problem, I want to add you to collaborators, what do you think?

praveenkumar commented 7 years ago

If I have time when re-fix regexp, also adding *_test.go unit testing.

That's sound like a plan 👍

BTW, if you no problem, I want to add you to collaborators, what do you think?

I am happy to test out stuff and do PR reviews if that's all you need from my side. I am not sure how much I can help you sending patches for listed issues right now due to my day job. If you think that would be enough for a collaborator then go ahead :)

zchee commented 7 years ago

@praveenkumar Thanks and np! You(and Red Hat team) was very helped me. I was very glad, so want to it. Not need sending many patches, that as a good contributor icon :) I'll adding you.

praveenkumar commented 7 years ago

Thanks and np! You(and Red Hat team) was very helped me. I was very glad, so want to it.

So generous of you, Thanks for quick feedback loops 👍

zchee commented 7 years ago

@praveenkumar

So generous of you, Thanks for quick feedback loops

:) But currently, I have no job. So quickly is normal lol


memo: travis-ci macOS build too slow. still not running. I'll consider disabling the travis, and using osxcross natively(without Docker build) on circleci which can cross-compiling with macOS Frameworks on the Linux. But hdiutil etc are can't test. only macOS system independent tests.

praveenkumar commented 7 years ago

memo: travis-ci macOS build too slow. still not running.

Sure, Can you update this PR so I can do testing again and let you know?

zchee commented 7 years ago

@praveenkumar Ah, sorry for delay. I'll research alter regexp pattern, and re-commit later.

I'll do it, now.

zchee commented 7 years ago

@praveenkumar Sorry, I want to test with centos iso. Could you tell me centos.iso url?

praveenkumar commented 7 years ago

I want to test with centos iso. Could you tell me centos.iso url?

https://www.dropbox.com/s/bytmy3p2skfwy11/minishift-centos.tar.gz?dl=0 currently image size is around ~315 MB but let's test it out.

zchee commented 7 years ago

@praveenkumar Thanks :) Now downloading...

zchee commented 7 years ago

@praveenkumar I can't boot using docker-machine(maybe need minishift?) but got below option. Is it correct?

Extracted Options "initrd=initrd0.img root=live:CDLABEL=minishift-centos rootfstype=auto ro rd.live.image quiet no_timer_check console=ttyS0 console=tty0 rhgb rd.luks=0 rd.md=0 rd.dm=0 "
praveenkumar commented 7 years ago

I can't boot using docker-machine(maybe need minishift?) but got below option.

Options looks as expected and yes you can boot using docker-machine this image.

Is it correct?

Absolutely.

zchee commented 7 years ago

@praveenkumar Ah, figured out. I was added --xhyve-qcow2 flag, but minishift-centos.iso does not support virtio-blk(maybe) https://github.com/kubernetes/minikube/commit/12d058573abb04db033a30e41e81b54102af4244

Without --xhyve-qcow2, works successfully. PTAL.

Edit: ref: https://regex101.com/r/LghmGX/5 Note that scanner.Scan scans line one by one. so boot2docker section is no problem.

zchee commented 7 years ago

@praveenkumar If you want to release new version immediately, I'll add the test to after the release new version.

praveenkumar commented 7 years ago

Without --xhyve-qcow2, works successfully. PTAL.

Tested this out and it worked as expected. LGTM :+1:

praveenkumar commented 7 years ago

If you want to release new version immediately, I'll add the test to after the release new version.

Agree on having a minor release version now and then start adding test cases.

zchee commented 7 years ago

@praveenkumar As an aside, I was got the circleci macOS OSS seed plan :) We can run the macOS testing on circleci with free(!)

I'll bump CHANGELOG.md. After that, I will concentrate on writing tests for a while.

praveenkumar commented 7 years ago

@zchee That's sound great :+1: will help you with tests whenever I have some spare time.