From af4117037bf75da101abfebad0a0f87e1b1475f9 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Oct 18 2022 09:49:29 +0000 Subject: Merge pull request #25004 from keszybz/transient-drop-ins Allow drop-ins for transient units --- diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 0e57bd9..a6b3c7b 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -957,7 +957,7 @@ static int transient_unit_from_message( if (!unit_is_pristine(u)) return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, - "Unit %s already exists.", name); + "Unit %s was already loaded or has a fragment file.", name); /* OK, the unit failed to load and is unreferenced, now let's * fill in the transient data instead */ diff --git a/src/core/unit.c b/src/core/unit.c index d6bea20..d0f7188 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4844,18 +4844,20 @@ int unit_fail_if_noncanonical(Unit *u, const char* where) { bool unit_is_pristine(Unit *u) { assert(u); - /* Check if the unit already exists or is already around, - * in a number of different ways. Note that to cater for unit - * types such as slice, we are generally fine with units that - * are marked UNIT_LOADED even though nothing was actually - * loaded, as those unit types don't require a file on disk. */ - - return !(!IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_LOADED) || - u->fragment_path || - u->source_path || - !strv_isempty(u->dropin_paths) || - u->job || - u->merged_into); + /* Check if the unit already exists or is already around, in a number of different ways. Note that to + * cater for unit types such as slice, we are generally fine with units that are marked UNIT_LOADED + * even though nothing was actually loaded, as those unit types don't require a file on disk. + * + * Note that we don't check for drop-ins here, because we allow drop-ins for transient units + * identically to non-transient units, both unit-specific and hierarchical. E.g. for a-b-c.service: + * service.d/….conf, a-.service.d/….conf, a-b-.service.d/….conf, a-b-c.service.d/….conf. + */ + + return IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_LOADED) && + !u->fragment_path && + !u->source_path && + !u->job && + !u->merged_into; } pid_t unit_control_pid(Unit *u) { diff --git a/test/units/testsuite-15.sh b/test/units/testsuite-15.sh index f847ada..079c8b2 100755 --- a/test/units/testsuite-15.sh +++ b/test/units/testsuite-15.sh @@ -3,30 +3,32 @@ set -eux set -o pipefail -_clear_service () { - local SERVICE_NAME="${1:?}" - systemctl stop "$SERVICE_NAME.service" 2>/dev/null || : - rm -f /{etc,run,usr/lib}/systemd/system/"$SERVICE_NAME".service - rm -fr /{etc,run,usr/lib}/systemd/system/"$SERVICE_NAME".service.d - rm -fr /{etc,run,usr/lib}/systemd/system/"$SERVICE_NAME".service.{wants,requires} - if [[ $SERVICE_NAME == *@ ]]; then - systemctl stop "$SERVICE_NAME"*.service 2>/dev/null || : - rm -f /{etc,run,usr/lib}/systemd/system/"$SERVICE_NAME"*.service - rm -fr /{etc,run,usr/lib}/systemd/system/"$SERVICE_NAME"*.service.d - rm -fr /{etc,run,usr/lib}/systemd/system/"$SERVICE_NAME"*.service.{wants,requires} +clear_unit () { + local UNIT_NAME="${1:?}" + systemctl stop "$UNIT_NAME" 2>/dev/null || : + rm -f /{etc,run,usr/lib}/systemd/system/"$UNIT_NAME" + rm -fr /{etc,run,usr/lib}/systemd/system/"$UNIT_NAME".d + rm -fr /{etc,run,usr/lib}/systemd/system/"$UNIT_NAME".{wants,requires} + if [[ $UNIT_NAME == *@ ]]; then + local base="${UNIT_NAME%@*}" + local suffix="${UNIT_NAME##*.}" + systemctl stop "$base@"*."$suffix" 2>/dev/null || : + rm -f /{etc,run,usr/lib}/systemd/system/"$base@"*."$suffix" + rm -fr /{etc,run,usr/lib}/systemd/system/"$base@"*."$suffix".d + rm -fr /{etc,run,usr/lib}/systemd/system/"$base@"*."$suffix".{wants,requires} fi } -clear_services () { +clear_units () { for u in "$@"; do - _clear_service "$u" + clear_unit "$u" done systemctl daemon-reload } create_service () { local SERVICE_NAME="${1:?}" - clear_services "$SERVICE_NAME" + clear_units "${SERVICE_NAME}".service cat >/etc/systemd/system/"$SERVICE_NAME".service </usr/lib/systemd/system/$dropin/override.conf systemctl daemon-reload - check_ok a-b-c ExecCondition "/bin/echo $dropin" + check_ok a-b-c ExecCondition "echo $dropin" + + # Check that we can start a transient service in presence of the drop-ins + systemd-run -u a-b-c2.service -p Description='sleepy' sleep infinity + + # The transient setting replaces the default + check_ok a-b-c2.service Description "sleepy" + + # The override takes precedence for ExecCondition + # (except the last iteration when it only applies to the other service) + if [ "$dropin" != "a-b-c.service.d" ]; then + check_ok a-b-c2.service ExecCondition "echo $dropin" + fi + + # Check that things are the same after a reload + systemctl daemon-reload + check_ok a-b-c2.service Description "sleepy" + if [ "$dropin" != "a-b-c.service.d" ]; then + check_ok a-b-c2.service ExecCondition "echo $dropin" + fi + + systemctl stop a-b-c2.service done for dropin in service.d a-.service.d a-b-.service.d a-b-c.service.d; do rm -rf /usr/lib/systemd/system/$dropin done - clear_services a-b-c + clear_units a-b-c.service +} + +test_hierarchical_slice_dropins () { + echo "Testing hierarchical slice dropins..." + echo "*** test slice.d/ top level drop-in" + # Slice units don't even need a fragment, so we test the defaults here + check_ok a-b-c.slice Description "Slice /a/b/c" + check_ok a-b-c.slice MemoryMax "infinity" + + # Test drop-ins + for dropin in slice.d a-.slice.d a-b-.slice.d a-b-c.slice.d; do + mkdir -p /usr/lib/systemd/system/$dropin + echo " +[Slice] +MemoryMax=1000000000 + " >/usr/lib/systemd/system/$dropin/override.conf + systemctl daemon-reload + check_ok a-b-c.slice MemoryMax "1000000000" + + busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + StartTransientUnit 'ssa(sv)a(sa(sv))' \ + 'a-b-c.slice' 'replace' \ + 2 \ + 'Description' s 'slice too' \ + 'MemoryMax' t 1000000002 \ + 0 + + # The override takes precedence for MemoryMax + check_ok a-b-c.slice MemoryMax "1000000000" + # The transient setting replaces the default + check_ok a-b-c.slice Description "slice too" + + # Check that things are the same after a reload + systemctl daemon-reload + check_ok a-b-c.slice MemoryMax "1000000000" + check_ok a-b-c.slice Description "slice too" + + busctl call \ + org.freedesktop.systemd1 \ + /org/freedesktop/systemd1 \ + org.freedesktop.systemd1.Manager \ + StopUnit 'ss' \ + 'a-b-c.slice' 'replace' + + rm /usr/lib/systemd/system/$dropin/override.conf + done + + # Test unit with a fragment + echo " +[Slice] +MemoryMax=1000000001 + " >/usr/lib/systemd/system/a-b-c.slice + systemctl daemon-reload + check_ok a-b-c.slice MemoryMax "1000000001" + + clear_units a-b-c.slice +} + +test_transient_service_dropins () { + echo "Testing dropins for a transient service..." + echo "*** test transient service drop-ins" + + mkdir -p /etc/systemd/system/service.d + mkdir -p /etc/systemd/system/a-.service.d + mkdir -p /etc/systemd/system/a-b-.service.d + mkdir -p /etc/systemd/system/a-b-c.service.d + + echo -e '[Service]\nStandardInputText=aaa' >/etc/systemd/system/service.d/drop1.conf + echo -e '[Service]\nStandardInputText=bbb' >/etc/systemd/system/a-.service.d/drop2.conf + echo -e '[Service]\nStandardInputText=ccc' >/etc/systemd/system/a-b-.service.d/drop3.conf + echo -e '[Service]\nStandardInputText=ddd' >/etc/systemd/system/a-b-c.service.d/drop4.conf + + # There's no fragment yet, so this fails + systemctl cat a-b-c.service && exit 1 + + # xxx → eHh4Cg== + systemd-run -u a-b-c.service -p StandardInputData=eHh4Cg== sleep infinity + + data=$(systemctl show -P StandardInputData a-b-c.service) + # xxx\naaa\n\bbb\nccc\nddd\n → eHh4… + test "$data" = "eHh4CmFhYQpiYmIKY2NjCmRkZAo=" + + # Do a reload and check again + systemctl daemon-reload + data=$(systemctl show -P StandardInputData a-b-c.service) + test "$data" = "eHh4CmFhYQpiYmIKY2NjCmRkZAo=" + + clear_units a-b-c.service + rm /etc/systemd/system/service.d/drop1.conf \ + /etc/systemd/system/a-.service.d/drop2.conf \ + /etc/systemd/system/a-b-.service.d/drop3.conf } test_template_dropins () { @@ -346,7 +463,7 @@ EOF check_ok bar-alias@3 Requires yup-template-requires.device check_ok bar-alias@3 Requires yup-3-requires.device - clear_services foo {bar,yup,bar-alias}@{,1,2,3} + clear_units foo.service {bar,yup,bar-alias}@{,1,2,3}.service } test_alias_dropins () { @@ -361,7 +478,7 @@ test_alias_dropins () { systemctl --quiet is-active test15-b systemctl stop test15-a test15-b rm /etc/systemd/system/test15-b1.service - clear_services test15-a test15-b + clear_units test15-{a,b}.service # Check that dependencies don't vary. echo "*** test 2" @@ -378,7 +495,7 @@ test_alias_dropins () { systemctl stop test15-a test15-x test15-y rm /etc/systemd/system/test15-a1.service - clear_services test15-a test15-x test15-y + clear_units test15-{a,x,y}.service } test_masked_dropins () { @@ -499,7 +616,7 @@ EOF ln -sf /dev/null /etc/systemd/system/test15-a.service.requires/test15-b.service check_ok test15-a Requires test15-b - clear_services test15-a test15-b + clear_units test15-{a,b}.service } test_invalid_dropins () { @@ -511,7 +628,7 @@ test_invalid_dropins () { # Assertion failed on earlier versions, command exits unsuccessfully on later versions systemctl cat a@.service || true systemctl stop a - clear_services a + clear_units a.service return 0 } @@ -531,13 +648,15 @@ EOF ln -s /tmp/testsuite-15-test15-a-dropin-directory-regular /usr/lib/systemd/system/test15-a.service.d check_ok test15-a Description hogehoge - clear_services test15-a + clear_units test15-a.service } test_basic_dropins test_linked_units test_template_alias -test_hierarchical_dropins +test_hierarchical_service_dropins +test_hierarchical_slice_dropins +test_transient_service_dropins test_template_dropins test_alias_dropins test_masked_dropins