projectatomic / container-storage-setup

Service to set up storage for Docker and other container systems
Apache License 2.0
153 stars 77 forks source link

docker-storage-setup.service not found #261

Open LongLiveCHIEF opened 7 years ago

LongLiveCHIEF commented 7 years ago

I'm getting issues with the docker service after reboot, (specifically a cannot take ownership of /vart/lib/docker error in journalctl). After doing some digging, it appears to be some systemd configurations don't quite point to each other correctly.

The primary issue seems to be that there is nothing telling the docker service to wait for css, or no value in a css unit file with the correct values to take ownership of container runtime directory in order to let docker start correctly.

I'm running container-storage-setup using the following template:

# /etc/sysconfig/docker-storage-setup
STORAGE_DRIVER=devicemapper
DEVS="/dev/sdb"
CONTAINER_THINPOOL=thinpool
VG=docker
ROOT_SIZE=10G
DATA_SIZE=40%FREE
MIN_DATA_SIZE=2G
POOL_META_SIZE=16M
CHUNK_SIZE=512K
AUTO_EXTEND_POOL=yes
POOL_AUTOEXTEND_THRESHOLD=60
POOL_AUTOEXTEND_PERCENT=20
WIPE_SIGNATURES=true
DEVICE_WAIT_TIMEOUT=60
CONTAINER_ROOT_LV_NAME=data
CONTAINER_ROOT_LV_MOUNT_PATH=/var/lib/docker
CONTAINER_ROOT_LV_SIZE=40%FREE

Which creates the following EnvironmentFile (loaded by docker.service):

# /etc/sysconfig/docker-storage
DOCKER_STORAGE_OPTIONS="--storage-driver devicemapper --storage-opt dm.fs=xfs --storage-opt dm.thinpooldev=/dev/mapper/docker-thinpool --storage-opt dm.use_deferred_removal=true --storage-opt dm.use_deferred_deletion=true "

My systemd files look like this (truncated):

drwx------.  2 root root   35 Oct 27 13:37 docker.service.d
drwxr-xr-x.  2 root root   33 Oct 27 14:15 docker-storage-setup.service.wants (symlink to var-lib-docker.mount)
-rw-------.  1 root root  347 Oct 30 10:09 var-lib-docker.mount

$ sudo ls -al /etc/systemd/system/docker-storage-setup.service.wants
total 4
drwxr-xr-x.  2 root root   33 Oct 27 14:15 .
drwxr-xr-x. 13 root root 4096 Oct 30 10:16 ..
lrwxrwxrwx.  1 root root   40 Oct 27 14:15 var-lib-docker.mount -> /etc/systemd/system/var-lib-docker.mount

Contents of var-lib-docker.mount:

[Unit]
Description=Mount data on /var/lib/docker directory.
Before=docker-storage-setup.service

[Mount]
What=/dev/docker/data
Where=/var/lib/docker
Type=xfs
Options=defaults

[Install]
WantedBy=docker-storage-setup.service

So in this setup:

  1. There is no docker-storage-setup.service as referenced in the var-lib-docker-mount
  2. There's nothing referencing the docker.service, thus allowing the docker.service to start before container-storage-setup has properly mounted the container root lv.

I was able to resolve this issue by changing two values in the var-lib-docker.mount. New content as follows:

   [Unit]
   Description=Mount data on /var/lib/docker directory.
- Before=docker-storage-setup.service
+ Before=docker.service

    [Install]
 - WantedBy=docker.storage-setup.service
 + WantedBy=docker.service

I'm guessing this isn't totally correct, but it does allow docker to start back up.

rhatdan commented 7 years ago

@rhvgoyal PTAL

LongLiveCHIEF commented 7 years ago

It looks like the unit file for the mount is created, but the .service definition file isn't. I've got no concerns with this needing to be added manually, but it would need to be clarified in the docs.

