Merge branch 'develop' into fix/libpod-spec-response-correctness

Author: Jaro-c

internal/quadlet/tests/units.rs

@@ -130,8 +130,7 @@ fn warns_for_every_unmapped_field() {
 services:
   s:
     image: x
-    network_mode: "container:other"
-    privileged: true
+    network_mode: "bridge:custom"
     profiles: [debug]
     volumes_from:
       - other
@@ -141,13 +140,7 @@ services:
 	let file = parse_str(yaml).unwrap();
 	let out = generate(&file, "p");
 	let joined = out.warnings.join("\n");
-	for needle in [
-		"network_mode",
-		"privileged",
-		"profiles",
-		"volumes_from",
-		"scale/replicas",
-	] {
+	for needle in ["network_mode", "profiles", "volumes_from", "scale/replicas"] {
 		assert!(joined.contains(needle), "expected warning for {needle}");
 	}
 }
@@ -334,3 +327,123 @@ volumes:
 	// An option with no dedicated key falls back to PodmanArgs=--opt.
 	assert!(vol.contains("PodmanArgs=--opt custom=extra"), "in:\n{vol}");
 }
+
+#[test]
+fn healthcheck_start_interval_is_warned_and_omitted() {
+	// `start_interval` has no Quadlet/Podman equivalent: it must not emit a
+	// `HealthStartupInterval=` (which drives an unrelated, no-op startup
+	// healthcheck) and must instead produce a warning.
+	let yaml = r#"
+services:
+  s:
+    image: x
+    healthcheck:
+      test: ["CMD", "true"]
+      interval: 5s
+      start_interval: 2s
+"#;
+	let file = parse_str(yaml).unwrap();
+	let out = generate(&file, "p");
+	let c = &unit_named(&out, "s.container").contents;
+	assert!(c.contains("HealthInterval=5s"), "in:\n{c}");
+	assert!(
+		!c.contains("HealthStartupInterval"),
+		"start_interval must not emit HealthStartupInterval=; got:\n{c}"
+	);
+	let joined = out.warnings.join("\n");
+	assert!(
+		joined.contains("start_interval"),
+		"start_interval must warn; got:\n{joined}"
+	);
+}
+
+#[test]
+fn dependency_unit_names_are_sanitized_in_ordering() {
+	// A dependency whose compose key sanitizes to a different stem must be
+	// referenced by that stem in After=/Requires=, matching the generated unit.
+	let yaml = r#"
+services:
+  web:
+    image: nginx
+    depends_on:
+      - "db:1"
+  ? "db:1"
+  : { image: postgres }
+"#;
+	let file = parse_str(yaml).unwrap();
+	let out = generate(&file, "proj");
+	let web = &unit_named(&out, "web.container").contents;
+	assert!(web.contains("After=db_1.service"), "in:\n{web}");
+	assert!(web.contains("Requires=db_1.service"), "in:\n{web}");
+	// The raw, unsanitized name must not leak into the ordering directives.
+	assert!(!web.contains("db:1.service"), "in:\n{web}");
+	// The dependency's own unit really is named with the sanitized stem.
+	unit_named(&out, "db_1.container");
+}
+
+#[test]
+fn network_mode_service_and_container_map_to_dot_container() {
+	// `network_mode: service:X` / `container:X` reuse another container's netns,
+	// which Quadlet expresses as `Network={X}.container`.
+	for (mode, target) in [("service:db", "db"), ("container:sidecar", "sidecar")] {
+		let yaml = format!("services:\n  s:\n    image: x\n    network_mode: \"{mode}\"\n");
+		let file = parse_str(&yaml).unwrap();
+		let out = generate(&file, "p");
+		let c = &unit_named(&out, "s.container").contents;
+		assert!(
+			c.contains(&format!("Network={target}.container")),
+			"{mode} must map to Network={target}.container; got:\n{c}"
+		);
+	}
+}
+
+#[test]
+fn network_mode_service_target_is_sanitized() {
+	let yaml = "services:\n  s:\n    image: x\n    network_mode: \"service:web:1\"\n";
+	let file = parse_str(yaml).unwrap();
+	let out = generate(&file, "p");
+	let c = &unit_named(&out, "s.container").contents;
+	assert!(c.contains("Network=web_1.container"), "in:\n{c}");
+}
+
+#[test]
+fn volume_and_network_units_have_no_install_section() {
+	let yaml = r#"
+services:
+  s:
+    image: x
+networks:
+  net:
+volumes:
+  vol:
+"#;
+	let file = parse_str(yaml).unwrap();
+	let out = generate(&file, "p");
+	let net = &unit_named(&out, "net.network").contents;
+	let vol = &unit_named(&out, "vol.volume").contents;
+	assert!(
+		!net.contains("[Install]") && !net.contains("WantedBy"),
+		".network must carry no [Install]; got:\n{net}"
+	);
+	assert!(
+		!vol.contains("[Install]") && !vol.contains("WantedBy"),
+		".volume must carry no [Install]; got:\n{vol}"
+	);
+	// The container unit still carries its [Install].
+	let c = &unit_named(&out, "s.container").contents;
+	assert!(c.contains("[Install]") && c.contains("WantedBy=default.target"));
+}
+
+#[test]
+fn privileged_maps_to_podman_arg() {
+	let yaml = "services:\n  s:\n    image: x\n    privileged: true\n";
+	let file = parse_str(yaml).unwrap();
+	let out = generate(&file, "p");
+	let c = &unit_named(&out, "s.container").contents;
+	assert!(c.contains("PodmanArgs=--privileged"), "in:\n{c}");
+	assert!(
+		!out.warnings.iter().any(|w| w.contains("privileged")),
+		"privileged must be mapped, not warned; got: {:?}",
+		out.warnings
+	);
+}

