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

Matthew Weber matthew.weber at rockwellcollins.com
Mon Nov 4 21:55:06 UTC 2019


Marcin,

On Mon, Nov 4, 2019 at 3:54 AM Marcin Niestrój
<m.niestroj at grinn-global.com> wrote:
>
> Hi Matt,
>
> Matthew Weber <matthew.weber at collins.com> writes:
>
> > Marcin,
> >
> > On Thu, Oct 31, 2019 at 11:18 AM Marcin Niestrój
> > <m.niestroj at grinn-global.com> wrote:
> >>
> >> Hi Matt,
> >>
> >> Matthew Weber <matthew.weber at collins.com> writes:
> >>
> >> > Marcin,
> >> >
> >> > On Wed, Oct 30, 2019 at 4:33 AM Marcin Niestroj
> >> > <m.niestroj at 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 at 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.
> >
> > Yeah, it's inheritly because of the way the base skeleton is setup
> > with symlinks in buildroot.
> >
> >>
> >> > To fix that target install issue, I'd propose the following patch be
> >> > added.
> >> >
> >> > https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.com_TL3znCJs&d=DwIFaQ&c=ilBQI1lupc9Y65XwNblLtw&r=y1sOV0GV8NZUkffv7oCRxs2Sd3nOBS-NxDM3NY8lOgs&m=QZL88tMKpKawDLFC9yPt80FEa5Ojbic8ue1DfxOQ3mY&s=U45yjsRvZGNjr4DcvyCulMnguBFPNn2KgFqHYPPQBpA&e=
> >>
> >> 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 :)
> >>
> >
> > Keep in mind, none of the items written to /var/log or /var/cache
> > during build time end up on the target.
>
> Yes, I am aware of that. So both /var/log/netdata and /var/cache/netdata
> point to non-existing (during system boot) /tmp/netdata directory.
>
> > If you want different runtime behavior with folders for cache and logs
> > you have to stage those with a startup script.
>
> Correct. And with current patch series we just install netdata within
> Buildroot and rely on configuring and starting netdata by user. I am not sure
>
> > Maybe it makes sense to add a S60netdata script in your new version of
> > the series which could setup the folders in the tmpfs to be used for
> > logging and cache accordingly?
>
> If we will provide S60netdata, then we also need some equivalent for
> systemd. What do you think about creating (mkdir -p) only
> /var/cache/netdata in such init script? This directory is used as
> current (home) directory for netdata. /var/log/cache looks like optional
> for netdata, because it will safely continue execution if log files
> cannot be opened. In default skeleton however both would point to
> /tmp/netdata, so there would be no problem with logs.

Yeah the init script would need to create that netdata folder at
runtime in /var/cache or /tmp or /var/log/ if not present.  That would
cover most of the use cases for tmpfs based or persistent.

You could just add a sysvinit script to start with and then let others
contribute systemd if that's not your focus for a init.

>
> > As part of this startup script you could also symlink the
> > /var/lib/netdata/cache folder to a tmpfs location.  I suggested the
> > patch to keep the changes minimal to the upstream package and then at
> > runtime we can tailor how netdata actually uses the folder.
>
> We can also simply remove the installation of .keep files with
> https://pastebin.com/raw/P9DQC1ry and it solves the problem with even
> less changes (in my opinion). This is also what you suggested below I
> think.

Looks good, I replied to your other email related to this patch and
strategy for now.

>
> >
> >> 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?
> >
> > Like previously mentioned you'll need an init script to setup any
> > folders in /var/cache or log as those locations are symlinked to /tmp
> > by default and the buildtime folders won't be present at runtime.  So
> > you could suggest a .keep cleanup script if that seems less intrusive
> > to the package and maybe you can even make it upstreamable?  In some
> > ways if we can just bypass this whole localstatedir folder setup
> > section of the makefile that would be ideal.
>
> The problem with netdata is that it is not clear which autotools or
> cmake they are targetting to support in the future. I don't feel
> comfortable in investing time now to support some feature in autotools
> (I barely know it as developer) if cmake will be the only supported
> solution in near future. On the other hand cmake seems to work
> differently, e.g. there is no .keep installation there at all, but
> unfortunately more dependencies are required instead of optional.
>
> So in order to do things properly we would need to ask netdata about
> their prefered build system for future and do improvements there :)

I'd go with what we have for now and the basic patch to remove .keeps.
I've noted in the other email thread about creating a issue ticket you
can reference as well.

Matt



More information about the buildroot mailing list