Merge ~andreserl/maas:1.9_lp1732703 into maas:1.9

Proposed by Andres Rodriguez on 2017-12-07
Status: Needs review
Proposed branch: ~andreserl/maas:1.9_lp1732703
Merge into: maas:1.9
Diff against target: 22 lines (+5/-7)
1 file modified
src/provisioningserver/utils/__init__.py (+5/-7)
Reviewer Review Type Date Requested Status
Mike Pontillo 2017-12-07 Approve on 2017-12-13
MAAS Lander Needs Fixing on 2017-12-08
Review via email: mp+334930@code.launchpad.net

Commit Message

LP: #1732703 - Correctly detect upstart when systemd is installed but not used.

To post a comment you must log in.
Mike Pontillo (mpontillo) wrote :

Where are the unit tests for this change?

That said, this code should work. But I kind of liked Alberto's suggestion (can't remember where he posted it, maybe IRC):

    # readlink /proc/1/exe
    /lib/systemd/systemd

That is, we could check the running PID 1 itself to see if 'systemd' is in the name.

Either way, I think if MAAS is running in Docker, all bets are off. ;-)

review: Needs Information
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b 1.9_lp1732703 lp:~andreserl/maas into -b 1.9 lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/924/console
COMMIT: 3726c5425945c6f7f42f0bb369659c0201710716

review: Needs Fixing
Mike Pontillo (mpontillo) wrote :

Now that I fully understand the issue, I don't think this solves the problem. On a machine with systemd installed, you'll see the following:

$ ls -la /sbin/init
lrwxrwxrwx 1 root root 20 Oct 27 03:11 /sbin/init -> /lib/systemd/systemd

$ dpkg -S /sbin/init
systemd-sysv: /sbin/init

So the fact that /sbin/init is a link that points to the systemd binary means one of two things:

 - systemd is installed and is being used
 - systemd has been installed, but is not yet being used (the system has not rebooted)

So the most accurate way to determine which is running is indeed to look at PID 1 itself.

review: Needs Fixing
Andres Rodriguez (andreserl) wrote :

First, and foremost, this only applies to *Trusty*.

Trusty uses upstart by default. When you install systemd, /sbin/init will *NOT* point to /lib/systemd/systemd/.

You need to manually modify it to use /sbin/init, hence this patch works.

This has been already tested in multiple environments.

Mike Pontillo (mpontillo) wrote :

After discussing this on IRC, I agree that since we cannot support a Trusty system that is running MAAS 1.9 but has been dist-upgraded to Xenial, this fix should work fine for our use case.

review: Approve
Mike Pontillo (mpontillo) wrote :

That said, you're still missing unit tests. ;-) Please update them before landing.

Unmerged commits

3726c54... by Andres Rodriguez on 2017-12-07

LP: #1732703 - Correctly detect upstart when systemd is installed but not used.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/provisioningserver/utils/__init__.py b/src/provisioningserver/utils/__init__.py
2index 76ddab5..ff22503 100644
3--- a/src/provisioningserver/utils/__init__.py
4+++ b/src/provisioningserver/utils/__init__.py
5@@ -502,12 +502,10 @@ def sudo(command_args):
6 return ['sudo', '-n'] + command_args
7
8
9-SYSTEMD_RUN_PATH = '/run/systemd/system'
10-
11-
12 def get_init_system():
13 """Returns 'upstart' or 'systemd'."""
14- if os.path.exists(SYSTEMD_RUN_PATH):
15- return 'systemd'
16- else:
17- return 'upstart'
18+ init_path = '/sbin/init'
19+ if os.path.islink(init_path):
20+ if os.readlink(init_path) == "/lib/systemd/systemd":
21+ return 'systemd'
22+ return 'upstart'

Subscribers

People subscribed via source and target branches