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

Implement extra-config for kubeadm components #1985

Merged
merged 1 commit into from
Oct 4, 2017

Conversation

r2d4
Copy link
Contributor

@r2d4 r2d4 commented Sep 20, 2017

These were all the changes needed to make

minikube start --kubernetes-version=v1.8.0-beta.0 --bootstrapper=kubeadm work.

The kubelet now will respect the extra config options set by minikube's --extra-config flags.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 20, 2017
@codecov-io
Copy link

codecov-io commented Sep 20, 2017

Codecov Report

Merging #1985 into master will decrease coverage by 1.1%.
The diff coverage is 42.7%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1985      +/-   ##
==========================================
- Coverage   29.69%   28.59%   -1.11%     
==========================================
  Files          77       80       +3     
  Lines        4842     5243     +401     
==========================================
+ Hits         1438     1499      +61     
- Misses       3218     3551     +333     
- Partials      186      193       +7
Impacted Files Coverage Δ
pkg/minikube/bootstrapper/kubeadm/kubeadm.go 11.91% <22.85%> (ø)
pkg/minikube/bootstrapper/kubeadm/versions.go 54.09% <54.09%> (ø)
pkg/minikube/bootstrapper/kubeadm/util.go 0% <0%> (ø)

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 4b595be...a139ccd. Read the comment docs.

func NewKubeletConfig(k8s bootstrapper.KubernetesConfig) (string, error) {
flags := []string{}
for _, opt := range k8s.ExtraOptions {
if opt.Component == "kubelet" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe make "kubelet" a constant if it's not already one?

}
}
// Strip leading 'v' prefix from version for semver parsing
fmt.Println(k8s.KubernetesVersion)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop the println?

}

// versions >= 1.8.0-alpha.0 require the --fail-swap-on flag set to false
if version.GTE(version18) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yuck, is this because or ISO has swap on? Should we just turn it off?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going forward this might be more maintainable if we have a map of special cased flags to versions, but this should be OK for now. Maybe just leave a TODO about that.

@r2d4 r2d4 changed the title Implement extra-config for kubeadm kubelet [WIP] Implement extra-config for kubeadm kubelet Sep 20, 2017
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 20, 2017
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 4, 2017
@r2d4
Copy link
Contributor Author

r2d4 commented Oct 4, 2017

Updated with:

  • Supports more configurable components through kubeadm: kubelet, controller-manager, apiserver, scheduler
  • Not supported yet: DNS and proxy
  • Introduces default "VersionedExtraOpts" which can be applied to a range of versions and overwritten by the --extra-option flag

@r2d4 r2d4 changed the title [WIP] Implement extra-config for kubeadm kubelet Implement extra-config for kubeadm kubelet Oct 4, 2017
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 4, 2017
@r2d4 r2d4 changed the title Implement extra-config for kubeadm kubelet Implement extra-config for kubeadm components Oct 4, 2017
@aaron-prindle
Copy link
Contributor

LGTM

@aaron-prindle aaron-prindle merged commit 8dbe63d into kubernetes:master Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants