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

[WIP] Kubeadm bootstrapper #1825

Closed
wants to merge 12 commits into from
Closed

Conversation

r2d4
Copy link
Contributor

@r2d4 r2d4 commented Aug 16, 2017

Some TODOs

  • Make custom storage provisioner work with the kubeadm bootstrapper. Currently it doesn't since we run it directly in localkube. I've skipped that particular integration test right now.
  • Implement the "extra config" with the kubeadm config file
  • Use an upstream version of kubeadm once one is released with the phase alpha controlplane command. This was recently merged.
  • Status should check the status of the apiserver. Currently it just checks the status of the kubelet
  • Logs should return logs of all the components of the control plane.

TODOs probably in scope for this PR

  • Add unit tests
  • Handle the none driver. There are a few obvious fixes needed, but I haven't done any extensive testing.
  • Figure out a persisted place for the etcd data directory (I just stuck it in /data)

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 16, 2017
@codecov-io
Copy link

codecov-io commented Aug 16, 2017

Codecov Report

Merging #1825 into master will decrease coverage by 5.66%.
The diff coverage is 24.7%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1825      +/-   ##
==========================================
- Coverage   36.25%   30.58%   -5.67%     
==========================================
  Files          51       70      +19     
  Lines        3462     4162     +700     
==========================================
+ Hits         1255     1273      +18     
- Misses       2024     2716     +692     
+ Partials      183      173      -10
Impacted Files Coverage Δ
pkg/util/constants.go 0% <ø> (ø) ⬆️
cmd/minikube/cmd/config/config.go 39.28% <ø> (ø)
pkg/minikube/bootstrapper/exec_runner.go 0% <0%> (ø)
cmd/minikube/cmd/logs.go 14.28% <0%> (-4.47%) ⬇️
pkg/minikube/bootstrapper/runner.go 0% <0%> (ø)
cmd/minikube/cmd/config/util.go 19.31% <0%> (ø)
pkg/minikube/bootstrapper/ssh_runner.go 0% <0%> (ø)
cmd/minikube/cmd/status.go 8.62% <0%> (-0.64%) ⬇️
pkg/minikube/sshutil/sshutil.go 19.11% <0%> (-26.25%) ⬇️
pkg/util/downloader.go 64.28% <100%> (ø) ⬆️
... and 31 more

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 1d715bf...9926776. Read the comment docs.

The command runner is a common interface for both commands ran on the
host and ssh commands.  Thsi way, the bootstrapper can set up the
cluster, unaware of the vm-driver being a hypervisor or none.
@r2d4 r2d4 force-pushed the kubeadm-bootstrapper branch 2 times, most recently from 5969a13 to 6649952 Compare August 17, 2017 22:16
@@ -114,6 +116,10 @@ var settings = []Setting{
set: SetString,
},
{
name: Bootstrapper,
set: SetString, //TODO(r2d4): more validation here?
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps validation function here that checks against a list of available bootstrappers

}

func (k *KubeadmBootstrapper) RestartCluster(k8s bootstrapper.KubernetesConfig) error {
if err := k.c.Run("rm -rf /etc/kubernets/*.conf"); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be /etc/kubernetes/*.conf?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Maybe I can remove this if it wasn't doing anything

description: "logs -f",
follow: true,
},
// TODO(r2d4): fix this test
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix

@@ -135,11 +136,31 @@ const (
)

const (
KubeletServiceFile = "/lib/systemd/system/kubelet.service"
KubeletSystemdConfFile = "/etc/systemd/system/kubelet.service.d/10-kubeadm.conf"
KubeadmConfigFile = "/var/lib/kubeadm.yaml"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

put in /etc/kubernetes

func DeleteFile(f assets.CopyableFile, client *ssh.Client) error {
return RunCommand(client, GetDeleteFileCommand(f))
}
func GetShell(session *ssh.Session, cmd string) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we actually need this

)

// SSHSession provides methods for running commands on a host.
type SSHSession interface {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where was this used?

}
//TODO(r2d4): Move this test to bootstrapper package

// func TestNewSSHClient(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix this

@MHBauer
Copy link
Contributor

MHBauer commented Aug 30, 2017

Is there something I can do to help/test?

For context: I mainly use https://github.com/Mirantis/kubeadm-dind-cluster as it is very helpful to be able to get a git HEAD dev version of kubernetes up and running with a bunch of sane defaults.

@r2d4
Copy link
Contributor Author

r2d4 commented Aug 30, 2017

@minikube-bot retest this please

@r2d4
Copy link
Contributor Author

r2d4 commented Aug 30, 2017

This branch is a bit out of a date now - as I split most of the changes into smaller PRs (command runner, localkube bootstrapped). The last part of this is just splitting out the kubeadm changes to a new PR, which I should hopefully get to soon. I've just been a bit busy with other things.

@r2d4 r2d4 mentioned this pull request Aug 31, 2017
@r2d4
Copy link
Contributor Author

r2d4 commented Aug 31, 2017

I've rebased and opened up #1903

@r2d4 r2d4 closed this Aug 31, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants