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

fix: update run test windows #587

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

vsiravar
Copy link
Contributor

Issue #, if available:

Description of changes:
Update run tests related to cgroup v2 resource flag tests for windows.

In wsl2 cgroup v2 is mounted at /sys/fs/cgroup/unified but containerd expects it to be mounted at /sys/fs/cgroup https://github.com/containerd/cgroups/blob/cc78c6c1e32dc5bde018d92999910fdace3cfa27/utils.go#L36.

Enabling cgroup v2 requires extra tweaks in wsl 2 based on https://stackoverflow.com/questions/73021599/how-to-enable-cgroup-v2-in-wsl2
Testing done:
Yes.

  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: Vishwas Siravara <vsiravar@gmail.com>
if runtime.GOOS == "windows" {
// wsl2 cgroup v2 is mounted at /sys/fs/cgroup/unified,
// containerd expects it at /sys/fs/cgroup based on https://github.com/containerd/cgroups/blob/cc78c6c1e32dc5bde018d92999910fdace3cfa27/utils.go#L36
tests.Run(&tests.RunOption{BaseOpt: o, CGMode: tests.Unavailable, DefaultHostGatewayIP: "192.168.5.2"})
Copy link
Contributor

Choose a reason for hiding this comment

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

Enabling cgroup v2 requires extra tweaks in wsl 2 based on https://stackoverflow.com/questions/73021599/how-to-enable-cgroup-v2-in-wsl2

This mentions that WSL has both cgroups v1 and cgroups v2 enabled. Could we instead use hybrid mode?

Suggested change
tests.Run(&tests.RunOption{BaseOpt: o, CGMode: tests.Unavailable, DefaultHostGatewayIP: "192.168.5.2"})
tests.Run(&tests.RunOption{BaseOpt: o, CGMode: tests.Hybrid, DefaultHostGatewayIP: "192.168.5.2"})

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think this is correct. From the findmnt output:

├─/sys                                      sysfs            sysfs       rw,nosuid,nodev,noexec,noatime
│ ├─/sys/fs/cgroup                          tmpfs            tmpfs       ro,nosuid,nodev,noexec,size=4096k,nr_inodes=1024,mode
│ │ ├─/sys/fs/cgroup/unified                cgroup2          cgroup2     rw,nosuid,nodev,noexec,relatime,nsdelegate
│ │ ├─/sys/fs/cgroup/cpuset                 cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,cpuset
│ │ ├─/sys/fs/cgroup/cpu                    cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,cpu
│ │ ├─/sys/fs/cgroup/cpuacct                cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,cpuacct
│ │ ├─/sys/fs/cgroup/blkio                  cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,blkio
│ │ ├─/sys/fs/cgroup/memory                 cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,memory
│ │ ├─/sys/fs/cgroup/devices                cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,devices
│ │ ├─/sys/fs/cgroup/freezer                cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,freezer
│ │ ├─/sys/fs/cgroup/net_cls                cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,net_cls
│ │ ├─/sys/fs/cgroup/perf_event             cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,perf_event
│ │ ├─/sys/fs/cgroup/net_prio               cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,net_prio
│ │ ├─/sys/fs/cgroup/hugetlb                cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,hugetlb
│ │ ├─/sys/fs/cgroup/pids                   cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,pids
│ │ ├─/sys/fs/cgroup/rdma                   cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,rdma
│ │ ├─/sys/fs/cgroup/misc                   cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,misc
│ │ └─/sys/fs/cgroup/systemd                cgroup           cgroup      rw,nosuid,nodev,noexec,relatime,xattr,name=systemd

@pendo324 pendo324 changed the title fix: Vsiravar/update run test windows fix: update run test windows Sep 29, 2023
Signed-off-by: Vishwas Siravara <vsiravar@gmail.com>
@vsiravar vsiravar merged commit 42aa755 into runfinch:windev Oct 4, 2023
2 checks passed
vsiravar added a commit that referenced this pull request Oct 17, 2023
Issue #, if available:

*Description of changes:*
Update run tests related to cgroup v2 resource flag tests for windows.

In wsl2 cgroup v2 is mounted at /sys/fs/cgroup/unified but containerd
expects it to be mounted at /sys/fs/cgroup
https://github.com/containerd/cgroups/blob/cc78c6c1e32dc5bde018d92999910fdace3cfa27/utils.go#L36.

Enabling cgroup v2 requires extra tweaks in wsl 2 based on
https://stackoverflow.com/questions/73021599/how-to-enable-cgroup-v2-in-wsl2
*Testing done:*
Yes. 


- [X] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Signed-off-by: Vishwas Siravara <vsiravar@gmail.com>
Co-authored-by: Vishwas Siravara <vsiravar@gmail.com>
vsiravar added a commit that referenced this pull request Oct 17, 2023
Issue #, if available:

*Description of changes:*
Update run tests related to cgroup v2 resource flag tests for windows.

In wsl2 cgroup v2 is mounted at /sys/fs/cgroup/unified but containerd
expects it to be mounted at /sys/fs/cgroup
https://github.com/containerd/cgroups/blob/cc78c6c1e32dc5bde018d92999910fdace3cfa27/utils.go#L36.

Enabling cgroup v2 requires extra tweaks in wsl 2 based on
https://stackoverflow.com/questions/73021599/how-to-enable-cgroup-v2-in-wsl2
*Testing done:*
Yes.

- [X] I've reviewed the guidance in CONTRIBUTING.md

#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Signed-off-by: Vishwas Siravara <vsiravar@gmail.com>
Co-authored-by: Vishwas Siravara <vsiravar@gmail.com>
Signed-off-by: Vishwas Siravara <siravara@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants