Skip to content
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.

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

Script _common.sh fails to detect correct CPU count in WSL (Windows Subsystem for Linux) distro #727

Closed
trallnag opened this issue Oct 30, 2024 · 13 comments · Fixed by #728

Comments

@trallnag
Copy link

trallnag commented Oct 30, 2024

Describe the bug

The script _common.sh fails to detect correct CPU count in WSL distributions.

How can we reproduce it?

Export the following two functions from the mentioned script:

  • common::get_cpu_num
  • common::colorify

Run common::get_cpu_num

You will see this output:

Unable to derive number of available CPU cores.
 Running inside K8s pod without limits or inside DinD without limits propagation.
 To avoid possible harm, parallelism is disabled.
 To re-enable it, set corresponding limits, or set the following for the current hook:
   args:
     - --hook-config=--parallelism-ci-cpu-cores=N
 where N is the number of CPU cores to allocate to pre-commit.
1

This is wrong. This snippet from get_cpu_num shows the correct CPU count:

> nproc 2> /dev/null || sysctl -n hw.ncpu 2> /dev/null || echo 1
20

But the script never reaches this point, because

/sys/fs/cgroup/cpu/cpu.cfs_quota_us

exists (and it contains -1).

So the problem is that the hook is not running inside K8s pod or inside DinD but inside WSL. It is possible to set CPU limits for WSL. But by default WSL is allowed to use all available CPUs with no limit.

Environment information

WSL-Version: 2.3.24.0
Kernelversion: 5.15.153.1-2
WSLg-Version: 1.0.65
MSRDC-Version: 1.2.5620
Direct3D-Version: 1.611.1-81528511
DXCore-Version: 10.0.26100.1-240331-1435.ge-release
Windows-Version: 10.0.22621.4317

Hook version is v4.6.0


Thanks for the hooks.

@trallnag trallnag added area/local_installation bug Something isn't working labels Oct 30, 2024
@yermulnik
Copy link
Collaborator

@trallnag Can you please confirm that /sys/fs/cgroup/cpu/cpu.cfs_quota_us file exists inside your WSL env?

@yermulnik
Copy link
Collaborator

yermulnik commented Oct 30, 2024

@MaxymVlasov If confirmed, we can additionally check for absence of /proc/sys/fs/binfmt_misc/WSLInterop file at https://github.com/antonbabenko/pre-commit-terraform/blob/master/hooks/_common.sh#L202 (ref: https://superuser.com/a/1749811/1241314) — this method looks to be in the "full compliance" with the existing check on that line. WDYT?

@trallnag I assume the /proc/sys/fs/binfmt_misc/WSLInterop file exists on your WSL installation, right?

@trallnag
Copy link
Author

@yermulnik both files exist in my WSL distribution (Ubuntu from Microsoft store, no customizations)

pringles:~# cat /sys/fs/cgroup/cpu/cpu.cfs_quota_us
-1

root@pringles:~# stat /sys/fs/cgroup/cpu/cpu.cfs_quota_us
  File: /sys/fs/cgroup/cpu/cpu.cfs_quota_us
  Size: 0               Blocks: 0          IO Block: 4096   regular empty file
Device: 0,60    Inode: 10          Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2024-10-25 19:53:10.198506900 +0200
Modify: 2024-10-25 19:53:10.198506900 +0200
Change: 2024-10-25 19:53:10.198506900 +0200
 Birth: -

pringles:~# stat /proc/sys/fs/binfmt_misc/WSLInterop
  File: /proc/sys/fs/binfmt_misc/WSLInterop
  Size: 0               Blocks: 0          IO Block: 4096   regular empty file
Device: 0,32    Inode: 18882       Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2024-10-25 19:51:08.700467298 +0200
Modify: 2024-10-25 19:51:08.700467298 +0200
Change: 2024-10-25 19:51:08.700467298 +0200
 Birth: -

@yermulnik
Copy link
Collaborator

@MaxymVlasov I went ahead and created quick PR: #728 ­— please let me know what do you think. Thanks.

@MaxymVlasov
Copy link
Collaborator

MaxymVlasov commented Oct 31, 2024

@trallnag do you have /sys/fs/cgroup/cpu.max in WSL?
If not, please make sure that nproc exist and provides number >1
If not, try sysctl -n hw.ncpu, it should provide number > 1

Please let me know if any of them works in WSL.

And if still nothing works - please send ls -a /sys/fs/cgroup/ and ls -a /sys/fs/cgroup/cpu/

@trallnag
Copy link
Author

I don't have it:

❯ cat /sys/fs/cgroup/cpu.max
cat: /sys/fs/cgroup/cpu.max: No such file or directory

❯ ls -a /sys/fs/cgroup/
.   blkio  cpuacct      cpuset   freezer  memory  net_cls           net_prio    pids  systemd
..  cpu    cpu,cpuacct  devices  hugetlb  misc    net_cls,net_prio  perf_event  rdma  unified

❯ ll /sys/fs/cgroup/
Permissions Links Size User Date Modified    Name
dr-xr-xr-x      3    - root 2024-10-31 17:46 blkio/
dr-xr-xr-x     11    - root 2024-10-31 17:46 cpu/
drwxr-xr-x      2    - root 2024-10-31 17:46 cpu,cpuacct/
dr-xr-xr-x      6    - root 2024-10-31 17:46 cpuacct/
dr-xr-xr-x      6    - root 2024-10-31 17:46 cpuset/
dr-xr-xr-x     14    - root 2024-10-31 17:46 devices/
dr-xr-xr-x      7    - root 2024-10-31 17:46 freezer/
dr-xr-xr-x      6    - root 2024-10-31 17:46 hugetlb/
dr-xr-xr-x     14    - root 2024-10-31 17:46 memory/
dr-xr-xr-x      6    - root 2024-10-31 17:46 misc/
dr-xr-xr-x      6    - root 2024-10-31 17:46 net_cls/
drwxr-xr-x      2    - root 2024-10-31 17:46 net_cls,net_prio/
dr-xr-xr-x      6    - root 2024-10-31 17:46 net_prio/
dr-xr-xr-x      6    - root 2024-10-31 17:46 perf_event/
dr-xr-xr-x     14    - root 2024-10-31 17:46 pids/
dr-xr-xr-x      6    - root 2024-10-31 17:46 rdma/
dr-xr-xr-x     14    - root 2024-10-31 17:46 systemd/
dr-xr-xr-x     16    - root 2024-10-31 17:46 unified/

Note that I don't have configured a processor limit for WSL, something which is supported: https://learn.microsoft.com/en-us/windows/wsl/wsl-config#main-wsl-settings.

Out of curiosity I just configured a limit, but the mentioned file still does not appear. But the limit definitly is applied:

❯ getconf _NPROCESSORS_ONLN
4

Without limit:

❯ getconf _NPROCESSORS_ONLN
16

@MaxymVlasov
Copy link
Collaborator

MaxymVlasov commented Oct 31, 2024

I edited my msg a few times, you probably missed the final revision.

Can you please also

If not, please make sure that nproc exist and provides number >1
If not, try sysctl -n hw.ncpu, it should provide number > 1

Please let me know if any of them works in WSL.

And if still nothing works - please send ls -a /sys/fs/cgroup/ and ls -a /sys/fs/cgroup/cpu/

@trallnag
Copy link
Author

❯ nproc
16

❯ sysctl -n hw.ncpu
sysctl: cannot stat /proc/sys/hw/ncpu: No such file or directory

❯ ls -a /sys/fs/cgroup/
.   blkio  cpuacct      cpuset   freezer  memory  net_cls           net_prio    pids  systemd
..  cpu    cpu,cpuacct  devices  hugetlb  misc    net_cls,net_prio  perf_event  rdma  unified

❯ ls -a /sys/fs/cgroup/cpu/
.                      cgroup.procs          cpu.cfs_period_us  cpu.rt_period_us   cpu.stat           tasks
..                     cgroup.sane_behavior  cpu.cfs_quota_us   cpu.rt_runtime_us  notify_on_release
cgroup.clone_children  cpu.cfs_burst_us      cpu.idle           cpu.shares         release_agent

@MaxymVlasov
Copy link
Collaborator

super, thank you. George fix should works for you, I'll edit a clarification comment, but you already can try it by

- repo: https://github.com/antonbabenko/pre-commit-terraform
  rev: 040b9771f828d52e14eff80c47b747b92db77579

@trallnag
Copy link
Author

Thanks, I will try later today and report back

@yermulnik
Copy link
Collaborator

Thanks, I will try later today and report back

Perfect. Keep us posted. Once you confirm the fix works for you, we'll merge it to default branch.

@trallnag
Copy link
Author

Can confirm that it works fine with the mentioned revision:

❯ task format
...
[INFO] Initializing environment for https://github.com/antonbabenko/pre-commit-terraform.
...
Terraform fmt............................................................Failed
- hook id: terraform_fmt
- files were modified by this hook

route53.tf
route53.tf
...

@antonbabenko
Copy link
Owner

This issue has been resolved in version 1.96.2 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants