diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index f4d46721c7..a13dd51833 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -1170,9 +1170,9 @@ func AutocompleteLogDriver(_ *cobra.Command, _ []string, _ string) ([]string, co } // AutocompleteLogOpt - Autocomplete log-opt options. -// -> "path=", "tag=" +// -> "path=", "tag=", "max-size=", "max-file=", "log-rotate=", "label=" func AutocompleteLogOpt(_ *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) { - logOptions := []string{"path=", "tag=", "max-size=", "label="} + logOptions := []string{"path=", "tag=", "max-size=", "max-file=", "log-rotate=", "label="} if strings.HasPrefix(toComplete, "path=") { return nil, cobra.ShellCompDirectiveDefault } diff --git a/docs/source/markdown/options/log-opt.md b/docs/source/markdown/options/log-opt.md index c554898cf8..c50a5b5089 100644 --- a/docs/source/markdown/options/log-opt.md +++ b/docs/source/markdown/options/log-opt.md @@ -18,6 +18,17 @@ Set custom logging configuration. The following *name*s are supported: **max-size**: specify a max size of the log file (e.g. << '**LogOpt=max-size=10mb**' if is_quadlet else '**--log-opt max-size=10mb**' >>); +**max-file**: specify the maximum number of rotated log files to keep when log +rotation is enabled (requires conmon >= 2.2.0) + (e.g. << '**LogOpt=max-file=5**' if is_quadlet else '**--log-opt max-file=5**' >>). +Setting this option implicitly enables log rotation. If log rotation is enabled +without setting **max-file**, conmon keeps 1 backup file by default. + +**log-rotate**: enable or disable log rotation for the container log file +(requires conmon >= 2.2.0). When enabled, the log file is rotated with a numbered +suffix instead of being truncated when it reaches the maximum size + (e.g. << '**LogOpt=log-rotate=true**' if is_quadlet else '**--log-opt log-rotate=true**' >>). + **tag**: specify a custom log tag for the container (e.g. << '**LogOpt=tag="{{.ImageName}}"**' if is_quadlet else '**--log-opt tag="{{.ImageName}}"**' >>. It supports the same keys as **podman inspect --format**. diff --git a/libpod/container.go b/libpod/container.go index 618c863a6d..c91fb41660 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -674,6 +674,16 @@ func (c *Container) LogLabels() map[string]string { return c.config.LogLabels } +// LogRotate returns whether log rotation is enabled for the container's log file. +func (c *Container) LogRotate() bool { + return c.config.LogRotate +} + +// LogMaxFiles returns the maximum number of rotated log files kept for the container. +func (c *Container) LogMaxFiles() uint { + return c.config.LogMaxFiles +} + // RestartPolicy returns the container's restart policy. func (c *Container) RestartPolicy() string { return c.config.RestartPolicy diff --git a/libpod/container_config.go b/libpod/container_config.go index 28f33ba292..516a71b9b4 100644 --- a/libpod/container_config.go +++ b/libpod/container_config.go @@ -381,6 +381,12 @@ type ContainerMiscConfig struct { LogSize int64 `json:"logSize"` // LogDriver driver for logs LogDriver string `json:"logDriver"` + // LogRotate indicates whether log files should be rotated instead of + // truncated when the maximum size is reached. + LogRotate bool `json:"logRotate,omitempty"` + // LogMaxFiles is the maximum number of rotated log files to keep when + // log rotation is enabled (LogRotate == true). + LogMaxFiles uint `json:"logMaxFiles,omitempty"` // File containing the conmon PID ConmonPidFile string `json:"conmonPidFile,omitempty"` // RestartPolicy indicates what action the container will take upon diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index c9c1f6d06e..368c029cad 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -7,6 +7,7 @@ import ( "fmt" "maps" "slices" + "strconv" "strings" "github.com/docker/go-units" @@ -496,6 +497,19 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named logConfig.Size = units.HumanSize(float64(c.LogSizeMax())) logConfig.Tag = c.config.LogTag + // Populate Config map with log rotation settings when configured. + if c.config.LogRotate || c.config.LogMaxFiles > 0 { + if logConfig.Config == nil { + logConfig.Config = make(map[string]string) + } + if c.config.LogRotate { + logConfig.Config["log-rotate"] = "true" + } + if c.config.LogMaxFiles > 0 { + logConfig.Config["max-file"] = strconv.FormatUint(uint64(c.config.LogMaxFiles), 10) + } + } + hostConfig.LogConfig = logConfig restartPolicy := new(define.InspectRestartPolicy) diff --git a/libpod/oci_conmon_common.go b/libpod/oci_conmon_common.go index d80f986287..5cc23ce58d 100644 --- a/libpod/oci_conmon_common.go +++ b/libpod/oci_conmon_common.go @@ -1389,6 +1389,16 @@ func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, p args = append(args, "--log-size-max", strconv.FormatInt(size, 10)) } + // Log rotation options: --log-rotate enables rotation; --log-max-files + // sets the number of backup files to keep. + // If max-file is not specified, conmon keeps 1 backup file by default when --log-rotate is passed. + if ctr.LogRotate() { + args = append(args, "--log-rotate") + } + if logMaxFiles := ctr.LogMaxFiles(); logMaxFiles > 0 { + args = append(args, "--log-max-files", strconv.FormatUint(uint64(logMaxFiles), 10)) + } + if ociLogPath != "" { args = append(args, "--runtime-arg", "--log-format=json", "--runtime-arg", "--log", fmt.Sprintf("--runtime-arg=%s", ociLogPath)) } diff --git a/libpod/options.go b/libpod/options.go index dba9e5d1fa..ea970c019b 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1065,6 +1065,34 @@ func WithLogLabels(logLabels map[string]string) CtrCreateOption { } } +// WithLogRotate sets whether the container's log file should be rotated +// instead of truncated when the maximum size is reached. +func WithLogRotate(rotate bool) CtrCreateOption { + return func(ctr *Container) error { + if ctr.valid { + return define.ErrCtrFinalized + } + + ctr.config.LogRotate = rotate + + return nil + } +} + +// WithLogMaxFiles sets the maximum number of rotated log files to keep. +// This is only meaningful when log rotation is enabled. +func WithLogMaxFiles(maxFiles uint) CtrCreateOption { + return func(ctr *Container) error { + if ctr.valid { + return define.ErrCtrFinalized + } + + ctr.config.LogMaxFiles = maxFiles + + return nil + } +} + // WithCgroupsMode disables the creation of Cgroups for the conmon process. func WithCgroupsMode(mode string) CtrCreateOption { return func(ctr *Container) error { diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index c27f649208..2ccb9d8329 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" "github.com/jinzhu/copier" @@ -597,7 +598,49 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l if len(s.LogConfiguration.Driver) > 0 { options = append(options, libpod.WithLogDriver(s.LogConfiguration.Driver)) } + // Log rotation options: accepted as --log-opt max-file= or + // --log-opt log-rotate=true|false from the CLI or REST API. + if len(s.LogConfiguration.Options) > 0 { + var logMaxFiles uint + var logRotate *bool + + if rotateStr, ok := s.LogConfiguration.Options["log-rotate"]; ok { + rotate, err := strconv.ParseBool(rotateStr) + if err != nil { + return nil, fmt.Errorf("invalid log option log-rotate %q: %w", rotateStr, err) + } + logRotate = &rotate + } + + if maxFileStr, ok := s.LogConfiguration.Options["max-file"]; ok { + n, err := strconv.ParseUint(maxFileStr, 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid log option max-file %q: %w", maxFileStr, err) + } + if n == 0 { + return nil, fmt.Errorf("invalid log option max-file %q: must be greater than 0", maxFileStr) + } + logMaxFiles = uint(n) + } + + if logMaxFiles > 0 { + if logRotate != nil && !*logRotate { + return nil, fmt.Errorf("conflicting log options: max-file cannot be set when log-rotate is false") + } + options = append(options, libpod.WithLogMaxFiles(logMaxFiles)) + // Setting max-file implicitly enables log rotation unless explicitly specified. + if logRotate == nil { + trueVal := true + logRotate = &trueVal + } + } + + if logRotate != nil { + options = append(options, libpod.WithLogRotate(*logRotate)) + } + } } + if s.LabelNested != nil { options = append(options, libpod.WithLabelNested(*s.LabelNested)) } diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index 38b800febf..1a20e187bc 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -896,6 +896,24 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions return err } s.LogConfiguration.Size = logSize + case "max-file": + // Validate that the value is a positive integer greater than 0, + // then store it under the canonical key so container_create.go can consume it. + maxFiles, err := strconv.ParseUint(val, 10, 64) + if err != nil { + return fmt.Errorf("invalid value for log option %q: must be a positive integer: %w", key, err) + } + if maxFiles == 0 { + return fmt.Errorf("invalid value for log option %q: must be greater than 0", key) + } + logOpts["max-file"] = val + case "log-rotate": + // Validate that the value is a boolean, then store it under the + // canonical key so container_create.go can consume it. + if _, err := strconv.ParseBool(val); err != nil { + return fmt.Errorf("invalid value for log option %q: must be true or false: %w", key, err) + } + logOpts["log-rotate"] = val case "label": labelKey, labelVal, hasVal := strings.Cut(val, "=") if !hasVal { @@ -908,6 +926,11 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions } if len(s.LogConfiguration.Options) == 0 || len(c.LogOptions) != 0 { + if mf, ok := logOpts["max-file"]; ok && mf != "" { + if lr, ok := logOpts["log-rotate"]; ok && lr == "false" { + return fmt.Errorf("conflicting log options: max-file cannot be set when log-rotate is false") + } + } s.LogConfiguration.Options = logOpts } if len(s.LogConfiguration.Labels) == 0 || len(c.LogOptions) != 0 { diff --git a/pkg/specgenutil/specgenutil_test.go b/pkg/specgenutil/specgenutil_test.go index 0cf8e1a9f2..cbe957d950 100644 --- a/pkg/specgenutil/specgenutil_test.go +++ b/pkg/specgenutil/specgenutil_test.go @@ -231,3 +231,109 @@ func TestFillOutSpecGenRecorsUserNs(t *testing.T) { assert.True(t, ok, "UserNsAnnotation is set") assert.Equal(t, "keep-id", v, "UserNsAnnotation is keep-id") } + +func TestLogOptMaxFile(t *testing.T) { + tests := []struct { + desc string + logOpts []string + wantVal string + wantErr bool + }{ + { + desc: "valid max-file integer", + logOpts: []string{"max-file=5"}, + wantVal: "5", + }, + { + desc: "max-file of 1", + logOpts: []string{"max-file=1"}, + wantVal: "1", + }, + { + desc: "invalid max-file string", + logOpts: []string{"max-file=notanumber"}, + wantErr: true, + }, + { + desc: "invalid max-file negative", + logOpts: []string{"max-file=-3"}, + wantErr: true, + }, + { + desc: "invalid max-file zero", + logOpts: []string{"max-file=0"}, + wantErr: true, + }, + { + desc: "conflicting max-file with log-rotate=false", + logOpts: []string{"max-file=5", "log-rotate=false"}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + sg := specgen.NewSpecGenerator("nothing", false) + sg.LogConfiguration = &specgen.LogConfig{} + err := FillOutSpecGen(sg, &entities.ContainerCreateOptions{ + ImageVolume: "ignore", + LogOptions: tt.logOpts, + }, []string{}) + if tt.wantErr { + assert.Error(t, err, "expected an error for %q", tt.logOpts) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.wantVal, sg.LogConfiguration.Options["max-file"], + "max-file stored in LogConfiguration.Options") + }) + } +} + +func TestLogOptLogRotate(t *testing.T) { + tests := []struct { + desc string + logOpts []string + wantVal string + wantErr bool + }{ + { + desc: "log-rotate true", + logOpts: []string{"log-rotate=true"}, + wantVal: "true", + }, + { + desc: "log-rotate false", + logOpts: []string{"log-rotate=false"}, + wantVal: "false", + }, + { + desc: "log-rotate 1", + logOpts: []string{"log-rotate=1"}, + wantVal: "1", + }, + { + desc: "invalid log-rotate value", + logOpts: []string{"log-rotate=maybe"}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + sg := specgen.NewSpecGenerator("nothing", false) + sg.LogConfiguration = &specgen.LogConfig{} + err := FillOutSpecGen(sg, &entities.ContainerCreateOptions{ + ImageVolume: "ignore", + LogOptions: tt.logOpts, + }, []string{}) + if tt.wantErr { + assert.Error(t, err, "expected an error for %q", tt.logOpts) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.wantVal, sg.LogConfiguration.Options["log-rotate"], + "log-rotate stored in LogConfiguration.Options") + }) + } +} diff --git a/rpm/podman.spec b/rpm/podman.spec index d51b60096e..83473fc8f8 100644 --- a/rpm/podman.spec +++ b/rpm/podman.spec @@ -99,7 +99,7 @@ BuildRequires: sqlite-devel BuildRequires: systemd BuildRequires: systemd-devel Requires: catatonit -Requires: conmon >= 2:2.1.7-2 +Requires: conmon >= 2:2.2.0 # Podman 6 requires the new config file layout. Requires: containers-common-extra >= 5:0.68.0 %if %{defined sequoia} diff --git a/test/apiv2/python/rest_api/test_v2_0_0_container.py b/test/apiv2/python/rest_api/test_v2_0_0_container.py index 868fd264a3..0f23f9cdd4 100644 --- a/test/apiv2/python/rest_api/test_v2_0_0_container.py +++ b/test/apiv2/python/rest_api/test_v2_0_0_container.py @@ -438,6 +438,62 @@ class ContainerTestCase(APITestCase): self.assertEqual(2000, out["HostConfig"]["MemorySwap"]) self.assertEqual(1000, out["HostConfig"]["Memory"]) + def test_log_rotation(self): + # Libpod create endpoint with log rotation options in SpecGenerator + r = requests.post( + self.podman_url + "/v1.4.0/libpod/containers/create", + json={ + "image": "alpine:latest", + "name": "log_rotate_libpod", + "command": ["top"], + "log_configuration": { + "driver": "k8s-file", + "options": { + "max-file": "5", + "log-rotate": "true", + }, + }, + }, + ) + self.assertEqual(r.status_code, 201, r.text) + payload = r.json() + container_id = payload["Id"] + self.assertIsNotNone(container_id) + + r = requests.get(self.podman_url + f"/v1.40/containers/{container_id}/json") + self.assertEqual(r.status_code, 200, r.text) + out = r.json() + self.assertEqual("5", out["HostConfig"]["LogConfig"]["Config"]["max-file"]) + self.assertEqual("true", out["HostConfig"]["LogConfig"]["Config"]["log-rotate"]) + + # Compat create endpoint with log rotation options in HostConfig.LogConfig + r = requests.post( + self.podman_url + "/v1.40/containers/create?name=log_rotate_compat", + json={ + "Image": "alpine:latest", + "Cmd": ["top"], + "HostConfig": { + "LogConfig": { + "Type": "k8s-file", + "Config": { + "max-file": "3", + "log-rotate": "true", + }, + }, + }, + }, + ) + self.assertEqual(r.status_code, 201, r.text) + payload = r.json() + container_id = payload["Id"] + self.assertIsNotNone(container_id) + + r = requests.get(self.podman_url + f"/v1.40/containers/{container_id}/json") + self.assertEqual(r.status_code, 200, r.text) + out = r.json() + self.assertEqual("3", out["HostConfig"]["LogConfig"]["Config"]["max-file"]) + self.assertEqual("true", out["HostConfig"]["LogConfig"]["Config"]["log-rotate"]) + def test_host_config_port_bindings(self): # create a container with two ports exposed, but only one of the ports bound r = requests.post( diff --git a/test/e2e/log_rotation_test.go b/test/e2e/log_rotation_test.go new file mode 100644 index 0000000000..7c21eb82bc --- /dev/null +++ b/test/e2e/log_rotation_test.go @@ -0,0 +1,117 @@ +//go:build linux || freebsd + +package integration + +import ( + "fmt" + "os" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "go.podman.io/podman/v6/test/utils" +) + +var _ = Describe("Podman log rotation options", func() { + It("podman run --log-opt max-file sets rotation file count", func() { + SkipIfConmonVersionLessThan("2.2.0") + + logDir := GinkgoT().TempDir() + logPath := filepath.Join(logDir, "ctr.log") + + podmanTest.PodmanExitCleanly( + "run", "--rm", + "--log-driver", "k8s-file", + "--log-opt", fmt.Sprintf("path=%s", logPath), + "--log-opt", "max-file=3", + ALPINE, "echo", "hello rotation", + ) + + // The log file must exist after the container exits. + _, err := os.Stat(logPath) + Expect(err).ShouldNot(HaveOccurred(), "log file should exist at %s", logPath) + + // Verify max-file appears in podman inspect output. + ctrName := "log-rotate-inspect" + podmanTest.PodmanExitCleanly( + "create", + "--name", ctrName, + "--log-driver", "k8s-file", + "--log-opt", "max-size=1mb", + "--log-opt", "max-file=5", + ALPINE, "echo", "hello", + ) + + inspect := podmanTest.PodmanExitCleanly( + "inspect", "--format", + "{{index .HostConfig.LogConfig.Config \"max-file\"}}", + ctrName, + ) + Expect(inspect.OutputToString()).To(Equal("5")) + + inspectRotate := podmanTest.PodmanExitCleanly( + "inspect", "--format", + "{{index .HostConfig.LogConfig.Config \"log-rotate\"}}", + ctrName, + ) + Expect(inspectRotate.OutputToString()).To(Equal("true")) + }) + + It("podman run --log-opt log-rotate=true sets log rotation", func() { + ctrName := "log-rotate-true" + podmanTest.PodmanExitCleanly( + "create", + "--name", ctrName, + "--log-driver", "k8s-file", + "--log-opt", "max-size=1mb", + "--log-opt", "log-rotate=true", + ALPINE, "echo", "hello", + ) + + inspect := podmanTest.PodmanExitCleanly( + "inspect", "--format", + "{{index .HostConfig.LogConfig.Config \"log-rotate\"}}", + ctrName, + ) + Expect(inspect.OutputToString()).To(Equal("true")) + }) + + It("podman run rejects conflicting --log-opt max-file and log-rotate=false", func() { + session := podmanTest.Podman([]string{ + "run", "--rm", + "--log-opt", "max-file=5", + "--log-opt", "log-rotate=false", + ALPINE, "true", + }) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitWithError(125, "conflicting log options")) + }) + + It("podman run --log-opt log-rotate=true without max-size", func() { + ctrName := "log-rotate-no-maxsize" + podmanTest.PodmanExitCleanly( + "create", + "--name", ctrName, + "--log-driver", "k8s-file", + "--log-opt", "log-rotate=true", + ALPINE, "echo", "hello", + ) + + inspect := podmanTest.PodmanExitCleanly( + "inspect", "--format", + "{{index .HostConfig.LogConfig.Config \"log-rotate\"}}", + ctrName, + ) + Expect(inspect.OutputToString()).To(Equal("true")) + }) + + It("podman run rejects invalid --log-opt max-file=0", func() { + session := podmanTest.Podman([]string{ + "run", "--rm", + "--log-opt", "max-file=0", + ALPINE, "true", + }) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitWithError(125, "invalid value for log option")) + }) +})