[Buildroot] [External] [PATCH v2 1/2] package/netdata: new package

["Marcin Niestrój" <m.niestroj@grinn-global.com>]

Hi Matt,

Matthew Weber <matthew.weber@collins.com> writes:

> Marcin,
>
> On Wed, Oct 30, 2019 at 4:33 AM Marcin Niestroj
> <m.niestroj@grinn-global.com> wrote:
>>
>> Always provide --disable-dbengine configuration option, because we do
>> not support libjudy dependency that is required otherwise.
>>
>> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> [snip]
>
>> --- /dev/null
>> +++ b/package/netdata/netdata.mk
>> @@ -0,0 +1,56 @@
>> +################################################################################
>> +#
>> +# netdata
>> +#
>> +################################################################################
>> +
>
> The package's default install step attempts to create folders in
> $(TARGET_DIR)/var/lib/netdata, $(TARGET_DIR)/var/cache/netdata, and
> $(TARGET_DIR)/var/log/netdata.  For the cache and log cases in the
> default Buildroot skeleton, those point to ../tmp and results in a
> race condition causing a "file exists error".

I'll try to clarify what happens there. This is what we see in build
log:

   /usr/bin/install -c -m 644 packaging/installer/.keep '/buildroot/test-netdata/TestNetdata/target/var/lib/netdata'
    /usr/bin/install -c netdata '/buildroot/test-netdata/TestNetdata/target/usr/sbin'
   /usr/bin/install -c -m 644 packaging/installer/.keep '/buildroot/test-netdata/TestNetdata/target/var/lib/netdata/registry'
   /usr/bin/install -c -m 644 packaging/installer/.keep '/buildroot/test-netdata/TestNetdata/target/var/log/netdata'
   /usr/bin/install -c -m 644 packaging/installer/.keep '/buildroot/test-netdata/TestNetdata/target/var/cache/netdata'
  /usr/bin/install: cannot change permissions of '/buildroot/test-netdata/TestNetdata/target/var/cache/netdata/.keep': No such file or directory

The problem is that `install` is called in parallel. Looking at what
`install` really does with strace:

  stat("/buildroot/test-netdata/TestNetdata/target/var/cache/netdata", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
  stat("packaging/installer/.keep", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
  newfstatat(AT_FDCWD, "/buildroot/test-netdata/TestNetdata/target/var/cache/netdata/.keep", {st_mode=S_IFREG|0644, st_size=0, ...}, AT_SYMLINK_NOFOLLOW) = 0
  unlink("/buildroot/test-netdata/TestNetdata/target/var/cache/netdata/.keep") = 0
  openat(AT_FDCWD, "packaging/installer/.keep", O_RDONLY) = 3
  fstat(3, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
  openat(AT_FDCWD, "/buildroot/test-netdata/TestNetdata/target/var/cache/netdata/.keep", O_WRONLY|O_CREAT|O_EXCL, 0600) = 4
  fstat(4, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
  fadvise64(3, 0, 0, POSIX_FADV_SEQUENTIAL) = 0
  mmap(NULL, 139264, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f6dfa7ee000
  read(3, "", 131072)                     = 0
  fsetxattr(4, "system.posix_acl_access", "\2\0\0\0\1\0\6\0\377\377\377\377\4\0\0\0\377\377\377\377 \0\0\0\377\377\377\377", 28, 0) = 0
  close(4)                                = 0
  close(3)                                = 0
  munmap(0x7f6dfa7ee000, 139264)          = 0
  lstat("/buildroot/test-netdata/TestNetdata/target/var/cache/netdata/.keep", {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
  chmod("/buildroot/test-netdata/TestNetdata/target/var/cache/netdata/.keep", 0644) = 0

So what (most probably) happens is that one `install` process has
created the `.keep` file, but didn't make it to change permission,
because another `install` process has already unlinked it.

> To fix that target install issue, I'd propose the following patch be
> added.
>
> https://pastebin.com/TL3znCJs

With this patch cache files would be written in persistent storage under
/var/lib/netdata/cache. I would rather leave those files in tmpfs.

I also do not like the fact that logs would be put under /tmp/debug.log,
/tmp/error.log and /tmp/access.log files. There are plenty of other
daemons which would like to do the same :)

So far we depend on creating /var/cache/netdata (because netdata tries
to cd into it by default during init). So we have already the needed
structure for both cache and log (both point to /tmp/netdata). So the
only thing I would do is to remove .keep installation from Makefile.am
to get rid of race condition. This means simple patch to maintain within
Buildroot. What do you think?

>
> Regards,
> Matt
>
>
>> --
>> 2.23.0
>>


--
Marcin Niestr?j