summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatthew Heon <matthew.heon@pm.me>2020-05-05 16:19:30 -0400
committerMatthew Heon <matthew.heon@pm.me>2020-05-05 20:21:01 -0400
commitf7c3cfde77a4bc8b241fb36c4d39cbe2fde0c45c (patch)
tree1955cdfcb7ef8705944bcb1b0f8b42286863224e
parentfb6eca50ba9e2dc652da0c33c72db70ab9da85e9 (diff)
downloadpodman-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.go38
-rw-r--r--pkg/specgen/generate/container_create.go7
-rw-r--r--pkg/specgen/specgen.go4
-rw-r--r--test/e2e/create_test.go49
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 = &quota
+ 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)))
+ })
})