quadlet: Add a network requirement on .image units

If a container unit starts on boot with a dependency on `default.target`
the image unit may start too soon, before network is ready. This cause
the unit to fail to pull the image.
- Add a dependency on `network-online.target` to make sure image pulls
don't fail.
See containers#21873

- Document the hardcoded dependency on `network-online.target` for images unit
and explain how it can be overriden if necessary.

- tests/e2e/quadlet: Add `assert-last-key-regex`

Required to test the `After=` override in [Unit] section
See containers#22057 (comment)

- quadlet/unitfile: add a prepenUnitLine method

Requirements on networks should be inserted at the top of the
section so the user can override them.

Signed-off-by: jbtrystram <[email protected]>

docs/source/markdown/podman-systemd.unit.5.md

@@ -1333,6 +1333,11 @@ exists on the host, pulling it if needed.
 Using image units allows containers and volumes to depend on images being automatically pulled. This is
 particularly interesting when using special options to control image pulls.
 
+Note: The generated service have a dependency on `network-online.target` assuring the network is reachable if
+an image needs to be pulled.
+If the image service needs to run without available network (e.g. early in boot), the requirement can be
+overriden simply by adding an empty `After=` in the unit file. This will unset all previously set After's.
+
 Valid options for `[Image]` are listed below:
 
 | **[Image] options**                    | **podman image pull equivalent**                 |

pkg/systemd/parser/unitfile.go

@@ -90,6 +90,11 @@ func (g *unitGroup) addLine(line *unitLine) {
 	g.lines = append(g.lines, line)
 }
 
+func (g *unitGroup) prependLine(line *unitLine) {
+	n := []*unitLine{line}
+	g.lines = append(n, g.lines...)
+}
+
 func (g *unitGroup) addComment(line *unitLine) {
 	g.comments = append(g.comments, line)
 }
@@ -923,6 +928,17 @@ func (f *UnitFile) PrependComment(groupName string, comments ...string) {
 	}
 }
 