If it's not supposed to be created (needs to be provided by user), i think it's going to cause some confusion. It would be inconsistent (some things are set up in systemd, others aren't)

Most of the packaged distributions of this seem to install src packages and not binaries with the service file attached.

rhvgoyal commented 7 years ago

docker.service has Wants= dependency on docker-storage-setup.service. That should take care of it, isn't it?

LongLiveCHIEF commented 7 years ago

No, but only because there isn't a systemd unit for docker-storage-setup.service. The script creates all the other systemd units except that one.

rhvgoyal commented 7 years ago

We do ship docker-storage-setup.service with docker package. Something else is not right.

rhvgoyal commented 7 years ago

I think these wants directories are added only if we use WantedBy= or RequiredBy= directives. Given we are already using Wants= in docker.service, we don't need to put WantedBy= in docker-storage-setup.service, AFAIK.

I did systemctl list-dependencies docker and it does list docker-storage-setup.serivce as a dependency.

docker.service ● ├─docker-containerd.service ● ├─docker-storage-setup.service

So I believe, that docker will wait for docker-storage-setup.service to start before it continues.

LongLiveCHIEF commented 7 years ago

can you run systemctl list-units |grep docker please?

Here's what I get:

$ sudo systemctl list-units |grep docker
  sys-devices-virtual-net-docker0.device                                              loaded active plugged   /sys/devices/virtual/net/docker0
  sys-subsystem-net-devices-docker0.device                                            loaded active plugged   /sys/subsystem/net/devices/docker0
  var-lib-docker-devicemapper.mount                                                   loaded active mounted   /var/lib/docker/devicemapper
  var-lib-docker-plugins.mount                                                        loaded active mounted   /var/lib/docker/plugins
  var-lib-docker.mount                                                                loaded active mounted   Mount data on /var/lib/docker directory.
  docker.service                                                                      loaded active running   Docker Application Container Engine
● systemd-fsck@dev-mapper-docker\x2ddata.service                                      loaded failed failed    File System Check on /dev/mapper/docker-data

Notice: there is no docker-storage-setup.service. The problem isn't the WantedBy or RequiredBy unit attributes, it's the fact that the unit they are targeting (docker-storage-setup.service) doesn't exist when you install container-storage-setup as a package (rpm/yum)

rhvgoyal commented 7 years ago

Right, container-storage-setup does not install docker-storage-setup.service. It is the docker package which install docker-storage-setup.service

LongLiveCHIEF commented 7 years ago

but the docker package doesn't install docker-storage-setup.service. It hasn't in a really long time. If it did, systemctl list-units would show it.

rhvgoyal commented 7 years ago

systemctl list-units --all | grep docker

docker-storage-setup.service loaded inactive dead Docker Storage Setup
docker.service loaded active running Docker Application Container Engine

rhvgoyal commented 7 years ago

systemctl status docker-storage-setup ● docker-storage-setup.service - Docker Storage Setup Loaded: loaded (/usr/lib/systemd/system/docker-storage-setup.service; enabled; vendor preset: dis Active: inactive (dead) since Tue 2017-10-31 11:10:27 EDT; 27min ago Process: 2925 ExecStart=/usr/bin/container-storage-setup (code=exited, status=0/SUCCESS) Main PID: 2925 (code=exited, status=0/SUCCESS)

rhvgoyal commented 7 years ago

rpm -ql docker | grep docker-storage-setup

/usr/lib/systemd/system/docker-storage-setup.service

LongLiveCHIEF commented 7 years ago

What docker distribution/version are you using? Because only the RedHat versions of docker, and docker pre 1.12 install that service. I don't get that unit when installing upstream docker

rhvgoyal commented 7 years ago

upstream docker does not install it. It is only fedora/rhel/centos docker which installs this service

rhvgoyal commented 7 years ago

This is what I am using on my F26 VM right now.

docker-1.13.1-22.gitb5e3294.fc26.x86_64

LongLiveCHIEF commented 7 years ago

That version is nearly a year old. The docs don't say anything about requiring a specific source for the docker package (redhat distributed docker packages or docker < 17.x).

So we're back to "missing systemd unit", unless you want to make a specification in the docs that upstream docker isn't supported?

LongLiveCHIEF commented 7 years ago

i'm ok with either, I think it just needs to be documented if you go with the latter.

I think that's the wrong route though, since it's inconsistent with setting up some of the systemd units and not others.

I also think this can and should be done without modifying the docker.service unit. All i need to do is add the docker-storage-setup.service file and specify the service should be before docker.service, and things work beatifully.

Most docker installations are managed by orchestration or automation tools, and the unit files for the docker installation will constantly be fighting with CSS altering the state of the docker.service file.

We should not alter the docker.service file (which we don't currently AFAICT), but we should ensure the script creates a docker-storage-setup.service file. (might as well be a container-storage-setup.service file at this point)

rhatdan commented 7 years ago

We work with the docker shipped with the distribution. If you have an issue with docker shipped from somewhere else, then you should take it up with those packagers.

rhvgoyal commented 7 years ago

I think those who ship docker package they need to decide whether they want to be dependent on docker-storage-setup for their storage needs or not. If they want to be dependent, then they need to ship right default config file and systemd unit file accordingly.

W.r.t dynmacially generating var-lib-docker.mount, it has to be done dynamically because name of this file is determined by the path of mount (specified in config file). Otherwise I would have generated it statically as well.

LongLiveCHIEF commented 7 years ago

docker isn't shipped with any distribution. You have to add repos that have the docker package. Even on RedHat family Operating Systems

LongLiveCHIEF commented 7 years ago

I think those who ship docker package they need to decide whether they want to be dependent on docker-storage-setup for their storage needs or not. If they want to be dependent, then they need to ship right default config file and systemd unit file accordingly.

None of the current distributions of docker ship with css as a dependency. And the README bills this product as a way to configure container storage.

What you're telling me about the usage here is very different from how the usage in the README is described. README says nothing about this project only being compatible with docker distributions that have a dependency for css built in.

rhatdan commented 7 years ago

docker package is available by default on Fedora and Centos. Only on RHEL is it on the extras repo. But it availabel as part of the default subscription.

rhatdan commented 7 years ago

Each distribution of the package sets up different configurations of the unit file that starts the service. If upstream does not match the distributions version, then the user is responsible to make the packages work together.
The distribution is responsible for making all packages that it ships work together. Not random upstream packages.

LongLiveCHIEF commented 7 years ago

All i'm asking you to do is remove any ambiguity from your README. As it reads right now, i would expect it to work with any container engine, no caveats.

LongLiveCHIEF commented 7 years ago

I can even make the PR if that's something you're willing to accept. something along the lines of

"User will need to hook up .service to the docker-storage-setup.service" if it is not already done so by the docker/container package.

LongLiveCHIEF commented 7 years ago

For the record, I'm approaching this as a maintainer/contributor to the puppet forge module for docker.

this is related to:

It's not a problem to implement the service itself with puppet dynamically based on the OS and location of the docker package, I was just trying to connect some dots based on the information in this project's README and wanted to make sure this responsibility was solved in the right place so that I didn't create a conflict in system automation by adding the service configuration to the automation tooling.

rhvgoyal commented 7 years ago

Please generate a PR for README. I will have a look. Lets see what text are you looking for.