nestybox / sysbox

An open-source, next-generation "runc" that empowers rootless containers to run workloads such as Systemd, Docker, Kubernetes, just like VMs.
Apache License 2.0
2.78k stars 152 forks source link

Improve the Sysbox installer #221

Closed ctalledo closed 2 years ago

ctalledo commented 3 years ago

The sysbox package installer could be improved as follows:

1) Update the Sysbox systemd units to log into the systemd journal by default, as opposed to log files (e.g., /var/log/sysbox-mgr.log).

2) Install executables in /usr/bin instead of /usr/local/bin. The former is a more appropriate directory, and it's the directory where the binaries for docker, containerd, etc. reside.

nudgegoonies commented 3 years ago

Regarding the journald logging. It works only when the --log parameter is used and a filename for a logfile is given. Using "--log /dev/stdout" explicit or implicit without --log parameter it does not work and fails with:

open /dev/stdout: no such device or address
ctalledo commented 3 years ago

Using "--log /dev/stdout" explicit or implicit without --log parameter it does not work

Yes this is what I had noticed when I briefly played around with this some weeks ago. I wonder if it's related to the fact that systemd sets /dev/stdout to be a socket rather than a file, as described here:

https://github.com/envoyproxy/envoy/issues/8297#issuecomment-620659781

nudgegoonies commented 3 years ago

Interesting issue. So the problem is that sysbox-mgr and sysbox-fs try to open the /dev/stdout filename instead writing to file descriptor 1?

I am not a go programmer, but could it be that that these lines try to open the file in the wrong mode (see create flag, mode 666)? On my Debian 10 system /dev/stdout is a symlink with lrwxrwxrwx permission to /proc/self/fd/1 with is a symlink with lrwx------ permissions to /dev/pts/4 with crw--w---- permission. By the way, why not just use PrintLn when no filename is given? https://github.com/nestybox/sysbox-mgr/blob/95010ecb3e813d75ec8b523bd8f6d8890dfa26e6/main.go#L110 https://github.com/nestybox/sysbox-fs/blob/4610f0be058e9121e91db9e9642079c4f12ae926/cmd/sysbox-fs/main.go#L252

ctalledo commented 3 years ago

In Sysbox we use the Golang logrus package for logging. It seems this package has no problem logging to files (or /dev/stdout when it points to a file). But when /dev/stdout points to a socket (e.g., when logging to the systemd journal log), it apparently does not work correctly. I am speculating here a bit as I've not had the chance to take a closer look. But there must be some fairly simple way to fix this.

mfriedenhagen commented 3 years ago

One other remark: I would go for logging to stderr instead of stdout. Both are picked up by journald just fine, but if you ever decide to have an option which generates non-logging output, the separation is useful. E.g. some programs may dump their current configuration and then you would use stdout for that while still being able to log stuff in parallel.

nudgegoonies commented 3 years ago

I found a way to use journald. See Merge Requests above.

If you want to keep logging to logfiles by default you should add a logrotate configuration as well.

ctalledo commented 2 years ago

The sysbox package installer could be improved as follows:

  1. Update the Sysbox systemd units to log into the systemd journal by default, as opposed to log files (e.g., /var/log/sysbox-mgr.log).
  2. Install executables in /usr/bin instead of /usr/local/bin. The former is a more appropriate directory, and it's the directory where the binaries for docker, containerd, etc. reside.

Both of these are already implemented (since Sysbox v0.4.1 IIRC). Closing.