-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Kubeadm bootstrapper #1903
Kubeadm bootstrapper #1903
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1903 +/- ##
=========================================
- Coverage 30.11% 30% -0.12%
=========================================
Files 76 77 +1
Lines 4722 4740 +18
=========================================
Hits 1422 1422
- Misses 3120 3138 +18
Partials 180 180
Continue to review full report at Codecov.
|
Much smaller than the previous PR, nice. Any special pitfalls I should know about if I try to give this a test? |
There is no default hostpath storage provisioner (yet). Planning to run that as its own container once we get #1881 in |
Do I have to do the 'make an iso' step as well as the standard build? |
No, if you build the minikube binary from HEAD, it should be pointing to the correct ISO image. |
Stop, then restart functionality here is still broken - due to the fact that kube-proxy uses a configmap with the hardcoded IP to the apiserver. On stop/start, the IP can change and kube-proxy won't be updated with the new IP. I think I have a few ideas on how to handle this. |
I think I have to do |
Thats correct @MHBauer. The following are all equivalent:
|
I can confirm it starts up!
Not sure what's up with the version string mismatch there. I see a bunch of containerized bits of kube started inside. Makes me happy. rbac is working nicely. service-catalog installs correctly. service-catalog e2e's run and pass. I'm making a note here: HUGE SUCCESS! 🎉 |
|
c7484f5
to
b92a23e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kubeadmTmpl := "sudo /usr/bin/kubeadm init --config {{.KubeadmConfigFile}} --skip-preflight-checks" | ||
t := template.Must(template.New("kubeadmTmpl").Parse(kubeadmTmpl)) | ||
b := bytes.Buffer{} | ||
if err := t.Execute(&b, struct{ KubeadmConfigFile string }{constants.KubeadmConfigFile}); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you like, you can just use map[string]string{"KubeadmConfigFile: constants.KubeadmConfigFile}
instead of the anonymous struct. it might read cleaner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that's a good idea. I'll do that
sudo /usr/bin/kubeadm alpha phase kubeconfig all --config {{.KubeadmConfigFile}} && | ||
sudo /usr/bin/kubeadm alpha phase controlplane all --config {{.KubeadmConfigFile}} && | ||
sudo /usr/bin/kubeadm alpha phase etcd local --config {{.KubeadmConfigFile}} | ||
` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
each line in this string are going to be indented. is that ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll undent it. I don't think it matters either way
func (k *KubeadmBootstrapper) UpdateCluster(cfg bootstrapper.KubernetesConfig) error { | ||
if cfg.ShouldLoadCachedImages { | ||
// Make best effort to load any cached images | ||
go machine.LoadImages(k.c, constants.GetKubeadmCachedImages(cfg.KubernetesVersion), constants.ImageCacheDir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for my understanding ... do you know that this goroutine will finish before this function returns (do you need to)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, there no guarantee. Since this is just a "best efforts" feature. The main function exits when the apiserver is up, so at that point caching and loading is won't have much effect. In practice usually we have more than enough time to cache and load the images
} | ||
|
||
func (k *KubeadmBootstrapper) generateConfig(k8s bootstrapper.KubernetesConfig) (string, error) { | ||
t := template.Must(template.New("kubeadmConfigTmpl").Parse(kubeadmConfigTmpl)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as a defensive measure, you could put this (and the other function-scoped template.Must
calls above) at global scope, so that minikube
panics on startup if any template becomes invalid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea. Will do
Until we get caching and can run an external storage provisioner.
Add status for kubeadm integration tests
func GetKubernetesReleaseURL(binaryName, version string) string { | ||
// TODO(r2d4): change this to official releases when the alpha controlplane commands are released. | ||
// We are working with unreleased kubeadm changes at HEAD. | ||
return fmt.Sprintf("https://storage.googleapis.com/minikube-builds/v1.7.3/%s", binaryName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be v1.7.3?
Also does kubeadm bootstrapper respect the k8s version flags?
LGTM, just tried this and it works great! |
This is the last phase of and supersedes #1825