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

[Paul Spooren]

Hi,

On 5/23/19 12:30 PM, Petr Štetiar wrote:
> 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 latter, running OpenWrt within a Docker container (for CI). I'm
aware of the docker-ce package porting, however did not do anything in
this area.
>> 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]?

No, thanks for the link. Seems that Docker now uses LXC anyway which
results in the availability of container=lxc in /proc/1/environ:

    root at df9992e4c518:/# cat /proc/1/environ
   
HOSTNAME=df9992e4c518SHLVL=1HOME=/rootcontainer=lxcTERM=xtermPATH=/usr/sbin:/usr/bin:/sbin:/binPWD=/root at df9992e4c518:/#


Meaning, the whole patch is superfluous!

>> --- 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.

Thanks, I'll look into that. As I understand, the containers don't need
*init*, but the deactivation would be required on run time, not?

Thanks for you time,
Paul


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