internal/quadlet/unit/container.rs

@@ -21,11 +21,15 @@ pub(crate) fn container_unit(
 	let mut unit = Section::new("Unit");
 	unit.add("Description", format!("{name} (podup)"));
 	for dep in service.depends_on.service_names() {
-		unit.add("After", format!("{dep}.service"));
+		// The dependency's generated unit is named `{safe_unit_stem(dep)}.container`,
+		// so its service is `{safe_unit_stem(dep)}.service`; reference that, not the
+		// raw compose key, or the ordering would target a non-existent unit.
+		let dep_service = format!("{}.service", safe_unit_stem(&dep));
+		unit.add("After", dep_service.clone());
 		if service.depends_on.required_for(&dep) {
-			unit.add("Requires", format!("{dep}.service"));
+			unit.add("Requires", dep_service);
 		} else {
-			unit.add("Wants", format!("{dep}.service"));
+			unit.add("Wants", dep_service);
 		}
 	}
 
@@ -61,6 +65,11 @@ pub(crate) fn container_unit(
 	if service.read_only == Some(true) {
 		container.add("ReadOnly", "true".to_string());
 	}
+	if service.privileged == Some(true) {
+		// No dedicated [Container] key exists for privileged mode; pass it through
+		// as a raw podman flag, like the other escape-hatch fields.
+		container.add("PodmanArgs", "--privileged".to_string());
+	}
 	if service.init == Some(true) {
 		container.add("RunInit", "true".to_string());
 	}
@@ -215,13 +224,23 @@ pub(crate) fn container_unit(
 			container.add("StopTimeout", secs.to_string());
 		}
 	}
-	// `network_mode: host`/`none` map to `Network=host`/`Network=none`; other
-	// modes (service:/container:) have no Quadlet key and are reported by
+	// `network_mode: host`/`none` map to `Network=host`/`Network=none`.
+	// `service:X`/`container:X` reuse another container's netns, which Quadlet
+	// expresses as `Network={X}.container` (the `.container` special case);
+	// other modes (bridge:, custom, …) have no key and are reported by
 	// collect_warnings.
 	match service.network_mode.as_deref() {
 		Some("host") => container.add("Network", "host".to_string()),
 		Some("none") => container.add("Network", "none".to_string()),
-		_ => {}
+		Some(m) => {
+			if let Some(target) = m
+				.strip_prefix("service:")
+				.or_else(|| m.strip_prefix("container:"))
+			{
+				container.add("Network", format!("{}.container", safe_unit_stem(target)));
+			}
+		}
+		None => {}
 	}
 	for group in &service.group_add {
 		container.add("GroupAdd", group.clone());
@@ -284,7 +303,7 @@ pub(crate) fn container_unit(
 	for secret in &service.secrets {
 		container.add("Secret", render_secret(secret));
 	}
-	render_healthcheck(service, &mut container);
+	render_healthcheck(name, service, &mut container, warnings);
 
 	let mut svc = Section::new("Service");
 	if let Some(restart) = &service.restart {

internal/quadlet/unit/health.rs

@@ -7,8 +7,14 @@ use super::Section;
 /// Map a compose `healthcheck:` onto the Quadlet `Health*=` keys. A disabled
 /// healthcheck emits `HealthCmd=none`; otherwise the compose test (with any
 /// leading `CMD`/`CMD-SHELL`/`NONE` sentinel stripped) and the timing fields
-/// are rendered.
-pub(super) fn render_healthcheck(service: &Service, container: &mut Section) {
+/// are rendered. Fields with no Quadlet/Podman equivalent push a warning into
+/// `warnings` instead of being silently dropped.
+pub(super) fn render_healthcheck(
+	name: &str,
+	service: &Service,
+	container: &mut Section,
+	warnings: &mut Vec<String>,
+) {
 	let Some(hc) = &service.healthcheck else {
 		return;
 	};
@@ -43,7 +49,15 @@ pub(super) fn render_healthcheck(service: &Service, container: &mut Section) {
 	if let Some(v) = &hc.start_period {
 		container.add("HealthStartPeriod", v.clone());
 	}
-	if let Some(v) = &hc.start_interval {
-		container.add("HealthStartupInterval", v.clone());
+	if hc.start_interval.is_some() {
+		// Compose `start_interval` (the probe interval during the start period)
+		// has no Quadlet/Podman equivalent. The Quadlet `HealthStartupInterval=`
+		// key drives Podman's separate *startup healthcheck* feature, which is a
+		// no-op without a `HealthStartupCmd=`; Podman 5.x exposes no
+		// `--health-start-interval`. Skip it and warn rather than emit a key that
+		// silently does nothing.
+		warnings.push(format!(
+			"{name}: healthcheck.start_interval has no Quadlet/Podman equivalent and is skipped"
+		));
 	}
 }

internal/quadlet/unit/network.rs

@@ -54,8 +54,10 @@ pub(crate) fn network_unit(
 			net.add("Label", format!("{key}={val}"));
 		}
 	}
-	let mut contents = net.render();
-	contents.push_str("\n[Install]\nWantedBy=default.target\n");
+	// No [Install] section: the spec defines none for `.network` units, which are
+	// pulled in automatically as dependencies of the `.container` units that use
+	// them. Only `.container` units carry [Install].
+	let contents = net.render();
 	QuadletUnit {
 		filename: format!("{}.network", safe_unit_stem(name)),
 		contents,

internal/quadlet/unit/volume.rs

@@ -32,8 +32,10 @@ pub(crate) fn volume_unit(name: &str, project: &str, config: Option<&VolumeConfi
 			vol.add("Label", format!("{key}={val}"));
 		}
 	}
-	let mut contents = vol.render();
-	contents.push_str("\n[Install]\nWantedBy=default.target\n");
+	// No [Install] section: the spec defines none for `.volume` units, which are
+	// pulled in automatically as dependencies of the `.container` units that use
+	// them. Only `.container` units carry [Install].
+	let contents = vol.render();
 	QuadletUnit {
 		filename: format!("{}.volume", safe_unit_stem(name)),
 		contents,

internal/quadlet/warnings.rs

@@ -30,26 +30,20 @@ pub(super) fn collect_warnings(name: &str, service: &Service, warnings: &mut Vec
 	if !service.volumes_from.is_empty() {
 		warn("volumes_from", "has no Quadlet equivalent and is skipped");
 	}
-	// `network_mode: host`/`none` map to `Network=`; other modes have no key.
-	if service
-		.network_mode
-		.as_deref()
-		.is_some_and(|m| m != "host" && m != "none")
-	{
+	// `host`/`none` map to `Network=`, and `service:X`/`container:X` map to
+	// `Network=X.container`; only the remaining modes (bridge:, custom, …) have
+	// no key.
+	if service.network_mode.as_deref().is_some_and(|m| {
+		m != "host" && m != "none" && !m.starts_with("service:") && !m.starts_with("container:")
+	}) {
 		warn(
 			"network_mode",
-			"is not mapped (only `host`/`none` are supported); use networks instead",
+			"is not mapped (only `host`/`none`/`service:`/`container:` are supported); use networks instead",
 		);
 	}
 	if !service.profiles.is_empty() {
 		warn("profiles", "have no Quadlet equivalent and are ignored");
 	}
-	if service.privileged == Some(true) {
-		warn(
-			"privileged",
-			"is not mapped; add PodmanArgs manually if required",
-		);
-	}
 	if !service.post_start.is_empty() {
 		warn(
 			"post_start",
@@ -163,8 +157,7 @@ services:
     image: app:1.0
     build: .
     scale: 3
-    privileged: true
-    network_mode: "container:other"
+    network_mode: "bridge:custom"
     volumes_from:
       - other
     profiles:
@@ -193,7 +186,6 @@ configs:
 			"volumes_from",
 			"network_mode",
 			"profiles",
-			"privileged",
 		] {
 			assert!(
 				joined.contains(field),
@@ -205,6 +197,26 @@ configs:
 			!joined.contains("secrets"),
 			"secrets should be mapped, not warned; got:\n{joined}"
 		);
+		// privileged is now mapped to PodmanArgs=--privileged, not warned.
+		assert!(
+			!joined.contains("privileged"),
+			"privileged should be mapped, not warned; got:\n{joined}"
+		);
+	}
+
+	#[test]
+	fn service_and_container_network_modes_are_mapped_not_warned() {
+		// `service:X`/`container:X` map to `Network=X.container`, so they must not
+		// warn; only other unmapped modes (bridge:, custom, …) warn.
+		for mode in ["service:db", "container:other"] {
+			let yaml = format!("services:\n  s:\n    image: x\n    network_mode: \"{mode}\"\n");
+			let file = parse_str(&yaml).unwrap();
+			let joined = generate(&file, "proj").warnings.join("\n");
+			assert!(
+				!joined.contains("network_mode"),
+				"{mode} should be mapped, not warned; got:\n{joined}"
+			);
+		}
 	}
 
 	#[test]

tests/quadlet.rs

@@ -84,11 +84,11 @@ networks:
 
 	assert_eq!(
 		unit(&out, "cache.volume"),
-		"[Volume]\nVolumeName=myproj_cache\n\n[Install]\nWantedBy=default.target\n"
+		"[Volume]\nVolumeName=myproj_cache\n"
 	);
 	assert_eq!(
 		unit(&out, "backend.network"),
-		"[Network]\nNetworkName=myproj_backend\n\n[Install]\nWantedBy=default.target\n"
+		"[Network]\nNetworkName=myproj_backend\n"
 	);
 }