Merge pull request containers#12272 from hshiina/memory-swappiness

Enable 'podman run --memory-swappiness=0'
rhatdan · Nov 13, 2021 · cca6df4 · cca6df4
2 parents 8f3fb74 + 9226ccb
commit cca6df4
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 14 deletions.
diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go
@@ -376,7 +376,9 @@ func createPodIfNecessary(s *specgen.SpecGenerator, netOpts *entities.NetOptions
 		return nil, err
 	}
 
-	infraOpts := entities.ContainerCreateOptions{ImageVolume: "bind", Net: netOpts, Quiet: true}
+	infraOpts := entities.NewInfraContainerCreateOptions()
+	infraOpts.Net = netOpts
+	infraOpts.Quiet = true
 	imageName := config.DefaultInfraImage
 	podGen.InfraImage = imageName
 	podGen.InfraContainerSpec = specgen.NewSpecGenerator(imageName, false)

diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go
@@ -46,7 +46,7 @@ var (
 
 var (
 	createOptions     entities.PodCreateOptions
-	infraOptions      entities.ContainerCreateOptions
+	infraOptions      = entities.NewInfraContainerCreateOptions()
 	infraImage        string
 	labels, labelFile []string
 	podIDFile         string
@@ -61,7 +61,6 @@ func init() {
 	})
 	flags := createCommand.Flags()
 	flags.SetInterspersed(false)
-	infraOptions.IsInfra = true
 	common.DefineCreateFlags(createCommand, &infraOptions, true)
 	common.DefineNetFlags(createCommand)
 

diff --git a/pkg/api/handlers/libpod/pods.go b/pkg/api/handlers/libpod/pods.go
@@ -39,8 +39,10 @@ func PodCreate(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if !psg.NoInfra {
-		infraOptions := &entities.ContainerCreateOptions{ImageVolume: "bind", IsInfra: true, Net: &entities.NetOptions{}, Devices: psg.Devices} // options for pulling the image and FillOutSpec
-		err = specgenutil.FillOutSpecGen(psg.InfraContainerSpec, infraOptions, []string{})                                                      // necessary for default values in many cases (userns, idmappings)
+		infraOptions := entities.NewInfraContainerCreateOptions() // options for pulling the image and FillOutSpec
+		infraOptions.Net = &entities.NetOptions{}
+		infraOptions.Devices = psg.Devices
+		err = specgenutil.FillOutSpecGen(psg.InfraContainerSpec, &infraOptions, []string{}) // necessary for default values in many cases (userns, idmappings)
 		if err != nil {
 			utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error filling out specgen"))
 			return

diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go
@@ -266,6 +266,15 @@ type ContainerCreateOptions struct {
 	CgroupConf []string
 }
 
+func NewInfraContainerCreateOptions() ContainerCreateOptions {
+	options := ContainerCreateOptions{
+		IsInfra:          true,
+		ImageVolume:      "bind",
+		MemorySwappiness: -1,
+	}
+	return options
+}
+
 type PodCreateReport struct {
 	Id string //nolint
 }

diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go
@@ -270,7 +270,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 
 	if podOpt.Infra {
 		infraImage := util.DefaultContainerConfig().Engine.InfraImage
-		infraOptions := entities.ContainerCreateOptions{ImageVolume: "bind"}
+		infraOptions := entities.NewInfraContainerCreateOptions()
 		podSpec.PodSpecGen.InfraImage = infraImage
 		podSpec.PodSpecGen.NoInfra = false
 		podSpec.PodSpecGen.InfraContainerSpec = specgen.NewSpecGenerator(infraImage, false)

diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go
@@ -172,7 +172,7 @@ func getMemoryLimits(s *specgen.SpecGenerator, c *entities.ContainerCreateOption
 		memory.Kernel = &mk
 		hasLimits = true
 	}
-	if c.MemorySwappiness > 0 {
+	if c.MemorySwappiness >= 0 {
 		swappiness := uint64(c.MemorySwappiness)
 		memory.Swappiness = &swappiness
 		hasLimits = true

diff --git a/test/e2e/run_memory_test.go b/test/e2e/run_memory_test.go
@@ -1,6 +1,7 @@
 package integration
 
 import (
+	"fmt"
 	"os"
 	"strconv"
 
@@ -67,13 +68,17 @@ var _ = Describe("Podman run memory", func() {
 		Expect(session.OutputToString()).To(Equal("41943040"))
 	})
 
-	It("podman run memory-swappiness test", func() {
-		SkipIfCgroupV2("memory-swappiness not supported on cgroupV2")
-		session := podmanTest.Podman([]string{"run", "--memory-swappiness=15", ALPINE, "cat", "/sys/fs/cgroup/memory/memory.swappiness"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(Exit(0))
-		Expect(session.OutputToString()).To(Equal("15"))
-	})
+	for _, limit := range []string{"0", "15", "100"} {
+		limit := limit // Keep this value in a proper scope
+		testName := fmt.Sprintf("podman run memory-swappiness test(%s)", limit)
+		It(testName, func() {
+			SkipIfCgroupV2("memory-swappiness not supported on cgroupV2")
+			session := podmanTest.Podman([]string{"run", fmt.Sprintf("--memory-swappiness=%s", limit), ALPINE, "cat", "/sys/fs/cgroup/memory/memory.swappiness"})
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(Exit(0))
+			Expect(session.OutputToString()).To(Equal(limit))
+		})
+	}
 
 	It("podman run kernel-memory test", func() {
 		if podmanTest.Host.Distribution == "ubuntu" {