diff options
| -rw-r--r-- | NEWS | 8 | ||||
| -rw-r--r-- | man/systemd.exec.xml | 84 | ||||
| -rw-r--r-- | src/core/execute.c | 25 | ||||
| -rw-r--r-- | src/core/job.c | 12 | ||||
| -rw-r--r-- | src/core/job.h | 4 | ||||
| -rw-r--r-- | src/core/service.c | 148 | ||||
| -rw-r--r-- | src/core/unit-dependency-atom.c | 24 | ||||
| -rw-r--r-- | src/core/unit-dependency-atom.h | 35 | ||||
| -rw-r--r-- | src/core/unit.c | 118 | ||||
| -rw-r--r-- | src/core/unit.h | 3 | ||||
| -rw-r--r-- | src/test/test-engine.c | 39 | ||||
| -rwxr-xr-x | test/units/testsuite-68.sh | 253 |
12 files changed, 354 insertions, 399 deletions
@@ -21,6 +21,14 @@ CHANGES WITH 251: for breaking backward compatibility instead, as nobody should be affected, given the state of the current interface. + * Service monitor environment variables will only be passed to OnFailure=/OnSuccess= + handlers if exactly one unit lists the handler unit as OnFailure=/OnSuccess=. + Therefore, $MONITOR_METADATA is no longer used, and instead separate + variables are used: $MONITOR_SERVICE_RESULT, $MONITOR_EXIT_CODE, + $MONITOR_EXIT_STATUS, $MONITOR_INVOCATION_ID and $MONITOR_UNIT. + For cases when a single handler needs to watch multiple units, use a + templated handler. + * Services with Restart=always and a failing ExecCondition= will no longer be restarted, to bring ExecCondition= in line with Condition*= settings. diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 3b57f8d2f1..defca12d1b 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -3404,7 +3404,7 @@ StandardInputData=SWNrIHNpdHplIGRhIHVuJyBlc3NlIEtsb3BzLAp1ZmYgZWVtYWwga2xvcHAncy <varlistentry> <term><varname>$SERVICE_RESULT</varname></term> - <listitem><para>Only defined for the service unit type, this environment variable is passed to all + <listitem><para>Only used for the service unit type. This environment variable is passed to all <varname>ExecStop=</varname> and <varname>ExecStopPost=</varname> processes, and encodes the service "result". Currently, the following values are defined:</para> @@ -3472,7 +3472,7 @@ StandardInputData=SWNrIHNpdHplIGRhIHVuJyBlc3NlIEtsb3BzLAp1ZmYgZWVtYWwga2xvcHAncy <term><varname>$EXIT_CODE</varname></term> <term><varname>$EXIT_STATUS</varname></term> - <listitem><para>Only defined for the service unit type, these environment variables are passed to all + <listitem><para>Only defined for the service unit type. These environment variables are passed to all <varname>ExecStop=</varname>, <varname>ExecStopPost=</varname> processes and contain exit status/code information of the main process of the service. For the precise definition of the exit code and status, see <citerefentry><refentrytitle>wait</refentrytitle><manvolnum>2</manvolnum></citerefentry>. <varname>$EXIT_CODE</varname> @@ -3585,38 +3585,28 @@ StandardInputData=SWNrIHNpdHplIGRhIHVuJyBlc3NlIEtsb3BzLAp1ZmYgZWVtYWwga2xvcHAncy </varlistentry> <varlistentry> - <term><varname>$MONITOR_METADATA</varname></term> - - <listitem><para>Only defined for the service unit type, this environment variable is passed to all - <varname>ExecStart=</varname> and <varname>ExecStartPre=</varname> processes which run in services - triggered by <varname>OnFailure=</varname> or <varname>OnSuccess=</varname> dependencies.</para> - - <para> - The contents of this variable consists of a semi-colon separated list of metadata fields associated with the triggering - service. For each service which triggered the <varname>OnFailure=</varname> or <varname>OnSuccess=</varname> - dependency the following fields will be set: + <term><varname>$MONITOR_SERVICE_RESULT</varname></term> + <term><varname>$MONITOR_EXIT_CODE</varname></term> + <term><varname>$MONITOR_EXIT_STATUS</varname></term> + <term><varname>$MONITOR_INVOCATION_ID</varname></term> + <term><varname>$MONITOR_UNIT</varname></term> + + <listitem><para>Only defined for the service unit type. Those environment variable are passed to + all <varname>ExecStart=</varname> and <varname>ExecStartPre=</varname> processes which run in + services triggered by <varname>OnFailure=</varname> or <varname>OnSuccess=</varname> dependencies. </para> - <itemizedlist> - <listitem><para><constant>SERVICE_RESULT</constant></para></listitem> - <listitem><para><constant>EXIT_CODE</constant></para></listitem> - <listitem><para><constant>EXIT_STATUS</constant></para></listitem> - <listitem><para><constant>INVOCATION_ID</constant></para></listitem> - <listitem><para><constant>UNIT</constant></para></listitem> - </itemizedlist> - - <para>The fields <constant>SERVICE_RESULT</constant>, <constant>EXIT_CODE</constant> and - <constant>EXIT_STATUS</constant> may take the same values that are allowed when set for - <varname>ExecStop=</varname> and <varname>ExecStopPost=</varname> processes. The fields - <constant>INVOCATION_ID</constant> and <constant>UNIT</constant> are the invocaton id and unit - name of the service which triggered the dependency. Each field is comma separated, i.e.</para> - - <programlisting> -SERVICE_RESULT=result-string,EXIT_CODE=exit-code,EXIT_STATUS=exit-status,INVOCATION_ID=invocation-id,UNIT=triggering.service - </programlisting> - - </listitem> - + <para>Variables <varname>$MONITOR_SERVICE_RESULT</varname>, <varname>$MONITOR_EXIT_CODE</varname> + and <varname>$MONITOR_EXIT_STATUS</varname> take the same values as for + <varname>ExecStop=</varname> and <varname>ExecStopPost=</varname> processes. Variables + <varname>$MONITOR_INVOCATION_ID</varname> and <varname>$MONITOR_UNIT</varname> are set to the + invocaton id and unit name of the service which triggered the dependency.</para> + + <para>Note that when multiple services trigger the same unit, those variables will be + <emphasis>not</emphasis> be passed. Consider using a template handler unit for that case instead: + <literal>OnFailure=<replaceable>handler</replaceable>@%n.service</literal> for non-templated units, + or <literal>OnFailure=<replaceable>handler</replaceable>@%p-%i.service</literal> for templated + units.</para></listitem> </varlistentry> <varlistentry> @@ -4060,7 +4050,7 @@ SERVICE_RESULT=result-string,EXIT_CODE=exit-code,EXIT_STATUS=exit-status,INVOCAT <title>Examples</title> <example> - <title><varname>$MONITOR_METADATA</varname> usage</title> + <title><varname>$MONITOR_<replaceable>*</replaceable></varname> usage</title> <para>A service <filename index="false">myfailer.service</filename> which can trigger an <varname>OnFailure=</varname> dependency.</para> @@ -4094,33 +4084,31 @@ ExecStart=/bin/mysecondprogram Description=Acts on service failing or succeeding [Service] -ExecStart=/bin/bash -c "echo $MONITOR_METADATA" +ExecStart=/bin/bash -c "echo $MONITOR_SERVICE_RESULT $MONITOR_EXIT_CODE $MONITOR_EXIT_STATUS $MONITOR_INVOCATION_ID $MONITOR_UNIT" </programlisting> <para>If <filename index="false">myfailer.service</filename> were to run and exit in failure, then <filename index="false">myhandler.service</filename> would be triggered and the - <varname>$MONITOR_METADATA</varname> variable would be set as follows:</para> + monitor variables would be set as follows:</para> <programlisting> -MONITOR_METADATA=SERVICE_RESULT=result-string,EXIT_CODE=exit-code,EXIT_STATUS=exit-status,INVOCATION_ID=invocation-id,UNIT=myfailer.service +MONITOR_SERVICE_RESULT=exit-code +MONITOR_EXIT_CODE=exited +MONITOR_EXIT_STATUS=1 +MONITOR_INVOCATION_ID=cc8fdc149b2b4ca698d4f259f4054236 +MONITOR_UNIT=myfailer.service </programlisting> <para>If <filename index="false">mysuccess.service</filename> were to run and exit in success, then <filename index="false">myhandler.service</filename> would be triggered and the - <varname>$MONITOR_METADATA</varname> variable would be set as follows:</para> - - <programlisting> -MONITOR_METADATA=SERVICE_RESULT=result-string,EXIT_CODE=exit-code,EXIT_STATUS=exit-status,INVOCATION_ID=invocation-id,UNIT=mysuccess.service - </programlisting> - - <para>If <filename index="false">myfailer.service</filename> and <filename index="false">mysuccess.service</filename> were to run and exit, - there is a chance that the triggered dependency start job might be merged. Thus only a single invocation of - <filename index="false">myhandler.service</filename> would be triggered. In this case the <varname>$MONITOR_METADATA</varname> variable - would be a list containing exit metadata for both of <filename index="false">myfailer.service</filename> - and <filename index="false">mysuccess.service</filename>.</para> + monitor variables would be set as follows:</para> <programlisting> -MONITOR_METADATA=SERVICE_RESULT=result-string,EXIT_CODE=exit-code,EXIT_STATUS=exit-status,INVOCATION_ID=invocation-id,UNIT=myfailer.service;SERVICE_RESULT=result-string,EXIT_CODE=exit-code,EXIT_STATUS=exit-status,INVOCATION_ID=invocation-id,UNIT=mysuccess.service +MONITOR_SERVICE_RESULT=success +MONITOR_EXIT_CODE=exited +MONITOR_EXIT_STATUS=0 +MONITOR_INVOCATION_ID=6ab9af147b8c4a3ebe36e7a5f8611697 +MONITOR_UNIT=mysuccess.service </programlisting> </example> diff --git a/src/core/execute.c b/src/core/execute.c index c731dec0bd..027e5473b6 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -4459,12 +4459,9 @@ static int exec_child( return log_oom(); } - /* The PATH variable is set to the default path in params->environment. - * However, this is overridden if user specified fields have PATH set. - * The intention is to also override PATH if the user does - * not specify PATH and the user has specified ExecSearchPath - */ - + /* The $PATH variable is set to the default path in params->environment. However, this is overridden + * if user-specified fields have $PATH set. The intention is to also override $PATH if the unit does + * not specify PATH but the unit has ExecSearchPath. */ if (!strv_isempty(context->exec_search_path)) { _cleanup_free_ char *joined = NULL; @@ -4501,22 +4498,26 @@ static int exec_child( return log_unit_error_errno(unit, r, "Failed to set up kernel keyring: %m"); } - /* We need sandboxing if the caller asked us to apply it and the command isn't explicitly excepted from it */ + /* We need sandboxing if the caller asked us to apply it and the command isn't explicitly excepted + * from it. */ needs_sandboxing = (params->flags & EXEC_APPLY_SANDBOXING) && !(command->flags & EXEC_COMMAND_FULLY_PRIVILEGED); - /* We need the ambient capability hack, if the caller asked us to apply it and the command is marked for it, and the kernel doesn't actually support ambient caps */ + /* We need the ambient capability hack, if the caller asked us to apply it and the command is marked + * for it, and the kernel doesn't actually support ambient caps. */ needs_ambient_hack = (params->flags & EXEC_APPLY_SANDBOXING) && (command->flags & EXEC_COMMAND_AMBIENT_MAGIC) && !ambient_capabilities_supported(); - /* We need setresuid() if the caller asked us to apply sandboxing and the command isn't explicitly excepted from either whole sandboxing or just setresuid() itself, and the ambient hack is not desired */ + /* We need setresuid() if the caller asked us to apply sandboxing and the command isn't explicitly + * excepted from either whole sandboxing or just setresuid() itself, and the ambient hack is not + * desired. */ if (needs_ambient_hack) needs_setuid = false; else needs_setuid = (params->flags & EXEC_APPLY_SANDBOXING) && !(command->flags & (EXEC_COMMAND_FULLY_PRIVILEGED|EXEC_COMMAND_NO_SETUID)); if (needs_sandboxing) { - /* MAC enablement checks need to be done before a new mount ns is created, as they rely on /sys being - * present. The actual MAC context application will happen later, as late as possible, to avoid - * impacting our own code paths. */ + /* MAC enablement checks need to be done before a new mount ns is created, as they rely on + * /sys being present. The actual MAC context application will happen later, as late as + * possible, to avoid impacting our own code paths. */ #if HAVE_SELINUX use_selinux = mac_selinux_use(); diff --git a/src/core/job.c b/src/core/job.c index f28821071b..94ab381626 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -99,9 +99,6 @@ Job* job_free(Job *j) { assert(!j->subject_list); assert(!j->object_list); - while (!LIST_IS_EMPTY(j->triggered_by)) - LIST_POP(triggered_by, j->triggered_by); - job_unlink(j); sd_bus_track_unref(j->bus_track); @@ -110,13 +107,6 @@ Job* job_free(Job *j) { return mfree(j); } -void job_add_triggering_unit(Job *j, Unit *u) { - assert(j); - assert(u); - - LIST_APPEND(triggered_by, j->triggered_by, u); -} - static void job_set_state(Job *j, JobState state) { assert(j); assert(state >= 0); @@ -197,8 +187,6 @@ static void job_merge_into_installed(Job *j, Job *other) { j->irreversible = j->irreversible || other->irreversible; j->ignore_order = j->ignore_order || other->ignore_order; - if (other->triggered_by) - LIST_JOIN(triggered_by, j->triggered_by, other->triggered_by); } Job* job_install(Job *j) { diff --git a/src/core/job.h b/src/core/job.h index 762b0bb19b..a66e5985b8 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -124,8 +124,6 @@ struct Job { LIST_HEAD(JobDependency, subject_list); LIST_HEAD(JobDependency, object_list); - LIST_HEAD(Unit, triggered_by); - /* Used for graph algs as a "I have been here" marker */ Job* marker; unsigned generation; @@ -246,5 +244,3 @@ JobResult job_result_from_string(const char *s) _pure_; const char* job_type_to_access_method(JobType t); int job_compare(Job *a, Job *b, UnitDependencyAtom assume_dep); - -void job_add_triggering_unit(Job *j, Unit *u); diff --git a/src/core/service.c b/src/core/service.c index 92af448ff4..acf3df77da 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -43,6 +43,8 @@ #include "utf8.h" #include "util.h" +#define service_spawn(...) service_spawn_internal(__func__, __VA_ARGS__) + static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = { [SERVICE_DEAD] = UNIT_INACTIVE, [SERVICE_CONDITION] = UNIT_ACTIVATING, @@ -1447,84 +1449,40 @@ static bool service_exec_needs_notify_socket(Service *s, ExecFlags flags) { return s->notify_access != NOTIFY_NONE; } -static int service_create_monitor_md_env(Job *j, char **ret) { - _cleanup_free_ char *var = NULL; - const char *list_delim = ";"; - bool first = true; - Unit *tu; +static Service *service_get_triggering_service(Service *s) { + Unit *candidate = NULL, *other; - assert(j); - assert(ret); + assert(s); - /* Create an environment variable 'MONITOR_METADATA', if creation is successful - * a pointer to it is returned via ret. - * - * This variable contains a space separated set of fields which relate to - * the service(s) which triggered job 'j'. Job 'j' is the JOB_START job for - * an OnFailure= or OnSuccess= dependency. Format of the MONITOR_METADATA - * variable is as follows: - * - * MONITOR_METADATA="SERVICE_RESULT=<result-string0>,EXIT_CODE=<exit-code0>,EXIT_STATUS=<exit-status0>, - * INVOCATION_ID=<id>,UNIT=<triggering-unit0.service>; - * SERVICE_RESULT=<result-stringN>,EXIT_CODE=<exit-codeN>,EXIT_STATUS=<exit-statusN>, - * INVOCATION_ID=<id>,UNIT=<triggering-unitN.service>" - * - * Multiple results may be passed as in the above example if jobs are merged, i.e. - * some services a and b contain an OnFailure= or OnSuccess= dependency on the same - * service. - * - * For example: + /* Return the service which triggered service 's', this means dependency + * types which include the UNIT_ATOM_ON_{FAILURE,SUCCESS}_OF atoms. * - * MONITOR_METADATA="SERVICE_RESULT=exit-code,EXIT_CODE=exited,EXIT_STATUS=1,INVOCATION_ID=02dd868af2f344b18edaf74b618b2f90,UNIT=failure.service; - * SERVICE_RESULT=exit-code,EXIT_CODE=exited,EXIT_STATUS=1,INVOCATION_ID=80cb228bd7344f77a090eda603a3cfe2,UNIT=failure2.service" - */ - - LIST_FOREACH(triggered_by, tu, j->triggered_by) { - Service *env_source = SERVICE(tu); - int r; - - if (!env_source) - continue; - - /* Add the environment variable name first. */ - if (first && !strextend(&var, "MONITOR_METADATA=")) - return -ENOMEM; - - if (!strextend(&var, !first ? list_delim : "", "SERVICE_RESULT=", service_result_to_string(env_source->result))) - return -ENOMEM; - - first = false; - - if (env_source->main_exec_status.pid > 0 && - dual_timestamp_is_set(&env_source->main_exec_status.exit_timestamp)) { - if (!strextend(&var, ",EXIT_CODE=", sigchld_code_to_string(env_source->main_exec_status.code))) - return -ENOMEM; - - if (env_source->main_exec_status.code == CLD_EXITED) { - r = strextendf(&var, ",EXIT_STATUS=%i", - env_source->main_exec_status.status); - if (r < 0) - return r; - } else if (!strextend(&var, ",EXIT_STATUS=", signal_to_string(env_source->main_exec_status.status))) - return -ENOMEM; - } - - if (!sd_id128_is_null(UNIT(env_source)->invocation_id)) { - r = strextendf(&var, ",INVOCATION_ID=" SD_ID128_FORMAT_STR, - SD_ID128_FORMAT_VAL(UNIT(env_source)->invocation_id)); - if (r < 0) - return r; - } + * N.B. if there are multiple services which could trigger 's' via OnFailure= + * or OnSuccess= then we return NULL. This is since we don't know from which + * one to propagate the exit status. */ + + UNIT_FOREACH_DEPENDENCY(other, UNIT(s), UNIT_ATOM_ON_FAILURE_OF) { + if (candidate) + goto have_other; + candidate = other; + } - if (!strextend(&var, ",UNIT=", UNIT(env_source)->id)) - return -ENOMEM; + UNIT_FOREACH_DEPENDENCY(other, UNIT(s), UNIT_ATOM_ON_SUCCESS_OF) { + if (candidate) + goto have_other; + candidate = other; } - *ret = TAKE_PTR(var); - return 0; + return SERVICE(candidate); + + have_other: + log_unit_warning(UNIT(s), "multiple trigger source candidates for exit status propagation (%s, %s), skipping.", + candidate->id, other->id); + return NULL; } -static int service_spawn( +static int service_spawn_internal( + const char *caller, Service *s, ExecCommand *c, usec_t timeout, @@ -1544,10 +1502,13 @@ static int service_spawn( pid_t pid; int r; + assert(caller); assert(s); assert(c); assert(ret_pid); + log_unit_debug(UNIT(s), "Will spawn child (%s): %s", caller, c->path); + r = unit_prepare_exec(UNIT(s)); /* This realizes the cgroup, among other things */ if (r < 0) return r; @@ -1588,7 +1549,7 @@ static int service_spawn( if (r < 0) return r; - our_env = new0(char*, 10); + our_env = new0(char*, 12); if (!our_env) return -ENOMEM; @@ -1645,30 +1606,44 @@ static int service_spawn( } } + Service *env_source = NULL; + const char *monitor_prefix; if (flags & EXEC_SETENV_RESULT) { - if (asprintf(our_env + n_env++, "SERVICE_RESULT=%s", service_result_to_string(s->result)) < 0) + env_source = s; + monitor_prefix = ""; + } else if (flags & EXEC_SETENV_MONITOR_RESULT) { + env_source = service_get_triggering_service(s); + monitor_prefix = "MONITOR_"; + } + + if (env_source) { + if (asprintf(our_env + n_env++, "%sSERVICE_RESULT=%s", monitor_prefix, service_result_to_string(env_source->result)) < 0) return -ENOMEM; - if (s->main_exec_status.pid > 0 && - dual_timestamp_is_set(&s->main_exec_status.exit_timestamp)) { - if (asprintf(our_env + n_env++, "EXIT_CODE=%s", sigchld_code_to_string(s->main_exec_status.code)) < 0) + if (env_source->main_exec_status.pid > 0 && + dual_timestamp_is_set(&env_source->main_exec_status.exit_timestamp)) { + if (asprintf(our_env + n_env++, "%sEXIT_CODE=%s", monitor_prefix, sigchld_code_to_string(env_source->main_exec_status.code)) < 0) return -ENOMEM; - if (s->main_exec_status.code == CLD_EXITED) - r = asprintf(our_env + n_env++, "EXIT_STATUS=%i", s->main_exec_status.status); + if (env_source->main_exec_status.code == CLD_EXITED) + r = asprintf(our_env + n_env++, "%sEXIT_STATUS=%i", monitor_prefix, env_source->main_exec_status.status); else - r = asprintf(our_env + n_env++, "EXIT_STATUS=%s", signal_to_string(s->main_exec_status.status)); + r = asprintf(our_env + n_env++, "%sEXIT_STATUS=%s", monitor_prefix, signal_to_string(env_source->main_exec_status.status)); if (r < 0) return -ENOMEM; } - } else if (flags & EXEC_SETENV_MONITOR_RESULT) { - Job *j = UNIT(s)->job; - if (j) { - r = service_create_monitor_md_env(j, our_env + n_env++); - if (r < 0) - return r; + if (env_source != s) { + if (!sd_id128_is_null(UNIT(env_source)->invocation_id)) { + r = asprintf(our_env + n_env++, "%sINVOCATION_ID=" SD_ID128_FORMAT_STR, + monitor_prefix, SD_ID128_FORMAT_VAL(UNIT(env_source)->invocation_id)); + if (r < 0) + return -ENOMEM; + } + + if (asprintf(our_env + n_env++, "%sUNIT=%s", monitor_prefix, UNIT(env_source)->id) < 0) + return -ENOMEM; } } @@ -2484,6 +2459,7 @@ static void service_run_next_control(Service *s) { EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_IS_CONTROL| (IN_SET(s->control_command_id, SERVICE_EXEC_CONDITION, SERVICE_EXEC_START_PRE, SERVICE_EXEC_STOP_POST) ? EXEC_APPLY_TTY_STDIN : 0)| (IN_SET(s->control_command_id, SERVICE_EXEC_STOP, SERVICE_EXEC_STOP_POST) ? EXEC_SETENV_RESULT : 0)| + (IN_SET(s->control_command_id, SERVICE_EXEC_START_PRE, SERVICE_EXEC_START) ? EXEC_SETENV_MONITOR_RESULT : 0)| (IN_SET(s->control_command_id, SERVICE_EXEC_START_POST, SERVICE_EXEC_RELOAD, SERVICE_EXEC_STOP, SERVICE_EXEC_STOP_POST) ? EXEC_CONTROL_CGROUP : 0), &s->control_pid); if (r < 0) @@ -2520,7 +2496,7 @@ static void service_run_next_main(Service *s) { r = service_spawn(s, s->main_command, s->timeout_start_usec, - EXEC_PASS_FDS|EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_SET_WATCHDOG, + EXEC_PASS_FDS|EXEC_APPLY_SANDBOXING|EXEC_APPLY_CHROOT|EXEC_APPLY_TTY_STDIN|EXEC_SET_WATCHDOG|EXEC_SETENV_MONITOR_RESULT, &pid); if (r < 0) goto fail; @@ -4671,7 +4647,7 @@ static const char* const service_type_table[_SERVICE_TYPE_MAX] = { DEFINE_STRING_TABLE_LOOKUP(service_type, ServiceType); static const char* const service_exit_type_table[_SERVICE_EXIT_TYPE_MAX] = { - [SERVICE_EXIT_MAIN] = "main", + [SERVICE_EXIT_MAIN] = "main", [SERVICE_EXIT_CGROUP] = "cgroup", }; diff --git a/src/core/unit-dependency-atom.c b/src/core/unit-dependency-atom.c index 333eea6c3d..81333523e7 100644 --- a/src/core/unit-dependency-atom.c +++ b/src/core/unit-dependency-atom.c @@ -82,13 +82,11 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { [UNIT_PROPAGATES_STOP_TO] = UNIT_ATOM_RETROACTIVE_STOP_ON_STOP | UNIT_ATOM_PROPAGATE_STOP, - [UNIT_ON_FAILURE] = UNIT_ATOM_ON_FAILURE | - UNIT_ATOM_BACK_REFERENCE_IMPLIED, - - [UNIT_ON_SUCCESS] = UNIT_ATOM_ON_SUCCESS | - UNIT_ATOM_BACK_REFERENCE_IMPLIED, - /* These are simple dependency types: they consist of a single atom only */ + [UNIT_ON_FAILURE] = UNIT_ATOM_ON_FAILURE, + [UNIT_ON_SUCCESS] = UNIT_ATOM_ON_SUCCESS, + [UNIT_ON_FAILURE_OF] = UNIT_ATOM_ON_FAILURE_OF, + [UNIT_ON_SUCCESS_OF] = UNIT_ATOM_ON_SUCCESS_OF, [UNIT_BEFORE] = UNIT_ATOM_BEFORE, [UNIT_AFTER] = UNIT_ATOM_AFTER, [UNIT_TRIGGERS] = UNIT_ATOM_TRIGGERS, @@ -104,8 +102,6 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = { * things discoverable/debuggable as they are the inverse dependencies to some of the above. As they * have no effect of their own, they all map to no atoms at all, i.e. the value 0. */ [UNIT_RELOAD_PROPAGATED_FROM] = 0, - [UNIT_ON_SUCCESS_OF] = 0, - [UNIT_ON_FAILURE_OF] = 0, [UNIT_STOP_PROPAGATED_FROM] = 0, }; @@ -200,17 +196,19 @@ UnitDependency unit_dependency_from_unique_atom(UnitDependencyAtom atom) { case UNIT_ATOM_PROPAGATE_STOP_FAILURE: return UNIT_CONFLICTED_BY; - case UNIT_ATOM_ON_FAILURE | - UNIT_ATOM_BACK_REFERENCE_IMPLIED: + /* And now, the simple ones */ + case UNIT_ATOM_ON_FAILURE: return UNIT_ON_FAILURE; - case UNIT_ATOM_ON_SUCCESS | - UNIT_ATOM_BACK_REFERENCE_IMPLIED: case UNIT_ATOM_ON_SUCCESS: return UNIT_ON_SUCCESS; - /* And now, the simple ones */ + case UNIT_ATOM_ON_SUCCESS_OF: + return UNIT_ON_SUCCESS_OF; + + case UNIT_ATOM_ON_FAILURE_OF: + return UNIT_ON_FAILURE_OF; case UNIT_ATOM_BEFORE: return UNIT_BEFORE; diff --git a/src/core/unit-dependency-atom.h b/src/core/unit-dependency-atom.h index 1cf9795786..02532e57d6 100644 --- a/src/core/unit-dependency-atom.h +++ b/src/core/unit-dependency-atom.h @@ -66,27 +66,22 @@ typedef enum UnitDependencyAtom { /* Recheck default target deps on other units (which are target units) */ UNIT_ATOM_DEFAULT_TARGET_DEPENDENCIES = UINT64_C(1) << 21, - /* Dependencies which include this atom automatically get a reverse - * REFERENCES/REFERENCED_BY dependency. */ - UNIT_ATOM_BACK_REFERENCE_IMPLIED = UINT64_C(1) << 22, - - /* Trigger a dependency on successful service exit. */ - UNIT_ATOM_ON_SUCCESS = UINT64_C(1) << 23, - /* Trigger a dependency on unsuccessful service exit. */ - UNIT_ATOM_ON_FAILURE = UINT64_C(1) << 24, - /* The remaining atoms map 1:1 to the equally named high-level deps */ - UNIT_ATOM_BEFORE = UINT64_C(1) << 25, - UNIT_ATOM_AFTER = UINT64_C(1) << 26, - UNIT_ATOM_TRIGGERS = UINT64_C(1) << 27, - UNIT_ATOM_TRIGGERED_BY = UINT64_C(1) << 28, - UNIT_ATOM_PROPAGATES_RELOAD_TO = UINT64_C(1) << 29, - UNIT_ATOM_JOINS_NAMESPACE_OF = UINT64_C(1) << 30, - UNIT_ATOM_REFERENCES = UINT64_C(1) << 31, - UNIT_ATOM_REFERENCED_BY = UINT64_C(1) << 32, - UNIT_ATOM_IN_SLICE = UINT64_C(1) << 33, - UNIT_ATOM_SLICE_OF = UINT64_C(1) << 34, - _UNIT_DEPENDENCY_ATOM_MAX = (UINT64_C(1) << 35) - 1, + UNIT_ATOM_ON_FAILURE = UINT64_C(1) << 22, + UNIT_ATOM_ON_SUCCESS = UINT64_C(1) << 23, + UNIT_ATOM_ON_FAILURE_OF = UINT64_C(1) << 24, + UNIT_ATOM_ON_SUCCESS_OF = UINT64_C(1) << 25, + UNIT_ATOM_BEFORE = UINT64_C(1) << 26, + UNIT_ATOM_AFTER = UINT64_C(1) << 27, + UNIT_ATOM_TRIGGERS = UINT64_C(1) << 28, + UNIT_ATOM_TRIGGERED_BY = UINT64_C(1) << 29, + UNIT_ATOM_PROPAGATES_RELOAD_TO = UINT64_C(1) << 30, + UNIT_ATOM_JOINS_NAMESPACE_OF = UINT64_C(1) << 31, + UNIT_ATOM_REFERENCES = UINT64_C(1) << 32, + UNIT_ATOM_REFERENCED_BY = UINT64_C(1) << 33, + UNIT_ATOM_IN_SLICE = UINT64_C(1) << 34, + UNIT_ATOM_SLICE_OF = UINT64_C(1) << 35, + _UNIT_DEPENDENCY_ATOM_MAX = (UINT64_C(1) << 36) - 1, _UNIT_DEPENDENCY_ATOM_INVALID = -EINVAL, } UnitDependencyAtom; diff --git a/src/core/unit.c b/src/core/unit.c index 2cddc924f3..bd11049c77 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -375,6 +375,20 @@ int unit_set_description(Unit *u, const char *description) { return 0; } +static bool unit_success_failure_handler_has_jobs(Unit *unit) { + Unit *other; + + UNIT_FOREACH_DEPENDENCY(other, unit, UNIT_ATOM_ON_SUCCESS) + if (other->job || other->nop_job) + return true; + + UNIT_FOREACH_DEPENDENCY(other, unit, UNIT_ATOM_ON_FAILURE) + if (other->job || other->nop_job) + return true; + + return false; +} + bool unit_may_gc(Unit *u) { UnitActiveState state; int r; @@ -389,10 +403,7 @@ bool unit_may_gc(Unit *u) { * in unit_gc_sweep(), but using markers to properly collect dependency loops. */ - if (u->job) - return false; - - if (u->nop_job) + if (u->job || u->nop_job) return false; state = unit_active_state(u); @@ -427,6 +438,10 @@ bool unit_may_gc(Unit *u) { assert_not_reached(); } + /* Check if any OnFailure= or on Success= jobs may be pending */ + if (unit_success_failure_handler_has_jobs(u)) + return false; + if (u->cgroup_path) { /* If the unit has a cgroup, then check whether there's anything in it. If so, we should stay * around. Units with active processes should never be collected. */ @@ -2210,7 +2225,7 @@ void unit_start_on_failure( UnitDependencyAtom atom, JobMode job_mode) { - bool logged = false; + int n_jobs = -1; Unit *other; int r; @@ -2222,28 +2237,23 @@ void unit_start_on_failure( UNIT_FOREACH_DEPENDENCY(other, u, atom) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - Job *job = NULL; - if (!logged) { + if (n_jobs < 0) { log_unit_info(u, "Triggering %s dependencies.", dependency_name); - logged = true; + n_jobs = 0; } - r = manager_add_job(u->manager, JOB_START, other, job_mode, NULL, &error, &job); + r = manager_add_job(u->manager, JOB_START, other, job_mode, NULL, &error, NULL); if (r < 0) log_unit_warning_errno( u, r, "Failed to enqueue %s job, ignoring: %s", dependency_name, bus_error_message(&error, r)); - else if (job) - /* u will be kept pinned since both UNIT_ON_FAILURE and UNIT_ON_SUCCESS includes - * UNIT_ATOM_BACK_REFERENCE_IMPLIED. We save the triggering unit here since we - * want to be able to reference it when we come to run the OnFailure= or OnSuccess= - * dependency. */ - job_add_triggering_unit(job, u); + n_jobs ++; } - if (logged) - log_unit_debug(u, "Triggering %s dependencies done.", dependency_name); + if (n_jobs >= 0) + log_unit_debug(u, "Triggering %s dependencies done (%u %s).", + dependency_name, n_jobs, n_jobs == 1 ? "job" : "jobs"); } void unit_trigger_notify(Unit *u) { @@ -3016,37 +3026,37 @@ int unit_add_dependency( UnitDependencyMask mask) { static const UnitDependency inverse_table[_UNIT_DEPENDENCY_MAX] = { - [UNIT_REQUIRES] = UNIT_REQUIRED_BY, - [UNIT_REQUISITE] = UNIT_REQUISITE_OF, - [UNIT_WANTS] = UNIT_WANTED_BY, - [UNIT_BINDS_TO] = UNIT_BOUND_BY, - [UNIT_PART_OF] = UNIT_CONSISTS_OF, - [UNIT_UPHOLDS] = UNIT_UPHELD_BY, - [UNIT_REQUIRED_BY] = UNIT_REQUIRES, - [UNIT_REQUISITE_OF] = UNIT_REQUISITE, - [UNIT_WANTED_BY] = UNIT_WANTS, - [UNIT_BOUND_BY] = UNIT_BINDS_TO, - [UNIT_CONSISTS_OF] = UNIT_PART_OF, - [UNIT_UPHELD_BY] = UNIT_UPHOLDS, - [UNIT_CONFLICTS] = UNIT_CONFLICTED_BY, - [UNIT_CONFLICTED_BY] = UNIT_CONFLICTS, - [UNIT_BEFORE] = UNIT_AFTER, - [UNIT_AFTER] = UNIT_BEFORE, - [UNIT_ON_SUCCESS] = UNIT_ON_SUCCESS_OF, - [UNIT_ON_SUCCESS_OF] = UNIT_ON_SUCCESS, - [UNIT_ON_FAILURE] = UNIT_ON_FAILURE_OF, - [UNIT_ON_FAILURE_OF] = UNIT_ON_FAILURE, - [UNIT_TRIGGERS] = UNIT_TRIGGERED_BY, - [UNIT_TRIGGERED_BY] = UNIT_TRIGGERS, - [UNIT_PROPAGATES_RELOAD_TO] = UNIT_RELOAD_PROPAGATED_FROM, + [UNIT_REQUIRES] = UNIT_REQUIRED_BY, + [UNIT_REQUISITE] = UNIT_REQUISITE_OF, + [UNIT_WANTS] = UNIT_WANTED_BY, + [UNIT_BINDS_TO] = UNIT_BOUND_BY, + [UNIT_PART_OF] = UNIT_CONSISTS_OF, + [UNIT_UPHOLDS] = UNIT_UPHELD_BY, + [UNIT_REQUIRED_BY] = UNIT_REQUIRES, + [UNIT_REQUISITE_OF] = UNIT_REQUISITE, + [UNIT_WANTED_BY] = UNIT_WANTS, + [UNIT_BOUND_BY] = UNIT_BINDS_TO, + [UNIT_CONSISTS_OF] = UNIT_PART_OF, + [UNIT_UPHELD_BY] = UNIT_UPHOLDS, + [UNIT_CONFLICTS] = UNIT_CONFLICTED_BY, + [UNIT_CONFLICTED_BY] = UNIT_CONFLICTS, + [UNIT_BEFORE] = UNIT_AFTER, + [UNIT_AFTER] = UNIT_BEFORE, + [UNIT_ON_SUCCESS] = UNIT_ON_SUCCESS_OF, + [UNIT_ON_SUCCESS_OF] = UNIT_ON_SUCCESS, + [UNIT_ON_FAILURE] = UNIT_ON_FAILURE_OF, + [UNIT_ON_FAILURE_OF] = UNIT_ON_FAILURE, + [UNIT_TRIGGERS] = UNIT_TRIGGERED_BY, + [UNIT_TRIGGERED_BY] = UNIT_TRIGGERS, + [UNIT_PROPAGATES_RELOAD_TO] = UNIT_RELOAD_PROPAGATED_FROM, [UNIT_RELOAD_PROPAGATED_FROM] = UNIT_PROPAGATES_RELOAD_TO, - [UNIT_PROPAGATES_STOP_TO] = UNIT_STOP_PROPAGATED_FROM, - [UNIT_STOP_PROPAGATED_FROM] = UNIT_PROPAGATES_STOP_TO, - [UNIT_JOINS_NAMESPACE_OF] = UNIT_JOINS_NAMESPACE_OF, /* symmetric! 👓 */ - [UNIT_REFERENCES] = UNIT_REFERENCED_BY, - [UNIT_REFERENCED_BY] = UNIT_REFERENCES, - [UNIT_IN_SLICE] = UNIT_SLICE_OF, - [UNIT_SLICE_OF] = UNIT_IN_SLICE, + [UNIT_PROPAGATES_STOP_TO] = UNIT_STOP_PROPAGATED_FROM, + [UNIT_STOP_PROPAGATED_FROM] = UNIT_PROPAGATES_STOP_TO, + [UNIT_JOINS_NAMESPACE_OF] = UNIT_JOINS_NAMESPACE_OF, /* symmetric! 👓 */ + [UNIT_REFERENCES] = UNIT_REFERENCED_BY, + [UNIT_REFERENCED_BY] = UNIT_REFERENCES, + [UNIT_IN_SLICE] = UNIT_SLICE_OF, + [UNIT_SLICE_OF] = UNIT_IN_SLICE, }; Unit *original_u = u, *original_other = other; UnitDependencyAtom a; @@ -3121,20 +3131,6 @@ int unit_add_dependency( noop = false; } - if (FLAGS_SET(a, UNIT_ATOM_BACK_REFERENCE_IMPLIED)) { - r = unit_add_dependency_hashmap(&other->dependencies, UNIT_REFERENCES, u, 0, mask); - if (r < 0) - return r; - if (r) - noop = false; - - r = unit_add_dependency_hashmap(&u->dependencies, UNIT_REFERENCED_BY, other, 0, mask); - if (r < 0) - return r; - if (r) - noop = false; - } - if (add_reference) { r = unit_add_dependency_hashmap(&u->dependencies, UNIT_REFERENCES, other, mask, 0); if (r < 0) diff --git a/src/core/unit.h b/src/core/unit.h index 786c15d623..94f2180951 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -242,9 +242,6 @@ typedef struct Unit { /* Queue of units that have a BindTo= dependency on some other unit, and should possibly be shut down */ LIST_FIELDS(Unit, stop_when_bound_queue); - /* Queue of units which have triggered an OnFailure= or OnSuccess= dependency job. */ - LIST_FIELDS(Unit, triggered_by); - /* PIDs we keep an eye on. Note that a unit might have many * more, but these are the ones we care enough about to * process SIGCHLD for */ diff --git a/src/test/test-engine.c b/src/test/test-engine.c index 673c665612..cd01d1358b 100644 --- a/src/test/test-engine.c +++ b/src/test/test-engine.c @@ -192,17 +192,17 @@ int main(int argc, char *argv[]) { assert_se(unit_add_dependency(a, UNIT_PROPAGATES_RELOAD_TO, b, true, UNIT_DEPENDENCY_UDEV) == 0); assert_se(unit_add_dependency(a, UNIT_PROPAGATES_RELOAD_TO, c, true, UNIT_DEPENDENCY_PROC_SWAP) == 0); - assert_se(hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), b)); - assert_se(hashmap_get(unit_get_dependencies(b, UNIT_RELOAD_PROPAGATED_FROM), a)); - assert_se(hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), c)); - assert_se(hashmap_get(unit_get_dependencies(c, UNIT_RELOAD_PROPAGATED_FROM), a)); + assert_se( hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), b)); + assert_se( hashmap_get(unit_get_dependencies(b, UNIT_RELOAD_PROPAGATED_FROM), a)); + assert_se( hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), c)); + assert_se( hashmap_get(unit_get_dependencies(c, UNIT_RELOAD_PROPAGATED_FROM), a)); unit_remove_dependencies(a, UNIT_DEPENDENCY_UDEV); assert_se(!hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), b)); assert_se(!hashmap_get(unit_get_dependencies(b, UNIT_RELOAD_PROPAGATED_FROM), a)); - assert_se(hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), c)); - assert_se(hashmap_get(unit_get_dependencies(c, UNIT_RELOAD_PROPAGATED_FROM), a)); + assert_se( hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), c)); + assert_se( hashmap_get(unit_get_dependencies(c, UNIT_RELOAD_PROPAGATED_FROM), a)); unit_remove_dependencies(a, UNIT_DEPENDENCY_PROC_SWAP); @@ -223,6 +223,7 @@ int main(int argc, char *argv[]) { assert_se(unit_add_dependency_by_name(stub, UNIT_AFTER, SPECIAL_ROOT_SLICE, true, UNIT_DEPENDENCY_FILE) >= 0); assert_se(unit_add_dependency_by_name(stub, UNIT_REQUIRES, "non-existing.mount", true, UNIT_DEPENDENCY_FILE) >= 0); assert_se(unit_add_dependency_by_name(stub, UNIT_ON_FAILURE, "non-existing-on-failure.target", true, UNIT_DEPENDENCY_FILE) >= 0); + assert_se(unit_add_dependency_by_name(stub, UNIT_ON_SUCCESS, "non-existing-on-success.target", true, UNIT_DEPENDENCY_FILE) >= 0); log_info("/* Merging a+stub, dumps before */"); unit_dump(a, stderr, NULL); @@ -231,13 +232,19 @@ int main(int argc, char *argv[]) { log_info("/* Dump of merged a+stub */"); unit_dump(a, stderr, NULL); - assert_se(unit_has_dependency(a, UNIT_ATOM_AFTER, manager_get_unit(m, SPECIAL_BASIC_TARGET))); - assert_se(unit_has_dependency(a, UNIT_ATOM_AFTER, manager_get_unit(m, "quux.target"))); - assert_se(unit_has_dependency(a, UNIT_ATOM_AFTER, manager_get_unit(m, SPECIAL_ROOT_SLICE))); - assert_se(unit_has_dependency(a, UNIT_ATOM_PULL_IN_START, manager_get_unit(m, "non-existing.mount"))); - assert_se(unit_has_dependency(a, UNIT_ATOM_RETROACTIVE_START_REPLACE, manager_get_unit(m, "non-existing.mount"))); - assert_se(unit_has_dependency(a, UNIT_ATOM_ON_FAILURE, manager_get_unit(m, "non-existing-on-failure.target"))); + assert_se( unit_has_dependency(a, UNIT_ATOM_AFTER, manager_get_unit(m, SPECIAL_BASIC_TARGET))); + assert_se( unit_has_dependency(a, UNIT_ATOM_AFTER, manager_get_unit(m, "quux.target"))); + assert_se( unit_has_dependency(a, UNIT_ATOM_AFTER, manager_get_unit(m, SPECIAL_ROOT_SLICE))); + assert_se( unit_has_dependency(a, UNIT_ATOM_PULL_IN_START, manager_get_unit(m, "non-existing.mount"))); + assert_se( unit_has_dependency(a, UNIT_ATOM_RETROACTIVE_START_REPLACE, manager_get_unit(m, "non-existing.mount"))); + assert_se( unit_has_dependency(a, UNIT_ATOM_ON_FAILURE, manager_get_unit(m, "non-existing-on-failure.target"))); + assert_se( unit_has_dependency(manager_get_unit(m, "non-existing-on-failure.target"), UNIT_ATOM_ON_FAILURE_OF, a)); + assert_se( unit_has_dependency(a, UNIT_ATOM_ON_SUCCESS, manager_get_unit(m, "non-existing-on-success.target"))); + assert_se( unit_has_dependency(manager_get_unit(m, "non-existing-on-success.target"), UNIT_ATOM_ON_SUCCESS_OF, a)); assert_se(!unit_has_dependency(a, UNIT_ATOM_ON_FAILURE, manager_get_unit(m, "basic.target"))); + assert_se(!unit_has_dependency(a, UNIT_ATOM_ON_SUCCESS, manager_get_unit(m, "basic.target"))); + assert_se(!unit_has_dependency(a, UNIT_ATOM_ON_FAILURE_OF, manager_get_unit(m, "basic.target"))); + assert_se(!unit_has_dependency(a, UNIT_ATOM_ON_SUCCESS_OF, manager_get_unit(m, "basic.target"))); assert_se(!unit_has_dependency(a, UNIT_ATOM_PROPAGATES_RELOAD_TO, manager_get_unit(m, "non-existing-on-failure.target"))); assert_se(unit_has_name(a, "a.service")); @@ -275,19 +282,19 @@ int main(int argc, char *argv[]) { assert_se(UNIT_GET_SLICE(tomato) == zupa); assert_se(!unit_has_dependency(tomato, UNIT_ATOM_IN_SLICE, sauce)); assert_se(!unit_has_dependency(tomato, UNIT_ATOM_IN_SLICE, fruit)); - assert_se(unit_has_dependency(tomato, UNIT_ATOM_IN_SLICE, zupa)); + assert_se( unit_has_dependency(tomato, UNIT_ATOM_IN_SLICE, zupa)); assert_se(!unit_has_dependency(tomato, UNIT_ATOM_REFERENCES, sauce)); assert_se(!unit_has_dependency(tomato, UNIT_ATOM_REFERENCES, fruit)); - assert_se(unit_has_dependency(tomato, UNIT_ATOM_REFERENCES, zupa)); + assert_se( unit_has_dependency(tomato, UNIT_ATOM_REFERENCES, zupa)); assert_se(!unit_has_dependency(sauce, UNIT_ATOM_SLICE_OF, tomato)); assert_se(!unit_has_dependency(fruit, UNIT_ATOM_SLICE_OF, tomato)); - assert_se(unit_has_dependency(zupa, UNIT_ATOM_SLICE_OF, tomato)); + assert_se( unit_has_dependency(zupa, UNIT_ATOM_SLICE_OF, tomato)); assert_se(!unit_has_dependency(sauce, UNIT_ATOM_REFERENCED_BY, tomato)); assert_se(!unit_has_dependency(fruit, UNIT_ATOM_REFERENCED_BY, tomato)); - assert_se(unit_has_dependency(zupa, UNIT_ATOM_REFERENCED_BY, tomato)); + assert_se( unit_has_dependency(zupa, UNIT_ATOM_REFERENCED_BY, tomato)); return 0; } diff --git a/test/units/testsuite-68.sh b/test/units/testsuite-68.sh index 15e2e0353f..3072a94575 100755 --- a/test/units/testsuite-68.sh +++ b/test/units/testsuite-68.sh @@ -24,7 +24,6 @@ wait_on_state_or_fail () { } systemd-analyze log-level debug -systemd-analyze log-target console # Trigger testservice-failure-exit-handler-68.service cat >/run/systemd/system/testservice-failure-68.service <<EOF @@ -33,17 +32,16 @@ Description=TEST-68-PROPAGATE-EXIT-STATUS with OnFailure= trigger OnFailure=testservice-failure-exit-handler-68.service [Service] -ExecStart=/bin/bash -c "exit 1" +ExecStart=sh -c "exit 1" EOF -# Another service which triggers testservice-failure-exit-handler-68.service -cat >/run/systemd/system/testservice-failure-68-additional.service <<EOF +cat >/run/systemd/system/testservice-failure-68-template.service <<EOF [Unit] -Description=TEST-68-PROPAGATE-EXIT-STATUS Additional service with OnFailure= trigger -OnFailure=testservice-failure-exit-handler-68.service +Description=TEST-68-PROPAGATE-EXIT-STATUS with OnFailure= trigger (template) +OnFailure=testservice-failure-exit-handler-68-template@%n.service [Service] -ExecStart=/bin/bash -c "exit 1" +ExecStart=sh -c "exit 1" EOF # Trigger testservice-success-exit-handler-68.service @@ -53,72 +51,54 @@ Description=TEST-68-PROPAGATE-EXIT-STATUS with OnSuccess= trigger OnSuccess=testservice-success-exit-handler-68.service [Service] -ExecStart=/bin/bash -c "exit 0" +ExecStart=sh -c "exit 0" EOF -# Trigger testservice-success-exit-handler-68.service -cat >/run/systemd/system/testservice-success-68-additional.service <<EOF +cat >/run/systemd/system/testservice-success-68-template.service <<EOF [Unit] -Description=TEST-68-PROPAGATE-EXIT-STATUS Addition service with OnSuccess= trigger -OnSuccess=testservice-success-exit-handler-68.service +Description=TEST-68-PROPAGATE-EXIT-STATUS with OnSuccess= trigger (template) +OnSuccess=testservice-success-exit-handler-68-template@%n.service [Service] -ExecStart=/bin/bash -c "exit 0" +ExecStart=sh -c "exit 0" EOF # Script to check that when an OnSuccess= dependency fires, the correct -# MONITOR* env variables are passed. This script handles the case where -# multiple services triggered the unit that calls this script. In this -# case we need to check the MONITOR_METADATA variable for >= 1 service -# details since jobs may merge. -cat >/tmp/check_on_success.sh <<EOF -#!/usr/bin/env bash +# MONITOR* env variables are passed. +cat >/tmp/check_on_success.sh <<"EOF" +#!/bin/sh set -ex +env | sort +if [ "$MONITOR_SERVICE_RESULT" != "success" ]; then + echo "MONITOR_SERVICE_RESULT was '$MONITOR_SERVICE_RESULT', expected 'success'" + exit 1 +fi + +if [ "$MONITOR_EXIT_CODE" != "exited" ]; then + echo "MONITOR_EXIT_CODE was '$MONITOR_EXIT_CODE', expected 'exited'" + exit 1 +fi + +if [ "$MONITOR_EXIT_STATUS" != "0" ]; then + echo "MONITOR_EXIT_STATUS was '$MONITOR_EXIT_STATUS', expected '0'" + exit 1 +fi + +if [ -z "$MONITOR_INVOCATION_ID" ]; then + echo "MONITOR_INVOCATION_ID unset" + exit 1 +fi + +if [ "$MONITOR_UNIT" != "testservice-success-68.service" ] && + [ "$MONITOR_UNIT" != "testservice-success-68-template.service" ] && + [ "$MONITOR_UNIT" != "testservice-transient-success-68.service" ]; then + + echo "MONITOR_UNIT was '$MONITOR_UNIT', expected 'testservice[-transient]-success-68[-template].service'" + exit 1 +fi -echo "MONITOR_METADATA=\$MONITOR_METADATA" - -IFS=';' read -ra ALL_SERVICE_MD <<< "\$MONITOR_METADATA" -for SERVICE_MD in "\${ALL_SERVICE_MD[@]}"; do - IFS=',' read -ra METADATA <<< "\$SERVICE_MD" - IFS='=' read -ra SERVICE_RESULT <<< "\${METADATA[0]}" - SERVICE_RESULT=\${SERVICE_RESULT[1]} - IFS='=' read -ra EXIT_CODE <<< "\${METADATA[1]}" - EXIT_CODE=\${EXIT_CODE[1]} - IFS='=' read -ra EXIT_STATUS <<< "\${METADATA[2]}" - EXIT_STATUS=\${EXIT_STATUS[1]} - IFS='=' read -ra INVOCATION_ID <<< "\${METADATA[3]}" - INVOCATION_ID=\${INVOCATION_ID[1]} - IFS='=' read -ra UNIT <<< "\${METADATA[4]}" - UNIT=\${UNIT[1]} - - if [ "\$SERVICE_RESULT" != "success" ]; then - echo 'SERVICE_RESULT was "\$SERVICE_RESULT", expected "success"'; - exit 1; - fi - - if [ "\$EXIT_CODE" != "exited" ]; then - echo 'EXIT_CODE was "\$EXIT_CODE", expected "exited"'; - exit 1; - fi - - if [ "\$EXIT_STATUS" != "0" ]; then - echo 'EXIT_STATUS was "\$EXIT_STATUS", expected "0"'; - exit 1; - fi - - if [ -z "\$INVOCATION_ID" ]; then - echo 'INVOCATION_ID unset'; - exit 1; - fi - - if [[ "\$UNIT" != "testservice-success-68.service" && "\$UNIT" != "testservice-success-68-additional.service" && "\$UNIT" != "testservice-transient-success-68.service" ]]; then - echo 'UNIT was "\$UNIT", expected "testservice-success-68{-additional,-transient}.service"'; - exit 1; - fi -done - -exit 0; +exit 0 EOF chmod +x /tmp/check_on_success.sh @@ -132,59 +112,53 @@ ExecStartPre=/tmp/check_on_success.sh ExecStart=/tmp/check_on_success.sh EOF +# Template version. +cat >/run/systemd/system/testservice-success-exit-handler-68-template@.service <<EOF +[Unit] +Description=TEST-68-PROPAGATE-EXIT-STATUS handle service exiting in success (template) + +[Service] +ExecStartPre=echo "triggered by %i" +ExecStartPre=/tmp/check_on_success.sh +ExecStart=/tmp/check_on_success.sh +EOF + # Script to check that when an OnFailure= dependency fires, the correct -# MONITOR* env variables are passed. This script handles the case where -# multiple services triggered the unit that calls this script. In this -# case we need to check the MONITOR_METADATA variable for >=1 service -# details since jobs may merge. -cat >/tmp/check_on_failure.sh <<EOF -#!/usr/bin/env bash +# MONITOR* env variables are passed. +cat >/tmp/check_on_failure.sh <<"EOF" +#!/bin/sh set -ex +env | sort +if [ "$MONITOR_SERVICE_RESULT" != "exit-code" ]; then + echo "MONITOR_SERVICE_RESULT was '$MONITOR_SERVICE_RESULT', expected 'exit-code'" + exit 1 +fi + +if [ "$MONITOR_EXIT_CODE" != "exited" ]; then + echo "MONITOR_EXIT_CODE was '$MONITOR_EXIT_CODE', expected 'exited'" + exit 1 +fi + +if [ "$MONITOR_EXIT_STATUS" != "1" ]; then + echo "MONITOR_EXIT_STATUS was '$MONITOR_EXIT_STATUS', expected '1'" + exit 1 +fi + +if [ -z "$MONITOR_INVOCATION_ID" ]; then + echo "MONITOR_INVOCATION_ID unset" + exit 1 +fi + +if [ "$MONITOR_UNIT" != "testservice-failure-68.service" ] && + [ "$MONITOR_UNIT" != "testservice-failure-68-template.service" ] && + [ "$MONITOR_UNIT" != "testservice-transient-failure-68.service" ]; then + + echo "MONITOR_UNIT was '$MONITOR_UNIT', expected 'testservice[-transient]-failure-68[-template].service'" + exit 1 +fi -echo "MONITOR_METADATA=\$MONITOR_METADATA" - -IFS=';' read -ra ALL_SERVICE_MD <<< "\$MONITOR_METADATA" -for SERVICE_MD in "\${ALL_SERVICE_MD[@]}"; do - IFS=',' read -ra METADATA <<< "\$SERVICE_MD" - IFS='=' read -ra SERVICE_RESULT <<< "\${METADATA[0]}" - SERVICE_RESULT=\${SERVICE_RESULT[1]} - IFS='=' read -ra EXIT_CODE <<< "\${METADATA[1]}" - EXIT_CODE=\${EXIT_CODE[1]} - IFS='=' read -ra EXIT_STATUS <<< "\${METADATA[2]}" - EXIT_STATUS=\${EXIT_STATUS[1]} - IFS='=' read -ra INVOCATION_ID <<< "\${METADATA[3]}" - INVOCATION_ID=\${INVOCATION_ID[1]} - IFS='=' read -ra UNIT <<< "\${METADATA[4]}" - UNIT=\${UNIT[1]} - - if [ "\$SERVICE_RESULT" != "exit-code" ]; then - echo 'SERVICE_RESULT was "\$SERVICE_RESULT", expected "exit-code"'; - exit 1; - fi - - if [ "\$EXIT_CODE" != "exited" ]; then - echo 'EXIT_CODE was "\$EXIT_CODE", expected "exited"'; - exit 1; - fi - - if [ "\$EXIT_STATUS" != "1" ]; then - echo 'EXIT_STATUS was "\$EXIT_STATUS", expected "1"'; - exit 1; - fi - - if [ -z "\$INVOCATION_ID" ]; then - echo 'INVOCATION_ID unset'; - exit 1; - fi - - if [[ "\$UNIT" != "testservice-failure-68.service" && "\$UNIT" != "testservice-failure-68-additional.service" && "\$UNIT" != "testservice-transient-failure-68.service" ]]; then - echo 'UNIT was "\$UNIT", expected "testservice-failure-68{-additional,-transient}.service"'; - exit 1; - fi -done - -exit 0; +exit 0 EOF chmod +x /tmp/check_on_failure.sh @@ -195,34 +169,65 @@ cat >/run/systemd/system/testservice-failure-exit-handler-68.service <<EOF Description=TEST-68-PROPAGATE-EXIT-STATUS handle service exiting in failure [Service] +# repeat the check to make sure that values are set correctly on repeated invocations +Type=oneshot +ExecStartPre=/tmp/check_on_failure.sh +ExecStartPre=/tmp/check_on_failure.sh +ExecStart=/tmp/check_on_failure.sh +ExecStart=/tmp/check_on_failure.sh +ExecStartPost=test -z '$MONITOR_SERVICE_RESULT' +EOF + +# Template version. +cat >/run/systemd/system/testservice-failure-exit-handler-68-template@.service <<EOF +[Unit] +Description=TEST-68-PROPAGATE-EXIT-STATUS handle service exiting in failure (template) + +[Service] +Type=oneshot +ExecStartPre=echo "triggered by %i" ExecStartPre=/tmp/check_on_failure.sh +ExecStartPre=/tmp/check_on_failure.sh +ExecStart=/tmp/check_on_failure.sh ExecStart=/tmp/check_on_failure.sh +ExecStartPost=test -z '$MONITOR_SERVICE_RESULT' EOF systemctl daemon-reload -# The running of the OnFailure= and OnSuccess= jobs for all of these services -# may result in jobs being merged. +: "-------I----------------------------------------------------" systemctl start testservice-failure-68.service wait_on_state_or_fail "testservice-failure-exit-handler-68.service" "inactive" "10" -systemctl start testservice-failure-68-additional.service -wait_on_state_or_fail "testservice-failure-exit-handler-68.service" "inactive" "10" + +: "-------II---------------------------------------------------" systemctl start testservice-success-68.service wait_on_state_or_fail "testservice-success-exit-handler-68.service" "inactive" "10" -systemctl start testservice-success-68-additional.service -wait_on_state_or_fail "testservice-success-exit-handler-68.service" "inactive" "10" + +# Let's get rid of the failed units so the tests below don't fail, and daemon-reload +# to force garbace collection of the units. +systemctl reset-failed +systemctl daemon-reload # Test some transient units since these exit very quickly. -systemd-run --unit=testservice-transient-success-68 --property=OnSuccess=testservice-success-exit-handler-68.service /bin/bash -c "exit 0;" +: "-------III--------------------------------------------------" +systemd-run --unit=testservice-transient-success-68 --property=OnSuccess=testservice-success-exit-handler-68.service sh -c "exit 0" wait_on_state_or_fail "testservice-success-exit-handler-68.service" "inactive" "10" -systemd-run --unit=testservice-transient-failure-68 --property=OnFailure=testservice-failure-exit-handler-68.service /bin/bash -c "exit 1;" -wait_on_state_or_fail "testservice-failure-exit-handler-68.service" "inactive" "10" -# These yield a higher chance of resulting in jobs merging. -systemctl start testservice-failure-68.service testservice-failure-68-additional.service --no-block +: "-------IIII-------------------------------------------------" +systemd-run --unit=testservice-transient-failure-68 --property=OnFailure=testservice-failure-exit-handler-68.service sh -c "exit 1" wait_on_state_or_fail "testservice-failure-exit-handler-68.service" "inactive" "10" -systemctl start testservice-success-68.service testservice-success-68-additional.service --no-block -wait_on_state_or_fail "testservice-success-exit-handler-68.service" "inactive" "10" + +systemctl reset-failed +systemctl daemon-reload + +# Test template handlers too +: "-------V---------------------------------------------------" +systemctl start testservice-success-68-template.service +wait_on_state_or_fail "testservice-success-exit-handler-68-template@testservice-success-68-template.service.service" "inactive" "10" + +: "-------VI----------------------------------------------------" +systemctl start testservice-failure-68-template.service +wait_on_state_or_fail "testservice-failure-exit-handler-68-template@testservice-failure-68-template.service.service" "inactive" "10" systemd-analyze log-level info echo OK >/testok |