Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libpod: correctly pass env so alternative locales work #19635

Merged
merged 5 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a linter to catch such cases. Not sure whether it's enabled though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clearly not otherwise it should have flagged this one

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")
}
}
}