From 2597eeae7001812fdc2417ca7f879bacea524ddc Mon Sep 17 00:00:00 2001 From: "Farya L. Maerten" Date: Thu, 31 Oct 2024 22:17:40 +0100 Subject: [PATCH] Add key to control if a container can get started by its pod By default today, the container is always started if its pod is also started. This prevents to create custom with systemd where containers in a pod could be started through their `[Install]` section. We add a key `StartWithPod=`, enabled by default, that enables one to disable that behavior. This prevents the pod service from changing the state of the container service. Fixes #24401 Signed-off-by: Farya L. Maerten --- cmd/quadlet/main.go | 6 +++--- docs/source/markdown/podman-systemd.unit.5.md | 11 +++++++++++ pkg/systemd/quadlet/quadlet.go | 14 ++++++++++---- test/e2e/quadlet/startwithpod.pod | 7 +++++++ test/e2e/quadlet/startwithpod_no.container | 7 +++++++ test/e2e/quadlet/startwithpod_yes.container | 7 +++++++ test/e2e/quadlet_test.go | 7 +++++++ 7 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 test/e2e/quadlet/startwithpod.pod create mode 100644 test/e2e/quadlet/startwithpod_no.container create mode 100644 test/e2e/quadlet/startwithpod_yes.container diff --git a/cmd/quadlet/main.go b/cmd/quadlet/main.go index e8ea99ed47..bf584c10c9 100644 --- a/cmd/quadlet/main.go +++ b/cmd/quadlet/main.go @@ -615,9 +615,9 @@ func generateUnitsInfoMap(units []*parser.UnitFile) map[string]*quadlet.UnitInfo } unitsInfoMap[unit.Filename] = &quadlet.UnitInfo{ - ServiceName: serviceName, - Containers: containers, - ResourceName: resourceName, + ServiceName: serviceName, + ContainersToStart: containers, + ResourceName: resourceName, } } diff --git a/docs/source/markdown/podman-systemd.unit.5.md b/docs/source/markdown/podman-systemd.unit.5.md index e2cafd9b55..0623e89b84 100644 --- a/docs/source/markdown/podman-systemd.unit.5.md +++ b/docs/source/markdown/podman-systemd.unit.5.md @@ -333,6 +333,7 @@ Valid options for `[Container]` are listed below: | SecurityLabelNested=true | --security-opt label=nested | | SecurityLabelType=spc_t | --security-opt label=type:spc_t | | ShmSize=100m | --shm-size=100m | +| StartWithPod=true | If Pod= is defined, container is started by pod | | StopSignal=SIGINT | --stop-signal=SIGINT | | StopTimeout=20 | --stop-timeout=20 | | SubGIDMap=gtest | --subgidname=gtest | @@ -813,6 +814,16 @@ Size of /dev/shm. This is equivalent to the Podman `--shm-size` option and generally has the form `number[unit]` +### `StartWithPod=` + +Start the container after the associated pod is created. Default to **true**. + +If `true`, container will be started/stopped/restarted alongside the pod. + +If `false`, the container will not be started when the pod starts. The container will be stopped with the pod. Restarting the pod will also restart the container as long as the container was also running before. + +Note, the container can still be started manually or through a target by configuring the `[Install]` section. The pod will be started as needed in any case. + ### `StopSignal=` Signal to stop a container. Default is **SIGTERM**. diff --git a/pkg/systemd/quadlet/quadlet.go b/pkg/systemd/quadlet/quadlet.go index 3e957200de..7eee1311ab 100644 --- a/pkg/systemd/quadlet/quadlet.go +++ b/pkg/systemd/quadlet/quadlet.go @@ -154,6 +154,7 @@ const ( KeyServiceName = "ServiceName" KeySetWorkingDirectory = "SetWorkingDirectory" KeyShmSize = "ShmSize" + KeyStartWithPod = "StartWithPod" KeyStopSignal = "StopSignal" KeyStopTimeout = "StopTimeout" KeySubGIDMap = "SubGIDMap" @@ -185,8 +186,8 @@ type UnitInfo struct { ResourceName string // For .pod units - // List of containers in a pod - Containers []string + // List of containers to start with the pod + ContainersToStart []string } var ( @@ -267,6 +268,7 @@ var ( KeyServiceName: true, KeyShmSize: true, KeyStopSignal: true, + KeyStartWithPod: true, KeyStopTimeout: true, KeySubGIDMap: true, KeySubUIDMap: true, @@ -1563,7 +1565,7 @@ func ConvertPod(podUnit *parser.UnitFile, name string, unitsInfoMap map[string]* // Need the containers filesystem mounted to start podman service.Add(UnitGroup, "RequiresMountsFor", "%t/containers") - for _, containerService := range unitInfo.Containers { + for _, containerService := range unitInfo.ContainersToStart { service.Add(UnitGroup, "Wants", containerService) service.Add(UnitGroup, "Before", containerService) } @@ -2133,7 +2135,11 @@ func handlePod(quadletUnitFile, serviceUnitFile *parser.UnitFile, groupName stri serviceUnitFile.Add(UnitGroup, "BindsTo", podServiceName) serviceUnitFile.Add(UnitGroup, "After", podServiceName) - podInfo.Containers = append(podInfo.Containers, serviceUnitFile.Filename) + // 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. + if quadletUnitFile.LookupBooleanWithDefault(groupName, KeyStartWithPod, true) { + podInfo.ContainersToStart = append(podInfo.ContainersToStart, serviceUnitFile.Filename) + } } return nil } diff --git a/test/e2e/quadlet/startwithpod.pod b/test/e2e/quadlet/startwithpod.pod new file mode 100644 index 0000000000..75505f9e55 --- /dev/null +++ b/test/e2e/quadlet/startwithpod.pod @@ -0,0 +1,7 @@ +## assert-key-contains "Unit" "Wants" "startwithpod_yes.service" +## assert-key-contains "Unit" "Before" "startwithpod_yes.service" + +## assert-key-not-contains "Unit" "Wants" "startwithpod_no.service" +## assert-key-not-contains "Unit" "Before" "startwithpod_no.service" + +[Pod] diff --git a/test/e2e/quadlet/startwithpod_no.container b/test/e2e/quadlet/startwithpod_no.container new file mode 100644 index 0000000000..0c07cfdb03 --- /dev/null +++ b/test/e2e/quadlet/startwithpod_no.container @@ -0,0 +1,7 @@ +# assert-key-contains "Unit" "After" "startwithpod-pod.service" +# assert-key-contains "Unit" "BindsTo" "startwithpod-pod.service" + +[Container] +Image=localhost/image +Pod=startwithpod.pod +StartWithPod=no diff --git a/test/e2e/quadlet/startwithpod_yes.container b/test/e2e/quadlet/startwithpod_yes.container new file mode 100644 index 0000000000..d5c608f266 --- /dev/null +++ b/test/e2e/quadlet/startwithpod_yes.container @@ -0,0 +1,7 @@ +# assert-key-contains "Unit" "After" "startwithpod-pod.service" +# assert-key-contains "Unit" "BindsTo" "startwithpod-pod.service" + +[Container] +Image=localhost/image +Pod=startwithpod.pod +StartWithPod=yes diff --git a/test/e2e/quadlet_test.go b/test/e2e/quadlet_test.go index ffed49a8a2..27d623bea5 100644 --- a/test/e2e/quadlet_test.go +++ b/test/e2e/quadlet_test.go @@ -227,6 +227,10 @@ func (t *quadletTestcase) assertKeyContains(args []string, unit *parser.UnitFile return ok && strings.Contains(realValue, value) } +func (t *quadletTestcase) assertKeyNotContains(args []string, unit *parser.UnitFile) bool { + return !t.assertKeyContains(args, unit) +} + func (t *quadletTestcase) assertPodmanArgs(args []string, unit *parser.UnitFile, key string, allowRegex, globalOnly bool) bool { podmanArgs, _ := unit.LookupLastArgs("Service", key) if globalOnly { @@ -516,6 +520,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-key-not-contains": + ok = t.assertKeyNotContains(args, unit) case "assert-last-key-is-regex": ok = t.assertLastKeyIsRegex(args, unit) case "assert-podman-args": @@ -1122,6 +1128,7 @@ BOGUS=foo Entry("Pod - Quadlet Volume", "volume.pod", []string{"basic.volume"}), Entry("Pod - Quadlet Network overriding service name", "network.servicename.quadlet.pod", []string{"service-name.network"}), Entry("Pod - Quadlet Volume overriding service name", "volume.servicename.pod", []string{"service-name.volume"}), + Entry("Pod - Do not autostart a container with pod", "startwithpod.pod", []string{"startwithpod_no.container", "startwithpod_yes.container"}), ) })