Skip to content

Add verification code to verify user input on resources. #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rhatdan wants to merge 2 commits into from
Closed

Add verification code to verify user input on resources. #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f6eded
Update vendoring
rhatdan Nov 22, 2017
616667b
Copy some verification code out of Docker to verify user input
rhatdan Nov 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/kpod/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ var createFlags = []cli.Flag{
cli.Int64Flag{
Name: "memory-swappiness",
Usage: "Tune container memory swappiness (0 to 100) (default -1)",
Value: -1,
},
cli.StringFlag{
Name: "name",
Expand Down
24 changes: 15 additions & 9 deletions cmd/kpod/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ var (
)

type createResourceConfig struct {
blkioDevice []string // blkio-weight-device
blkioWeight uint16 // blkio-weight
blkioWeightDevice []string // blkio-weight-device
cpuPeriod uint64 // cpu-period
cpuQuota int64 // cpu-quota
cpuRtPeriod uint64 // cpu-rt-period
Expand All @@ -46,15 +46,15 @@ type createResourceConfig struct {
cpusetCpus string
cpusetMems string // cpuset-mems
deviceReadBps []string // device-read-bps
deviceReadIops []string // device-read-iops
deviceReadIOps []string // device-read-iops
deviceWriteBps []string // device-write-bps
deviceWriteIops []string // device-write-iops
deviceWriteIOps []string // device-write-iops
disableOomKiller bool // oom-kill-disable
kernelMemory int64 // kernel-memory
memory int64 //memory
memoryReservation int64 // memory-reservation
memorySwap int64 //memory-swap
memorySwapiness uint64 // memory-swappiness
memorySwappiness int // memory-swappiness
oomScoreAdj int //oom-score-adj
pidsLimit int64 // pids-limit
shmSize string
Expand Down Expand Up @@ -373,7 +373,7 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime) (*createConfig, er
readOnlyRootfs: c.Bool("read-only"),
resources: createResourceConfig{
blkioWeight: blkioWeight,
blkioDevice: c.StringSlice("blkio-weight-device"),
blkioWeightDevice: c.StringSlice("blkio-weight-device"),
cpuShares: c.Uint64("cpu-shares"),
cpuPeriod: c.Uint64("cpu-period"),
cpusetCpus: c.String("cpu-period"),
Expand All @@ -383,15 +383,15 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime) (*createConfig, er
cpuRtRuntime: c.Int64("cpu-rt-runtime"),
cpus: c.String("cpus"),
deviceReadBps: c.StringSlice("device-read-bps"),
deviceReadIops: c.StringSlice("device-read-iops"),
deviceReadIOps: c.StringSlice("device-read-iops"),
deviceWriteBps: c.StringSlice("device-write-bps"),
deviceWriteIops: c.StringSlice("device-write-iops"),
deviceWriteIOps: c.StringSlice("device-write-iops"),
disableOomKiller: c.Bool("oom-kill-disable"),
shmSize: c.String("shm-size"),
memory: memoryLimit,
memoryReservation: memoryReservation,
memorySwap: memorySwap,
memorySwapiness: c.Uint64("memory-swapiness"),
memorySwappiness: c.Int("memory-swappiness"),
kernelMemory: memoryKernel,
oomScoreAdj: c.Int("oom-score-adj"),

Expand All @@ -418,6 +418,12 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime) (*createConfig, er
return nil, err
}
}

warnings, err := verifyContainerResources(config, false)
if err != nil {
return nil, err
}
for _, warning := range warnings {
fmt.Fprintln(os.Stderr, warning)
}
return config, nil
}
148 changes: 148 additions & 0 deletions cmd/kpod/create_cli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
package main

import (
"fmt"
"strings"

"github.com/docker/docker/pkg/sysinfo"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

const (
// It's not kernel limit, we want this 4M limit to supply a reasonable functional container
linuxMinMemory = 4194304
)

func getAllLabels(labelFile, inputLabels []string) (map[string]string, error) {
Expand All @@ -26,3 +34,143 @@ func convertStringSliceToMap(strSlice []string, delimiter string) (map[string]st
}
return sysctl, nil
}

func addWarning(warnings []string, msg string) []string {
logrus.Warn(msg)
return append(warnings, msg)
}

func verifyContainerResources(config *createConfig, update bool) ([]string, error) {
warnings := []string{}
sysInfo := sysinfo.New(true)

// memory subsystem checks and adjustments
if config.resources.memory != 0 && config.resources.memory < linuxMinMemory {
return warnings, fmt.Errorf("minimum memory limit allowed is 4MB")
}
if config.resources.memory > 0 && !sysInfo.MemoryLimit {
warnings = addWarning(warnings, "Your kernel does not support memory limit capabilities or the cgroup is not mounted. Limitation discarded.")
config.resources.memory = 0
config.resources.memorySwap = -1
}
if config.resources.memory > 0 && config.resources.memorySwap != -1 && !sysInfo.SwapLimit {
warnings = addWarning(warnings, "Your kernel does not support swap limit capabilities,or the cgroup is not mounted. Memory limited without swap.")
config.resources.memorySwap = -1
}
if config.resources.memory > 0 && config.resources.memorySwap > 0 && config.resources.memorySwap < config.resources.memory {
return warnings, fmt.Errorf("minimum memoryswap limit should be larger than memory limit, see usage")
}
if config.resources.memory == 0 && config.resources.memorySwap > 0 && !update {
return warnings, fmt.Errorf("you should always set the Memory limit when using Memoryswap limit, see usage")
}
if config.resources.memorySwappiness != -1 {
if !sysInfo.MemorySwappiness {
msg := "Your kernel does not support memory swappiness capabilities, or the cgroup is not mounted. Memory swappiness discarded."
warnings = addWarning(warnings, msg)
config.resources.memorySwappiness = -1
} else {
swappiness := config.resources.memorySwappiness
if swappiness < -1 || swappiness > 100 {
return warnings, fmt.Errorf("invalid value: %v, valid memory swappiness range is 0-100", swappiness)
}
}
}
if config.resources.memoryReservation > 0 && !sysInfo.MemoryReservation {
warnings = addWarning(warnings, "Your kernel does not support memory soft limit capabilities or the cgroup is not mounted. Limitation discarded.")
config.resources.memoryReservation = 0
}
if config.resources.memoryReservation > 0 && config.resources.memoryReservation < linuxMinMemory {
return warnings, fmt.Errorf("minimum memory reservation allowed is 4MB")
}
if config.resources.memory > 0 && config.resources.memoryReservation > 0 && config.resources.memory < config.resources.memoryReservation {
return warnings, fmt.Errorf("minimum memory limit can not be less than memory reservation limit, see usage")
}
if config.resources.kernelMemory > 0 && !sysInfo.KernelMemory {
warnings = addWarning(warnings, "Your kernel does not support kernel memory limit capabilities or the cgroup is not mounted. Limitation discarded.")
config.resources.kernelMemory = 0
}
if config.resources.kernelMemory > 0 && config.resources.kernelMemory < linuxMinMemory {
return warnings, fmt.Errorf("minimum kernel memory limit allowed is 4MB")
}
if config.resources.disableOomKiller == true && !sysInfo.OomKillDisable {
// only produce warnings if the setting wasn't to *disable* the OOM Kill; no point
// warning the caller if they already wanted the feature to be off
warnings = addWarning(warnings, "Your kernel does not support OomKillDisable. OomKillDisable discarded.")
config.resources.disableOomKiller = false
}

if config.resources.pidsLimit != 0 && !sysInfo.PidsLimit {
warnings = addWarning(warnings, "Your kernel does not support pids limit capabilities or the cgroup is not mounted. PIDs limit discarded.")
config.resources.pidsLimit = 0
}

if config.resources.cpuShares > 0 && !sysInfo.CPUShares {
warnings = addWarning(warnings, "Your kernel does not support CPU shares or the cgroup is not mounted. Shares discarded.")
config.resources.cpuShares = 0
}
if config.resources.cpuPeriod > 0 && !sysInfo.CPUCfsPeriod {
warnings = addWarning(warnings, "Your kernel does not support CPU cfs period or the cgroup is not mounted. Period discarded.")
config.resources.cpuPeriod = 0
}
if config.resources.cpuPeriod != 0 && (config.resources.cpuPeriod < 1000 || config.resources.cpuPeriod > 1000000) {
return warnings, fmt.Errorf("CPU cfs period can not be less than 1ms (i.e. 1000) or larger than 1s (i.e. 1000000)")
}
if config.resources.cpuQuota > 0 && !sysInfo.CPUCfsQuota {
warnings = addWarning(warnings, "Your kernel does not support CPU cfs quota or the cgroup is not mounted. Quota discarded.")
config.resources.cpuQuota = 0
}
if config.resources.cpuQuota > 0 && config.resources.cpuQuota < 1000 {
return warnings, fmt.Errorf("CPU cfs quota can not be less than 1ms (i.e. 1000)")
}
// cpuset subsystem checks and adjustments
if (config.resources.cpusetCpus != "" || config.resources.cpusetMems != "") && !sysInfo.Cpuset {
warnings = addWarning(warnings, "Your kernel does not support cpuset or the cgroup is not mounted. Cpuset discarded.")
config.resources.cpusetCpus = ""
config.resources.cpusetMems = ""
}
cpusAvailable, err := sysInfo.IsCpusetCpusAvailable(config.resources.cpusetCpus)
if err != nil {
return warnings, fmt.Errorf("invalid value %s for cpuset cpus", config.resources.cpusetCpus)
}
if !cpusAvailable {
return warnings, fmt.Errorf("requested CPUs are not available - requested %s, available: %s", config.resources.cpusetCpus, sysInfo.Cpus)
}
memsAvailable, err := sysInfo.IsCpusetMemsAvailable(config.resources.cpusetMems)
if err != nil {
return warnings, fmt.Errorf("invalid value %s for cpuset mems", config.resources.cpusetMems)
}
if !memsAvailable {
return warnings, fmt.Errorf("requested memory nodes are not available - requested %s, available: %s", config.resources.cpusetMems, sysInfo.Mems)
}

// blkio subsystem checks and adjustments
if config.resources.blkioWeight > 0 && !sysInfo.BlkioWeight {
warnings = addWarning(warnings, "Your kernel does not support Block I/O weight or the cgroup is not mounted. Weight discarded.")
config.resources.blkioWeight = 0
}
if config.resources.blkioWeight > 0 && (config.resources.blkioWeight < 10 || config.resources.blkioWeight > 1000) {
return warnings, fmt.Errorf("range of blkio weight is from 10 to 1000")
}
if len(config.resources.blkioWeightDevice) > 0 && !sysInfo.BlkioWeightDevice {
warnings = addWarning(warnings, "Your kernel does not support Block I/O weight_device or the cgroup is not mounted. Weight-device discarded.")
config.resources.blkioWeightDevice = []string{}
}
if len(config.resources.deviceReadBps) > 0 && !sysInfo.BlkioReadBpsDevice {
warnings = addWarning(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")
config.resources.deviceReadBps = []string{}
}
if len(config.resources.deviceWriteBps) > 0 && !sysInfo.BlkioWriteBpsDevice {
warnings = addWarning(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.")
config.resources.deviceWriteBps = []string{}
}
if len(config.resources.deviceReadIOps) > 0 && !sysInfo.BlkioReadIOpsDevice {
warnings = addWarning(warnings, "Your kernel does not support IOPS Block read limit or the cgroup is not mounted. Block I/O IOPS read limit discarded.")
config.resources.deviceReadIOps = []string{}
}
if len(config.resources.deviceWriteIOps) > 0 && !sysInfo.BlkioWriteIOpsDevice {
warnings = addWarning(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.")
config.resources.deviceWriteIOps = []string{}
}

return warnings, nil
}
Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat Nov 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have multiple warnings packed into 'warnings' will it be readable? Can we stuff a \n at the end of each?

Copy link
Member Author

@rhatdan rhatdan Nov 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warnings is a list of strings, I believe. The caller would be responsible for how he wants to display the warnings.

Copy link
Member Author

@rhatdan rhatdan Nov 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an addWarnings function

20 changes: 10 additions & 10 deletions cmd/kpod/spec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) {
if config.resources.kernelMemory != 0 {
g.SetLinuxResourcesMemoryKernel(config.resources.kernelMemory)
}
if config.resources.memorySwapiness != 0 {
g.SetLinuxResourcesMemorySwappiness(config.resources.memorySwapiness)
if config.resources.memorySwappiness != -1 {
Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat Nov 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would "<0" be better here? I think it's pretty controlled now, but might be a good hedge long-term.

Copy link
Member Author

@rhatdan rhatdan Nov 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 seems to be the special value (Default which indicates whether to use the Swappiness or not. (At least in Docker.)

g.SetLinuxResourcesMemorySwappiness(uint64(config.resources.memorySwappiness))
}
g.SetLinuxResourcesMemoryDisableOOMKiller(config.resources.disableOomKiller)
g.SetProcessOOMScoreAdj(config.resources.oomScoreAdj)
Expand Down Expand Up @@ -263,9 +263,9 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) {
func (c *createConfig) CreateBlockIO() (spec.LinuxBlockIO, error) {
bio := spec.LinuxBlockIO{}
bio.Weight = &c.resources.blkioWeight
if len(c.resources.blkioDevice) > 0 {
if len(c.resources.blkioWeightDevice) > 0 {
var lwds []spec.LinuxWeightDevice
for _, i := range c.resources.blkioDevice {
for _, i := range c.resources.blkioWeightDevice {
wd, err := validateweightDevice(i)
if err != nil {
return bio, errors.Wrapf(err, "invalid values for blkio-weight-device")
Expand Down Expand Up @@ -293,19 +293,19 @@ func (c *createConfig) CreateBlockIO() (spec.LinuxBlockIO, error) {
}
bio.ThrottleWriteBpsDevice = writeBpds
}
if len(c.resources.deviceReadIops) > 0 {
readIops, err := makeThrottleArray(c.resources.deviceReadIops)
if len(c.resources.deviceReadIOps) > 0 {
readIOps, err := makeThrottleArray(c.resources.deviceReadIOps)
if err != nil {
return bio, err
}
bio.ThrottleReadIOPSDevice = readIops
bio.ThrottleReadIOPSDevice = readIOps
}
if len(c.resources.deviceWriteIops) > 0 {
writeIops, err := makeThrottleArray(c.resources.deviceWriteIops)
if len(c.resources.deviceWriteIOps) > 0 {
writeIOps, err := makeThrottleArray(c.resources.deviceWriteIOps)
if err != nil {
return bio, err
}
bio.ThrottleWriteIOPSDevice = writeIops
bio.ThrottleWriteIOPSDevice = writeIOps
}

return bio, nil
Expand Down
10 changes: 2 additions & 8 deletions vendor.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
k8s.io/kubernetes v1.7.8 https://github.com/kubernetes/kubernetes
k8s.io/client-go release-4.0 https://github.com/kubernetes/client-go
k8s.io/apimachinery release-1.7 https://github.com/kubernetes/apimachinery
k8s.io/apiserver release-1.7 https://github.com/kubernetes/apiserver
#
github.com/sirupsen/logrus v1.0.0
github.com/containers/image storage-update https://github.com/nalind/image
Expand All @@ -24,7 +20,7 @@ github.com/tchap/go-patricia v2.2.6
gopkg.in/cheggaaa/pb.v1 v1.0.7
gopkg.in/inf.v0 v0.9.0
gopkg.in/yaml.v2 v2
github.com/docker/docker d4f6db83c21cfc6af54fffb1f13e8acb7199f96a
github.com/docker/docker ce452fb72ffcdb7605ce98bde9302238f47c63c5
github.com/docker/spdystream ed496381df8283605c435b86d4fdd6f4f20b8c6e
github.com/docker/distribution 7a8efe719e55bbfaff7bc5718cdf0ed51ca821df
github.com/docker/go-units v0.3.1
Expand All @@ -41,7 +37,6 @@ github.com/seccomp/libseccomp-golang v0.9.0
github.com/syndtr/gocapability e7cb7fa329f456b3855136a2642b197bad7366ba
github.com/blang/semver v3.5.0
github.com/BurntSushi/toml v0.2.0
github.com/mitchellh/go-wordwrap ad45545899c7b13c020ea92b2072220eefad42b8
github.com/golang/glog 23def4e6c14b4da8ac2ed8007337bc5eb5007998
github.com/davecgh/go-spew v1.1.0
github.com/go-openapi/spec 6aced65f8501fe1217321abf0749d354824ba2ff
Expand All @@ -64,7 +59,6 @@ github.com/golang/protobuf 748d386b5c1ea99658fd69fe9f03991ce86a90c1
github.com/coreos/go-systemd v14
github.com/coreos/pkg v3
github.com/golang/groupcache b710c8433bd175204919eb38776e944233235d03
github.com/fsnotify/fsnotify 7d7316ed6e1ed2de075aab8dfc76de5d158d66e1
github.com/Azure/go-ansiterm 19f72df4d05d31cbe1c56bfc8045c96babff6c7e
github.com/Microsoft/go-winio 78439966b38d69bf38227fbf57ac8a6fee70f69a
github.com/Microsoft/hcsshim 43f9725307998e09f2e3816c2c0c36dc98f0c982
Expand Down Expand Up @@ -97,9 +91,9 @@ github.com/prometheus/procfs 65c1f6f8f0fc1e2185eb9863a3bc751496404259
github.com/matttproud/golang_protobuf_extensions fc2b8d3a73c4867e51861bbdd5ae3c1f0869dd6a
github.com/beorn7/perks 3ac7bf7a47d159a033b107610db8a1b6575507a4
github.com/containerd/cgroups 7a5fdd8330119dc70d850260db8f3594d89d6943
github.com/go-zoo/bone 031b4005dfe248ccba241a0c9de0f9e112fd6b7c
github.com/hashicorp/go-multierror 83588e72410abfbe4df460eeb6f30841ae47d4c4
github.com/hashicorp/errwrap 7554cd9344cec97297fa6649b055a8c98c2a1e55
github.com/pquerna/ffjson d49c2bc1aa135aad0c6f4fc2056623ec78f5d5ac
github.com/stretchr/testify 4d4bfba8f1d1027c4fdbe371823030df51419987
github.com/pmezard/go-difflib 792786c7400a136282c1664665ae0a8db921c6c2
github.com/containerd/continuity master
Loading