kinvolk / kube-spawn

A tool for creating multi-node Kubernetes clusters on a Linux machine using kubeadm & systemd-nspawn. Brought to you by the Kinvolk team.
https://kinvolk.io
Apache License 2.0
442 stars 41 forks source link

Incorrect assumption that overlayfs is only docker storage driver #302

Open donbowman opened 6 years ago

donbowman commented 6 years ago

https://github.com/kinvolk/kube-spawn/pull/301#discussion_r211939630

in pkg/bootstrap/node.go we have function ensureOverlayfs(). However, users may be using e.g. devicemapper driver for docker.

I personally don't believe these system checks belong in this tool, its too easy to get them wrong.

In this case, we'd have to look in /etc/docker/daemon.json for:

{
  "storage-driver": "devicemapper"
}

But, on some systems, this would be setup in systemd (e.g. /lib/systemd/system/docker.service) or in an environment or add-on for it.

Another method would be to parse docker info (as below). But I'm still not sure why we are doing this, I think its simpler to document 'docker is installed and working', its not really our business whether they use btrfs / zfs / overlayfs / ...

$ docker info
Containers: 1
 Running: 1
 Paused: 0
 Stopped: 0
Images: 14
Server Version: 18.06.0-ce
Storage Driver: overlay2
 Backing Filesystem: extfs
 Supports d_type: true
 Native Overlay Diff: true
Logging Driver: json-file
Cgroup Driver: cgroupfs
Plugins:
 Volume: local
 Network: bridge host macvlan null overlay
 Log: awslogs fluentd gcplogs gelf journald json-file logentries splunk syslog
Swarm: inactive
Runtimes: runc
Default Runtime: runc
Init Binary: docker-init
containerd version: d64c661f1d51c48782c9cec8fda7604785f93587
runc version: 69663f0bd4b60df09991c08812a60108003fa340
init version: fec3683
Security Options:
 apparmor
 seccomp
  Profile: default
Kernel Version: 4.18.0-041800-generic
Operating System: Ubuntu 18.04.1 LTS
OSType: linux
Architecture: x86_64
CPUs: 16
Total Memory: 31.4GiB
Name: cube
ID: MMIA:334X:MD65:7BKQ:WYL3:XGCV:RYQO:OTAS:PJPP:362I:VG63:L2AN
Docker Root Dir: /var/lib/docker
Debug Mode (client): false
Debug Mode (server): false
Username: donbowman
Registry: https://index.docker.io/v1/
Labels:
Experimental: false
Insecure Registries:
 127.0.0.0/8
Live Restore Enabled: false

WARNING: No swap limit support
dongsupark commented 6 years ago

Ok, it seems to be actually multiple issues.

First of all, basically I agree that we should check for more details before loading the overlay kernel module. Maybe then it's an issue about not only docker storage driver, but also whether the container runtime is docker or rkt. Or there could be other factors.

And the next issue about whether we can just ignore all of these issues and simply write documents: I doubt that it's doable. Of course in that case, we system engineers are still capable of fulfilling the system requirements on our own. (I'm sure you are able to do that as well :-)) But in practice, most newcomers would not be able to run kube-spawn at all. That's one of the reasons why we added the various ensure* functions to meet the requirements.

donbowman commented 6 years ago

its one thing to do a sanity check, another to try and infer the config of the system or fix it.

I think we are headed down a bad path w/ things like 'assume /var/lib/machines is a loopback of /var/lib/machines,raw, that we are the only ones using it, that we can umount/resize' etc. Similarly the docker storage.

A better way might be to just run a simple sanity check. If docker info returns OK, then docker is running OK, why do the additional checks for how?

Imagine if every docker container in the world had an install mechanism that groped around is 'lsmod' etc to see what modules were loaded? I think its better to assume the system is properly setup rather than to assume we know better.

On Thu, 23 Aug 2018 at 06:20, Dongsu Park notifications@github.com wrote:

Ok, it seems to be actually multiple issues.

First of all, basically I agree that we should check for more details before loading the overlay kernel module. Maybe then it's an issue about not only docker storage driver, but also whether the container runtime is docker or rkt. Or there could be other factors.

And the next issue about whether we can just ignore all of these issues and simply write documents: I doubt that it's doable. Of course in that case, we system engineers are still capable of fulfilling the system requirements on our own. (I'm sure you are able to do that as well :-)) But in practice, most newcomers would not be able to run kube-spawn at all. That's one of the reasons why we added the various ensure* functions to meet the requirements.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kinvolk/kube-spawn/issues/302#issuecomment-415366545, or mute the thread https://github.com/notifications/unsubscribe-auth/AE5Ok7EzpYtPE0CGPFt5KqNFBwUrgRC4ks5uToHSgaJpZM4WHscf .

dongsupark commented 6 years ago

I would call it a "necessary evil" instead of a "bad path". ;-) Anyway your suggestion of running docker info looks good in the docker world.

donbowman commented 6 years ago

maybe at least make it such behaviour off by default.

e.g. a '--run-checks-and-change-your-system-if-it-matches-my-assumptions'

On Thu, 23 Aug 2018 at 07:59, Dongsu Park notifications@github.com wrote:

I would call it a "necessary evil" instead of a "bad path". ;-) Anyway your suggestion of running docker info looks good in the docker world.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kinvolk/kube-spawn/issues/302#issuecomment-415389548, or mute the thread https://github.com/notifications/unsubscribe-auth/AE5Ok-ViA6RDMzrhobDHjnhwNaQeH4w2ks5uTpktgaJpZM4WHscf .