+func (f *UnitFile) PrependUnitLine(groupName string, key string, value string) {
+	var group *unitGroup
+	if groupName == "" && len(f.groups) > 0 {
+		group = f.groups[0]
+	} else {
+		// Uses magic "" for first comment-only group if no other groups
+		group = f.ensureGroup(groupName)
+	}
+	group.prependLine(newUnitLine(key, value, false))
+}
+
 func (f *UnitFile) GetTemplateParts() (string, string) {
 	ext := filepath.Ext(f.Filename)
 	basename := strings.TrimSuffix(f.Filename, ext)

pkg/systemd/quadlet/quadlet.go

@@ -411,6 +411,14 @@ func ConvertContainer(container *parser.UnitFile, names map[string]string, isUse
 	service := container.Dup()
 	service.Filename = replaceExtension(container.Filename, ".service", "", "")
 
+	// Add a dependency on network-online.target so the image pull does not happen
+	// before network is ready
+	// https://github.com/containers/podman/issues/21873
+	// Prepend the lines, so the user-provided values
+	// override the default ones.
+	service.PrependUnitLine(UnitGroup, "After", "network-online.target")
+	service.PrependUnitLine(UnitGroup, "Wants", "network-online.target")
+
 	if container.Path != "" {
 		service.Add(UnitGroup, "SourcePath", container.Path)
 	}
@@ -1182,6 +1190,14 @@ func ConvertImage(image *parser.UnitFile) (*parser.UnitFile, string, error) {
 	service := image.Dup()
 	service.Filename = replaceExtension(image.Filename, ".service", "", "-image")
 
+	// Add a dependency on network-online.target so the image pull does not happen
+	// before network is ready
+	// https://github.com/containers/podman/issues/21873
+	// Prepend the lines, so the user-provided values
+	// override the default ones.
+	service.PrependUnitLine(UnitGroup, "After", "network-online.target")
+	service.PrependUnitLine(UnitGroup, "Wants", "network-online.target")
+
 	if image.Path != "" {
 		service.Add(UnitGroup, "SourcePath", image.Path)
 	}

test/e2e/quadlet/basic.container

@@ -15,6 +15,8 @@
 ## assert-key-is-regex "Service" "ExecStopPost" "-[/S].*/podman rm -v -f -i --cidfile=%t/%N.cid"
 ## assert-key-is-regex "Service" "ExecStop" ".*/podman rm -v -f -i --cidfile=%t/%N.cid"
 ## assert-key-is "Service" "Environment" "PODMAN_SYSTEMD_UNIT=%n"
+## assert-key-is "Unit" "After" "network-online.target"
+## assert-key-is "Unit" "Wants" "network-online.target"
 
 [Container]
 Image=localhost/imagename

test/e2e/quadlet/basic.image

@@ -1,4 +1,6 @@
 ## assert-podman-final-args localhost/imagename
+## assert-key-is "Unit" "After" "network-online.target"
+## assert-key-is "Unit" "Wants" "network-online.target"
 ## assert-key-is "Unit" "RequiresMountsFor" "%t/containers"
 ## assert-key-is "Service" "Type" "oneshot"
 ## assert-key-is "Service" "RemainAfterExit" "yes"

test/e2e/quadlet/mount.container

@@ -10,7 +10,7 @@ Mount=type=bind,src=/path/on/host,dst=/path/in/container,relabel=shared,U=true
 Mount=type=volume,source=vol1,destination=/path/in/container,ro=true
 ## assert-podman-args-key-val "--mount" "," "type=volume,source=systemd-vol2,destination=/path/in/container,ro=true"
 ## assert-key-is "Unit" "Requires" "vol2-volume.service"
-## assert-key-is "Unit" "After" "vol2-volume.service"
+## assert-key-is "Unit" "After" "network-online.target" "vol2-volume.service"
 Mount=type=volume,source=vol2.volume,destination=/path/in/container,ro=true
 ## assert-podman-args-key-val "--mount" "," "type=tmpfs,tmpfs-size=512M,destination=/path/in/container"
 Mount=type=tmpfs,tmpfs-size=512M,destination=/path/in/container

test/e2e/quadlet/network.quadlet.container

@@ -1,6 +1,6 @@
 ## assert-podman-args "--network=systemd-basic"
 ## assert-key-is "Unit" "Requires" "basic-network.service"
-## assert-key-is "Unit" "After" "basic-network.service"
+## assert-key-is "Unit" "After" "network-online.target" "basic-network.service"
 
 [Container]
 Image=localhost/imagename

test/e2e/quadlet/unit-after-override.container

@@ -0,0 +1,7 @@
+## assert-last-key-is-regex "Unit" "After" "^$"
+
+[Unit]
+After=
+
+[Container]
+Image=localhost/imagename

test/e2e/quadlet/unit-after-override.image

@@ -0,0 +1,7 @@
+## assert-last-key-is-regex "Unit" "After" "^$"
+
+[Unit]
+After=
+
+[Image]
+Image=localhost/imagename

test/e2e/quadlet_test.go

@@ -172,6 +172,24 @@ func (t *quadletTestcase) assertKeyIsRegex(args []string, unit *parser.UnitFile)
 	return true
 }
 
+func (t *quadletTestcase) assertLastKeyIsRegex(args []string, unit *parser.UnitFile) bool {
+	Expect(len(args)).To(BeNumerically(">=", 3))
+	group := args[0]
+	key := args[1]
+	regex := args[2]
+
+	value, ok := unit.LookupLast(group, key)
+	if !ok {
+		return false
+	}
+
+	matched, err := regexp.MatchString(regex, value)
+	if err != nil || !matched {
+		return false
+	}
+	return true
+}
+
 func (t *quadletTestcase) assertKeyContains(args []string, unit *parser.UnitFile) bool {
 	Expect(args).To(HaveLen(3))
 	group := args[0]
@@ -469,6 +487,8 @@ func (t *quadletTestcase) doAssert(check []string, unit *parser.UnitFile, sessio
 		ok = t.assertKeyIsRegex(args, unit)
 	case "assert-key-contains":
 		ok = t.assertKeyContains(args, unit)
+	case "assert-last-key-is-regex":
+		ok = t.assertLastKeyIsRegex(args, unit)
 	case "assert-podman-args":
 		ok = t.assertStartPodmanArgs(args, unit)
 	case "assert-podman-args-regex":
@@ -847,6 +867,7 @@ BOGUS=foo
 		Entry("merged-override.container", "merged-override.container", 0, ""),
 		Entry("[email protected]", "[email protected]", 0, ""),
 		Entry("[email protected]", "[email protected]", 0, ""),
+		Entry("Unit After Override", "unit-after-override.container", 0, ""),
 
 		Entry("basic.volume", "basic.volume", 0, ""),
 		Entry("device-copy.volume", "device-copy.volume", 0, ""),
@@ -921,6 +942,7 @@ BOGUS=foo
 		Entry("Image - Arch and OS", "arch-os.image", 0, ""),
 		Entry("Image - global args", "globalargs.image", 0, ""),
 		Entry("Image - Containers Conf Modules", "containersconfmodule.image", 0, ""),
+		Entry("Image - Unit After Override", "unit-after-override.image", 0, ""),
 
 		Entry("basic.pod", "basic.pod", 0, ""),
 		Entry("name.pod", "name.pod", 0, ""),