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>

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
 

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")

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.

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)))
+	})
 })