Restart policy "Always" doesn't work under certain circumstances

Bug #1821255 reported by Cody Shepherd on 2019-03-21
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
open-iscsi (Ubuntu)
High
Heitor Alves de Siqueira
Bionic
High
Heitor Alves de Siqueira
Cosmic
High
Heitor Alves de Siqueira
Disco
High
Heitor Alves de Siqueira

Bug Description

[Impact]
iscsid restart fails when killed with SIGTERM

[Description]
If systemd tries to execute a command that talks to iscsid via iscsid.socket, it can hang if iscsid is not running (or is in the process of being stopped). This can happen due to the current ExecStop= directive, which calls iscsiadm to kill iscsid, and prevents the service from being restarted even when we set Restart=always in the iscsid unit file.

The solution is to let systemd terminate iscsid by itself. The default action
when omitting the ExecStop directive is to send SIGTERM to the process group,
which is equivalent to invoking "iscsiadm -k" (the current ExecStop command).

[Test Case]
1) Deploy a Disco VM e.g. with uvt-kvm
    $ uvt-kvm create disco release=disco

2) Run the deploy-tgt.sh script in the VM to setup an iSCSI target in the
localhost. The script will install tgt and open-iscsi, configure a 1G
file-backed iSCSI target, login and restart iscsid.service
    ubuntu@disco:~$ ./deploy-tgt.sh

3) Kill iscsid with SIGTERM
    ubuntu@disco:~$ sudo pkill iscsid

4) Try to stop iscsid.service and check whether it hangs
    ubuntu@disco:~$ sudo systemctl stop iscsid

If we remove the ExecStop= directive, it works as expected:
    ubuntu@disco:~$ sudo systemctl stop iscsid
    ubuntu@disco:~$
This also causes Restart=always to work as expected.

[Regression Potential]
This shouldn't introduce any regressions, since iscsiadm -k just sends SIGTERM to iscsid's process group and that's equivalent to the default systemd ExecStop action. Nonetheless, changes will be tested with autopkgtests and different iscsi scenarios.

Steve Langasek (vorlon) wrote :

We triaged this to the fact that ExecStop runs a command that tries to talk to iscsid, and iscsid is socket-activated on Ubuntu, so the command hangs while trying to talk to a socket that systemd is waiting to dispatch to the iscsid service that is in the process of being stopped.

From reading the manpage on iscsiadm, we should just omit the ExecStop command in favor of using the default semantics of KillSignal=SIGTERM.

Changed in open-iscsi (Ubuntu):
importance: Undecided → High
status: New → Triaged
Changed in open-iscsi (Ubuntu Bionic):
status: New → Triaged
importance: Undecided → High
description: updated
Changed in open-iscsi (Ubuntu Bionic):
status: Triaged → Confirmed
Changed in open-iscsi (Ubuntu Cosmic):
status: New → Confirmed
Changed in open-iscsi (Ubuntu Disco):
status: Triaged → Confirmed
Changed in open-iscsi (Ubuntu Cosmic):
importance: Undecided → High
Changed in open-iscsi (Ubuntu Disco):
assignee: nobody → Heitor R. Alves de Siqueira (halves)
Changed in open-iscsi (Ubuntu Cosmic):
assignee: nobody → Heitor R. Alves de Siqueira (halves)
Changed in open-iscsi (Ubuntu Bionic):
assignee: nobody → Heitor R. Alves de Siqueira (halves)
description: updated

I've updated the description with a simpler test case that doesn't require an
override file, but this bug can also be verified with the Restart=always directive:

ubuntu@disco:~$ cat /etc/systemd/system/iscsid.service.d/override.conf
[Service]
Restart=always

ubuntu@disco:~$ sudo pkill iscsid

