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

no need to implement log rotation internally #7

Closed
laf0rge opened this issue Oct 21, 2017 · 3 comments
Closed

no need to implement log rotation internally #7

laf0rge opened this issue Oct 21, 2017 · 3 comments
Labels
type:enhancement Enhance performance or improve usability of original features.

Comments

@laf0rge
Copy link
Contributor

laf0rge commented Oct 21, 2017

It seems like nextepc is implementing log rotation internally, at least judging from the below log messages:

Can't rename '/var/log/nextepc.log' to '/var/log/nextepc.log.0'
freeDiameter[1]: Prepared 1 sets of connection parameters to peer hss.localdomain
Cannot write 82 bytes to log file (0 written)

This means that nextepc would have to run with permissions not only to the single log file it is writing to, but also with permissions to write/create new files in /var/log, which normally only root can do.

I would suggest to leave log file rotation up to logrotate, and simply re-open the log files on SIGHUP. This meachanism has worked for decades and is well-understood by sysadmins all over the planet.

https://linux.die.net/man/8/logrotate
https://wiki.archlinux.org/index.php/logrotate

This way, only logrotate needs to run with higher privileges, while the individual daemon (like nextepc) can run with regular user privilege, only with a single /var/log/nextepc.log writable to the nextepc user.

@acetcom
Copy link
Member

acetcom commented Oct 22, 2017

You made a good point!

Basically, I didn't test installations with root privileges and running with user privileges. And also, I realized that It should be considered because binary distribution like debian packaging is goes on in different privileges.

Yes, our logger thread cannot be properly working in this setup as you described. So I have no objection to use logrotate in Linux/FreeBSD. For MacOSX, newsyslog will be used.

I'll start this work from this week to solve this problem for accelerating debian package project in r0.2 branch. Of course, I'll try to solve Issue #4 at the same time.

@acetcom
Copy link
Member

acetcom commented Oct 24, 2017

I've updated r0.2 for supporting logrotate. Now, 'sudo make install' and './nextepc-epcd' is properly working.

There are two things that is different from other open sources. Mostly, the followings are for developer. Let me know if it could be a problem to the end-user.

  1. configure.ac
221         systemd_unit_dir="${sysconfdir}/systemd/system/"
         ...
224         logrotate_conf_dir="${sysconfdir}/logrotate.d/"

I used ${sysconfdir} instead of /etc to permit developer to install nextepc with the user-privilege.

  1. support/logrotate/nextepc.in
7    create 640 @current_user@ @current_group@

So, if user installs this package with sudo make install, the above value is set to current user and group. For example, support/logrotate/nextepc file is generated as follows.

7    create 640 acetcom acetcom

I know most open source project generally uses specific user. In our case, it will be a 'nextepc:nextepc'. Yeah!... It could be changed with ./configure --enable_user=acetcom --enable-group=acetcom. However, in my case, I'd just like to use ./configure --prefix=`pwd`/install without any other options.

I'm not sure what is good for us. Just it's my first trial.
Feel free to raise any suggestions.

@acetcom acetcom added the type:enhancement Enhance performance or improve usability of original features. label Oct 25, 2017
@acetcom
Copy link
Member

acetcom commented Oct 30, 2017

Please, forget about the previous message. I think that above two things are not good for us. In debian package, I made a nextepc user for systemd to execute nextepc's daemon. All things follow the method which is widely used in other open sources .

BTW, I've initially uploaded debian package using LaunchPad PPA. Currently, I've tested only in Ubuntu 16.04 LTS.

sudo add-apt-repository ppa:acetcom/nextepc
sudo apt-get update
sudo apt-get install nextepc

Thank you for your contribution related to debian package.

@acetcom acetcom closed this as completed Jan 2, 2018
acetcom pushed a commit that referenced this issue Dec 13, 2019
aseaudi pushed a commit to aseaudi/open5gs that referenced this issue Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Enhance performance or improve usability of original features.
Projects
None yet
Development

No branches or pull requests

2 participants