Merge pull request #22057 from jbtrystram/quadlet-image-network

quadlet: Add a network requirement on .image and .containers units

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("template@.container", "template@.container", 0, ""),
 		Entry("template@instance.container", "template@instance.container", 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, ""),