docker-ce: add uci support

[feckert]

Maintainer: @G-M0N3Y-2503
Compile tested: only script changes
Run tested: x86_64, APU3, OpenWrt master

Description:
Recently we also have the application luci-app-dockerman in the LuCI. In this app we have also an service which reads the configuration from /etc/docker/daemon.json and compares this against his own uci configuration in /etc/config/dockerman. To reconcile configuration differences a script is used, so that everything works again.
To simplify things I think it is better to move the uci into the docker-ce package. So that we have a clear responsibility.

@lisaac To get everything working again we have to make some changes to the luci-app-dockerman as well.

Since docker's configuration options are very large, I don't think we can display all configuration options here. But I guess it doesn't hurt that we can configure at least a few things.

[G-M0N3Y-2503]

Otherwise, thanks for the work, sounds like a great idea going forward!

[lisaac]

Sounds great!

Now, we can get rid of dockerd-config.lua for the luci-app-dockerman.

@feckert And how about the access control?

[feckert]

@lisaac I have to see how you do it first.
But I could imagine that we add a firewall object to the docker-ce service and then send a reload to the firewall, which should take care of setting the right rules.
The advantage would be that we have no dependency on the firewall tools and can work with the openwrt tool set.

[feckert]

I have now added a proof of concept firewall ac handling in the dockerd service with this commit. The advantage we do not need any iptables commands. The firewall handling is abstracted by procd.

If we set the following option in the dockerd uci config

config globals 'globals'
        list ac_allowed_interface 'lan'

then the following rule is added to the firewall by fw3.

iptables-save | grep docker0
-A zone_lan_input -i docker0 -p tcp -m comment --comment "!fw3: ubus:dockerd[instance1] rule 0" -j ACCEPT

Unfortunately, it is not yet clear to me what you @lisaac want to achieve with ac.

• AC to the containers
  or
• AC to the config interface over the network.

The way I see it, this has nothing to do with the configuration interface. Did I get that right?

If so is that firewall handling really necessary in docker?
I think we could add the following configuration section to /etc/config/network and /etc/config/firewall and then we achieve the same. Or did I get something wrong.

/etc/config/network

config interface 'docker0'
        option proto 'none'
        option ifname 'docker0'
        option force_link '0

/etc/config/firewall

config zone
        option name 'lan'
        option input 'ACCEPT'
        option output 'ACCEPT'
        option forward 'ACCEPT'
        option network 'lan docker0'

[lisaac]

Without the access control, WAN can easily access the ports exposed by the container using WAN_IP:PORT, which is big security problems.

On another hand, sometimes, you would like some ports exposed by containers could be access from WAN.

For more details, please see lisaac/blog#7 , https://docs.docker.com/network/iptables/

In order to let those works, I add the access control in luci-app-dockerman.

-A zone_lan_input -i docker0 -p tcp -m comment --comment "!fw3: ubus:dockerd[instance1] rule 0" -j ACCEPT

@feckert I don't think it will work, and maybe we could let dockerman achieve with ac, let docker-ce do his own job

[G-M0N3Y-2503]

The docker documentation @lisaac linked leads me to believe that this kind of setup is outside the scope of docker-ce, at least in general. However, I think it is also inside the scope of this package to get docker-ce running on OpenWrt.

It could be argued either way really, but since one of the reasons to use OpenWrt is "Security" and being able to access from the WAN by default seems to go against that. I lean slightly in favour of configuring it from docker-ce. Though the only case I can think of would be where someone doesn't have luci-app-dockerman for whatever reason...
But, I'm open to suggestions.

Also, if we decide to do AC here, it looks like this PR would close #12358

[feckert]

@G-M0N3Y-2503 thanks for the docker iptables link. That was so not clear to me that the IP packages that are for docker are not handled by the fw3 of openwrt firewall. Then we can forget about the procd (firewall_array)?

@lisaac If I see this correctly, would it be better to add the rules for the docker to DOCKER-USER and not do the new DOCKER-MAN chain?

[lisaac]

@feckert
iptables -I DOCKER-USER -j DOCKER-MAN

Use a custom chain to facilitate maintenance

[feckert]

I have update the pullrequest so that on docker stop the related pullrquests for dockerd are removed.
The next step would be to update luci-app-dockerman to add the rule iptables -I DOCKER-USER -j DOCKER-MAN to iptables on luci-app-dockerman service start. @lisaac Is there anything else we need to consider?

[jmarcet]

Hi everybody,

My two cents with respect to this PR, with apologies for the length, but there
is a lot to be said.

I have been using docker extensively in openwrt for more than 6 months. I am
the maintainer of the docker-compose package. With that said, I would like to
do some observations:

• I love luci-app-dockerman, for me it was a surprise when I saw it merged,
  and a pretty good one. The only thing I needed to change in my systems was
  launching dockerman instead of dockerd.

• I am all in favor of a proper integration of docker in openwrt.

• In principle I like the idea of controlling dockerd parameters (the ones
  which end up in /etc/docker/daemon.json) from uci, but there is a
  plethora of potentially interesting options. How do you envision supporting
  them all?

• dockerd does its own iptables handling. The fact that you can end up
  exposing ports on wan easily is the standard dockerd behavior. Instead of
  writing a new set of iptables rules outside of docker, there are other two
  sensible ways to avoid it, all while keeping the standard docker behavior.
  You can either map ports to an internal IP (be it localhost or lan) container
  by container, or you can change the default docker bridge binding address,
  which defaults to 0.0.0.0, with a dockerd parameter. Hence I favor not doing
  any new iptables outside of docker by default.

$ dockerd --help | grep bind
--ip ip  Default IP when binding container ports (default 0.0.0.0)

Or in a docker-compose.yml, instead of doing:

    ports:
      - '8087:80'

We would do something like:

    ports:
      - '127.0.0.1:8087:80'

[jmarcet]

• There is quite a lot of room for improvement in other areas to have a cleaner
  integration in openwrt. Namely the integration of docker networks in openwrt's
  interfaces. This could come in as a new type of network interface for externally
  managed networks, as well as offering an easy way to use specially crafted docker
  networks like macvlan bridges.

Let me give you a few examples. I currently have 20+ containers running,
connected to different network bridges. By default, dockerd assigns (partially)
random mac and ip addresses to those bridges, and the interfaces are not
considered to be up by openwrt.

All those random addresses are a PITA from a security and administration point
of view. Instead of having a controlled list of mac addresses in the network,
you end up with an ever growing list. Using something like nlbwmon turns really
ugly.

I first tried adding the bridges to /etc/config/network, and it sort of
worked, I could even set static mac addresses there and have the bridges always
use the same macs. However dockerd often failed to start some containers at
boot saying the network already existed.

I ended up adding the networks this way:

/etc/docker/daemon.json
{
  "debug": false,
  "data-root": "/opt/docker",
  "log-level": "warn",
  "bip": "10.XX.YY.1/24"
}

/etc/config/network
config interface 'docker'
       option ifname 'docker0'
       option proto 'static'
       option netmask '255.255.255.0'
       option ipaddr '10.XX.YY.1'
       option auto '0'

config interface 'nextcloud'
       option ifname 'br-nextcloud'
       option proto 'static'
       option netmask '255.255.255.0'
       option ipaddr '10.AA.BB.1'
       option auto '0'

/etc/config/firewall
config zone
        option network 'docker'
        option input 'REJECT'
        option output 'ACCEPT'
        option forward 'REJECT'
        option name 'docker'

config zone
        option network 'nextcloud'
        option input 'REJECT'
        option output 'ACCEPT'
        option forward 'REJECT'
        option name 'nextcloud'

/etc/hotplug.d/net/11-docker-bridges
#!/bin/sh

IFACES="docker0 br-nextcloud"

[ -n "$INTERFACE" -a -n "$( echo $IFACES | grep $INTERFACE )" ] || exit 0

if [ "$ACTION" = add ]; then
        MAC="$( grep $INTERFACE /etc/ethers | cut -d' ' -f1 )"
        [ "$INTERFACE" = "docker0" ] && IFACE=docker || IFACE="${INTERFACE/br-}"
        logger -t docker-bridges "Setting $IFACE network up"
        ifup $IFACE
        logger -t docker-bridges "Setting $INTERFACE mac address to $MAC"
        ifconfig $INTERFACE hw ether $MAC
fi

nextcloud's docker-compose.yml

networks:
  default:
    driver: bridge
    ipam:
      driver: default
      config:
        - subnet: 10.AA.BB.0/24
    driver_opts:
      com.docker.network.bridge.name: br-nextcloud

/etc/ethers

ZZ:ZZ:ZZ:ZZ:00:00 docker0
ZZ:ZZ:ZZ:ZZ:60:00 br-nextcloud

With this I have a nice view of all the interfaces (real and virtual) on my
network from luci. I also get proper metrics from all the bridges with
node_exporter.

docker0, lan and the docker bridges are in different zones. Access between
them is not allowed by default. If I want to allow something I can easily
add a new firewall rule (all from luci if you like). So this adds to docker
infrastructure instead of duplicating it.

It could be much cleaner if we could set the bridges as externally managed
in /etc/config/network. That would erase the need to add the hotplug
script and the macs to /etc/ethers.

With respect to the access control, we could probably change the default
binding address from 0.0.0.0 to the router lan's ip. Anyhow this all should
be well documented in the wiki, since the docker way (from reading blogs
everywhere about it) would be specifying an IP when doing any port mapping.

[jmarcet]

Last, we could add support for defining macvlan bridges directly in
/etc/config/network. At the moment you have to manually add the bridge if you
want to use it from docker. So, for example, say you want to have a container
directly on your lan, with its own mac and an ip address within your lan's
subnet. You can define it in docker-compose like this (where 192.168.1.1/24
is your lan subnet and eth0 your lan interface):

docker-compose.yml
version: '2.4'

services:
  someservice:
    [...]
    networks:
      somelan:
        ipv4_address: 192.168.1.65

networks:
  somelan:
    driver: macvlan
    driver_opts:
      parent: eth0
      macvlan_mode: bridge
    ipam:
      config:
        - subnet: 192.168.1.64/29

But you need to set the macvlan bridge yourself:

config interface 'macvlan'
       option ifname 'veth'
       option proto 'static'
       option ipaddr '192.168.1.64'
       option netmask '255.255.255.248'
       option defaultroute '0'

ip link add veth link eth0 type macvlan mode bridge

This can be added as a hotplug script to make it automatic.

Again, this could be much nicer if we had a macvlan bridge option in openwrt's
interfaces.

For reference, you can see the complete set of docker-compose files I am using
in my openwrt managed home lan here

What do you think?

[feckert]

@jmarcet Thanks for your feedback and your thoughts
I still have to look at the other commentaries

But for the last one:

Last, we could add support for defining macvlan bridges directly in
/etc/config/network. At the moment you have to manually add the bridge if you
want to use it from docker.

I think this is already possible. The service netifd could setup this. I have the following uci config in /etc/config/network to setup a macvlan docker setup.

config device 'container1'
		option name 'container1'
		option type 'macvlan'
		option ifname 'eth1'
		option mode 'bridge'

config interface 'docker1'
		option proto 'static'
		option ipaddr '192.168.0.249'
		option netmask '255.255.255.252'
		option ifname 'container1'

config route 'route_docker1'
		option interface 'docker1'
		option target '192.168.0.248'
		option netmask '255.255.255.252

After an reload_config I could start the container and setup the network with the following commands.

docker network create -d macvlan -o parent=eth1 --subnet 192.168.0.0/24 --gateway 192.168.0.249 --ip-range 192.168.0.248/30 container1
docker run -it --rm --net container1 --ip 192.168.0.250 alpine

[jmarcet]

@feckert Wow, that was fast!

@jmarcet Thanks for your feedback and your thoughts
I still have to look at the other commentaries

But for the last one:

Last, we could add support for defining macvlan bridges directly in
/etc/config/network. At the moment you have to manually add the bridge if you
want to use it from docker.

I think this is already possible. The service netifd could setup this. I have the following uci config in /etc/config/network to setup a macvlan docker setup.

Really interesting. This just needs to be documented then.

[lisaac]

@jmarcet @feckert
Thanks for your sharing!

this could be much nicer if we had a macvlan bridge option in openwrt's
interfaces.

