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.

Sign up for GitHub

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

Jump to bottom

Remove localkube from bindata #2089

Merged
merged 4 commits into from
Oct 20, 2017
Merged

Remove localkube from bindata #2089

merged 4 commits into from
Oct 20, 2017

Conversation

r2d4
Copy link
Contributor

@r2d4 r2d4 commented Oct 19, 2017

This makes the minikube build simpler and the the binary smaller.
Localkube will always be remotely fetched and locally cached on the
host.

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 19, 2017
@r2d4
Copy link
Contributor Author

r2d4 commented Oct 19, 2017

@minikube-bot retest this please

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 19, 2017
@r2d4 r2d4 force-pushed the no-localkube branch 2 times, most recently from a6375eb to df927ad Compare October 19, 2017 16:49
@r2d4
Remove localkube from bindata
a7d5684 
This makes the minikube build simpler and the the binary smaller.
Localkube will always be remotely fetched and locally cached on the
host.
@dlorenc
Copy link
Contributor

dlorenc commented Oct 19, 2017

@minikube-bot test this please

@r2d4
Copy link
Contributor Author

r2d4 commented Oct 19, 2017

Ah theres an error since minikube reference the hyperkit package, which then references go-vmnet, which requires CGO_ENABLED=1. I'll see if i I can figure out how to remove this dependency

@r2d4
Split out hyperkit into CGO needed stub
02f0d41 
This allows us to use the hyperkit Driver struct in minikube, without
needing CGO_ENABLED=1.  For the hyperkit driver binary, the
CGO_ENABLED=1 function will work.
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 19, 2017
@codecov-io
Copy link

Codecov Report

Merging #2089 into master will decrease coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2089      +/-   ##
==========================================
- Coverage   28.92%   28.84%   -0.08%     
==========================================
  Files          81       81              
  Lines        5311     5307       -4     
==========================================
- Hits         1536     1531       -5     
- Misses       3582     3583       +1     
  Partials      193      193
Impacted Files Coverage Δ
pkg/minikube/bootstrapper/localkube/localkube.go 30.76% <100%> (+0.24%) ⬆️
...nikube/bootstrapper/localkube/localkube_caching.go 39.62% <0%> (-5.67%) ⬇️
pkg/util/kubeconfig/config.go 46.93% <0%> (-0.69%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b23ac9...a19f90f. Read the comment docs.

@r2d4
Copy link
Contributor Author

r2d4 commented Oct 19, 2017 •
edited
Loading

This also removes CGO_ENABLED=1 from minikube (but not localkube), making it easier to cross build and compile

@r2d4
Copy link
Contributor Author

r2d4 commented Oct 20, 2017

An additional note: if you want to test localkube changes now, you'll need to set --kubernetes-version=file://$PWD/out/localkube

@r2d4 r2d4 merged commit d9089c3 into kubernetes:master Oct 20, 2017
@r2d4 r2d4 deleted the no-localkube branch October 20, 2017 16:29
@r2d4 r2d4 mentioned this pull request Oct 20, 2017
Merged
@NeQuissimus NeQuissimus mentioned this pull request Nov 13, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dlorenc dlorenc dlorenc approved these changes

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@r2d4 @dlorenc @codecov-io @k8s-ci-robot