Re-add resource limit warnings to Specgen

These were part of Podman v1.9, but were lost in the transition
to using Specgen to create containers. Most resource limits are
checked via the sysinfo package to ensure they are safe to use
(the cgroup is mounted, kernel support is present, etc) and
removed if not safe. Further, bounds checks are performed to
ensure that values are valid.

Ensure these warnings are printed client-side when they occur.
This part is a little bit gross, as it happens in pkg/infra and
not cmd/podman, which is largely down to how we implemented
`podman run` - all the work is done in pkg/infra and it returns
only once the container has exited, and we need warnings to print
*before* the container runs. The solution here, while inelegant,
avoid the need to extensively refactor our handling of run.

Should fix blkio-limit warnings that were identified by the FCOS
test suite.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

cmd/podman/common/specgen.go

@@ -635,8 +635,6 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
 	s.Remove = c.Rm
 	s.StopTimeout = &c.StopTimeout
 
-	// TODO where should we do this?
-	// func verifyContainerResources(config *cc.CreateConfig, update bool) ([]string, error) {
 	return nil
 }
 

pkg/api/handlers/libpod/containers_create.go

@@ -22,7 +22,8 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Decode()"))
 		return
 	}
-	if err := generate.CompleteSpec(r.Context(), runtime, &sg); err != nil {
+	warn, err := generate.CompleteSpec(r.Context(), runtime, &sg)
+	if err != nil {
 		utils.InternalServerError(w, err)
 		return
 	}
@@ -31,6 +32,6 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 		utils.InternalServerError(w, err)
 		return
 	}
-	response := entities.ContainerCreateResponse{ID: ctr.ID()}
+	response := entities.ContainerCreateResponse{ID: ctr.ID(), Warnings: warn}
 	utils.WriteJSON(w, http.StatusCreated, response)
 }

pkg/domain/infra/abi/containers.go