We do have macvlan bridge option in it, and if you want, you can use dockerman to add macvlan network, dockerman will add macvlan gateway interface automatically(with the option).

Maybe we could edit /etc/config/network /etc/config/firewall automatically when we create the custom docker network, just like we did before.

[G-M0N3Y-2503]

• In principle I like the idea of controlling dockerd parameters (the ones
  which end up in /etc/docker/daemon.json) from uci, but there is a
  plethora of potentially interesting options. How do you envision supporting
  them all?

I Imagine we'll only cover the options that are frequently used, at least until someone adds them all or writes something to serialize/deserialize the UCI to the daemon.json or CLI arguments.

But now I think of it, we should have a way to not use the UCI for advanced configurations.
If I'm not mistaken @feckert, the only way we can do this with this PR is by deleting /etc/config/dockerd and placing out own /tmp/dockerd/daemon.json. However, we probably want a persistent directory where a config file can be placed for advanced configurations. maybe we optionally add --config-file="$DOCKERD_CONF" only if a /etc/config/dockerd exists? that way people can just use the default configuration file.

Otherwise, it looks like we agree that docker-ce will focus on being manually configurable and expose aspects through the UCI to make integration easier with other applications.

[feckert]

But now I think of it, we should have a way to not use the UCI for advanced configurations.

That's what I thought too, that we won't get all configuration options into the uci.
I have now designed it in a way that nothing changes for the users who already use it.
If there is the config option in the globals uci section alt_config_file then this will be used.
cad0b13#diff-d2f590ffe4d65867a6968c82a4a2ed5aR20
This is default and we use /etc/daemon.json.

@G-M0N3Y-2503 I would like to finish this PR to get this in.
@lisaac noting would get changed for your luci-app-dockerman
After that we can discuss the next steps from @jmarcet
#12556 (comment)
#12556 (comment)
#12556 (comment)

[G-M0N3Y-2503]

I think it makes sense for process_config() to either return the config file or set DOCKERD_CONF directly, rather than using symlinks.

if /etc/config/dockerd doesn't exist DOCKERD_CONF equals "" because the daemon will just startup with its defaults and not mislead anyone looking at the process command line.

if alt_config_file is set in the UCI, DOCKERD_CONF will equal that if it exists or "" because it's most likely an error.

otherwise, we set DOCKERD_CONF to the one the UCI will generate "/tmp/dockerd/daemon.json"

[G-M0N3Y-2503]

This Implementation looks a little more complicated than it needs to be, perhaps I wasn't clear about what I think is important.

I think longterm we want the UCI config to be the default configuration file as it will probably be enough for most users. So we will have to break the previous configuration for this, preferably before a mainline release. I'm leaning towards ripping off the band-aid with this PR, but I'm open to suggestions as to when.

So with that, It might be nicer from a maintainers perspective to just have a default /etc/docker/daemon.json and the one generated by the UCI config. The simplest implementation I can think of would be to [ -f /etc/docker/daemon.json ] and use it if it exists otherwise we use the UCI generated one, then we could default to UCI by not installing a /etc/docker/daemon.json.

If we still want an alt_config_file that's fine, I'm just wondering what the use case is that justifies this extra complication is...
But with that, the breaking change we'd have would be by not having alt_config_file defined by default.

[feckert]

If we still want an alt_config_file that's fine, I'm just wondering what the use case is that justifies this extra complication is...
But with that, the breaking change we'd have would be by not having alt_config_file defined by default.

I should also mention that from my point of view it is important that we store the uci generated configuration in tmp.
This reduces the write cycles, OpenWrt is made for embedded systems in mind.

[G-M0N3Y-2503]

hmmm, I think I may not have been clear, I wasn't talking about whether the config should be in /tmp or not.
I've created a patch to help explain what I think should be done at a minimum.

In summary:

• Use /etc/config/dockerd as the default configuration going forward.
• If no config can be found let the daemon use its defaults.

diff --git a/utils/docker-ce/Makefile b/utils/docker-ce/Makefile
index 8ed184c40..f8b1d13e0 100644
--- a/utils/docker-ce/Makefile
+++ b/utils/docker-ce/Makefile
@@ -124,9 +124,6 @@ define Package/docker-ce/install
 	$(INSTALL_DIR) $(1)/etc/init.d
 	$(INSTALL_BIN) ./files/dockerd.init $(1)/etc/init.d/dockerd
 
