Merge pull request #28380 from simonbrauner/issue-26960

Handle uninstantiated template quadlets

cmd/podman/common/completion.go

@@ -1776,7 +1776,7 @@ func AutocompleteSDNotify(_ *cobra.Command, _ []string, _ string) ([]string, cob
 
 var containerStatuses = []string{"created", "running", "paused", "stopped", "exited", "unknown"}
 
-var quadletStatuses = []string{"Not loaded", "active/running", "inactive/dead", "failed/failed", "activating/start", "deactivating/stop"}
+var quadletStatuses = []string{entities.QuadletStatusNotLoaded, entities.QuadletStatusLoadedTemplate, "active/running", "inactive/dead", "failed/failed", "activating/start", "deactivating/stop"}
 
 // AutocompletePsFilters - Autocomplete ps filter options.
 func AutocompletePsFilters(cmd *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) {

docs/source/markdown/podman-quadlet-list.1.md.in

@@ -23,8 +23,8 @@ Supported filters:
 | Filter     | Description                                                                                      |
 |------------|--------------------------------------------------------------------------------------------------|
 | name       | Filter by quadlet name                                                                           |
-| status     | Filter by quadlet status. Valid values: `Not loaded`, `active/running`, `inactive/dead`, `failed/failed`, `activating/start`, `deactivating/stop` |
 | pod        | Filter by the `Pod=` value (displays only for .container units)                                  |
+| status     | Filter by quadlet status. Valid values: `Not loaded`, `loaded template`, `active/running`, `inactive/dead`, `failed/failed`, `activating/start`, `deactivating/stop` |
 
 #### **--format**=*format*
 
@@ -38,7 +38,7 @@ Print results with a Go template.
 | .Name           | Name of the Quadlet file                         |
 | .Path           | Quadlet file path on disk                        |
 | .Pod            | Pod quadlet file from `Pod=` in `[Container]` (empty if not set) |
-| .Status         | Quadlet status corresponding to systemd unit     |
+| .Status         | Quadlet status corresponding to systemd unit (`Not loaded` and `loaded template` are from podman, other values are from systemd)    |
 | .UnitName       | Systemd unit name corresponding to quadlet       |
 
 @@option noheading

docs/source/markdown/podman-quadlet-rm.1.md.in

@@ -11,6 +11,8 @@ podman\-quadlet\-rm - Removes an installed quadlet
 Remove one or more installed Quadlets from the current user. Following command also takes application name
 as input and removes all the Quadlets which belongs to that specific application.
 
+When the argument is uninstantiated template quadlet, this command removes the template quadlet file (e.g. `templateName@.container`) and the generated systemd template unit (e.g. `templateName@.service`). If there are running instances of that systemd template, the command fails if **--force** option is not set, and tries to stop the instances if **--force** option is set.
+
 Note: If a quadlet is part of an application, removing that specific quadlet will remove the entire application.
 When a quadlet is installed from a directory, all files installed from that directory—including both quadlet and non-quadlet files—are considered part
 of a single application.
@@ -23,7 +25,7 @@ Remove all Quadlets for the current user.
 
 #### **--force**, **-f**
 
-Remove running Quadlets.
+Remove running Quadlets (in case of uninstantiated template quadlets, stop its instances).
 
 #### **--ignore**, **-i**
 

pkg/api/server/register_quadlets.go

@@ -196,7 +196,7 @@ func (s *APIServer) registerQuadletHandlers(r *mux.Router) error {
 	//    name: force
 	//    type: boolean
 	//    default: false
-	//    description: Remove running quadlet by stopping it first
+	//    description: Remove running quadlets (in case of uninstantiated template quadlets, stop its instances).
 	//  - in: query
 	//    name: ignore
 	//    type: boolean

pkg/domain/entities/quadlet.go

@@ -28,6 +28,14 @@ type QuadletListOptions struct {
 	Filters []string
 }
 
+const (
+	// Quadlet does not have a corresponding loaded systemd unit.
+	QuadletStatusNotLoaded = "Not loaded"
+	// Quadlet is a template loaded into systemd (there is no status
+	// to display in the same sense as for concrete units).
+	QuadletStatusLoadedTemplate = "loaded template"
+)
+
 // A ListQuadlet is a single Quadlet to be listed by `podman quadlet list`
 type ListQuadlet struct {
 	// Name is the name of the Quadlet file
@@ -37,8 +45,9 @@ type ListQuadlet struct {
 	UnitName string
 	// Path to the Quadlet on disk
 	Path string
-	// What is the status of the Quadlet - if present in systemd, will be a
-	// systemd status, else will mention if the Quadlet has syntax errors
+	// What is the status of the Quadlet - either values from systemd
+	// (e.g. active/running), or podman-defined values "Not loaded"
+	// and "loaded template".
 	Status string
 	// If multiple quadlets were installed together they will belong
 	// to common App.

pkg/domain/infra/abi/quadlet_utils.go

@@ -116,7 +116,8 @@ func getAllQuadlets(ctx context.Context, conn *dbus.Conn) ([]*entities.ListQuadl
 	// Get the root paths of all quadlets available to the current user
 	quadletDirs := systemdquadlet.GetUnitDirs(rootless.IsRootless(), false)
 
-	allServiceNames := make([]string, 0)
+	concreteServiceNames := make([]string, 0)
+	templateServiceNames := make([]string, 0)
 
 	// for every quadlet dir, let's get the quadlets
 	for _, dir := range quadletDirs {
@@ -154,36 +155,69 @@ func getAllQuadlets(ctx context.Context, conn *dbus.Conn) ([]*entities.ListQuadl
 				report.Pod = pod
 			}
 
-			allServiceNames = append(allServiceNames, serviceName)
+			if systemdquadlet.IsTemplateUnitFileName(serviceName) {
+				templateServiceNames = append(templateServiceNames, serviceName)
+			} else {
+				concreteServiceNames = append(concreteServiceNames, serviceName)
+			}
 			partialReports[serviceName] = report
 		}
 	}
 
-	// Get status of all systemd units with given names.
-	statuses, err := conn.ListUnitsByNamesContext(ctx, allServiceNames)
-	if err != nil {
-		return nil, fmt.Errorf("querying systemd for unit status: %w", err)
-	}
-	if len(statuses) != len(allServiceNames) {
-		logrus.Warnf("Queried for %d services but received %d responses", len(allServiceNames), len(statuses))
+	// Get status of concrete systemd units with given names.
+	if len(concreteServiceNames) > 0 {
+		statuses, err := conn.ListUnitsByNamesContext(ctx, concreteServiceNames)
+		if err != nil {
+			return nil, fmt.Errorf("querying systemd for unit status: %w", err)
+		}
+		if len(statuses) != len(concreteServiceNames) {
+			logrus.Warnf("Queried for %d services but received %d responses", len(concreteServiceNames), len(statuses))
+		}
+
+		for _, unitStatus := range statuses {
+			report, ok := partialReports[unitStatus.Name]
+			if !ok {
+				logrus.Errorf("Unexpected unit returned by systemd - was not searching for %s", unitStatus.Name)
+			}
+			logrus.Debugf("Unit %s has status %s %s %s", unitStatus.Name, unitStatus.LoadState, unitStatus.ActiveState, unitStatus.SubState)
+			report.UnitName = unitStatus.Name
+
+			// Unit is not loaded
+			if unitStatus.LoadState != "loaded" {
+				report.Status = entities.QuadletStatusNotLoaded
+			} else {
+				report.Status = fmt.Sprintf("%s/%s", unitStatus.ActiveState, unitStatus.SubState)
+			}
+			reports = append(reports, &report)
+			delete(partialReports, unitStatus.Name)
+		}
 	}
 
-	for _, unitStatus := range statuses {
-		report, ok := partialReports[unitStatus.Name]
-		if !ok {
-			logrus.Errorf("Unexpected unit returned by systemd - was not searching for %s", unitStatus.Name)
+	// Uninstantiated template units need to be handled separately, because
+	// ListUnitsBy* functions do not return anything for templates.
+	if len(templateServiceNames) > 0 {
+		unitFiles, err := conn.ListUnitFilesByPatternsContext(ctx, []string{}, templateServiceNames)
+		if err != nil {
+			return nil, fmt.Errorf("querying systemd for unit file: %w", err)
 		}
-		logrus.Debugf("Unit %s has status %s %s %s", unitStatus.Name, unitStatus.LoadState, unitStatus.ActiveState, unitStatus.SubState)
-		report.UnitName = unitStatus.Name
+		unitFilesFound := make(map[string]struct{})
+		for _, unitFile := range unitFiles {
+			unitFilesFound[filepath.Base(unitFile.Path)] = struct{}{}
+		}
+		for _, templateServiceName := range templateServiceNames {
+			report := partialReports[templateServiceName]
 
-		// Unit is not loaded
-		if unitStatus.LoadState != "loaded" {
-			report.Status = "Not loaded"
-		} else {
-			report.Status = fmt.Sprintf("%s/%s", unitStatus.ActiveState, unitStatus.SubState)
+			report.UnitName = templateServiceName
+
+			if _, ok := unitFilesFound[templateServiceName]; ok {
+				report.Status = entities.QuadletStatusLoadedTemplate
+			} else {
+				report.Status = entities.QuadletStatusNotLoaded
+			}
+
+			reports = append(reports, &report)
+			delete(partialReports, templateServiceName)
 		}
-		reports = append(reports, &report)
-		delete(partialReports, unitStatus.Name)
 	}
 
 	// This should not happen.
@@ -191,7 +225,7 @@ func getAllQuadlets(ctx context.Context, conn *dbus.Conn) ([]*entities.ListQuadl
 	// We can find them with LoadState, as we do above.
 	// Handle it anyways because it's easy enough to do.
 	for _, report := range partialReports {
-		report.Status = "Not loaded"
+		report.Status = entities.QuadletStatusNotLoaded
 		reports = append(reports, &report)
 	}
 
@@ -199,12 +233,7 @@ func getAllQuadlets(ctx context.Context, conn *dbus.Conn) ([]*entities.ListQuadl
 }
 
 func removeQuadlet(ctx context.Context, conn *dbus.Conn, quadlet *entities.ListQuadlet, force bool) error {
-	switch quadlet.Status {
-	case "Not loaded":
-	case "inactive/dead":
-		// Nothing to do here if it doesn't exist in systemd
-		break
-	case "active/running":
+	if quadlet.Status == "active/running" {
 		if !force {
 			return fmt.Errorf("quadlet %s is running and force is not set, refusing to remove: %w", quadlet.Name, define.ErrQuadletRunning)
 		}
@@ -221,10 +250,81 @@ func removeQuadlet(ctx context.Context, conn *dbus.Conn, quadlet *entities.ListQ
 			return fmt.Errorf("unable to stop quadlet %s: %s", quadlet.Name, stopResult)
 		}
 	}
+	if quadlet.Status == entities.QuadletStatusLoadedTemplate {
+		if err := stopLoadedTemplateInstances(ctx, conn, quadlet, force); err != nil {
+			return err
+		}
+	}
 
 	return os.Remove(quadlet.Path)
 }
 
+// stopLoadedTemplateInstances handles the running instances of a template.
+// If force is set, the instances are stopped and error is returned on failure.
+// If force is not set, stopping is refused and error is returned on running instances.
+func stopLoadedTemplateInstances(ctx context.Context, conn *dbus.Conn, quadlet *entities.ListQuadlet, force bool) error {
+	extension := filepath.Ext(quadlet.UnitName)
+
+	// The regular expression in this pattern matches the template itself as well, but the
+	// ListUnitsBy* functions do not return anything for templates.
+	instancePattern := strings.TrimSuffix(quadlet.UnitName, extension) + "*" + extension
+	instances, err := conn.ListUnitsByPatternsContext(ctx, []string{}, []string{instancePattern})
+	if err != nil {
+		return fmt.Errorf("querying systemd for instances of %q: %w", quadlet.UnitName, err)
+	}
+
+	var runningInstanceErrors []error
+	for _, instanceStatus := range instances {
+		properties, err := conn.GetUnitPropertiesContext(ctx, instanceStatus.Name)
+		if err != nil {
+			logrus.Errorf("getting unit properties for %q: %v", instanceStatus.Name, err)
+			continue
+		}
+
+		sourcePath, ok := properties["SourcePath"].(string)
+		if !ok || sourcePath == "" {
+			logrus.Warnf("source path not found for unit %q", instanceStatus.Name)
+			continue
+		}
+
+		if filepath.Clean(sourcePath) != filepath.Clean(quadlet.Path) {
+			// Nothing to do here if the unit is not associated with this quadlet
+			continue
+		}
+
+		if instanceStatus.LoadState != "loaded" {
+			// Nothing to do here if the instance is not loaded
+			continue
+		}
+
+		if instanceStatus.ActiveState == "active" {
+			if !force {
+				runningInstanceErrors = append(runningInstanceErrors, fmt.Errorf("template %q has running instance %q and force is not set, refusing to remove: %w", quadlet.Name, instanceStatus.Name, define.ErrQuadletRunning))
+				continue
+			}
+			logrus.Debugf("Going to stop systemd unit %q (Instance of template %q)", instanceStatus.Name, quadlet.Name)
+
+			ch := make(chan string)
+			if _, err := conn.StopUnitContext(ctx, instanceStatus.Name, "replace", ch); err != nil {
+				runningInstanceErrors = append(runningInstanceErrors, fmt.Errorf("stopping instance %q of template %q: %w", instanceStatus.Name, quadlet.Name, err))
+				continue
+			}
+
+			logrus.Debugf("Waiting for systemd unit %q to stop", instanceStatus.Name)
+			stopResult := <-ch
+			if stopResult != "done" && stopResult != "skipped" {
+				runningInstanceErrors = append(runningInstanceErrors, fmt.Errorf("unable to stop instance %q of template %q: %q", instanceStatus.Name, quadlet.Name, stopResult))
+			}
+		}
+	}
+
+	if len(runningInstanceErrors) > 0 {
+		return errors.Join(runningInstanceErrors...)
+	}
+
+	return nil
+}
+
 // getQuadletServiceNameAndUnit parses a Quadlet file and returns both the
 // generated systemd service name and the parsed unit file.
 func getQuadletServiceNameAndUnit(quadletPath string) (string, *parser.UnitFile, error) {

pkg/systemd/quadlet/quadlet.go

@@ -943,8 +943,9 @@ func ConvertContainer(container *parser.UnitFile, unitsInfoMap map[string]*UnitI
 	return service, warnings, nil
 }
 
-func isTemplateUnit(unit *parser.UnitFile) bool {
-	base := strings.TrimSuffix(unit.Filename, filepath.Ext(unit.Filename))
+// Determine if the given file name belongs to an uninstantiated template unit.
+func IsTemplateUnitFileName(fileName string) bool {
+	base := strings.TrimSuffix(fileName, filepath.Ext(fileName))
 	return strings.HasSuffix(base, "@")
 }
 
@@ -2231,7 +2232,7 @@ func handlePod(quadletUnitFile, serviceUnitFile *parser.UnitFile, groupName stri
 		// If we want to start the container with the pod, we add it to this list.
 		// This creates corresponding Wants=/Before= statements in the pod service.
 		// Do not add this for template units as dependency cannot be created for them.
-		if !isTemplateUnit(quadletUnitFile) && quadletUnitFile.LookupBooleanWithDefault(groupName, KeyStartWithPod, true) {
+		if !IsTemplateUnitFileName(quadletUnitFile.Filename) && quadletUnitFile.LookupBooleanWithDefault(groupName, KeyStartWithPod, true) {
 			podInfo.ContainersToStart = append(podInfo.ContainersToStart, serviceUnitFile.Filename)
 		}
 	}

test/system/253-podman-quadlet.bats

@@ -156,6 +156,70 @@ EOF
     run_podman quadlet rm $ctr_unit
 }
 
+@test "quadlet verb - install, list, print, rm - template" {
+    # Determine the install directory path based on rootless/root
+    local install_dir
+    install_dir=$(get_quadlet_install_dir)
+    # Create a test quadlet file
+    local quadlet_name_without_extension="templated-quadlet@"
+    local quadlet_name=${quadlet_name_without_extension}.container
+    local quadlet_unit_name=${quadlet_name_without_extension}.service
+    local quadlet_instance_name=${quadlet_name_without_extension}instance.service
+    local quadlet_file=$PODMAN_TMPDIR/$quadlet_name
+    cat > "$quadlet_file" <<EOF
+[Container]
+Image=$IMAGE
+Exec=sh -c "echo STARTED CONTAINER; trap 'exit' SIGTERM; while :; do sleep 0.1; done"
+EOF
+    # Test quadlet install
+    run_podman quadlet install "$quadlet_file"
+    assert "$output" =~ "$quadlet_name" "install output should contain quadlet name"
+
+    # Test quadlet list
+    run_podman quadlet list --filter status='loaded template'
+    assert "$output" =~ "$quadlet_name" "list should contain $quadlet_name"
+    assert "$output" =~ "$quadlet_unit_name" "UNIT NAME should be $quadlet_unit_name"
+    assert "$output" =~ "loaded template" "STATUS should be 'loaded template'"
+    assert "$output" =~ "$install_dir/$quadlet_name" "PATH ON DISK should show the quadlet file path"
+
+    # Test quadlet print
+    run_podman quadlet print "$quadlet_name"
+    assert "$output" == "$(<"$quadlet_file")" "print output matches quadlet file"
+
+    # Regenerate the service manually, otherwise PODMAN path is not set correctly in CI
+    QUADLET_UNIT_DIRS="$install_dir" run \
+        timeout --foreground -v --kill=10 $PODMAN_TIMEOUT \
+        $QUADLET $_DASHUSER $UNIT_DIR
+    assert $status -eq 0 "Failed to regenerate the service manually"
+    systemctl daemon-reload
+
+    # Instantiate the template
+    systemctl_start "$quadlet_instance_name"
+
+    # Test quadlet rm without --force (should fail)
+    run_podman 125 quadlet rm "$quadlet_name"
+    assert "$output" =~ "$quadlet_instance_name" "error message should contain running instance name"
+    assert "$output" =~ "quadlet is running" "error message should contain the explanation"
+    # Verify template was not removed
+    run_podman quadlet list
+    assert "$output" =~ "$quadlet_name" "list should contain template"
+
+    # Test quadlet rm with --force (should succeed)
+    run_podman quadlet rm --force "$quadlet_name"
+    assert "$output" =~ "$quadlet_name" "remove output should contain quadlet name"
+    # Verify template was removed
+    run_podman quadlet list
+    assert "$output" !~ "$quadlet_name" "list should not contain removed template"
+
+    # Check that removing template also works without --force when there are no running instances
+    run_podman quadlet install "$quadlet_file"
+    assert "$output" =~ "$quadlet_name" "install output should contain quadlet name"
+    run_podman quadlet rm "$quadlet_name"
+    assert "$output" =~ "$quadlet_name" "remove output should contain quadlet name"
+    # Verify template was removed
+    run_podman quadlet list
+    assert "$output" !~ "$quadlet_name" "list should not contain removed template"
+}
 
 @test "quadlet verb - install multiple files from directory and remove by app name" {
     # Create a directory for multiple quadlet files