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

Systemd unit update #1889

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@sergey-safarov
Copy link
Contributor

commented Mar 12, 2019

Pre-Submission Checklist

  • Commit message has the format required by CONTRIBUTING guide
  • Commits are split per component (core, individual modules, libs, utils, ...)
  • Each component has a single commit (if not, squash them into one commit)
  • No commits to README files for modules (changes must be done to docbook files
    in doc/ subfolder, the README file is autogenerated)

Type Of Change

  • Small bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would change existing functionality)

Checklist:

  • PR should be backported to stable branches
  • Tested changes locally
  • Related to issue #1827

Description

  1. tmpfiles creation is implemented via systemd
  2. owner of kamailio process implemented via systemd drop-in
  3. kamailio started as simple process, that allow systemd check kamailio process without PID file.

sergey-safarov added some commits Mar 12, 2019

Init scripts always create kamailio user and group.
If anybody wants use any other user/group, then they must create new user/group.
Also they be able change username using systemd drop-in /etc/systemd/system/kamailio.service.d/10-user_group.conf file, like
```
[Service]
User=kamuser
Group=kamgroup
```
Added kamailio.tmpfiles. Directory /run/kamailio is created by command
systemd-tmpfiles --create kamailio.conf

systemd-tmpfiles command is executed by dh_installinit
@sergey-safarov

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2019

Hello Victor @linuxmaniac
Could you look this PR.
Changes is spited by commit.
After you approve i will squash commit into one and create new commit message like

pkg/kamailio/deb: Updated systemd service and tmpfile files

@sergey-safarov sergey-safarov requested a review from linuxmaniac Mar 12, 2019

@miconda miconda changed the title Unit update Systemd unit update Mar 12, 2019

@@ -4,18 +4,16 @@ Wants=network-online.target
After=network-online.target

[Service]
Type=forking
Type=simple
User=kamailio

This comment has been minimized.

Copy link
@mckaygerhard

mckaygerhard Mar 12, 2019

Hi!, thanks for this pull request.. finally after many fights and ignored mails, but now a small request (if it can be done): there's a way to pass the user and group from environment or invoke command? i mean for packaging.. in debian exits the /etc/default/kamailio file that set user and group...

This comment has been minimized.

Copy link
@sergey-safarov

sergey-safarov Mar 12, 2019

Author Contributor

To change username at runtime need place drop-in into /run/systemd/system/kamailio.service.d directory and apply changes by systemctl daemon-reload
For dynamic unit updates you cannot use /etc/systemd/system/kamailio.service.d directory.

Also you can use service templates

This comment has been minimized.

Copy link
@sergey-safarov

sergey-safarov Mar 12, 2019

Author Contributor

there's a way to pass the user and group from environment or invoke command?
Please look d3cc4e4
kamailio.default file have complete description how to change user/group of kamailio process.

@sergey-safarov sergey-safarov added the pkg label Mar 12, 2019

@@ -4,18 +4,16 @@ Wants=network-online.target
After=network-online.target

[Service]
Type=forking
Type=simple

This comment has been minimized.

Copy link
@verticelo

verticelo Mar 22, 2019

Contributor

How will this interact with the default setting of fork=yes in the config?

This comment has been minimized.

Copy link
@sergey-safarov

sergey-safarov Mar 22, 2019

Author Contributor

This systemd setting not kamailio process.
Interactions is not exist.

This comment has been minimized.

Copy link
@verticelo

verticelo May 20, 2019

Contributor

But, doesn't this determine how systemd monitors the process after the process has started, ie. if the kamailio process forks or not?

This comment has been minimized.

Copy link
@sergey-safarov

sergey-safarov May 20, 2019

Author Contributor

yes, that is determines how to monitor kamailio process.
I suggest to start kamailio with -DD option.
In this case kamailio main process control own child's, and systemd monitors only kamailio main process.

# ExecStart requires a full absolute path
ExecStart=/usr/sbin/kamailio -P /var/run/kamailio/kamailio.pid -f $CFGFILE -m $SHM_MEMORY -M $PKG_MEMORY -u $USER -g $GROUP
ExecStart=/usr/sbin/kamailio -DD -P /var/run/kamailio/kamailio.pid -f $CFGFILE -m $SHM_MEMORY -M $PKG_MEMORY

This comment has been minimized.

Copy link
@verticelo

verticelo Mar 22, 2019

Contributor

Changing to this mode, will it have any impact on performance of Kamailio? If so, in what direction?

This comment has been minimized.

Copy link
@sergey-safarov

sergey-safarov Mar 22, 2019

Author Contributor

I did not observe the influence

@mckaygerhard

This comment has been minimized.

Copy link

commented Mar 26, 2019

seems systemd made more problems thatn solves #1898

@sergey-safarov

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2019

@mckaygerhard your reference not relevant to systemd.

@miconda

This comment has been minimized.

Copy link
Member

commented May 20, 2019

@linuxmaniac - these changes are on systemd unit for debian packaging. Any conclusion on what to do with this PR?

If decided to merge, quash in a single commit and set a proper formatted commit message, because now the commits do not follow the rules.

@sergey-safarov

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

I ready to squash and update commit after review.

@miconda

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

@linuxmaniac - any conclusion this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.