diff options
author | Matthew Heon <matthew.heon@pm.me> | 2020-05-05 16:19:30 -0400 |
---|---|---|
committer | Matthew Heon <matthew.heon@pm.me> | 2020-05-05 20:21:01 -0400 |
commit | f7c3cfde77a4bc8b241fb36c4d39cbe2fde0c45c (patch) | |
tree | 1955cdfcb7ef8705944bcb1b0f8b42286863224e | |
parent | fb6eca50ba9e2dc652da0c33c72db70ab9da85e9 (diff) | |
download | podman-f7c3cfde77a4bc8b241fb36c4d39cbe2fde0c45c.tar.gz podman-f7c3cfde77a4bc8b241fb36c4d39cbe2fde0c45c.tar.bz2 podman-f7c3cfde77a4bc8b241fb36c4d39cbe2fde0c45c.zip |
Add small fixes for 'podman run' from diffing inspect
To try and identify differences between Podman v1.9 and master,
I ran a series of `podman run` commands with various flags
through each, then inspecting the resulting containers and diffed
the inspect JSON between each. This identified a number of issues
which are fixed in this PR.
In order of discovery:
- Podman v2 gave short names for images, where Podman v1 gave the
fully-qualified name. Simple enough fix (get image tags and use
the first one if they're available)
- The --restart flag was not being parsed correctly when a number
of retries was specified. Parsing has been corrected.
- The -m flag was not setting the swap limit (simple fix to set
swap in that case if it's not explicitly set by the user)
- The --cpus flag was completely nonfunctional (wired in its
logic)
Tests have been added for all of these to catch future
regressions.
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
-rw-r--r-- | cmd/podman/common/specgen.go | 38 | ||||
-rw-r--r-- | pkg/specgen/generate/container_create.go | 7 | ||||
-rw-r--r-- | pkg/specgen/specgen.go | 4 | ||||
-rw-r--r-- | test/e2e/create_test.go | 49 |
4 files changed, 92 insertions, 6 deletions
diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index 7467c184a..3681804ea 100644 --- a/cmd/podman/common/specgen.go +++ b/cmd/podman/common/specgen.go @@ -26,6 +26,16 @@ func getCPULimits(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string) cpu := &specs.LinuxCPU{} hasLimits := false + const cpuPeriod = 100000 + + if c.CPUS > 0 { + quota := int64(c.CPUS * cpuPeriod) + period := uint64(cpuPeriod) + + cpu.Period = &period + cpu.Quota = "a + hasLimits = true + } if c.CPUShares > 0 { cpu.Shares = &c.CPUShares hasLimits = true @@ -142,6 +152,10 @@ func getMemoryLimits(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []strin return nil, errors.Wrapf(err, "invalid value for memory") } memory.Limit = &ml + if c.MemorySwap == "" { + limit := 2 * ml + memory.Swap = &(limit) + } hasLimits = true } if m := c.MemoryReservation; len(m) > 0 { @@ -606,7 +620,29 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string s.Name = c.Name s.OOMScoreAdj = &c.OOMScoreAdj - s.RestartPolicy = c.Restart + if c.Restart != "" { + splitRestart := strings.Split(c.Restart, ":") + switch len(splitRestart) { + case 1: + // No retries specified + case 2: + if strings.ToLower(splitRestart[0]) != "on-failure" { + return errors.Errorf("restart policy retries can only be specified with on-failure restart policy") + } + retries, err := strconv.Atoi(splitRestart[1]) + if err != nil { + return errors.Wrapf(err, "error parsing restart policy retry count") + } + if retries < 0 { + return errors.Errorf("must specify restart policy retry count as a number greater than 0") + } + var retriesUint uint = uint(retries) + s.RestartRetries = &retriesUint + default: + return errors.Errorf("invalid restart policy: may specify retries at most once") + } + s.RestartPolicy = splitRestart[0] + } s.Remove = c.Rm s.StopTimeout = &c.StopTimeout diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index be54b60d2..f3aaf96bf 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -85,7 +85,12 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener if err != nil { return nil, err } - options = append(options, libpod.WithRootFSFromImage(newImage.ID(), s.Image, s.RawImageName)) + imgName := s.Image + names := newImage.Names() + if len(names) > 0 { + imgName = names[0] + } + options = append(options, libpod.WithRootFSFromImage(newImage.ID(), imgName, s.Image)) } if err := s.Validate(); err != nil { return nil, errors.Wrap(err, "invalid config provided") diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go index 4f1c4fde1..4ad6dd6fb 100644 --- a/pkg/specgen/specgen.go +++ b/pkg/specgen/specgen.go @@ -140,10 +140,6 @@ type ContainerStorageConfig struct { // Conflicts with Rootfs. // At least one of Image or Rootfs must be specified. Image string `json:"image"` - // RawImageName is the unprocessed and not-normalized user-specified image - // name. One use case for having this data at hand are auto-updates where - // the _exact_ user input is needed in order to look-up the correct image. - RawImageName string `json:"raw_image_name,omitempty"` // Rootfs is the path to a directory that will be used as the // container's root filesystem. No modification will be made to the // directory, it will be directly mounted into the container as root. diff --git a/test/e2e/create_test.go b/test/e2e/create_test.go index 10742a0e8..1041b30bb 100644 --- a/test/e2e/create_test.go +++ b/test/e2e/create_test.go @@ -342,4 +342,53 @@ var _ = Describe("Podman create", func() { Expect(ok2).To(BeTrue()) Expect(val2).To(Equal("bar")) }) + + It("podman create with --restart=on-failure:5 parses correctly", func() { + ctrName := "testctr" + session := podmanTest.Podman([]string{"create", "-t", "--restart", "on-failure:5", "--name", ctrName, ALPINE, "/bin/sh"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + inspect := podmanTest.Podman([]string{"inspect", ctrName}) + inspect.WaitWithDefaultTimeout() + data := inspect.InspectContainerToJSON() + Expect(len(data)).To(Equal(1)) + Expect(data[0].HostConfig.RestartPolicy.Name).To(Equal("on-failure")) + Expect(data[0].HostConfig.RestartPolicy.MaximumRetryCount).To(Equal(uint(5))) + }) + + It("podman create with --restart-policy=always:5 fails", func() { + session := podmanTest.Podman([]string{"create", "-t", "--restart", "always:5", ALPINE, "/bin/sh"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Not(Equal(0))) + }) + + It("podman create with -m 1000000 sets swap to 2000000", func() { + numMem := 1000000 + ctrName := "testCtr" + session := podmanTest.Podman([]string{"create", "-t", "-m", fmt.Sprintf("%db", numMem), "--name", ctrName, ALPINE, "/bin/sh"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + inspect := podmanTest.Podman([]string{"inspect", ctrName}) + inspect.WaitWithDefaultTimeout() + data := inspect.InspectContainerToJSON() + Expect(len(data)).To(Equal(1)) + Expect(data[0].HostConfig.MemorySwap).To(Equal(int64(2 * numMem))) + }) + + It("podman create --cpus 5 sets nanocpus", func() { + numCpus := 5 + nanoCPUs := numCpus * 1000000000 + ctrName := "testCtr" + session := podmanTest.Podman([]string{"create", "-t", "--cpus", fmt.Sprintf("%d", numCpus), "--name", ctrName, ALPINE, "/bin/sh"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + inspect := podmanTest.Podman([]string{"inspect", ctrName}) + inspect.WaitWithDefaultTimeout() + data := inspect.InspectContainerToJSON() + Expect(len(data)).To(Equal(1)) + Expect(data[0].HostConfig.NanoCpus).To(Equal(int64(nanoCPUs))) + }) }) |