From 14509a92bb87bc328c67182b9e985fd81f6f7b53 Mon Sep 17 00:00:00 2001 From: Urvashi Mohnani Date: Wed, 29 Sep 2021 16:22:55 -0400 Subject: Allow a value of -1 to set unlimited pids limit Users can set --pids-limit to -1 now to set unlimited pids limit for a container - this matches the convention. [NO TESTS NEEDED] Signed-off-by: Urvashi Mohnani --- pkg/specgen/generate/validate.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'pkg') diff --git a/pkg/specgen/generate/validate.go b/pkg/specgen/generate/validate.go index 50efe7fa3..b0d84825e 100644 --- a/pkg/specgen/generate/validate.go +++ b/pkg/specgen/generate/validate.go @@ -72,10 +72,9 @@ func verifyContainerResourcesCgroupV1(s *specgen.SpecGenerator) ([]string, error // Pids checks if s.ResourceLimits.Pids != nil { - pids := s.ResourceLimits.Pids // TODO: Should this be 0, or checking that ResourceLimits.Pids // is set at all? - if pids.Limit > 0 && !sysInfo.PidsLimit { + if s.ResourceLimits.Pids.Limit >= 0 && !sysInfo.PidsLimit { warnings = append(warnings, "Your kernel does not support pids limit capabilities or the cgroup is not mounted. PIDs limit discarded.") s.ResourceLimits.Pids = nil } -- cgit v1.2.3-54-g00ecf From c6be71486d4357cb3cbb380e846e32780fcb37f5 Mon Sep 17 00:00:00 2001 From: Aditya Rajan Date: Thu, 30 Sep 2021 15:45:32 +0530 Subject: machine: silently cleanup dangling sockets before rm if possible Try to cleanup dandling pid and machine socket if possible silently before `rm`. [NO TESTS NEEDED] Signed-off-by: Aditya Rajan --- pkg/machine/qemu/machine.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'pkg') diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index d0f48da5f..03197fef1 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -460,6 +460,22 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun for _, msg := range files { confirmationMessage += msg + "\n" } + + // Get path to socket and pidFile before we do any cleanups + qemuSocketFile, pidFile, errSocketFile := v.getSocketandPid() + //silently try to delete socket and pid file + //remove socket and pid file if any: warn at low priority if things fail + if errSocketFile == nil { + // Remove the pidfile + if err := os.Remove(pidFile); err != nil && !errors.Is(err, os.ErrNotExist) { + logrus.Debugf("Error while removing pidfile: %v", err) + } + // Remove socket + if err := os.Remove(qemuSocketFile); err != nil && !errors.Is(err, os.ErrNotExist) { + logrus.Debugf("Error while removing podman-machine-socket: %v", err) + } + } + confirmationMessage += "\n" return confirmationMessage, func() error { for _, f := range files { -- cgit v1.2.3-54-g00ecf From 62d12a2adf1a33060e935c09fdbdabf4ecc16f33 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Thu, 30 Sep 2021 10:36:17 -0700 Subject: Add guard for BuildOptions.CommonBuildOpts Existing images.Build() bindings code panicked when field was not initialized. Signed-off-by: Jhon Honce --- pkg/bindings/images/build.go | 5 ++ pkg/bindings/test/fixture/Containerfile | 1 + pkg/bindings/test/images_test.go | 100 ++++++++++++++++++-------------- 3 files changed, 61 insertions(+), 45 deletions(-) create mode 100644 pkg/bindings/test/fixture/Containerfile (limited to 'pkg') diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index 4d667d90a..4c3c83837 100644 --- a/pkg/bindings/images/build.go +++ b/pkg/bindings/images/build.go @@ -16,6 +16,7 @@ import ( "strconv" "strings" + "github.com/containers/buildah/define" "github.com/containers/podman/v3/pkg/auth" "github.com/containers/podman/v3/pkg/bindings" "github.com/containers/podman/v3/pkg/domain/entities" @@ -39,6 +40,10 @@ var ( // Build creates an image using a containerfile reference func Build(ctx context.Context, containerFiles []string, options entities.BuildOptions) (*entities.BuildReport, error) { + if options.CommonBuildOpts == nil { + options.CommonBuildOpts = new(define.CommonBuildOptions) + } + params := url.Values{} if caps := options.AddCapabilities; len(caps) > 0 { diff --git a/pkg/bindings/test/fixture/Containerfile b/pkg/bindings/test/fixture/Containerfile new file mode 100644 index 000000000..3a1031f32 --- /dev/null +++ b/pkg/bindings/test/fixture/Containerfile @@ -0,0 +1 @@ +From quay.io/libpod/alpine_nginx diff --git a/pkg/bindings/test/images_test.go b/pkg/bindings/test/images_test.go index ff8f72c85..aa8ff0537 100644 --- a/pkg/bindings/test/images_test.go +++ b/pkg/bindings/test/images_test.go @@ -9,9 +9,11 @@ import ( "github.com/containers/podman/v3/pkg/bindings" "github.com/containers/podman/v3/pkg/bindings/containers" "github.com/containers/podman/v3/pkg/bindings/images" + "github.com/containers/podman/v3/pkg/domain/entities" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/onsi/gomega/gexec" + . "github.com/onsi/gomega/gexec" + . "github.com/onsi/gomega/gstruct" ) var _ = Describe("Podman images", func() { @@ -20,7 +22,7 @@ var _ = Describe("Podman images", func() { // err error // podmanTest *PodmanTestIntegration bt *bindingTest - s *gexec.Session + s *Session err error ) @@ -37,7 +39,7 @@ var _ = Describe("Podman images", func() { s = bt.startAPIService() time.Sleep(1 * time.Second) err := bt.NewConnection() - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) }) AfterEach(func() { @@ -57,19 +59,19 @@ var _ = Describe("Podman images", func() { // Inspect by short name data, err := images.GetImage(bt.conn, alpine.shortName, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Inspect with full ID _, err = images.GetImage(bt.conn, data.ID, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Inspect with partial ID _, err = images.GetImage(bt.conn, data.ID[0:12], nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Inspect by long name _, err = images.GetImage(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // TODO it looks like the images API always returns size regardless // of bool or not. What should we do ? // Expect(data.Size).To(BeZero()) @@ -77,7 +79,7 @@ var _ = Describe("Podman images", func() { options := new(images.GetOptions).WithSize(true) // Enabling the size parameter should result in size being populated data, err = images.GetImage(bt.conn, alpine.name, options) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(data.Size).To(BeNumerically(">", 0)) }) @@ -90,7 +92,7 @@ var _ = Describe("Podman images", func() { // Remove an image by name, validate image is removed and error is nil inspectData, err := images.GetImage(bt.conn, busybox.shortName, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) response, errs = images.Remove(bt.conn, []string{busybox.shortName}, nil) Expect(len(errs)).To(BeZero()) @@ -101,10 +103,10 @@ var _ = Describe("Podman images", func() { // Start a container with alpine image var top string = "top" _, err = bt.RunTopContainer(&top, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // we should now have a container called "top" running containerResponse, err := containers.Inspect(bt.conn, "top", nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(containerResponse.Name).To(Equal("top")) // try to remove the image "alpine". This should fail since we are not force @@ -115,7 +117,7 @@ var _ = Describe("Podman images", func() { // Removing the image "alpine" where force = true options := new(images.RemoveOptions).WithForce(true) response, errs = images.Remove(bt.conn, []string{alpine.shortName}, options) - Expect(errs).To(BeNil()) + Expect(errs).To(Or(HaveLen(0), BeNil())) // To be extra sure, check if the previously created container // is gone as well. _, err = containers.Inspect(bt.conn, "top", nil) @@ -141,11 +143,11 @@ var _ = Describe("Podman images", func() { // Validates if the image is tagged successfully. err = images.Tag(bt.conn, alpine.shortName, "demo", alpine.shortName, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Validates if name updates when the image is retagged. _, err := images.GetImage(bt.conn, "alpine:demo", nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) }) @@ -154,7 +156,7 @@ var _ = Describe("Podman images", func() { // Array to hold the list of images returned imageSummary, err := images.List(bt.conn, nil) // There Should be no errors in the response. - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Since in the begin context two images are created the // list context should have only 2 images Expect(len(imageSummary)).To(Equal(2)) @@ -163,23 +165,23 @@ var _ = Describe("Podman images", func() { // And the count should be three now. bt.Pull("testimage:20200929") imageSummary, err = images.List(bt.conn, nil) - Expect(err).To(BeNil()) - Expect(len(imageSummary)).To(Equal(3)) + Expect(err).ToNot(HaveOccurred()) + Expect(len(imageSummary)).To(BeNumerically(">=", 2)) // Validate the image names. var names []string for _, i := range imageSummary { names = append(names, i.RepoTags...) } - Expect(StringInSlice(alpine.name, names)).To(BeTrue()) - Expect(StringInSlice(busybox.name, names)).To(BeTrue()) + Expect(names).To(ContainElement(alpine.name)) + Expect(names).To(ContainElement(busybox.name)) // List images with a filter filters := make(map[string][]string) filters["reference"] = []string{alpine.name} options := new(images.ListOptions).WithFilters(filters).WithAll(false) filteredImages, err := images.List(bt.conn, options) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(len(filteredImages)).To(BeNumerically("==", 1)) // List images with a bad filter @@ -194,17 +196,17 @@ var _ = Describe("Podman images", func() { It("Image Exists", func() { // exists on bogus image should be false, with no error exists, err := images.Exists(bt.conn, "foobar", nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeFalse()) // exists with shortname should be true exists, err = images.Exists(bt.conn, alpine.shortName, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeTrue()) // exists with fqname should be true exists, err = images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeTrue()) }) @@ -213,37 +215,37 @@ var _ = Describe("Podman images", func() { _, errs := images.Remove(bt.conn, []string{alpine.name}, nil) Expect(len(errs)).To(BeZero()) exists, err := images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeFalse()) f, err := os.Open(filepath.Join(ImageCacheDir, alpine.tarballName)) defer f.Close() - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) names, err := images.Load(bt.conn, f) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(names.Names[0]).To(Equal(alpine.name)) exists, err = images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeTrue()) // load with a repo name f, err = os.Open(filepath.Join(ImageCacheDir, alpine.tarballName)) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) _, errs = images.Remove(bt.conn, []string{alpine.name}, nil) Expect(len(errs)).To(BeZero()) exists, err = images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeFalse()) names, err = images.Load(bt.conn, f) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(names.Names[0]).To(Equal(alpine.name)) // load with a bad repo name should trigger a 500 f, err = os.Open(filepath.Join(ImageCacheDir, alpine.tarballName)) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) _, errs = images.Remove(bt.conn, []string{alpine.name}, nil) Expect(len(errs)).To(BeZero()) exists, err = images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeFalse()) }) @@ -252,11 +254,11 @@ var _ = Describe("Podman images", func() { exportPath := filepath.Join(bt.tempDirPath, alpine.tarballName) w, err := os.Create(filepath.Join(bt.tempDirPath, alpine.tarballName)) defer w.Close() - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) err = images.Export(bt.conn, []string{alpine.name}, w, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) _, err = os.Stat(exportPath) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // TODO how do we verify that a format change worked? }) @@ -266,21 +268,21 @@ var _ = Describe("Podman images", func() { _, errs := images.Remove(bt.conn, []string{alpine.name}, nil) Expect(len(errs)).To(BeZero()) exists, err := images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeFalse()) f, err := os.Open(filepath.Join(ImageCacheDir, alpine.tarballName)) defer f.Close() - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) changes := []string{"CMD /bin/foobar"} testMessage := "test_import" options := new(images.ImportOptions).WithMessage(testMessage).WithChanges(changes).WithReference(alpine.name) _, err = images.Import(bt.conn, f, options) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) exists, err = images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeTrue()) data, err := images.GetImage(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(data.Comment).To(Equal(testMessage)) }) @@ -294,9 +296,9 @@ var _ = Describe("Podman images", func() { var foundID bool data, err := images.GetImage(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) history, err := images.History(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) for _, i := range history { if i.ID == data.ID { foundID = true @@ -308,7 +310,7 @@ var _ = Describe("Podman images", func() { It("Search for an image", func() { reports, err := images.Search(bt.conn, "alpine", nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(len(reports)).To(BeNumerically(">", 1)) var foundAlpine bool for _, i := range reports { @@ -322,7 +324,7 @@ var _ = Describe("Podman images", func() { // Search for alpine with a limit of 10 options := new(images.SearchOptions).WithLimit(10) reports, err = images.Search(bt.conn, "docker.io/alpine", options) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(len(reports)).To(BeNumerically("<=", 10)) filters := make(map[string][]string) @@ -330,7 +332,7 @@ var _ = Describe("Podman images", func() { // Search for alpine with stars greater than 100 options = new(images.SearchOptions).WithFilters(filters) reports, err = images.Search(bt.conn, "docker.io/alpine", options) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) for _, i := range reports { Expect(i.Stars).To(BeNumerically(">=", 100)) } @@ -367,4 +369,12 @@ var _ = Describe("Podman images", func() { _, err = images.Pull(bt.conn, "bogus-transport:bogus.com/image:reference", nil) Expect(err).To(HaveOccurred()) }) + + It("Build no options", func() { + results, err := images.Build(bt.conn, []string{"fixture/Containerfile"}, entities.BuildOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(*results).To(MatchFields(IgnoreMissing, Fields{ + "ID": Not(BeEmpty()), + })) + }) }) -- cgit v1.2.3-54-g00ecf From 2cd206d0fce477d123ab8b7acf19fdf34ca0a7aa Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 5 Oct 2021 11:13:49 +0200 Subject: libpod: fix race when closing STDIN There is a race where `conn.Close()` was called before `conn.CloseWrite()`. In this case `CloseWrite` will fail and an useless error is printed. To fix this we move the the `CloseWrite()` call to the same goroutine to remove the race. This ensures that `CloseWrite()` is called before `Close()` and never afterwards. Also fixed podman-remote run where the STDIN was never was closed. This is causing flakes in CI testing. [NO TESTS NEEDED] Fixes #11856 Signed-off-by: Paul Holzinger Signed-off-by: Matthew Heon --- libpod/oci_attach_linux.go | 17 +++++++++-------- libpod/oci_conmon_exec_linux.go | 7 ++++--- libpod/oci_conmon_linux.go | 4 ++++ pkg/bindings/containers/attach.go | 14 +++++++------- test/system/030-run.bats | 6 ++++++ 5 files changed, 30 insertions(+), 18 deletions(-) (limited to 'pkg') diff --git a/libpod/oci_attach_linux.go b/libpod/oci_attach_linux.go index de435b58a..702628aa8 100644 --- a/libpod/oci_attach_linux.go +++ b/libpod/oci_attach_linux.go @@ -84,7 +84,7 @@ func (c *Container) attach(streams *define.AttachStreams, keys string, resize <- if attachRdy != nil { attachRdy <- true } - return readStdio(streams, receiveStdoutError, stdinDone) + return readStdio(conn, streams, receiveStdoutError, stdinDone) } // Attach to the given container's exec session @@ -165,7 +165,7 @@ func (c *Container) attachToExec(streams *define.AttachStreams, keys *string, se return err } - return readStdio(streams, receiveStdoutError, stdinDone) + return readStdio(conn, streams, receiveStdoutError, stdinDone) } func processDetachKeys(keys string) ([]byte, error) { @@ -208,11 +208,6 @@ func setupStdioChannels(streams *define.AttachStreams, conn *net.UnixConn, detac var err error if streams.AttachInput { _, err = utils.CopyDetachable(conn, streams.InputStream, detachKeys) - if err == nil { - if connErr := conn.CloseWrite(); connErr != nil { - logrus.Errorf("unable to close conn: %q", connErr) - } - } } stdinDone <- err }() @@ -265,7 +260,7 @@ func redirectResponseToOutputStreams(outputStream, errorStream io.Writer, writeO return err } -func readStdio(streams *define.AttachStreams, receiveStdoutError, stdinDone chan error) error { +func readStdio(conn *net.UnixConn, streams *define.AttachStreams, receiveStdoutError, stdinDone chan error) error { var err error select { case err = <-receiveStdoutError: @@ -274,6 +269,12 @@ func readStdio(streams *define.AttachStreams, receiveStdoutError, stdinDone chan if err == define.ErrDetach { return err } + if err == nil { + // copy stdin is done, close it + if connErr := conn.CloseWrite(); connErr != nil { + logrus.Errorf("Unable to close conn: %v", connErr) + } + } if streams.AttachOutput || streams.AttachError { return <-receiveStdoutError } diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index 5a7677b04..553c91833 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -609,9 +609,6 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp _, err := utils.CopyDetachable(conn, httpBuf, detachKeys) logrus.Debugf("STDIN copy completed") stdinChan <- err - if connErr := conn.CloseWrite(); connErr != nil { - logrus.Errorf("Unable to close conn: %v", connErr) - } }() } @@ -654,6 +651,10 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp if err != nil { return err } + // copy stdin is done, close it + if connErr := conn.CloseWrite(); connErr != nil { + logrus.Errorf("Unable to close conn: %v", connErr) + } case <-cancel: return nil } diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index c2b472f76..58aa3557a 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -699,6 +699,10 @@ func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, req *http.Request, w http. if err != nil { return err } + // copy stdin is done, close it + if connErr := conn.CloseWrite(); connErr != nil { + logrus.Errorf("Unable to close conn: %v", connErr) + } case <-cancel: return nil } diff --git a/pkg/bindings/containers/attach.go b/pkg/bindings/containers/attach.go index 6efbcb57b..725d06648 100644 --- a/pkg/bindings/containers/attach.go +++ b/pkg/bindings/containers/attach.go @@ -157,24 +157,24 @@ func Attach(ctx context.Context, nameOrID string, stdin io.Reader, stdout io.Wri } stdoutChan := make(chan error) - stdinChan := make(chan error) + stdinChan := make(chan error, 1) //stdin channel should not block if isSet.stdin { go func() { logrus.Debugf("Copying STDIN to socket") _, err := utils.CopyDetachable(socket, stdin, detachKeysInBytes) - if err != nil && err != define.ErrDetach { logrus.Error("failed to write input to service: " + err.Error()) } - stdinChan <- err - - if closeWrite, ok := socket.(CloseWriter); ok { - if err := closeWrite.CloseWrite(); err != nil { - logrus.Warnf("Failed to close STDIN for writing: %v", err) + if err == nil { + if closeWrite, ok := socket.(CloseWriter); ok { + if err := closeWrite.CloseWrite(); err != nil { + logrus.Warnf("Failed to close STDIN for writing: %v", err) + } } } + stdinChan <- err }() } diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 3d9d834b3..8640d427a 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -723,4 +723,10 @@ EOF is "$output" "Error: strconv.ParseInt: parsing \"a\": invalid syntax" } +@test "podman run closes stdin" { + random_1=$(random_string 25) + run_podman run -i --rm $IMAGE cat <<<"$random_1" + is "$output" "$random_1" "output matches STDIN" +} + # vim: filetype=sh -- cgit v1.2.3-54-g00ecf From ca33df1468b79dc6dc27798a242b22926c220e6d Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 6 Oct 2021 11:17:43 -0400 Subject: Ensure `podman ps --sync` functions The backend for `ps --sync` has been nonfunctional for a long while now - probably since v2.0. It's questionable how useful the flag is in modern Podman (the original case it was intended to catch, Conmon gone via SIGKILL, should be handled now via pinging the process with a signal to ensure it's still alive) but having the ability to force a refresh of container state from the OCI runtime is still useful. Signed-off-by: Matthew Heon --- libpod/container_api.go | 2 +- libpod/oci_conmon_linux.go | 6 ++++++ pkg/ps/ps.go | 6 ++++++ 3 files changed, 13 insertions(+), 1 deletion(-) (limited to 'pkg') diff --git a/libpod/container_api.go b/libpod/container_api.go index cc845fe2e..c6f459fbd 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -686,7 +686,7 @@ func (c *Container) Sync() error { // If runtime knows about the container, update its status in runtime // And then save back to disk - if c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStatePaused, define.ContainerStateStopped) { + if c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStatePaused, define.ContainerStateStopped, define.ContainerStateStopping) { oldState := c.state.State if err := c.ociRuntime.UpdateContainerStatus(c); err != nil { return err diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 58aa3557a..ea0ef842d 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -351,6 +351,12 @@ func (r *ConmonOCIRuntime) UpdateContainerStatus(ctr *Container) error { return ctr.handleExitFile(exitFile, fi) } + // Handle ContainerStateStopping - keep it unless the container + // transitioned to no longer running. + if oldState == define.ContainerStateStopping && (ctr.state.State == define.ContainerStatePaused || ctr.state.State == define.ContainerStateRunning) { + ctr.state.State = define.ContainerStateStopping + } + return nil } diff --git a/pkg/ps/ps.go b/pkg/ps/ps.go index e65400555..21ce818cb 100644 --- a/pkg/ps/ps.go +++ b/pkg/ps/ps.go @@ -127,6 +127,12 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities ) batchErr := ctr.Batch(func(c *libpod.Container) error { + if opts.Sync { + if err := c.Sync(); err != nil { + return errors.Wrapf(err, "unable to update container state from OCI runtime") + } + } + conConfig = c.Config() conState, err = c.State() if err != nil { -- cgit v1.2.3-54-g00ecf From 338e01f04877578c7fdd0c847da81333fcc5b00b Mon Sep 17 00:00:00 2001 From: Reinhard Tartler Date: Sat, 9 Oct 2021 11:26:31 -0400 Subject: [CI:DOCS] oci-hooks.5.md: fixup section in header This fixes the autodetection of where to install the manpages Signed-off-by: Reinhard Tartler --- pkg/hooks/docs/oci-hooks.5.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/hooks/docs/oci-hooks.5.md b/pkg/hooks/docs/oci-hooks.5.md index d6b866231..9a1a35682 100644 --- a/pkg/hooks/docs/oci-hooks.5.md +++ b/pkg/hooks/docs/oci-hooks.5.md @@ -1,4 +1,4 @@ -% oci-hooks(5) OCI Hooks Configuration +% oci-hooks 5 OCI Hooks Configuration % W. Trevor King % MAY 2018 -- cgit v1.2.3-54-g00ecf From fbe94088fd3dea66afcd7c7d26f1e2454b782b4d Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 11 Oct 2021 16:09:11 +0200 Subject: podman stats: move cgroup validation to server Podman stats is not supported for rootless cgroupv1 setups. The check for this must be on the server side and not the client. [NO NEW TESTS NEEDED] we cannot test this because remote and server are always on the same machine in CI Fixes #11909 Signed-off-by: Paul Holzinger --- cmd/podman/containers/stats.go | 12 ------------ pkg/domain/infra/abi/containers.go | 9 +++++++++ 2 files changed, 9 insertions(+), 12 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/containers/stats.go b/cmd/podman/containers/stats.go index 11e8f6870..d21feaabc 100644 --- a/cmd/podman/containers/stats.go +++ b/cmd/podman/containers/stats.go @@ -11,9 +11,7 @@ import ( "github.com/containers/podman/v3/cmd/podman/registry" "github.com/containers/podman/v3/cmd/podman/validate" "github.com/containers/podman/v3/libpod/define" - "github.com/containers/podman/v3/pkg/cgroups" "github.com/containers/podman/v3/pkg/domain/entities" - "github.com/containers/podman/v3/pkg/rootless" "github.com/containers/podman/v3/utils" "github.com/docker/go-units" "github.com/pkg/errors" @@ -113,16 +111,6 @@ func checkStatOptions(cmd *cobra.Command, args []string) error { } func stats(cmd *cobra.Command, args []string) error { - if rootless.IsRootless() { - unified, err := cgroups.IsCgroup2UnifiedMode() - if err != nil { - return err - } - if !unified { - return errors.New("stats is not supported in rootless mode without cgroups v2") - } - } - // Convert to the entities options. We should not leak CLI-only // options into the backend and separate concerns. opts := entities.ContainerStatsOptions{ diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 774362d03..d1e430f3b 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -1316,6 +1316,15 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri if options.Interval < 1 { return nil, errors.New("Invalid interval, must be a positive number greater zero") } + if rootless.IsRootless() { + unified, err := cgroups.IsCgroup2UnifiedMode() + if err != nil { + return nil, err + } + if !unified { + return nil, errors.New("stats is not supported in rootless mode without cgroups v2") + } + } statsChan = make(chan entities.ContainerStatsReport, 1) containerFunc := ic.Libpod.GetRunningContainers -- cgit v1.2.3-54-g00ecf From 8b87793d4878026d5d4f734409f33fe873160115 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Mon, 11 Oct 2021 09:11:32 -0700 Subject: Use SplitN(2) when copying env variables Environment variables whose value contained an equal sign where truncated Fixes #11891 Signed-off-by: Jhon Honce Signed-off-by: Matthew Heon --- pkg/specgen/generate/kube/kube.go | 2 +- test/e2e/play_kube_test.go | 41 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/specgen/generate/kube/kube.go b/pkg/specgen/generate/kube/kube.go index 27a1e5a72..7072a5821 100644 --- a/pkg/specgen/generate/kube/kube.go +++ b/pkg/specgen/generate/kube/kube.go @@ -254,7 +254,7 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener // Environment Variables envs := map[string]string{} for _, env := range imageData.Config.Env { - keyval := strings.Split(env, "=") + keyval := strings.SplitN(env, "=", 2) envs[keyval[0]] = keyval[1] } diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 0d5b9d52c..c31b89650 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -18,6 +18,7 @@ import ( "github.com/containers/storage/pkg/stringid" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" . "github.com/onsi/gomega/gexec" "github.com/opencontainers/selinux/go-selinux" ) @@ -2722,4 +2723,44 @@ invalid kube kind exists.WaitWithDefaultTimeout() Expect(exists).To(Exit(0)) }) + + Describe("verify environment variables", func() { + var maxLength int + BeforeEach(func() { + maxLength = format.MaxLength + format.MaxLength = 0 + }) + AfterEach(func() { + format.MaxLength = maxLength + }) + + It("values containing equal sign", func() { + javaToolOptions := `-XX:+IgnoreUnrecognizedVMOptions -XX:+IdleTuningGcOnIdle -Xshareclasses:name=openj9_system_scc,cacheDir=/opt/java/.scc,readonly,nonFatal` + openj9JavaOptions := `-XX:+IgnoreUnrecognizedVMOptions -XX:+IdleTuningGcOnIdle -Xshareclasses:name=openj9_system_scc,cacheDir=/opt/java/.scc,readonly,nonFatal -Dosgi.checkConfiguration=false` + + containerfile := fmt.Sprintf(`FROM %s +ENV JAVA_TOOL_OPTIONS=%q +ENV OPENJ9_JAVA_OPTIONS=%q +`, + ALPINE, javaToolOptions, openj9JavaOptions) + + image := "podman-kube-test:env" + podmanTest.BuildImage(containerfile, image, "false") + ctnr := getCtr(withImage(image)) + pod := getPod(withCtr(ctnr)) + Expect(generateKubeYaml("pod", pod, kubeYaml)).Should(Succeed()) + + play := podmanTest.Podman([]string{"play", "kube", "--start", kubeYaml}) + play.WaitWithDefaultTimeout() + Expect(play).Should(Exit(0)) + + inspect := podmanTest.Podman([]string{"container", "inspect", "--format=json", getCtrNameInPod(pod)}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).Should(Exit(0)) + + contents := string(inspect.Out.Contents()) + Expect(contents).To(ContainSubstring(javaToolOptions)) + Expect(contents).To(ContainSubstring(openj9JavaOptions)) + }) + }) }) -- cgit v1.2.3-54-g00ecf From b470de05b072fff1d7a70459a0d4543d0dbc9b0a Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 12 Oct 2021 12:16:32 +0200 Subject: cgroups: use cgroup.controllers to read controllers use the cgroup.controllers file instead of cgroup.subtree_control to read the list of controllers available in the current cgroup. Closes: https://github.com/containers/podman/issues/11931 [NO TESTS NEEDED] we have disabled this test in the CI because it is difficult to know what controllers are going to be enabled for rootless under all conditions we test. Signed-off-by: Giuseppe Scrivano --- pkg/cgroups/cgroups.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'pkg') diff --git a/pkg/cgroups/cgroups.go b/pkg/cgroups/cgroups.go index 4bb8de69b..f1ef538e4 100644 --- a/pkg/cgroups/cgroups.go +++ b/pkg/cgroups/cgroups.go @@ -129,8 +129,8 @@ func init() { func getAvailableControllers(exclude map[string]controllerHandler, cgroup2 bool) ([]controller, error) { if cgroup2 { controllers := []controller{} - subtreeControl := cgroupRoot + "/cgroup.subtree_control" - // rootless cgroupv2: check available controllers for current user ,systemd or servicescope will inherit + controllersFile := cgroupRoot + "/cgroup.controllers" + // rootless cgroupv2: check available controllers for current user, systemd or servicescope will inherit if rootless.IsRootless() { userSlice, err := getCgroupPathForCurrentProcess() if err != nil { @@ -138,13 +138,13 @@ func getAvailableControllers(exclude map[string]controllerHandler, cgroup2 bool) } //userSlice already contains '/' so not adding here basePath := cgroupRoot + userSlice - subtreeControl = fmt.Sprintf("%s/cgroup.subtree_control", basePath) + controllersFile = fmt.Sprintf("%s/cgroup.controllers", basePath) } - subtreeControlBytes, err := ioutil.ReadFile(subtreeControl) + controllersFileBytes, err := ioutil.ReadFile(controllersFile) if err != nil { - return nil, errors.Wrapf(err, "failed while reading controllers for cgroup v2 from %q", subtreeControl) + return nil, errors.Wrapf(err, "failed while reading controllers for cgroup v2 from %q", controllersFile) } - for _, controllerName := range strings.Fields(string(subtreeControlBytes)) { + for _, controllerName := range strings.Fields(string(controllersFileBytes)) { c := controller{ name: controllerName, symlink: false, -- cgit v1.2.3-54-g00ecf From b5dd62f31558caeb3c8cb4b83c0563db9f07cf20 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 14 Oct 2021 10:24:02 -0400 Subject: Don't use docker/pkg/archive, use containers/storage/pkg/archive [NO NEW TESTS NEEDED] Signed-off-by: Daniel J Walsh --- cmd/podman/diff/diff.go | 2 +- libpod/container_copy_linux.go | 2 +- pkg/machine/pull.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/diff/diff.go b/cmd/podman/diff/diff.go index 81bbb6c43..fba4ea540 100644 --- a/cmd/podman/diff/diff.go +++ b/cmd/podman/diff/diff.go @@ -8,7 +8,7 @@ import ( "github.com/containers/common/pkg/report" "github.com/containers/podman/v3/cmd/podman/registry" "github.com/containers/podman/v3/pkg/domain/entities" - "github.com/docker/docker/pkg/archive" + "github.com/containers/storage/pkg/archive" "github.com/pkg/errors" "github.com/spf13/cobra" ) diff --git a/libpod/container_copy_linux.go b/libpod/container_copy_linux.go index 7d4dd0d46..954d54a1d 100644 --- a/libpod/container_copy_linux.go +++ b/libpod/container_copy_linux.go @@ -15,8 +15,8 @@ import ( "github.com/containers/buildah/util" "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/pkg/rootless" + "github.com/containers/storage/pkg/archive" "github.com/containers/storage/pkg/idtools" - "github.com/docker/docker/pkg/archive" "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" diff --git a/pkg/machine/pull.go b/pkg/machine/pull.go index f79ac6ec4..3c8422a30 100644 --- a/pkg/machine/pull.go +++ b/pkg/machine/pull.go @@ -15,7 +15,7 @@ import ( "time" "github.com/containers/image/v5/pkg/compression" - "github.com/docker/docker/pkg/archive" + "github.com/containers/storage/pkg/archive" "github.com/sirupsen/logrus" "github.com/vbauerster/mpb/v6" "github.com/vbauerster/mpb/v6/decor" -- cgit v1.2.3-54-g00ecf