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.

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

Jump to bottom

Fix tests/cgroup on cgroup2 #299

Merged
merged 3 commits into from
Oct 4, 2024
Merged

Fix tests/cgroup on cgroup2 #299

merged 3 commits into from
Oct 4, 2024

Conversation

mihalicyn
Copy link
Member

When cgroup2 is used there is a difference between root of cgroupfs tree and sub-cgroups. When we do a cgroup feature checking we should check for memory.swap.max file existence in a /sys/fs/cgroup/system.slice/memory.swap.max instead of /sys/fs/cgroup/memory.swap.max.

See also:
lxc/lxcfs@f496e62

@mihalicyn
Copy link
Member Author

After merging this we will get test regressions on older LXD versions (when run on cgroup2-enabled hosts). But really those are not regressions and we should update LXCFS to v6.0.2 or backport lxc/lxcfs#617

@mihalicyn mihalicyn marked this pull request as draft October 1, 2024 15:42
@mihalicyn mihalicyn force-pushed the cgroup_test_imp branch 2 times, most recently from 2108767 to af02993 Compare October 1, 2024 15:49
@simondeziel
Copy link
Member

latest/edge which has the proper LXCFS version is now failing at a later test: https://github.com/canonical/lxd-ci/actions/runs/11128901521/job/30924728690?pr=299#step:8:432

@mihalicyn
Copy link
Member Author

latest/edge which has the proper LXCFS version is now failing at a later test: https://github.com/canonical/lxd-ci/actions/runs/11128901521/job/30924728690?pr=299#step:8:432

yeah, I'm on it ;-)

@mihalicyn
Copy link
Member Author

Sigh... lxc/lxcfs#661

@mihalicyn
Copy link
Member Author

mihalicyn commented Oct 2, 2024 •
edited
Loading 

==> Testing memory limits
+ MEM_LIMIT_MIB=512
+ lxc config set c1 limits.memory=512MiB
++ lxc exec c1 -- grep '^MemTotal' /proc/meminfo
+ '[' 'MemTotal:       16373812 kB' = 'MemTotal:         524288 kB' ']'

on latest/edge - 24.04 will be fixed by lxc/lxcfs#661

Test failed
++ grep '^SwapTotal' /proc/meminfo
+ '[' 'SwapTotal:             0 kB' = 'SwapTotal:       4194300 kB' ']'

on 5.21/edge - 22.04, 5.0/edge - 22.04, 4.0/edge - 22.04, 5.21/edge - 24.04

should be fixed by lxc/lxcfs#663

@mihalicyn
Copy link
Member Author

I don't think that we should backport lxc/lxcfs#663 to LXD LTS releases (which using LXCFS versions 4.0.X and 5.0.X) as it was never working there. But this fix will be included into the next stable 6.0.X LXCFS release, for sure.

@tomponline
Copy link
Member

But this fix will be included into the next stable 6.0.X LXCFS release, for sure.

Can we patch it for latest/candidate and latest/edge so we dont have persistent failures on CI and a regression in the next release?

@mihalicyn
Copy link
Member Author

Can we patch it for latest/candidate and latest/edge so we dont have persistent failures on CI and a regression in the next release?

latest/candidate and latest/edge are using main branch of LXCFS so they should be fine.

I have also fixed tests to handle this differences properly.

But it's not the end of a story yet. I need to figure out one more thing which I found weird in there...

@mihalicyn
Copy link
Member Author

ah, it's the same thing which I fixed by lxc/lxcfs#661 but now it fails on swap checks. Should be fine. Let's wait until latest/edge is rebuilt.

@mihalicyn mihalicyn marked this pull request as ready for review October 2, 2024 16:49
@mihalicyn
bin/helpers: add more diagnostic info on failures
2864b9b 
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
@mihalicyn
tests/cgroup: check for memory.swap.max in non-root cgroup
eeab22d 
There is no /sys/fs/cgroup/memory.swap.max file in cgroup tree root
we should look for it in a non-root cgroup instead.

See also LXCFS fix:
lxc/lxcfs@f496e62

Signed-off-by: Alexander Mikhalitsyn <[email protected]>
@mihalicyn
tests/cgroup: take into account LXCFS support for swap accounting
49c9c07 
Older LXCFS versions (4.0.x and 5.0.x branches) do not handle
swap accounting on cgroup2 properly. While we are improving cgroup2
tests for a newer LXD versions we don't want to make tests red on
the older ones. So, let's check if LXCFS declares support for
swap accounting and only in this case make any checks for it.

See also:
lxc/lxcfs#663

Signed-off-by: Alexander Mikhalitsyn <[email protected]>
@mihalicyn
Copy link
Member Author

@tomponline I think it is good to go now

tomponline
tomponline reviewed Oct 4, 2024
View reviewed changes
tests/cgroup Outdated Show resolved Hide resolved
@tomponline tomponline merged commit ad7765d into canonical:main Oct 4, 2024
136 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simondeziel simondeziel simondeziel approved these changes

@tomponline tomponline tomponline approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mihalicyn @simondeziel @tomponline