Merge pull request #19635 from Luap99/utf8-log-tag

libpod: correctly pass env so alternative locales work

contrib/cirrus/lib.sh

@@ -96,7 +96,7 @@ EPOCH_TEST_COMMIT="$CIRRUS_BASE_SHA"
 PASSTHROUGH_ENV_EXACT='CGROUP_MANAGER|DEST_BRANCH|DISTRO_NV|GOCACHE|GOPATH|GOSRC|NETWORK_BACKEND|OCI_RUNTIME|ROOTLESS_USER|SCRIPT_BASE|SKIP_USERNS|EC2_INST_TYPE|PODMAN_DB'
 
 # List of envariable patterns which must match AT THE BEGINNING of the name.
-PASSTHROUGH_ENV_ATSTART='CI|TEST'
+PASSTHROUGH_ENV_ATSTART='CI|LANG|LC_|TEST'
 
 # List of envariable patterns which can match ANYWHERE in the name
 PASSTHROUGH_ENV_ANYWHERE='_NAME|_FQIN'

contrib/cirrus/setup_environment.sh

@@ -150,6 +150,11 @@ case "$OS_RELEASE_ID" in
         # (Checked on 2023-08-08 and it's still too old: 1.1.5)
         # FIXME: please remove this once runc >= 1.2 makes it into debian.
         modprobe tun
+
+        # TODO: move this into image build process
+        # We need the "en_US.UTF-8" locale for the "podman logs with non ASCII log tag" tests
+        sed -i '/en_US.UTF-8/s/^#//g' /etc/locale.gen
+        locale-gen
         ;;
     fedora)
         if ((CONTAINER==0)); then

libpod/oci_conmon_common.go

@@ -1261,12 +1261,20 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
 		return 0, err
 	}
 	if err := r.moveConmonToCgroupAndSignal(ctr, cmd, parentStartPipe); err != nil {
-		return 0, err
+		// The child likely already exited in which case the cmd.Wait() below should return the proper error.
+		// EPIPE is expected if the child already exited so not worth to log and kill the process.
+		if !errors.Is(err, syscall.EPIPE) {
+			logrus.Errorf("Failed to signal conmon to start: %v", err)
+			if err := cmd.Process.Kill(); err != nil && !errors.Is(err, syscall.ESRCH) {
+				logrus.Errorf("Failed to kill conmon after error: %v", err)
+			}
+		}
 	}
+
 	/* Wait for initial setup and fork, and reap child */
 	err = cmd.Wait()
 	if err != nil {
-		return 0, err
+		return 0, fmt.Errorf("conmon failed: %w", err)
 	}
 
 	pid, err := readConmonPipeData(r.name, parentSyncPipe, ociLog)
@@ -1311,6 +1319,9 @@ func (r *ConmonOCIRuntime) configureConmonEnv(runtimeDir string) []string {
 		if strings.HasPrefix(e, "LC_") {
 			env = append(env, e)
 		}
+		if strings.HasPrefix(e, "LANG=") {
+			env = append(env, e)
+		}
 	}
 	if path, ok := os.LookupEnv("PATH"); ok {
 		env = append(env, fmt.Sprintf("PATH=%s", path))

libpod/oci_conmon_linux.go

@@ -162,10 +162,7 @@ func (r *ConmonOCIRuntime) moveConmonToCgroupAndSignal(ctr *Container, cmd *exec
 	}
 
 	/* We set the cgroup, now the child can start creating children */
-	if err := writeConmonPipeData(startFd); err != nil {
+	return writeConmonPipeData(startFd)
-		return err
-	}
-	return nil
 }
 
 // GetLimits converts spec resource limits to cgroup consumable limits

test/e2e/logs_test.go

@@ -2,6 +2,7 @@ package integration
 
 import (
 	"fmt"
+	"os"
 	"os/exec"
 	"time"
 
@@ -584,6 +585,38 @@ var _ = Describe("Podman logs", func() {
 		Expect(logs.ErrorToString()).To(ContainSubstring("this container is using the 'none' log driver, cannot read logs: this container is not logging output"))
 	})
 
+	It("podman logs with non ASCII log tag fails without correct LANG", func() {
+		SkipIfJournaldUnavailable()
+		// need to set the LANG to something that does not support german umlaute to trigger the failure case
+		cleanup := setLangEnv("C")
+		if IsRemote() {
+			podmanTest.RestartRemoteService()
+		}
+		defer cleanup()
+		logc := podmanTest.Podman([]string{"run", "--log-driver", "journald", "--log-opt", "tag=äöüß", ALPINE, "echo", "podman"})
+		logc.WaitWithDefaultTimeout()
+		Expect(logc).To(Exit(126))
+		if !IsRemote() {
+			// Error is only seen on local client
+			// Why does conmon log this to stdout? This must be fixed after https://github.com/containers/conmon/pull/447.
+			Expect(logc.OutputToString()).To(Equal("conmon: option parsing failed: Invalid byte sequence in conversion input"))
+		}
+		Expect(logc.ErrorToString()).To(ContainSubstring("conmon failed: exit status 1"))
+	})
+
+	It("podman logs with non ASCII log tag succeeds with proper env", func() {
+		SkipIfJournaldUnavailable()
+		cleanup := setLangEnv("en_US.UTF-8")
+		if IsRemote() {
+			podmanTest.RestartRemoteService()
+		}
+		defer cleanup()
+		logc := podmanTest.Podman([]string{"run", "--log-driver", "journald", "--log-opt", "tag=äöüß", ALPINE, "echo", "podman"})
+		logc.WaitWithDefaultTimeout()
+		Expect(logc).To(Exit(0))
+		Expect(logc.OutputToString()).To(Equal("podman"))
+	})
+
 	It("podman pod logs with container names", func() {
 		SkipIfRemote("Remote can only process one container at a time")
 		podName := "testPod"
@@ -638,5 +671,26 @@ var _ = Describe("Podman logs", func() {
 			g.Expect(output[1]).To(MatchRegexp(`\x1b\[3[0-9a-z ]+\x1b\[0m`))
 		}).Should(Succeed())
 	})
-
 })
+
+func setLangEnv(lang string) func() {
+	oldLang, okLang := os.LookupEnv("LANG")
+	oldLcAll, okLc := os.LookupEnv("LC_ALL")
+	err := os.Setenv("LANG", lang)
+	Expect(err).ToNot(HaveOccurred())
+	err = os.Setenv("LC_ALL", "")
+	Expect(err).ToNot(HaveOccurred())
+
+	return func() {
+		if okLang {
+			os.Setenv("LANG", oldLang)
+		} else {
+			os.Unsetenv("LANG")
+		}
+		if okLc {
+			os.Setenv("LC_ALL", oldLcAll)
+		} else {
+			os.Unsetenv("LC_ALL")
+		}
+	}
+}