From 092ff38129cd31badda0ed8801a89f3c032fa08b Mon Sep 17 00:00:00 2001 From: zhangguanzhang Date: Thu, 16 Jul 2020 23:22:22 +0800 Subject: fix play kube doesn't override dockerfile ENTRYPOINT Signed-off-by: zhangguanzhang --- pkg/domain/infra/abi/play.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'pkg/domain') diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index f82da2c95..888811958 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -453,11 +453,16 @@ func kubeContainerToCreateConfig(ctx context.Context, containerYAML v1.Container containerConfig.Command = []string{} if imageData != nil && imageData.Config != nil { - containerConfig.Command = append(containerConfig.Command, imageData.Config.Entrypoint...) + containerConfig.Command = imageData.Config.Entrypoint } if len(containerYAML.Command) != 0 { - containerConfig.Command = append(containerConfig.Command, containerYAML.Command...) - } else if imageData != nil && imageData.Config != nil { + containerConfig.Command = containerYAML.Command + } + // doc https://kubernetes.io/docs/tasks/inject-data-application/define-command-argument-container/#notes + if len(containerYAML.Args) != 0 { + containerConfig.Command = append(containerConfig.Command, containerYAML.Args...) + } else if len(containerYAML.Command) == 0 { + // Add the Cmd from the image config only if containerYAML.Command and containerYAML.Args are empty containerConfig.Command = append(containerConfig.Command, imageData.Config.Cmd...) } if imageData != nil && len(containerConfig.Command) == 0 { -- cgit v1.2.3-54-g00ecf From 2e35dfa20c60a80ca12ffab68a7fcdac797feb3d Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Sun, 19 Jul 2020 22:55:27 +0200 Subject: fix: system df error when an image has no name When an image has no name/tag system df will error because it tries to parse an empty name. This commit makes sure we only parse non empty names and set the repository and tag to "" otherwise. Closes #7015 Signed-off-by: Paul Holzinger --- pkg/domain/infra/abi/system.go | 19 ++++++++++++------- test/e2e/system_df_test.go | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) (limited to 'pkg/domain') diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index 855ca7ff9..be14f52b8 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -252,13 +252,18 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System } } - named, err := reference.ParseNormalizedNamed(name) - if err != nil { - return nil, err - } - repository = named.Name() - if tagged, isTagged := named.(reference.NamedTagged); isTagged { - tag = tagged.Tag() + if len(name) > 0 { + named, err := reference.ParseNormalizedNamed(name) + if err != nil { + return nil, err + } + repository = named.Name() + if tagged, isTagged := named.(reference.NamedTagged); isTagged { + tag = tagged.Tag() + } + } else { + repository = "" + tag = "" } report := entities.SystemDfImageReport{ diff --git a/test/e2e/system_df_test.go b/test/e2e/system_df_test.go index e882756f9..3d7aaf659 100644 --- a/test/e2e/system_df_test.go +++ b/test/e2e/system_df_test.go @@ -60,4 +60,18 @@ var _ = Describe("podman system df", func() { Expect(containers[1]).To(Equal("2")) Expect(volumes[2]).To(Equal("1")) }) + + It("podman system df image with no tag", func() { + session := podmanTest.PodmanNoCache([]string{"create", ALPINE}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.PodmanNoCache([]string{"image", "untag", ALPINE}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.PodmanNoCache([]string{"system", "df"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + }) }) -- cgit v1.2.3-54-g00ecf From e21a6368f9308292641ed5ce58321b1cd46abdc9 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Fri, 17 Jul 2020 13:58:22 +0200 Subject: abi: set default umask and rlimits the code got lost in the migration to podman 2.0, reintroduce it. Closes: https://github.com/containers/podman/issues/6989 Signed-off-by: Giuseppe Scrivano Signed-off-by: Matthew Heon --- cmd/podman/early_init_linux.go | 39 ++++++++++++++++++++++++++++++++++++ cmd/podman/early_init_unsupported.go | 6 ++++++ cmd/podman/root.go | 1 + libpod/define/config.go | 10 +++++++++ pkg/domain/infra/abi/system.go | 22 -------------------- pkg/spec/spec.go | 15 +++++++------- pkg/specgen/generate/oci.go | 15 +++++++------- 7 files changed, 70 insertions(+), 38 deletions(-) create mode 100644 cmd/podman/early_init_linux.go create mode 100644 cmd/podman/early_init_unsupported.go (limited to 'pkg/domain') diff --git a/cmd/podman/early_init_linux.go b/cmd/podman/early_init_linux.go new file mode 100644 index 000000000..b43450a7f --- /dev/null +++ b/cmd/podman/early_init_linux.go @@ -0,0 +1,39 @@ +package main + +import ( + "fmt" + "os" + "syscall" + + "github.com/containers/libpod/v2/libpod/define" + "github.com/pkg/errors" +) + +func setRLimits() error { + rlimits := new(syscall.Rlimit) + rlimits.Cur = define.RLimitDefaultValue + rlimits.Max = define.RLimitDefaultValue + if err := syscall.Setrlimit(syscall.RLIMIT_NOFILE, rlimits); err != nil { + if err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, rlimits); err != nil { + return errors.Wrapf(err, "error getting rlimits") + } + rlimits.Cur = rlimits.Max + if err := syscall.Setrlimit(syscall.RLIMIT_NOFILE, rlimits); err != nil { + return errors.Wrapf(err, "error setting new rlimits") + } + } + return nil +} + +func setUMask() { + // Be sure we can create directories with 0755 mode. + syscall.Umask(0022) +} + +func earlyInitHook() { + if err := setRLimits(); err != nil { + fmt.Fprint(os.Stderr, "Failed to set rlimits: "+err.Error()) + } + + setUMask() +} diff --git a/cmd/podman/early_init_unsupported.go b/cmd/podman/early_init_unsupported.go new file mode 100644 index 000000000..4e748559f --- /dev/null +++ b/cmd/podman/early_init_unsupported.go @@ -0,0 +1,6 @@ +// +build !linux + +package main + +func earlyInitHook() { +} diff --git a/cmd/podman/root.go b/cmd/podman/root.go index 7c54da91a..b2c9f9c2c 100644 --- a/cmd/podman/root.go +++ b/cmd/podman/root.go @@ -77,6 +77,7 @@ func init() { cobra.OnInitialize( loggingHook, syslogHook, + earlyInitHook, ) rootFlags(rootCmd, registry.PodmanConfig()) diff --git a/libpod/define/config.go b/libpod/define/config.go index 900a363d8..64b24d9e2 100644 --- a/libpod/define/config.go +++ b/libpod/define/config.go @@ -75,3 +75,13 @@ const JSONLogging = "json-file" // NoLogging is the string conmon expects when specifying to use no log driver whatsoever const NoLogging = "none" + +// Strings used for --sdnotify option to podman +const ( + SdNotifyModeContainer = "container" + SdNotifyModeConmon = "conmon" + SdNotifyModeIgnore = "ignore" +) + +// DefaultRlimitValue is the value set by default for nofile and nproc +const RLimitDefaultValue = uint64(1048576) diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index be14f52b8..435902ded 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -8,7 +8,6 @@ import ( "os/exec" "path/filepath" "strconv" - "syscall" "github.com/containers/common/pkg/config" "github.com/containers/libpod/v2/libpod/define" @@ -146,27 +145,6 @@ func movePauseProcessToScope() error { return utils.RunUnderSystemdScope(int(pid), "user.slice", "podman-pause.scope") } -func setRLimits() error { // nolint:deadcode,unused - rlimits := new(syscall.Rlimit) - rlimits.Cur = 1048576 - rlimits.Max = 1048576 - if err := syscall.Setrlimit(syscall.RLIMIT_NOFILE, rlimits); err != nil { - if err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, rlimits); err != nil { - return errors.Wrapf(err, "error getting rlimits") - } - rlimits.Cur = rlimits.Max - if err := syscall.Setrlimit(syscall.RLIMIT_NOFILE, rlimits); err != nil { - return errors.Wrapf(err, "error setting new rlimits") - } - } - return nil -} - -func setUMask() { // nolint:deadcode,unused - // Be sure we can create directories with 0755 mode. - syscall.Umask(0022) -} - // checkInput can be used to verify any of the globalopt values func checkInput() error { // nolint:deadcode,unused return nil diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 6f52b88b1..b974772d5 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -505,10 +505,9 @@ func BlockAccessToKernelFilesystems(privileged, pidModeIsHost bool, g *generate. func addRlimits(config *CreateConfig, g *generate.Generator) error { var ( - kernelMax uint64 = 1048576 - isRootless = rootless.IsRootless() - nofileSet = false - nprocSet = false + isRootless = rootless.IsRootless() + nofileSet = false + nprocSet = false ) for _, u := range config.Resources.Ulimit { @@ -538,8 +537,8 @@ func addRlimits(config *CreateConfig, g *generate.Generator) error { // files and number of processes to the maximum they can be set to // (without overriding a sysctl) if !nofileSet { - max := kernelMax - current := kernelMax + max := define.RLimitDefaultValue + current := define.RLimitDefaultValue if isRootless { var rlimit unix.Rlimit if err := unix.Getrlimit(unix.RLIMIT_NOFILE, &rlimit); err != nil { @@ -555,8 +554,8 @@ func addRlimits(config *CreateConfig, g *generate.Generator) error { g.AddProcessRlimits("RLIMIT_NOFILE", max, current) } if !nprocSet { - max := kernelMax - current := kernelMax + max := define.RLimitDefaultValue + current := define.RLimitDefaultValue if isRootless { var rlimit unix.Rlimit if err := unix.Getrlimit(unix.RLIMIT_NPROC, &rlimit); err != nil { diff --git a/pkg/specgen/generate/oci.go b/pkg/specgen/generate/oci.go index f770b0582..b62e851b3 100644 --- a/pkg/specgen/generate/oci.go +++ b/pkg/specgen/generate/oci.go @@ -20,10 +20,9 @@ import ( func addRlimits(s *specgen.SpecGenerator, g *generate.Generator) error { var ( - kernelMax uint64 = 1048576 - isRootless = rootless.IsRootless() - nofileSet = false - nprocSet = false + isRootless = rootless.IsRootless() + nofileSet = false + nprocSet = false ) if s.Rlimits == nil { @@ -45,8 +44,8 @@ func addRlimits(s *specgen.SpecGenerator, g *generate.Generator) error { // files and number of processes to the maximum they can be set to // (without overriding a sysctl) if !nofileSet { - max := kernelMax - current := kernelMax + max := define.RLimitDefaultValue + current := define.RLimitDefaultValue if isRootless { var rlimit unix.Rlimit if err := unix.Getrlimit(unix.RLIMIT_NOFILE, &rlimit); err != nil { @@ -62,8 +61,8 @@ func addRlimits(s *specgen.SpecGenerator, g *generate.Generator) error { g.AddProcessRlimits("RLIMIT_NOFILE", max, current) } if !nprocSet { - max := kernelMax - current := kernelMax + max := define.RLimitDefaultValue + current := define.RLimitDefaultValue if isRootless { var rlimit unix.Rlimit if err := unix.Getrlimit(unix.RLIMIT_NPROC, &rlimit); err != nil { -- cgit v1.2.3-54-g00ecf From 0630d19b34c6cea674fc5f830fe308b8a1206259 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 7 Jul 2020 17:19:59 -0400 Subject: Fix container and pod create commands for remote create In `podman inspect` output for containers and pods, we include the command that was used to create the container. This is also used by `podman generate systemd --new` to generate unit files. With remote podman, the generated create commands were incorrect since we sourced directly from os.Args on the server side, which was guaranteed to be `podman system service` (or some variant thereof). The solution is to pass the command along in the Specgen or PodSpecgen, where we can source it from the client's os.Args. This will still be VERY iffy for mixed local/remote use (doing a `podman --remote run ...` on a remote client then a `podman generate systemd --new` on the server on the same container will not work, because the `--remote` flag will slip in) but at the very least the output of `podman inspect` will be correct. We can look into properly handling `--remote` (parsing it out would be a little iffy) in a future PR. Signed-off-by: Matthew Heon Signed-off-by: Matthew Heon --- cmd/podman/common/specgen.go | 3 +++ cmd/podman/pods/create.go | 2 ++ libpod/options.go | 8 ++++---- pkg/domain/entities/pods.go | 2 ++ pkg/spec/createconfig.go | 4 ---- pkg/specgen/generate/container_create.go | 4 +++- pkg/specgen/generate/pod_create.go | 4 +++- pkg/specgen/podspecgen.go | 6 ++++++ pkg/specgen/specgen.go | 7 +++++++ pkg/varlinkapi/create.go | 4 ---- 10 files changed, 30 insertions(+), 14 deletions(-) (limited to 'pkg/domain') diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index b4f786da2..3722d45b2 100644 --- a/cmd/podman/common/specgen.go +++ b/cmd/podman/common/specgen.go @@ -378,6 +378,9 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string } var command []string + // Include the command used to create the container. + s.ContainerCreateCommand = os.Args + // Build the command // If we have an entry point, it goes first if c.Entrypoint != nil { diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go index 0e2a085fd..d57a2f2f7 100644 --- a/cmd/podman/pods/create.go +++ b/cmd/podman/pods/create.go @@ -149,6 +149,8 @@ func create(cmd *cobra.Command, args []string) error { } } + createOptions.CreateCommand = os.Args + if replace { if err := replacePod(createOptions.Name); err != nil { return err diff --git a/libpod/options.go b/libpod/options.go index 3120a35d7..bff3f3c18 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1371,12 +1371,12 @@ func WithHealthCheck(healthCheck *manifest.Schema2HealthConfig) CtrCreateOption // WithCreateCommand adds the full command plus arguments of the current // process to the container config. -func WithCreateCommand() CtrCreateOption { +func WithCreateCommand(cmd []string) CtrCreateOption { return func(ctr *Container) error { if ctr.valid { return define.ErrCtrFinalized } - ctr.config.CreateCommand = os.Args + ctr.config.CreateCommand = cmd return nil } } @@ -1553,12 +1553,12 @@ func WithPodHostname(hostname string) PodCreateOption { // WithPodCreateCommand adds the full command plus arguments of the current // process to the pod config. -func WithPodCreateCommand() PodCreateOption { +func WithPodCreateCommand(createCmd []string) PodCreateOption { return func(pod *Pod) error { if pod.valid { return define.ErrPodFinalized } - pod.config.CreateCommand = os.Args + pod.config.CreateCommand = createCmd return nil } } diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go index 8023034ef..9e9b834ef 100644 --- a/pkg/domain/entities/pods.go +++ b/pkg/domain/entities/pods.go @@ -104,6 +104,7 @@ type PodRmReport struct { type PodCreateOptions struct { CGroupParent string + CreateCommand []string Hostname string Infra bool InfraImage string @@ -133,6 +134,7 @@ func (p PodCreateOptions) ToPodSpecGen(s *specgen.PodSpecGenerator) { } s.InfraImage = p.InfraImage s.SharedNamespaces = p.Share + s.PodCreateCommand = p.CreateCommand // Networking config s.NetNS = p.Net.Network diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go index 879c66895..55c3238d2 100644 --- a/pkg/spec/createconfig.go +++ b/pkg/spec/createconfig.go @@ -406,10 +406,6 @@ func CreateContainerFromCreateConfig(ctx context.Context, r *libpod.Runtime, cre return nil, err } - // Set the CreateCommand explicitly. Some (future) consumers of libpod - // might not want to set it. - options = append(options, libpod.WithCreateCommand()) - ctr, err := r.NewContainer(ctx, runtimeSpec, options...) if err != nil { return nil, err diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index daf7f84d4..be1e3b48e 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -78,7 +78,9 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener } options := []libpod.CtrCreateOption{} - options = append(options, libpod.WithCreateCommand()) + if s.ContainerCreateCommand != nil { + options = append(options, libpod.WithCreateCommand(s.ContainerCreateCommand)) + } var newImage *image.Image if s.Rootfs != "" { diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index 690651a23..4fe1b6435 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -93,7 +93,9 @@ func createPodOptions(p *specgen.PodSpecGenerator) ([]libpod.PodCreateOption, er options = append(options, libpod.WithInfraContainerPorts(ports)) } options = append(options, libpod.WithPodCgroups()) - options = append(options, libpod.WithPodCreateCommand()) + if p.PodCreateCommand != nil { + options = append(options, libpod.WithPodCreateCommand(p.PodCreateCommand)) + } if len(p.InfraConmonPidFile) > 0 { options = append(options, libpod.WithInfraConmonPidFile(p.InfraConmonPidFile)) } diff --git a/pkg/specgen/podspecgen.go b/pkg/specgen/podspecgen.go index 600d27004..3c32ec365 100644 --- a/pkg/specgen/podspecgen.go +++ b/pkg/specgen/podspecgen.go @@ -49,6 +49,12 @@ type PodBasicConfig struct { // Conflicts with NoInfra=true. // Optional. SharedNamespaces []string `json:"shared_namespaces,omitempty"` + // PodCreateCommand is the command used to create this pod. + // This will be shown in the output of Inspect() on the pod, and may + // also be used by some tools that wish to recreate the pod + // (e.g. `podman generate systemd --new`). + // Optional. + PodCreateCommand []string `json:"pod_create_command,omitempty"` } // PodNetworkConfig contains networking configuration for a pod. diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go index 91bc36709..bd738f5a7 100644 --- a/pkg/specgen/specgen.go +++ b/pkg/specgen/specgen.go @@ -131,6 +131,13 @@ type ContainerBasicConfig struct { // Remove indicates if the container should be removed once it has been started // and exits Remove bool `json:"remove,omitempty"` + // ContainerCreateCommand is the command that was used to create this + // container. + // This will be shown in the output of Inspect() on the container, and + // may also be used by some tools that wish to recreate the container + // (e.g. `podman generate systemd --new`). + // Optional. + ContainerCreateCommand []string `json:"containerCreateCommand,omitempty"` } // ContainerStorageConfig contains information on the storage configuration of a diff --git a/pkg/varlinkapi/create.go b/pkg/varlinkapi/create.go index 5c5f075f7..ac93939d9 100644 --- a/pkg/varlinkapi/create.go +++ b/pkg/varlinkapi/create.go @@ -915,10 +915,6 @@ func CreateContainerFromCreateConfig(ctx context.Context, r *libpod.Runtime, cre return nil, err } - // Set the CreateCommand explicitly. Some (future) consumers of libpod - // might not want to set it. - options = append(options, libpod.WithCreateCommand()) - ctr, err := r.NewContainer(ctx, runtimeSpec, options...) if err != nil { return nil, err -- cgit v1.2.3-54-g00ecf From 84076bf95f485bccfd9e03108fb69a1f72b42918 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 9 Jul 2020 13:32:20 +0200 Subject: log API: add context to allow for cancelling Add a `context.Context` to the log APIs to allow for cancelling streaming (e.g., via `podman logs -f`). This fixes issues for the remote API where some go routines of the server will continue writing and produce nothing but heat and waste CPU cycles. Signed-off-by: Valentin Rothberg --- libpod/container_api.go | 2 +- libpod/container_log.go | 23 ++++++--- libpod/container_log_linux.go | 16 ++++++- libpod/container_log_unsupported.go | 4 +- pkg/api/handlers/compat/containers_logs.go | 76 +++++++++++++++--------------- pkg/domain/infra/abi/containers.go | 2 +- pkg/varlinkapi/containers.go | 2 +- 7 files changed, 73 insertions(+), 52 deletions(-) (limited to 'pkg/domain') diff --git a/libpod/container_api.go b/libpod/container_api.go index d19dc651b..487f75e67 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -353,7 +353,7 @@ func (c *Container) HTTPAttach(httpCon net.Conn, httpBuf *bufio.ReadWriter, stre logOpts.WaitGroup.Wait() close(logChan) }() - if err := c.ReadLog(logOpts, logChan); err != nil { + if err := c.ReadLog(context.Background(), logOpts, logChan); err != nil { return err } logrus.Debugf("Done reading logs for container %s, %d bytes", c.ID(), logSize) diff --git a/libpod/container_log.go b/libpod/container_log.go index 97936c683..80f8e6e50 100644 --- a/libpod/container_log.go +++ b/libpod/container_log.go @@ -1,6 +1,7 @@ package libpod import ( + "context" "fmt" "os" "time" @@ -13,9 +14,9 @@ import ( ) // Log is a runtime function that can read one or more container logs. -func (r *Runtime) Log(containers []*Container, options *logs.LogOptions, logChannel chan *logs.LogLine) error { +func (r *Runtime) Log(ctx context.Context, containers []*Container, options *logs.LogOptions, logChannel chan *logs.LogLine) error { for _, ctr := range containers { - if err := ctr.ReadLog(options, logChannel); err != nil { + if err := ctr.ReadLog(ctx, options, logChannel); err != nil { return err } } @@ -23,25 +24,25 @@ func (r *Runtime) Log(containers []*Container, options *logs.LogOptions, logChan } // ReadLog reads a containers log based on the input options and returns loglines over a channel. -func (c *Container) ReadLog(options *logs.LogOptions, logChannel chan *logs.LogLine) error { +func (c *Container) ReadLog(ctx context.Context, options *logs.LogOptions, logChannel chan *logs.LogLine) error { switch c.LogDriver() { case define.NoLogging: return errors.Wrapf(define.ErrNoLogs, "this container is using the 'none' log driver, cannot read logs") case define.JournaldLogging: // TODO Skip sending logs until journald logs can be read - return c.readFromJournal(options, logChannel) + return c.readFromJournal(ctx, options, logChannel) case define.JSONLogging: // TODO provide a separate implementation of this when Conmon // has support. fallthrough case define.KubernetesLogging, "": - return c.readFromLogFile(options, logChannel) + return c.readFromLogFile(ctx, options, logChannel) default: return errors.Wrapf(define.ErrInternal, "unrecognized log driver %q, cannot read logs", c.LogDriver()) } } -func (c *Container) readFromLogFile(options *logs.LogOptions, logChannel chan *logs.LogLine) error { +func (c *Container) readFromLogFile(ctx context.Context, options *logs.LogOptions, logChannel chan *logs.LogLine) error { t, tailLog, err := logs.GetLogFile(c.LogPath(), options) if err != nil { // If the log file does not exist, this is not fatal. @@ -62,8 +63,17 @@ func (c *Container) readFromLogFile(options *logs.LogOptions, logChannel chan *l } go func() { + defer options.WaitGroup.Done() + var partial string for line := range t.Lines { + select { + case <-ctx.Done(): + // the consumer has cancelled + return + default: + // fallthrough + } nll, err := logs.NewLogLine(line.Text) if err != nil { logrus.Error(err) @@ -82,7 +92,6 @@ func (c *Container) readFromLogFile(options *logs.LogOptions, logChannel chan *l logChannel <- nll } } - options.WaitGroup.Done() }() // Check if container is still running or paused if options.Follow { diff --git a/libpod/container_log_linux.go b/libpod/container_log_linux.go index fad3bf87c..00b2039a9 100644 --- a/libpod/container_log_linux.go +++ b/libpod/container_log_linux.go @@ -4,6 +4,7 @@ package libpod import ( + "context" "fmt" "io" "math" @@ -29,7 +30,7 @@ const ( bufLen = 16384 ) -func (c *Container) readFromJournal(options *logs.LogOptions, logChannel chan *logs.LogLine) error { +func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOptions, logChannel chan *logs.LogLine) error { var config journal.JournalReaderConfig if options.Tail < 0 { config.NumFromTail = math.MaxUint64 @@ -65,13 +66,24 @@ func (c *Container) readFromJournal(options *logs.LogOptions, logChannel chan *l if options.Follow { go func() { + done := make(chan bool) + until := make(chan time.Time) + go func() { + select { + case <-ctx.Done(): + until <- time.Time{} + case <-done: + // nothing to do anymore + } + }() follower := FollowBuffer{logChannel} - err := r.Follow(nil, follower) + err := r.Follow(until, follower) if err != nil { logrus.Debugf(err.Error()) } r.Close() options.WaitGroup.Done() + done <- true return }() return nil diff --git a/libpod/container_log_unsupported.go b/libpod/container_log_unsupported.go index 18882720a..f3b36619e 100644 --- a/libpod/container_log_unsupported.go +++ b/libpod/container_log_unsupported.go @@ -3,11 +3,13 @@ package libpod import ( + "context" + "github.com/containers/libpod/v2/libpod/define" "github.com/containers/libpod/v2/libpod/logs" "github.com/pkg/errors" ) -func (c *Container) readFromJournal(options *logs.LogOptions, logChannel chan *logs.LogLine) error { +func (c *Container) readFromJournal(_ context.Context, _ *logs.LogOptions, _ chan *logs.LogLine) error { return errors.Wrapf(define.ErrOSNotSupported, "Journald logging only enabled with systemd on linux") } diff --git a/pkg/api/handlers/compat/containers_logs.go b/pkg/api/handlers/compat/containers_logs.go index 8147f4d38..30ee030e8 100644 --- a/pkg/api/handlers/compat/containers_logs.go +++ b/pkg/api/handlers/compat/containers_logs.go @@ -92,7 +92,7 @@ func LogsFromContainer(w http.ResponseWriter, r *http.Request) { options.WaitGroup = &wg logChannel := make(chan *logs.LogLine, tail+1) - if err := runtime.Log([]*libpod.Container{ctnr}, options, logChannel); err != nil { + if err := runtime.Log(r.Context(), []*libpod.Container{ctnr}, options, logChannel); err != nil { utils.InternalServerError(w, errors.Wrapf(err, "Failed to obtain logs for Container '%s'", name)) return } @@ -105,50 +105,48 @@ func LogsFromContainer(w http.ResponseWriter, r *http.Request) { var frame strings.Builder header := make([]byte, 8) - for ok := true; ok; ok = query.Follow { - for line := range logChannel { - if _, found := r.URL.Query()["until"]; found { - if line.Time.After(until) { - break - } + for line := range logChannel { + if _, found := r.URL.Query()["until"]; found { + if line.Time.After(until) { + break } + } - // Reset buffer we're ready to loop again - frame.Reset() - switch line.Device { - case "stdout": - if !query.Stdout { - continue - } - header[0] = 1 - case "stderr": - if !query.Stderr { - continue - } - header[0] = 2 - default: - // Logging and moving on is the best we can do here. We may have already sent - // a Status and Content-Type to client therefore we can no longer report an error. - log.Infof("unknown Device type '%s' in log file from Container %s", line.Device, ctnr.ID()) + // Reset buffer we're ready to loop again + frame.Reset() + switch line.Device { + case "stdout": + if !query.Stdout { continue } - - if query.Timestamps { - frame.WriteString(line.Time.Format(time.RFC3339)) - frame.WriteString(" ") + header[0] = 1 + case "stderr": + if !query.Stderr { + continue } - frame.WriteString(line.Msg) + header[0] = 2 + default: + // Logging and moving on is the best we can do here. We may have already sent + // a Status and Content-Type to client therefore we can no longer report an error. + log.Infof("unknown Device type '%s' in log file from Container %s", line.Device, ctnr.ID()) + continue + } - binary.BigEndian.PutUint32(header[4:], uint32(frame.Len())) - if _, err := w.Write(header[0:8]); err != nil { - log.Errorf("unable to write log output header: %q", err) - } - if _, err := io.WriteString(w, frame.String()); err != nil { - log.Errorf("unable to write frame string: %q", err) - } - if flusher, ok := w.(http.Flusher); ok { - flusher.Flush() - } + if query.Timestamps { + frame.WriteString(line.Time.Format(time.RFC3339)) + frame.WriteString(" ") + } + frame.WriteString(line.Msg) + + binary.BigEndian.PutUint32(header[4:], uint32(frame.Len())) + if _, err := w.Write(header[0:8]); err != nil { + log.Errorf("unable to write log output header: %q", err) + } + if _, err := io.WriteString(w, frame.String()); err != nil { + log.Errorf("unable to write frame string: %q", err) + } + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() } } } diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 596fc2cc1..8909f831d 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -924,7 +924,7 @@ func (ic *ContainerEngine) ContainerLogs(ctx context.Context, containers []strin } logChannel := make(chan *logs.LogLine, chSize) - if err := ic.Libpod.Log(ctrs, logOpts, logChannel); err != nil { + if err := ic.Libpod.Log(ctx, ctrs, logOpts, logChannel); err != nil { return err } diff --git a/pkg/varlinkapi/containers.go b/pkg/varlinkapi/containers.go index 8650ba000..07b492331 100644 --- a/pkg/varlinkapi/containers.go +++ b/pkg/varlinkapi/containers.go @@ -754,7 +754,7 @@ func (i *VarlinkAPI) GetContainersLogs(call iopodman.VarlinkCall, names []string if err != nil { return call.ReplyErrorOccurred(err.Error()) } - if err := i.Runtime.Log(containers, &options, logChannel); err != nil { + if err := i.Runtime.Log(getContext(), containers, &options, logChannel); err != nil { return err } go func() { -- cgit v1.2.3-54-g00ecf