Skip to content

Commit

Permalink
Merge pull request #19635 from Luap99/utf8-log-tag
Browse files Browse the repository at this point in the history
libpod: correctly pass env so alternative locales work
  • Loading branch information
openshift-merge-robot authored Aug 17, 2023
2 parents e0b8178 + c726cf8 commit 938a3e1
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 8 deletions.
2 changes: 1 addition & 1 deletion contrib/cirrus/lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
5 changes: 5 additions & 0 deletions contrib/cirrus/setup_environment.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 13 additions & 2 deletions libpod/oci_conmon_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
5 changes: 1 addition & 4 deletions libpod/oci_conmon_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 err
}
return nil
return writeConmonPipeData(startFd)
}

// GetLimits converts spec resource limits to cgroup consumable limits
Expand Down
56 changes: 55 additions & 1 deletion test/e2e/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package integration

import (
"fmt"
"os"
"os/exec"
"time"

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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")
}
}
}

0 comments on commit 938a3e1

Please sign in to comment.