From 01532c2d16ba62406362dcb4bae5030e05fd6502 Mon Sep 17 00:00:00 2001 From: Andrew Stone Date: Thu, 25 Aug 2022 16:31:56 -0700 Subject: [PATCH 1/2] test: Move testsuite-03 units to .units directory --- test/meson.build | 3 +++ test/{units => testsuite-03.units}/hello-after-sleep.target | 0 test/{units => testsuite-03.units}/hello.service | 0 test/{units => testsuite-03.units}/sleep.service | 0 test/{units => testsuite-03.units}/unstoppable.service | 0 5 files changed, 3 insertions(+) rename test/{units => testsuite-03.units}/hello-after-sleep.target (100%) rename test/{units => testsuite-03.units}/hello.service (100%) rename test/{units => testsuite-03.units}/sleep.service (100%) rename test/{units => testsuite-03.units}/unstoppable.service (100%) diff --git a/test/meson.build b/test/meson.build index 65cb52cd21..f2080b5a9a 100644 --- a/test/meson.build +++ b/test/meson.build @@ -27,6 +27,9 @@ if install_tests install_subdir('test-network-generator-conversion', exclude_files : '.gitattributes', install_dir : testdata_dir) + install_subdir('testsuite-03.units', + exclude_files : '.gitattributes', + install_dir : testdata_dir) install_subdir('testsuite-04.units', exclude_files : '.gitattributes', install_dir : testdata_dir) diff --git a/test/units/hello-after-sleep.target b/test/testsuite-03.units/hello-after-sleep.target similarity index 100% rename from test/units/hello-after-sleep.target rename to test/testsuite-03.units/hello-after-sleep.target diff --git a/test/units/hello.service b/test/testsuite-03.units/hello.service similarity index 100% rename from test/units/hello.service rename to test/testsuite-03.units/hello.service diff --git a/test/units/sleep.service b/test/testsuite-03.units/sleep.service similarity index 100% rename from test/units/sleep.service rename to test/testsuite-03.units/sleep.service diff --git a/test/units/unstoppable.service b/test/testsuite-03.units/unstoppable.service similarity index 100% rename from test/units/unstoppable.service rename to test/testsuite-03.units/unstoppable.service From dc06321fe3e44915b732248bbf93d01b2bb7819a Mon Sep 17 00:00:00 2001 From: Andrew Stone Date: Mon, 22 Aug 2022 15:03:42 -0700 Subject: [PATCH 2/2] job: Don't discard propagated restart jobs when unit is activating When a service unit Requires= a socket, and the socket is restarted while the service is in state=activating, the propagated restart is being discarded. This is contrary to the documentation for Requires=, which states "this unit will be stopped (or restarted) if one of the other units is explicitly stopped (or restarted)". --- src/core/job.c | 24 ++++++++++++++++--- .../always-activating.service | 8 +++++++ .../always-activating.socket | 5 ++++ test/units/testsuite-03.sh | 9 +++++++ 4 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 test/testsuite-03.units/always-activating.service create mode 100644 test/testsuite-03.units/always-activating.socket diff --git a/src/core/job.c b/src/core/job.c index 6653dbde84..9a75abb9e9 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -400,8 +400,22 @@ bool job_type_is_redundant(JobType a, UnitActiveState b) { b == UNIT_RELOADING; case JOB_RESTART: - return - b == UNIT_ACTIVATING; + /* Restart jobs must always be kept. + * + * For ACTIVE/RELOADING units, this is obvious. + * + * For ACTIVATING units, it's more subtle: + * + * Generally, if a service Requires= another unit, restarts of + * the unit must be propagated to the service. If the service is + * ACTIVATING, it must still be restarted since it might have + * stale information regarding the other unit. + * + * For example, consider a service that Requires= a socket: if + * the socket is restarted, but the service is still ACTIVATING, + * it's necessary to restart the service so that it gets the new + * socket. */ + return false; case JOB_NOP: return true; @@ -417,8 +431,12 @@ JobType job_type_collapse(JobType t, Unit *u) { switch (t) { case JOB_TRY_RESTART: + /* Be sure to keep the restart job even if the unit is + * ACTIVATING. + * + * See the job_type_is_redundant(JOB_RESTART) for more info */ s = unit_active_state(u); - if (!UNIT_IS_ACTIVE_OR_RELOADING(s)) + if (!UNIT_IS_ACTIVE_OR_ACTIVATING(s)) return JOB_NOP; return JOB_RESTART; diff --git a/test/testsuite-03.units/always-activating.service b/test/testsuite-03.units/always-activating.service new file mode 100644 index 0000000000..93ddb85c75 --- /dev/null +++ b/test/testsuite-03.units/always-activating.service @@ -0,0 +1,8 @@ +[Unit] +Description=Service that never leaves state ACTIVATING +Requires=always-activating.socket +After=always-activating.socket + +[Service] +Type=notify +ExecStart=bash -c 'sleep infinity' diff --git a/test/testsuite-03.units/always-activating.socket b/test/testsuite-03.units/always-activating.socket new file mode 100644 index 0000000000..c76fed2ca8 --- /dev/null +++ b/test/testsuite-03.units/always-activating.socket @@ -0,0 +1,5 @@ +[Unit] +Description=Socket that activates always-activating.service + +[Socket] +ListenStream=/run/test-03-always-activating.sock diff --git a/test/units/testsuite-03.sh b/test/units/testsuite-03.sh index 7c5a3b8f19..4b3b409760 100755 --- a/test/units/testsuite-03.sh +++ b/test/units/testsuite-03.sh @@ -50,6 +50,15 @@ systemctl stop hello.service sleep.service hello-after-sleep.target # TODO: add more job queueing/merging tests here. +# Test that restart propagates to activating units +systemctl -T --no-block start always-activating.service +systemctl list-jobs | grep 'always-activating.service' +ACTIVATING_ID_PRE=$(systemctl show -P InvocationID always-activating.service) +systemctl -T start always-activating.socket # Wait for the socket to come up +systemctl -T restart always-activating.socket +ACTIVATING_ID_POST=$(systemctl show -P InvocationID always-activating.service) +[ "$ACTIVATING_ID_PRE" != "$ACTIVATING_ID_POST" ] || exit 1 + # Test for irreversible jobs systemctl start unstoppable.service