validate cpus according to actual number of node cpus #2057
Conversation
rawdaGastan
requested review from
muhamadazmy,
delandtj,
MarioBassem,
ashraffouda and
xmonader
as code owners
pkg/vm.go
Outdated
| @@ -175,8 +176,16 @@ func (vm *VM) Validate() error { | |||
| return fmt.Errorf("invalid memory must not be less than 250M") | |||
| } | |||
|
|
|||
| if vm.CPU == 0 || vm.CPU > 32 { | |||
| return fmt.Errorf("invalid cpu must be between 1 and 32") | |||
| n, err := cpu.Counts(true) | |||
There was a problem hiding this comment.
can't we use runtime.NumCPU? https://pkg.go.dev/runtime#NumCPU, if so, let's not introduce a new dependency like gopsutil
There was a problem hiding this comment.
n -> numCpus or ncpus or cpus
There was a problem hiding this comment.
can't we use runtime.NumCPU? https://pkg.go.dev/runtime#NumCPU, if so, let's not introduce a new dependency like gopsutil
this is not the same, according to the docs, the runtime.NumCPU is logical number of cpus that can be used by the current process. this is a totally Go runtime thing, not related to the total number of CPUs available on the system
There was a problem hiding this comment.
But should definitely rename the variable from n to something else
pkg/vm.go
Outdated
| @@ -175,8 +176,16 @@ func (vm *VM) Validate() error { | |||
| return fmt.Errorf("invalid memory must not be less than 250M") | |||
| } | |||
|
|
|||
| if vm.CPU == 0 || vm.CPU > 32 { | |||
| return fmt.Errorf("invalid cpu must be between 1 and 32") | |||
| n, err := cpu.Counts(true) | |||
There was a problem hiding this comment.
can't we use runtime.NumCPU? https://pkg.go.dev/runtime#NumCPU, if so, let's not introduce a new dependency like gopsutil
this is not the same, according to the docs, the runtime.NumCPU is logical number of cpus that can be used by the current process. this is a totally Go runtime thing, not related to the total number of CPUs available on the system
pkg/vm.go
Outdated
| @@ -175,8 +176,16 @@ func (vm *VM) Validate() error { | |||
| return fmt.Errorf("invalid memory must not be less than 250M") | |||
| } | |||
|
|
|||
| if vm.CPU == 0 || vm.CPU > 32 { | |||
| return fmt.Errorf("invalid cpu must be between 1 and 32") | |||
| n, err := cpu.Counts(true) | |||
There was a problem hiding this comment.
But should definitely rename the variable from n to something else
Issues