@@ -511,9 +511,14 @@ func (ic *ContainerEngine) ContainerRestore(ctx context.Context, namesOrIds []st
 }
 
 func (ic *ContainerEngine) ContainerCreate(ctx context.Context, s *specgen.SpecGenerator) (*entities.ContainerCreateReport, error) {
-	if err := generate.CompleteSpec(ctx, ic.Libpod, s); err != nil {
+	warn, err := generate.CompleteSpec(ctx, ic.Libpod, s)
+	if err != nil {
 		return nil, err
 	}
+	// Print warnings
+	for _, w := range warn {
+		fmt.Fprintf(os.Stderr, "%s\n", w)
+	}
 	ctr, err := generate.MakeContainer(ctx, ic.Libpod, s)
 	if err != nil {
 		return nil, err
@@ -773,9 +778,14 @@ func (ic *ContainerEngine) ContainerDiff(ctx context.Context, nameOrID string, o
 }
 
 func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.ContainerRunOptions) (*entities.ContainerRunReport, error) {
-	if err := generate.CompleteSpec(ctx, ic.Libpod, opts.Spec); err != nil {
+	warn, err := generate.CompleteSpec(ctx, ic.Libpod, opts.Spec)
+	if err != nil {
 		return nil, err
 	}
+	// Print warnings
+	for _, w := range warn {
+		fmt.Fprintf(os.Stderr, "%s\n", w)
+	}
 	ctr, err := generate.MakeContainer(ctx, ic.Libpod, opts.Spec)
 	if err != nil {
 		return nil, err

pkg/domain/infra/tunnel/containers.go

@@ -340,6 +340,9 @@ func (ic *ContainerEngine) ContainerCreate(ctx context.Context, s *specgen.SpecG
 	if err != nil {
 		return nil, err
 	}
+	for _, w := range response.Warnings {
+		fmt.Fprintf(os.Stderr, "%s\n", w)
+	}
 	return &entities.ContainerCreateReport{Id: response.ID}, nil
 }
 
@@ -497,6 +500,9 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta
 	if err != nil {
 		return nil, err
 	}
+	for _, w := range con.Warnings {
+		fmt.Fprintf(os.Stderr, "%s\n", w)
+	}
 	report := entities.ContainerRunReport{Id: con.ID}
 	// Attach
 	if !opts.Detach {

pkg/specgen/generate/container.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/libpod/libpod"
+	"github.com/containers/libpod/libpod/image"
 	ann "github.com/containers/libpod/pkg/annotations"
 	envLib "github.com/containers/libpod/pkg/env"
 	"github.com/containers/libpod/pkg/signal"
@@ -13,91 +14,103 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerator) error {
+// Fill any missing parts of the spec generator (e.g. from the image).
-	// If a rootfs is used, then there is no image data
+// Returns a set of warnings or any fatal error that occurred.
-	if s.ContainerStorageConfig.Rootfs != "" {
+func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerator) ([]string, error) {
-		return nil
+	var (
-	}
+		newImage *image.Image
+		err      error
+	)
 
-	newImage, err := r.ImageRuntime().NewFromLocal(s.Image)
+	// Only add image configuration if we have an image
-	if err != nil {
+	if s.Image != "" {
-		return err
+		newImage, err = r.ImageRuntime().NewFromLocal(s.Image)
-	}
-
-	_, mediaType, err := newImage.Manifest(ctx)
-	if err != nil {
-		return err
-	}
-
-	if s.HealthConfig == nil && mediaType == manifest.DockerV2Schema2MediaType {
-		s.HealthConfig, err = newImage.GetHealthCheck(ctx)
 		if err != nil {
-			return err
+			return nil, err
 		}
-	}
 
-	// Image stop signal
+		_, mediaType, err := newImage.Manifest(ctx)
-	if s.StopSignal == nil {
-		stopSignal, err := newImage.StopSignal(ctx)
 		if err != nil {
-			return err
+			return nil, err
 		}
-		if stopSignal != "" {
+
-			sig, err := signal.ParseSignalNameOrNumber(stopSignal)
+		if s.HealthConfig == nil && mediaType == manifest.DockerV2Schema2MediaType {
+			s.HealthConfig, err = newImage.GetHealthCheck(ctx)
 			if err != nil {
-				return err
+				return nil, err
+			}
+		}
+
+		// Image stop signal
+		if s.StopSignal == nil {
+			stopSignal, err := newImage.StopSignal(ctx)
+			if err != nil {
+				return nil, err
+			}
+			if stopSignal != "" {
+				sig, err := signal.ParseSignalNameOrNumber(stopSignal)
+				if err != nil {
+					return nil, err
+				}
+				s.StopSignal = &sig
 			}
-			s.StopSignal = &sig
 		}
 	}
 
 	rtc, err := r.GetConfig()
 	if err != nil {
-		return err
+		return nil, err
 	}
 	// Get Default Environment
 	defaultEnvs, err := envLib.ParseSlice(rtc.Containers.Env)
 	if err != nil {
-		return errors.Wrap(err, "Env fields in containers.conf failed to parse")
+		return nil, errors.Wrap(err, "Env fields in containers.conf failed to parse")
 	}
 
-	// Image envs from the image if they don't exist
+	var envs map[string]string
-	// already, overriding the default environments
-	imageEnvs, err := newImage.Env(ctx)
-	if err != nil {
-		return err
-	}
 
-	envs, err := envLib.ParseSlice(imageEnvs)
+	if newImage != nil {
-	if err != nil {
+		// Image envs from the image if they don't exist
-		return errors.Wrap(err, "Env fields from image failed to parse")
+		// already, overriding the default environments
-	}
+		imageEnvs, err := newImage.Env(ctx)
-	s.Env = envLib.Join(envLib.Join(defaultEnvs, envs), s.Env)
+		if err != nil {
+			return nil, err
+		}
 
-	labels, err := newImage.Labels(ctx)
+		envs, err = envLib.ParseSlice(imageEnvs)
-	if err != nil {
+		if err != nil {
-		return err
+			return nil, errors.Wrap(err, "Env fields from image failed to parse")
-	}
-
-	// labels from the image that dont exist already
-	if len(labels) > 0 && s.Labels == nil {
-		s.Labels = make(map[string]string)
-	}
-	for k, v := range labels {
-		if _, exists := s.Labels[k]; !exists {
-			s.Labels[k] = v
 		}
 	}
 
-	// annotations
+	s.Env = envLib.Join(envLib.Join(defaultEnvs, envs), s.Env)
 
-	// Add annotations from the image
+	// Labels and Annotations
-	annotations, err := newImage.Annotations(ctx)
+	annotations := make(map[string]string)
-	if err != nil {
+	if newImage != nil {
-		return err
+		labels, err := newImage.Labels(ctx)
-	}
+		if err != nil {
-	for k, v := range annotations {
+			return nil, err
-		annotations[k] = v
+		}
+
+		// labels from the image that dont exist already
+		if len(labels) > 0 && s.Labels == nil {
+			s.Labels = make(map[string]string)
+		}
+		for k, v := range labels {
+			if _, exists := s.Labels[k]; !exists {
+				s.Labels[k] = v
+			}
+		}
+
+		// Add annotations from the image
+		imgAnnotations, err := newImage.Annotations(ctx)
+		if err != nil {
+			return nil, err
+		}
+		for k, v := range imgAnnotations {
+			annotations[k] = v
+		}
 	}
 
 	// in the event this container is in a pod, and the pod has an infra container
@@ -121,40 +134,42 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 	s.Annotations = annotations
 
 	// workdir
-	workingDir, err := newImage.WorkingDir(ctx)
+	if newImage != nil {
-	if err != nil {
+		workingDir, err := newImage.WorkingDir(ctx)
-		return err
+		if err != nil {
-	}
+			return nil, err
-	if len(s.WorkDir) < 1 && len(workingDir) > 1 {
+		}
-		s.WorkDir = workingDir
+		if len(s.WorkDir) < 1 && len(workingDir) > 1 {
+			s.WorkDir = workingDir
+		}
 	}
 
 	if len(s.SeccompProfilePath) < 1 {
 		p, err := libpod.DefaultSeccompPath()
 		if err != nil {
-			return err
+			return nil, err
 		}
 		s.SeccompProfilePath = p
 	}
 
-	if len(s.User) == 0 {
+	if len(s.User) == 0 && newImage != nil {
 		s.User, err = newImage.User(ctx)
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}
 	if err := finishThrottleDevices(s); err != nil {
-		return err
+		return nil, err
 	}
 	// Unless already set via the CLI, check if we need to disable process
 	// labels or set the defaults.
 	if len(s.SelinuxOpts) == 0 {
 		if err := setLabelOpts(s, r, s.PidNS, s.IpcNS); err != nil {
-			return err
+			return nil, err
 		}
 	}
 
-	return nil
+	return verifyContainerResources(s)
 }
 
 // finishThrottleDevices takes the temporary representation of the throttle

pkg/specgen/generate/container_create.go

@@ -16,7 +16,9 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-// MakeContainer creates a container based on the SpecGenerator
+// MakeContainer creates a container based on the SpecGenerator.
+// Returns the created, container and any warnings resulting from creating the
+// container, or an error.
 func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGenerator) (*libpod.Container, error) {
 	rtc, err := rt.GetConfig()
 	if err != nil {

pkg/specgen/generate/validate.go

@@ -0,0 +1,159 @@
+package generate
+
+import (
+	"github.com/containers/common/pkg/sysinfo"
+	"github.com/containers/libpod/pkg/cgroups"
+	"github.com/containers/libpod/pkg/specgen"
+	"github.com/pkg/errors"
+)
+
+// Verify resource limits are sanely set, removing any limits that are not
+// possible with the current cgroups config.
+func verifyContainerResources(s *specgen.SpecGenerator) ([]string, error) {
+	warnings := []string{}
+
+	cgroup2, err := cgroups.IsCgroup2UnifiedMode()
+	if err != nil || cgroup2 {
+		return warnings, err
+	}
+
+	sysInfo := sysinfo.New(true)
+
+	if s.ResourceLimits == nil {
+		return warnings, nil
+	}
+
+	// Memory checks
+	if s.ResourceLimits.Memory != nil {
+		memory := s.ResourceLimits.Memory
+		if memory.Limit != nil && !sysInfo.MemoryLimit {
+			warnings = append(warnings, "Your kernel does not support memory limit capabilities or the cgroup is not mounted. Limitation discarded.")
+			memory.Limit = nil
+			memory.Swap = nil
+		}
+		if memory.Limit != nil && memory.Swap != nil && !sysInfo.SwapLimit {
+			warnings = append(warnings, "Your kernel does not support swap limit capabilities,or the cgroup is not mounted. Memory limited without swap.")
+			memory.Swap = nil
+		}
+		if memory.Limit != nil && memory.Swap != nil && *memory.Swap < *memory.Limit {
+			return warnings, errors.New("minimum memoryswap limit should be larger than memory limit, see usage")
+		}
+		if memory.Limit == nil && memory.Swap != nil {
+			return warnings, errors.New("you should always set a memory limit when using a memoryswap limit, see usage")
+		}
+		if memory.Swappiness != nil {
+			if !sysInfo.MemorySwappiness {
+				warnings = append(warnings, "Your kernel does not support memory swappiness capabilities, or the cgroup is not mounted. Memory swappiness discarded.")
+				memory.Swappiness = nil
+			} else {
+				if *memory.Swappiness < 0 || *memory.Swappiness > 100 {
+					return warnings, errors.Errorf("invalid value: %v, valid memory swappiness range is 0-100", *memory.Swappiness)
+				}
+			}
+		}
+		if memory.Reservation != nil && !sysInfo.MemoryReservation {
+			warnings = append(warnings, "Your kernel does not support memory soft limit capabilities or the cgroup is not mounted. Limitation discarded.")
+			memory.Reservation = nil
+		}
+		if memory.Limit != nil && memory.Reservation != nil && *memory.Limit < *memory.Reservation {
+			return warnings, errors.New("minimum memory limit cannot be less than memory reservation limit, see usage")
+		}
+		if memory.Kernel != nil && !sysInfo.KernelMemory {
+			warnings = append(warnings, "Your kernel does not support kernel memory limit capabilities or the cgroup is not mounted. Limitation discarded.")
+			memory.Kernel = nil
+		}
+		if memory.DisableOOMKiller != nil && *memory.DisableOOMKiller && !sysInfo.OomKillDisable {
+			warnings = append(warnings, "Your kernel does not support OomKillDisable. OomKillDisable discarded.")
+			memory.DisableOOMKiller = nil
+		}
+	}
+
+	// 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 {
+			warnings = append(warnings, "Your kernel does not support pids limit capabilities or the cgroup is not mounted. PIDs limit discarded.")
+			s.ResourceLimits.Pids = nil
+		}
+	}
+
+	// CPU Checks
+	if s.ResourceLimits.CPU != nil {
+		cpu := s.ResourceLimits.CPU
+		if cpu.Shares != nil && !sysInfo.CPUShares {
+			warnings = append(warnings, "Your kernel does not support CPU shares or the cgroup is not mounted. Shares discarded.")
+			cpu.Shares = nil
+		}
+		if cpu.Period != nil && !sysInfo.CPUCfsPeriod {
+			warnings = append(warnings, "Your kernel does not support CPU cfs period or the cgroup is not mounted. Period discarded.")
+			cpu.Period = nil
+		}
+		if cpu.Period != nil && (*cpu.Period < 1000 || *cpu.Period > 1000000) {
+			return warnings, errors.New("CPU cfs period cannot be less than 1ms (i.e. 1000) or larger than 1s (i.e. 1000000)")
+		}
+		if cpu.Quota != nil && !sysInfo.CPUCfsQuota {
+			warnings = append(warnings, "Your kernel does not support CPU cfs quota or the cgroup is not mounted. Quota discarded.")
+			cpu.Quota = nil
+		}
+		if cpu.Quota != nil && *cpu.Quota < 1000 {
+			return warnings, errors.New("CPU cfs quota cannot be less than 1ms (i.e. 1000)")
+		}
+		if (cpu.Cpus != "" || cpu.Mems != "") && !sysInfo.Cpuset {
+			warnings = append(warnings, "Your kernel does not support cpuset or the cgroup is not mounted. CPUset discarded.")
+			cpu.Cpus = ""
+			cpu.Mems = ""
+		}
+
+		cpusAvailable, err := sysInfo.IsCpusetCpusAvailable(cpu.Cpus)
+		if err != nil {
+			return warnings, errors.Errorf("invalid value %s for cpuset cpus", cpu.Cpus)
+		}
+		if !cpusAvailable {
+			return warnings, errors.Errorf("requested CPUs are not available - requested %s, available: %s", cpu.Cpus, sysInfo.Cpus)
+		}
+
+		memsAvailable, err := sysInfo.IsCpusetMemsAvailable(cpu.Mems)
+		if err != nil {
+			return warnings, errors.Errorf("invalid value %s for cpuset mems", cpu.Mems)
+		}
+		if !memsAvailable {
+			return warnings, errors.Errorf("requested memory nodes are not available - requested %s, available: %s", cpu.Mems, sysInfo.Mems)
+		}
+	}
+
+	// Blkio checks
+	if s.ResourceLimits.BlockIO != nil {
+		blkio := s.ResourceLimits.BlockIO
+		if blkio.Weight != nil && !sysInfo.BlkioWeight {
+			warnings = append(warnings, "Your kernel does not support Block I/O weight or the cgroup is not mounted. Weight discarded.")
+			blkio.Weight = nil
+		}
+		if blkio.Weight != nil && (*blkio.Weight > 1000 || *blkio.Weight < 10) {
+			return warnings, errors.New("range of blkio weight is from 10 to 1000")
+		}
+		if len(blkio.WeightDevice) > 0 && !sysInfo.BlkioWeightDevice {
+			warnings = append(warnings, "Your kernel does not support Block I/O weight_device or the cgroup is not mounted. Weight-device discarded.")
+			blkio.WeightDevice = nil
+		}
+		if len(blkio.ThrottleReadBpsDevice) > 0 && !sysInfo.BlkioReadBpsDevice {
+			warnings = append(warnings, "Your kernel does not support BPS Block I/O read limit or the cgroup is not mounted. Block I/O BPS read limit discarded")
+			blkio.ThrottleReadBpsDevice = nil
+		}
+		if len(blkio.ThrottleWriteBpsDevice) > 0 && !sysInfo.BlkioWriteBpsDevice {
+			warnings = append(warnings, "Your kernel does not support BPS Block I/O write limit or the cgroup is not mounted. Block I/O BPS write limit discarded.")
+			blkio.ThrottleWriteBpsDevice = nil
+		}
+		if len(blkio.ThrottleReadIOPSDevice) > 0 && !sysInfo.BlkioReadIOpsDevice {
+			warnings = append(warnings, "Your kernel does not support IOPS Block read limit or the cgroup is not mounted. Block I/O IOPS read limit discarded.")
+			blkio.ThrottleReadIOPSDevice = nil
+		}
+		if len(blkio.ThrottleWriteIOPSDevice) > 0 && !sysInfo.BlkioWriteIOpsDevice {
+			warnings = append(warnings, "Your kernel does not support IOPS Block I/O write limit or the cgroup is not mounted. Block I/O IOPS write limit discarded.")
+			blkio.ThrottleWriteIOPSDevice = nil
+		}
+	}
+
+	return warnings, nil
+}