From 3c52ef43f57df73e4b5efbbebc790e19f766e88f Mon Sep 17 00:00:00 2001 From: benniekiss <63211101+benniekiss@users.noreply.github.com> Date: Fri, 12 Jul 2024 13:39:45 -0400 Subject: [PATCH] Expand drop-in search paths * top-level (pod.d) * truncated (unit-.container.d) Signed-off-by: Bennie Milburn-Town <63211101+benniekiss@users.noreply.github.com> --- cmd/quadlet/main.go | 27 ++++---- docs/source/markdown/podman-systemd.unit.5.md | 14 +++-- pkg/systemd/parser/unitfile.go | 55 +++++++++++++++- pkg/systemd/parser/unitfile_test.go | 49 +++++++++++++++ test/system/252-quadlet.bats | 62 ++++++++++++++++++- 5 files changed, 182 insertions(+), 25 deletions(-) diff --git a/cmd/quadlet/main.go b/cmd/quadlet/main.go index c443b05f26..eeab3d4c3f 100644 --- a/cmd/quadlet/main.go +++ b/cmd/quadlet/main.go @@ -158,9 +158,12 @@ func appendSubPaths(dirs []string, path string, isUserFlag bool, filterPtr func( } err = filepath.WalkDir(resolvedPath, func(_path string, info os.DirEntry, err error) error { - if info == nil || info.IsDir() { - if filterPtr == nil || filterPtr(_path, isUserFlag) { - dirs = append(dirs, _path) + // Ignore drop-in directory subpaths + if !strings.HasSuffix(_path, ".d") { + if info == nil || info.IsDir() { + if filterPtr == nil || filterPtr(_path, isUserFlag) { + dirs = append(dirs, _path) + } } } return err @@ -256,16 +259,11 @@ func loadUnitDropins(unit *parser.UnitFile, sourcePaths []string) error { } dropinDirs := []string{} + unitDropinPaths := unit.GetUnitDropinPaths() for _, sourcePath := range sourcePaths { - dropinDirs = append(dropinDirs, path.Join(sourcePath, unit.Filename+".d")) - } - - // For instantiated templates, also look in the non-instanced template dropin dirs - templateBase, templateInstance := unit.GetTemplateParts() - if templateBase != "" && templateInstance != "" { - for _, sourcePath := range sourcePaths { - dropinDirs = append(dropinDirs, path.Join(sourcePath, templateBase+".d")) + for _, dropinPath := range unitDropinPaths { + dropinDirs = append(dropinDirs, path.Join(sourcePath, dropinPath)) } } @@ -359,15 +357,14 @@ func enableServiceFile(outputPath string, service *parser.UnitFile) { } serviceFilename := service.Filename - templateBase, templateInstance := service.GetTemplateParts() + templateBase, templateInstance, isTemplate := service.GetTemplateParts() // For non-instantiated template service we only support installs if a // DefaultInstance is given. Otherwise we ignore the Install group, but // it is still useful when instantiating the unit via a symlink. - if templateBase != "" && templateInstance == "" { + if isTemplate && templateInstance == "" { if defaultInstance, ok := service.Lookup(quadlet.InstallGroup, "DefaultInstance"); ok { - parts := strings.SplitN(templateBase, "@", 2) - serviceFilename = parts[0] + "@" + defaultInstance + parts[1] + serviceFilename = templateBase + "@" + defaultInstance + filepath.Ext(serviceFilename) } else { serviceFilename = "" } diff --git a/docs/source/markdown/podman-systemd.unit.5.md b/docs/source/markdown/podman-systemd.unit.5.md index 56732e11c0..21e3bb2c27 100644 --- a/docs/source/markdown/podman-systemd.unit.5.md +++ b/docs/source/markdown/podman-systemd.unit.5.md @@ -50,11 +50,15 @@ other sections are passed on untouched, allowing the use of any normal systemd c like dependencies or cgroup limits. The source files also support drop-ins in the same [way systemd does](https://www.freedesktop.org/software/systemd/man/latest/systemd.unit.html). -For a given source file (say `foo.container`), the corresponding `.d`directory (in this -case `foo.container.d`) will be scanned for files with a `.conf` extension that are merged into -the base file in alphabetical order. The format of these drop-in files is the same as the base file. -This is useful to alter or add configuration settings for a unit, without having to modify unit -files. +For a given source file (`foo.container`), the corresponding `.d` directory (`foo.container.d`) will +be scanned for files with a `.conf` extension, which are then merged into the base file in alphabetical +order. Top-level type drop-ins (`container.d`) will also be included. If the unit contains dashes ("-") +in the name (`foo-bar-baz.container`), then the drop-in directories generated by truncating the name after +the dash are searched as well (`foo-.container.d` and `foo-bar-.container.d`). Drop-in files with the same name +further down the hierarchy override those further up (`foo-bar-baz.container.d/10-override.conf` overrides +`foo-bar-.container.d/10-override.conf`, which overrides `foo-.service.d/10-override.conf`, which overrides +`container.d/10-override.conf`). The format of these drop-in files is the same as the base file. This is useful +to alter or add configuration settings for a unit, without having to modify unit files. For rootless containers, when administrators place Quadlet files in the /etc/containers/systemd/users directory, all users' sessions execute the diff --git a/pkg/systemd/parser/unitfile.go b/pkg/systemd/parser/unitfile.go index 9ecbf4a158..84be6ab623 100644 --- a/pkg/systemd/parser/unitfile.go +++ b/pkg/systemd/parser/unitfile.go @@ -939,14 +939,63 @@ func (f *UnitFile) PrependUnitLine(groupName string, key string, value string) { group.prependLine(newUnitLine(key, value, false)) } -func (f *UnitFile) GetTemplateParts() (string, string) { +func (f *UnitFile) GetTemplateParts() (string, string, bool) { ext := filepath.Ext(f.Filename) basename := strings.TrimSuffix(f.Filename, ext) parts := strings.SplitN(basename, "@", 2) if len(parts) < 2 { - return "", "" + return parts[0], "", false } - return parts[0] + "@" + ext, parts[1] + return parts[0], parts[1], true +} + +func (f *UnitFile) GetUnitDropinPaths() []string { + unitName, instanceName, isTemplate := f.GetTemplateParts() + + ext := filepath.Ext(f.Filename) + dropinExt := ext + ".d" + + dropinPaths := []string{} + + // Add top-level drop-in location (pod.d, container.d, etc) + topLevelDropIn := strings.TrimPrefix(dropinExt, ".") + dropinPaths = append(dropinPaths, topLevelDropIn) + + truncatedParts := strings.Split(unitName, "-") + // If the unit contains any '-', then there are truncated paths to search. + if len(truncatedParts) > 1 { + // We don't need the last item because that would be the full path + truncatedParts = truncatedParts[:len(truncatedParts)-1] + // Truncated instance names are not included in the drop-in search path + // i.e. template-unit@base-instance.service does not search template-unit@base-.service + // So we only search truncations of the template name, i.e. template-@.service, and unit name, i.e. template-.service + // or only the unit name if it is not a template. + for i := range truncatedParts { + truncatedUnitPath := strings.Join(truncatedParts[:i+1], "-") + "-" + dropinPaths = append(dropinPaths, truncatedUnitPath+dropinExt) + // If the unit is a template, add the truncated template name as well. + if isTemplate { + truncatedTemplatePath := truncatedUnitPath + "@" + dropinPaths = append(dropinPaths, truncatedTemplatePath+dropinExt) + } + } + } + // For instanced templates, add the base template unit search path + if instanceName != "" { + dropinPaths = append(dropinPaths, unitName+"@"+dropinExt) + } + // Add the drop-in directory for the full filename + dropinPaths = append(dropinPaths, f.Filename+".d") + // Finally, reverse the list so that when drop-ins are parsed, + // the most specific are applied instead of the most broad. + // dropinPaths should be a list where the items are in order of specific -> broad + // i.e., the most specific search path is dropinPaths[0], and broadest search path is dropinPaths[len(dropinPaths)-1] + // Uses https://go.dev/wiki/SliceTricks#reversing + for i := len(dropinPaths)/2 - 1; i >= 0; i-- { + opp := len(dropinPaths) - 1 - i + dropinPaths[i], dropinPaths[opp] = dropinPaths[opp], dropinPaths[i] + } + return dropinPaths } func PathEscape(path string) string { diff --git a/pkg/systemd/parser/unitfile_test.go b/pkg/systemd/parser/unitfile_test.go index bb56d1b5fb..ea805ddcdc 100644 --- a/pkg/systemd/parser/unitfile_test.go +++ b/pkg/systemd/parser/unitfile_test.go @@ -1,6 +1,7 @@ package parser import ( + "reflect" "testing" "github.com/stretchr/testify/assert" @@ -226,6 +227,43 @@ Also=systemd-networkd-wait-online.service var samples = []string{memcachedService, systemdloginService, systemdnetworkdService} +const sampleDropinService string = "sample-unit.service" + +var sampleDropinServicePaths = []string{ + "sample-unit.service.d", + "sample-.service.d", + "service.d", +} + +const sampleDropinTemplate string = "sample-template-unit@.service" + +var sampleDropinTemplatePaths = []string{ + "sample-template-unit@.service.d", + "sample-template-@.service.d", + "sample-template-.service.d", + "sample-@.service.d", + "sample-.service.d", + "service.d", +} + +const sampleDropinTemplateInstance string = "sample-template-unit@base-instance.service" + +var sampleDropinTemplateInstancePaths = []string{ + "sample-template-unit@base-instance.service.d", + "sample-template-unit@.service.d", + "sample-template-@.service.d", + "sample-template-.service.d", + "sample-@.service.d", + "sample-.service.d", + "service.d", +} + +var sampleDropinPaths = map[string][]string{ + sampleDropinService: sampleDropinServicePaths, + sampleDropinTemplate: sampleDropinTemplatePaths, + sampleDropinTemplateInstance: sampleDropinTemplateInstancePaths, +} + func TestRanges_Roundtrip(t *testing.T) { for i := range samples { sample := samples[i] @@ -243,3 +281,14 @@ func TestRanges_Roundtrip(t *testing.T) { assert.Equal(t, sample, asStr) } } + +func TestUnitDropinPaths_Search(t *testing.T) { + for filename, expectedPaths := range sampleDropinPaths { + f := UnitFile{ + Filename: filename, + } + generatedPaths := f.GetUnitDropinPaths() + + assert.True(t, reflect.DeepEqual(expectedPaths, generatedPaths)) + } +} diff --git a/test/system/252-quadlet.bats b/test/system/252-quadlet.bats index c09e49ed9b..8d8494d35a 100644 --- a/test/system/252-quadlet.bats +++ b/test/system/252-quadlet.bats @@ -64,14 +64,18 @@ function run_quadlet() { assert $status -eq 0 "Failed to convert quadlet file: $sourcefile" is "$output" "" "quadlet should report no errors" + run cat $UNIT_DIR/$service + assert $status -eq 0 "Could not cat $UNIT_DIR/$service" + echo "$output" + local content="$output" + # Ensure this is teared down UNIT_FILES+=("$UNIT_DIR/$service") QUADLET_SERVICE_NAME="$service" + QUADLET_SERVICE_CONTENT="$content" QUADLET_SYSLOG_ID="$(basename $service .service)" QUADLET_CONTAINER_NAME="systemd-$QUADLET_SYSLOG_ID" - - cat $UNIT_DIR/$QUADLET_SERVICE_NAME } function service_setup() { @@ -1599,4 +1603,58 @@ EOF run_podman rmi $untagged_image:latest $built_image $(pause_image) run_podman network rm podman-default-kube-network } + +@test "quadlet - drop-in files" { + local quadlet_tmpdir="${PODMAN_TMPDIR}/dropins" + + local quadlet_file="truncated-$(random_string).container" + + local -A dropin_dirs=( + [toplevel]=container.d + [truncated]=truncated-.container.d + [quadlet]="${quadlet_file}.d" + ) + + # Table of drop-in .conf files. Format is: + # + # apply | dir | filename | [Section] | Content=... + local dropin_files=" +y | toplevel | 10 | [Unit] | Description=Test File for Dropin Configuration +n | toplevel | 99 | [Install] | WantedBy=default.target +y | truncated | 50 | [Container] | ContainerName=truncated-dropins +n | truncated | 99 | [Service] | Restart=always +n | truncated | 99 | [Install] | WantedBy=multiuser.target +y | quadlet | 99 | [Service] | RestartSec=60s +" + + # Pass 1: Create all drop-in directories and files + while read apply dir file section content; do + local d="${quadlet_tmpdir}/${dropin_dirs[${dir}]}" + mkdir -p "${d}" + + local f="${d}/${file}.conf" + echo "${section}" >>"${f}" + echo "${content}" >>"${f}" + done < <(parse_table "${dropin_files}") + + # Create the base quadlet file + quadlet_base="${PODMAN_TMPDIR}/${quadlet_file}" + cat > "${quadlet_base}" <