ubuntu@disco:~$ sudo systemctl status iscsid
● iscsid.service - iSCSI initiator daemon (iscsid)
   Loaded: loaded (/lib/systemd/system/iscsid.service; disabled; vendor pre
  Drop-In: /etc/systemd/system/iscsid.service.d
           └─override.conf
   Active: inactive (dead)
     Docs: man:iscsid(8)

tags: added: sts
tags: added: sts-sponsor
Eric Desrochers (slashd) wrote :

[sponsor note]
- Can you please submit the change to Debian ?
- Does Xenial would need the fix as well as it come with systemd by default and as the 'ExecStop=/sbin/iscsiadm -k 0 2' directive as well ?

Changed in open-iscsi (Ubuntu Disco):
status: Confirmed → In Progress

Hi Eric,

Xenial does not exhibit this bug since it does not ship iscsid.socket. This is also the reason Debian is not affected, since they also don't have socket activation for iscsid.
I did send a comment to the Debian bug that proposes changing iscsid to socket activation [0], just so they're aware of this issue.

Thanks,
Heitor

[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=900397

The attachment "lp1821255-bionic.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Eric Desrochers (slashd) wrote :

Sponsored for disco. Let's wait until this land in disco-releases, before proceeding with the SRU.

Changed in open-iscsi (Ubuntu Disco):
status: In Progress → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package open-iscsi - 2.0.874-5ubuntu15

---------------
open-iscsi (2.0.874-5ubuntu15) disco; urgency=medium

  * d/iscsid.service: Remove ExecStop= directive. Letting systemd
    handle termination prevents restart failures. (LP: #1821255)

 -- <email address hidden> (Heitor R. Alves de Siqueira) Thu, 28 Mar 2019 10:21:24 -0300

Changed in open-iscsi (Ubuntu Disco):
status: Fix Committed → Fix Released
Eric Desrochers (slashd) on 2019-04-02
Changed in open-iscsi (Ubuntu Cosmic):
status: Confirmed → In Progress
Changed in open-iscsi (Ubuntu Bionic):
status: Confirmed → In Progress
Eric Desrochers (slashd) wrote :

Sponsored for Cosmic, now that open-iscsi is "Fix Released" in disco.

Eric Desrochers (slashd) wrote :

Sponsored for Bionic.

Hello Cody, or anyone else affected,

Accepted open-iscsi into cosmic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/open-iscsi/2.0.874-5ubuntu9.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-cosmic to verification-done-cosmic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-cosmic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in open-iscsi (Ubuntu Cosmic):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-cosmic
Changed in open-iscsi (Ubuntu Bionic):
status: In Progress → Fix Committed
tags: added: verification-needed-bionic
Brian Murray (brian-murray) wrote :

Hello Cody, or anyone else affected,

Accepted open-iscsi into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/open-iscsi/2.0.874-5ubuntu2.7 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Confirmed fix in bionic, according to test case:

ubuntu@bionic:~$ ./deploy-tgt.sh

ubuntu@bionic:~$ dpkg -l | grep open-iscsi
ii open-iscsi 2.0.874-5ubuntu2.7 amd64 iSCSI initiator tools

ubuntu@bionic:~$ sudo pkill iscsid

ubuntu@bionic:~$ sudo systemctl stop iscsid
Warning: Stopping iscsid.service, but it can still be activated by:
  iscsid.socket

ubuntu@bionic:~$

The systemctl stop command doesn't hang anymore, and the service can be restarted normally.

tags: added: verification-done-bionic
removed: verification-needed-bionic

Confirmed fix in cosmic, according to test case:

ubuntu@cosmic:~$ ./deploy-tgt.sh

ubuntu@cosmic:~$ dpkg -l | grep open-iscsi
ii open-iscsi 2.0.874-5ubuntu9.4 amd64 iSCSI initiator tools

ubuntu@cosmic:~$ sudo pkill iscsid

ubuntu@cosmic:~$ sudo systemctl stop iscsid
Warning: Stopping iscsid.service, but it can still be activated by:
  iscsid.socket

ubuntu@cosmic:~$

The systemctl stop command doesn't hang anymore, and the service can be restarted normally.

tags: added: verification-done-cosmic
removed: verification-needed verification-needed-cosmic
Dan Streetman (ddstreet) on 2019-04-10
tags: removed: sts-sponsor
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package open-iscsi - 2.0.874-5ubuntu9.4

---------------
open-iscsi (2.0.874-5ubuntu9.4) cosmic; urgency=medium

  * d/iscsid.service: Remove ExecStop= directive. Letting systemd
    handle termination prevents restart failures. (LP: #1821255)

 -- <email address hidden> (Heitor R. Alves de Siqueira) Thu, 28 Mar 2019 15:12:50 -0300

Changed in open-iscsi (Ubuntu Cosmic):
status: Fix Committed → Fix Released

The verification of the Stable Release Update for open-iscsi has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package open-iscsi - 2.0.874-5ubuntu2.7

---------------
open-iscsi (2.0.874-5ubuntu2.7) bionic; urgency=medium

  * d/iscsid.service: Remove ExecStop= directive. Letting systemd
    handle termination prevents restart failures. (LP: #1821255)

 -- <email address hidden> (Heitor R. Alves de Siqueira) Thu, 28 Mar 2019 15:14:49 -0300

Changed in open-iscsi (Ubuntu Bionic):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.