Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the Sysbox installer #221

Closed
ctalledo opened this issue Feb 26, 2021 · 7 comments
Closed

Improve the Sysbox installer #221

ctalledo opened this issue Feb 26, 2021 · 7 comments
Labels
enhancement New feature or request
Projects
Milestone

Comments

@ctalledo
Copy link
Member

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.

@ctalledo ctalledo added the enhancement New feature or request label Feb 26, 2021
@ctalledo ctalledo added this to the v0.4 milestone Feb 26, 2021
@ctalledo ctalledo added this to To do in Sysbox Dev via automation Feb 26, 2021
@nudgegoonies
Copy link

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
Copy link
Member Author

ctalledo commented Mar 1, 2021

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:

envoyproxy/envoy#8297 (comment)

@nudgegoonies
Copy link

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
Copy link
Member Author

ctalledo commented Mar 3, 2021

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
Copy link

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 added a commit to nudgegoonies/sysbox-fs that referenced this issue Apr 12, 2021
* Use os.Stdout if no logging filename is given
* This makes it possible to use journald by setting --log ""
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
nudgegoonies added a commit to nudgegoonies/sysbox-fs that referenced this issue Apr 12, 2021
* Use os.Stdout if no logging filename is given
* This makes it possible to use journald by setting --log ""
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
nudgegoonies added a commit to nudgegoonies/sysbox-mgr that referenced this issue Apr 12, 2021
* Use os.Stdout if no logging filename is given
* This makes it possible to use journald by setting --log ""
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
@nudgegoonies
Copy link

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.

nudgegoonies added a commit to nudgegoonies/sysbox-fs that referenced this issue Apr 13, 2021
* Use os.Stdout if no logging filename is given
* This makes it possible to use journald by setting --log ""
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
nudgegoonies added a commit to nudgegoonies/sysbox-fs that referenced this issue Apr 13, 2021
* Use os.Stderr if no logging filename is given
* This makes it possible to use journald by setting --log ""
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
nudgegoonies added a commit to nudgegoonies/sysbox-mgr that referenced this issue Apr 13, 2021
* Use os.Stderr if no logging filename is given
* This makes it possible to use journald by setting --log ""
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
nudgegoonies added a commit to nudgegoonies/sysbox-fs that referenced this issue Apr 13, 2021
* Use os.Stderr if no logging filename is given
* Set default log to "" to enable stderr output
* This makes it possible to use journald by using default log value
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
nudgegoonies added a commit to nudgegoonies/sysbox-mgr that referenced this issue Apr 13, 2021
* Use os.Stderr if no logging filename is given
* Set default log to "" to enable stderr output
* This makes it possible to use journald by using default log value
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
nudgegoonies added a commit to nudgegoonies/sysbox-fs that referenced this issue Apr 14, 2021
* Use os.Stderr if no logging filename is given
* Set default log to "" to enable stderr output
* This makes it possible to use journald by using default log value
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
Signed-off-by: Andreas Wirooks <andreas.wirooks@web.de>
nudgegoonies added a commit to nudgegoonies/sysbox-mgr that referenced this issue Apr 14, 2021
* Use os.Stderr if no logging filename is given
* Set default log to "" to enable stderr output
* This makes it possible to use journald by using default log value
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
Signed-off-by: Andreas Wirooks <andreas.wirooks@web.de>
rodnymolina pushed a commit to nestybox/sysbox-fs that referenced this issue Apr 14, 2021
* Use os.Stderr if no logging filename is given
* Set default log to "" to enable stderr output
* This makes it possible to use journald by using default log value
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
Signed-off-by: Andreas Wirooks <andreas.wirooks@web.de>
rodnymolina pushed a commit to nestybox/sysbox-mgr that referenced this issue Apr 14, 2021
* Use os.Stderr if no logging filename is given
* Set default log to "" to enable stderr output
* This makes it possible to use journald by using default log value
* This solution is not optimal, but keeps backwards compatibility

Refs: nestybox/sysbox#221
Signed-off-by: Andreas Wirooks <andreas.wirooks@web.de>
rodnymolina added a commit that referenced this issue Apr 16, 2021
…usr/bin'

Address issue #221 requirement of changing the executables path to `/usr/bin` instead of the original `/usr/local/bin` path.

Signed-off-by: Rodny Molina <rmolina@nestybox.com>
rodnymolina added a commit to nestybox/sysbox-pkgr that referenced this issue Apr 16, 2021
Refer to Sysbox issue [#221](nestybox/sysbox#221) for details.

Signed-off-by: Rodny Molina <rmolina@nestybox.com>
rodnymolina added a commit to nestybox/sysbox-pkgr that referenced this issue Apr 16, 2021
Refer to Sysbox issue [#221](nestybox/sysbox#221) for details.

Signed-off-by: Rodny Molina <rmolina@nestybox.com>
rodnymolina added a commit that referenced this issue Apr 16, 2021
…usr/bin'

Address issue #221 requirement of changing the executables path to `/usr/bin` instead of the original `/usr/local/bin` path.

Signed-off-by: Rodny Molina <rmolina@nestybox.com>
@ctalledo
Copy link
Member Author

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.

Sysbox Dev automation moved this from To do to Done Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Sysbox Dev
  
Done
Development

No branches or pull requests

3 participants