-	$(INSTALL_DIR) $(1)/etc/docker
-	$(INSTALL_CONF) ./files/daemon.json $(1)/etc/docker/
-
 	$(INSTALL_DIR) $(1)/etc/config
 	$(INSTALL_CONF) ./files/etc/config/dockerd $(1)/etc/config/dockerd
 
diff --git a/utils/docker-ce/files/daemon.json b/utils/docker-ce/files/daemon.json
deleted file mode 100644
index 53c03211f..000000000
--- a/utils/docker-ce/files/daemon.json
+++ /dev/null
@@ -1,4 +0,0 @@
-{
-    "data-root": "/opt/docker/",
-    "log-level": "warn"
-}
diff --git a/utils/docker-ce/files/dockerd.init b/utils/docker-ce/files/dockerd.init
index c2e5c4dcc..c892c203b 100644
--- a/utils/docker-ce/files/dockerd.init
+++ b/utils/docker-ce/files/dockerd.init
@@ -12,14 +12,18 @@ json_add_array_string() {
 process_config() {
 	local alt_config_file data_root log_level
 
-	[ -f /etc/config/dockerd ] || return 0
-
 	rm -f "$DOCKERD_CONF"
+
+	[ -f /etc/config/dockerd ] || {
+		# Use the daemon default configuration
+		DOCKERD_CONF=""
+		return 0
+	}
+
 	config_load 'dockerd'
 
 	config_get alt_config_file globals alt_config_file
 	[ -n "$alt_config_file" ] && [ -f "$alt_config_file" ] && {
-		rm -f "$DOCKERD_CONF"
 		ln -s "$alt_config_file" "$DOCKERD_CONF"
 		return 0
 	}
diff --git a/utils/docker-ce/files/etc/config/dockerd b/utils/docker-ce/files/etc/config/dockerd
index f49860c6a..b89d5e4bf 100644
--- a/utils/docker-ce/files/etc/config/dockerd
+++ b/utils/docker-ce/files/etc/config/dockerd
@@ -1,7 +1,6 @@
-
 config globals 'globals'
-	option alt_config_file "/etc/daemon.json"
-#	option data_root "/opt/docker/"
-#	option log_level "warn"
+#	option alt_config_file "/etc/docker/daemon.json"
+	option data_root "/opt/docker/"
+	option log_level "warn"
 #	list registry_mirror "https://<my-docker-mirror-host>"
 #	list registry_mirror "https://hub.docker.com"

patch.txt

[feckert]

I understand now what you mean.

I agree with you that we use the uci variant as default.
So that we start with the uci on next openwrt release.

I added your suggestion, so we return if no uci config for docker is found, and use the docker config default location.

The one thing I did not change is the installation of the daemon.json file you suggested.
Why is this necessary?

@G-M0N3Y-2503 please check my changes. If I get a thumb up I will merge this.

[G-M0N3Y-2503]

The one thing I did not change is the installation of the daemon.json file you suggested.
Why is this necessary?

It isn't necessary exactly, just that the same config can now be achieved by the default or empty /etc/config/dockerd, so /etc/docker/daemon.json isn't needed as far as I can see.

But this PR looks good to me either way.

[feckert]

@G-M0N3Y-2503 Thanks for your input and review 👍

[lisaac]

@feckert @G-M0N3Y-2503
I just review the code.

In order to allow manually configures daemon.json, can we compare daemon.json with the UCI configuration, and modify the daemon.json, just like dockerd-config.lua did.

[G-M0N3Y-2503]

I don't think I understand @lisaac...

Do you mean that we should have the UCI override parts of the daemon.json?
If so, it might be easier just to add options to the UCI when there are commonly needed or just take ownership of an entire daemon.json and set alt_config_file to point to it.

[lisaac]

@G-M0N3Y-2503

Do you mean that we should have the UCI override parts of the daemon.json?

Yes, exactly.

If so, it might be easier just to add options to the UCI when there are commonly needed or just take ownership of an entire daemon.json and set alt_config_file to point to it.

Yes, it works, just different way.