diff options
-rw-r--r-- | cmd/podman/common/specgen.go | 2 | ||||
-rw-r--r-- | pkg/api/handlers/libpod/containers_create.go | 5 | ||||
-rw-r--r-- | pkg/domain/infra/abi/containers.go | 14 | ||||
-rw-r--r-- | pkg/domain/infra/tunnel/containers.go | 6 | ||||
-rw-r--r-- | pkg/specgen/generate/container.go | 159 | ||||
-rw-r--r-- | pkg/specgen/generate/container_create.go | 4 | ||||
-rw-r--r-- | pkg/specgen/generate/validate.go | 159 |
7 files changed, 270 insertions, 79 deletions
diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index 0b44ef544..2a070710a 100644 --- a/cmd/podman/common/specgen.go +++ b/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 } diff --git a/pkg/api/handlers/libpod/containers_create.go b/pkg/api/handlers/libpod/containers_create.go index 71f440bce..8fbff9be7 100644 --- a/pkg/api/handlers/libpod/containers_create.go +++ b/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) } diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index d2c8aefdc..da9be0946 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/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 diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 955149fcf..a69c6548c 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/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 { diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index 3d70571d5..df27f225b 100644 --- a/pkg/specgen/generate/container.go +++ b/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 { - // If a rootfs is used, then there is no image data - if s.ContainerStorageConfig.Rootfs != "" { - return nil - } - - newImage, err := r.ImageRuntime().NewFromLocal(s.Image) - if err != nil { - return err - } - - _, mediaType, err := newImage.Manifest(ctx) - if err != nil { - return err - } - - if s.HealthConfig == nil && mediaType == manifest.DockerV2Schema2MediaType { - s.HealthConfig, err = newImage.GetHealthCheck(ctx) +// Fill any missing parts of the spec generator (e.g. from the image). +// Returns a set of warnings or any fatal error that occurred. +func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerator) ([]string, error) { + var ( + newImage *image.Image + err error + ) + + // Only add image configuration if we have an image + if s.Image != "" { + newImage, err = r.ImageRuntime().NewFromLocal(s.Image) if err != nil { - return err + return nil, err } - } - // Image stop signal - if s.StopSignal == nil { - stopSignal, err := newImage.StopSignal(ctx) + _, mediaType, err := newImage.Manifest(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 - // already, overriding the default environments - imageEnvs, err := newImage.Env(ctx) - if err != nil { - return err - } + var envs map[string]string - envs, err := envLib.ParseSlice(imageEnvs) - if err != nil { - return errors.Wrap(err, "Env fields from image failed to parse") - } - s.Env = envLib.Join(envLib.Join(defaultEnvs, envs), s.Env) + if newImage != nil { + // Image envs from the image if they don't exist + // already, overriding the default environments + imageEnvs, err := newImage.Env(ctx) + if err != nil { + return nil, err + } - labels, err := newImage.Labels(ctx) - if err != nil { - return err + envs, err = envLib.ParseSlice(imageEnvs) + if err != nil { + 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 + s.Env = envLib.Join(envLib.Join(defaultEnvs, envs), s.Env) + + // Labels and Annotations + annotations := make(map[string]string) + if newImage != nil { + labels, err := newImage.Labels(ctx) + if err != nil { + return nil, err } - } - // annotations + // 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 - annotations, err := newImage.Annotations(ctx) - if err != nil { - return err - } - for k, v := range annotations { - annotations[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 err != nil { - return err - } - if len(s.WorkDir) < 1 && len(workingDir) > 1 { - s.WorkDir = workingDir + if newImage != nil { + workingDir, err := newImage.WorkingDir(ctx) + if err != nil { + return nil, err + } + 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 diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 33075b543..869601e93 100644 --- a/pkg/specgen/generate/container_create.go +++ b/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 { diff --git a/pkg/specgen/generate/validate.go b/pkg/specgen/generate/validate.go new file mode 100644 index 000000000..bb3ca9907 --- /dev/null +++ b/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 +} |