[OpenWrt-Devel] [PATCH 1/2] procd: add docker support

[Petr Štetiar]

Paul Spooren <mail at aparcar.org> [2019-05-22 19:24:18]:

I'm wondering if this Docker related changes shouldn't be included as a part
of a patch series which would add complete Docker support to OpenWrt (do you
plan to do this?), so it could be tested together.

Or is it now possible to just download OpenWrt image and use it inside the
Docker as it is?

> The additional exit(0) treatment in state.c is based on @mikma code[0].
> It should fix stopping problems of containers.
> 
> [0]: https://github.com/mikma/lxd-openwrt/blob/master/patches/procd-master/0003-docker-fix-problem-stopping-container.patch

This patch looks like a hack, which was probably just cherry-picked without
actually looking into the surrounding code. See bellow.

> --- a/container.h
> +++ b/container.h
> @@ -16,9 +16,11 @@
>  
>  #include <stdlib.h>
>  #include <stdbool.h>
> +#include <sys/stat.h>
>  
>  static inline bool is_container() {
> -	return !!getenv("container");
> +	struct stat s;
> +	return !!getenv("container") || !!stat("/.dockerinit", &s);
>  }

Have you noticed "Remove dockerinit once and for all"[1]?

> --- a/state.c
> +++ b/state.c
> @@ -21,6 +21,7 @@
>  #include <signal.h>
>  
>  #include "procd.h"
> +#include "container.h"
>  #include "syslog.h"
>  #include "plug/hotplug.h"
>  #include "watchdog.h"
> @@ -157,6 +158,9 @@ static void state_enter(void)
>  		else
>  			LOG("- reboot -\n");
>  
> +		if (is_container())
> +			exit(0);

if you look at that file, you'll notice following:

 #ifndef DISABLE_INIT
                ...
 #else
		exit(0);
 #endif

which makes me wonder if this is proper approach as maybe the proper fix would
be conversion of compile time DISABLE_INIT functionality into runtime one,
something like following: 

 inline bool is_init_disabled()
 {
 #ifdef DISABLE_INIT
	return true;
 #else
	return is_container();
 #endif
 }

It seems like that DISABLE_INIT touches more parts of procd which probably
should be considered in case if procd is running in container as well (or
maybe not):

 $ git grep DISABLE_INIT

 plug/hotplug.h:#ifndef DISABLE_INIT
 signal.c:#ifndef DISABLE_INIT
 signal.c:#ifndef DISABLE_INIT
 state.c:#ifndef DISABLE_INIT
 watchdog.h:#ifndef DISABLE_INIT

This will probably incur more work, but it will likely result in a proper
solution.

1. https://github.com/moby/moby/pull/19490

-- ynezz

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel