Skip to content

Conversation

Creatone
Copy link
Owner

Signed-off-by: Paweł Szulik [email protected]

}

return stats, nil
return nil, nil

Choose a reason for hiding this comment

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

It looks strange but I see that nil, nil is returned when type was different than CTRL_MON and different than MON so maybe it will be useful to add log with proper information (maybe info level log).

Copy link
Owner Author

Choose a reason for hiding this comment

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

It will be easier to return the error.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I also provided a unit test for that.

@Creatone Creatone force-pushed the resctrl-mon-group branch 8 times, most recently from b6266be to 2b63c34 Compare July 27, 2020 13:07
@Creatone Creatone force-pushed the resctrl-mon-group branch 3 times, most recently from 711c104 to 507ecec Compare August 4, 2020 11:43
@Creatone Creatone force-pushed the resctrl-mon-group branch 3 times, most recently from 397e410 to 2852ea3 Compare August 25, 2020 12:29
@Creatone Creatone force-pushed the resctrl-mon-group branch 2 times, most recently from 3d6ca74 to ffe869e Compare October 5, 2020 12:35
@Creatone Creatone force-pushed the resctrl-mon-group branch 5 times, most recently from f570ef0 to 1fd7fdc Compare October 26, 2020 11:41
@Creatone Creatone force-pushed the resctrl-mon-group branch 2 times, most recently from 5b2c6b3 to 6276975 Compare October 28, 2020 17:26
kolyshkin and others added 28 commits April 3, 2023 11:27
In code that checks that the resource name is in the for
Using strings.SplitN is an overkill in this case, resulting in
allocations and thus garbage to collect.

Using strings.IndexByte and checking that result is not less than 1
(meaning there is a period, and it is not the first character) is
sufficient here.

Fixes: 0cb8bf6
Signed-off-by: Kir Kolyshkin <[email protected]>
Systemd v252 (available in CentOS Stream 9 in our CI) added support
for setting cpu.idle (see [1]). The way it works is:
 - if CPUWeight == 0, cpu.idle is set to 1;
 - if CPUWeight != 0, cpu.idle is set to 0.

This behavior breaks the existing test case, as described in [2].

To fix, skip the last check in the test case in case a newer systemd
is used.

Fixes: opencontainers#3786

[1] systemd/systemd#23299
[2] opencontainers#3786

Signed-off-by: Kir Kolyshkin <[email protected]>
Values other than 1 or 0 are ignored by the kernel,
see sched_group_set_idle() in kernel/sched/fair.c

If the added test case ever fails, it means that the kernel now accepts
values other than 0 or 1, and runc needs to adopt to that.

Signed-off-by: Kir Kolyshkin <[email protected]>
Systemd v252 (available in CentOS Stream 9 in our CI) added support
for setting cpu.idle (see [1]). The way it works is:
 - if CPUWeight == 0, cpu.idle is set to 1;
 - if CPUWeight != 0, cpu.idle is set to 0.

This commit implements setting cpu.idle in systemd cgroup driver via a
unit property. In case CPUIdle is set to non-zero value, the driver sets
adds CPUWeight=0 property, which will result in systemd setting cpu.idle
to 1.

Unfortunately, there's no way to set cpu.idle to 0 without also changing
the CPUWeight value, so the driver doesn't do anything if CPUIdle is
explicitly set to 0. This case is handled by the fs driver which is
always used as a followup to setting systemd unit properties.

Also, handle cpu.idle set via unified map. In case it is set to non-zero
value, add CPUWeight=0 property, and ignore cpu.weight (otherwise we'll
get two different CPUWeight properties set).

Add a unit test for new values in unified map, and an integration test case.

[1] systemd/systemd#23299
[2] opencontainers#3786

Signed-off-by: Kir Kolyshkin <[email protected]>
cg/sd: support CPUWeight=idle (aka cpu.idle=1)
…-no-init

libctr/cgroups: don't take init's cgroup into account
Bumps [lumaxis/shellcheck-problem-matchers](https://github.com/lumaxis/shellcheck-problem-matchers) from 1 to 2.
- [Release notes](https://github.com/lumaxis/shellcheck-problem-matchers/releases)
- [Commits](lumaxis/shellcheck-problem-matchers@v1...v2)

---
updated-dependencies:
- dependency-name: lumaxis/shellcheck-problem-matchers
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
These functions were added in ancient times, facilitating the
docker-in-docker case when cgroup namespace was not available.

As pointed out in commit 2b28b3c, using init 1 cgroup is not
correct because it won't work in case of host PID namespace.

The last user of GetInitCgroup was removed by commit
54e2021. GetInitCgroupPath was never used
as far as I can see, nor was I able to find any external users.

Remove both functions. Modify the comment in libct/cg/fs.subsysPath
to not refer to GetInitCgroupPath.

Signed-off-by: Kir Kolyshkin <[email protected]>
tests: Fix fuzzer location in oss-fuzz config
…/github_actions/lumaxis/shellcheck-problem-matchers-2

build(deps): bump lumaxis/shellcheck-problem-matchers from 1 to 2
…oup-path

libct/cg: rm GetInitCgroup[Path]
Bumps [golang.org/x/sys](https://github.com/golang/sys) from 0.6.0 to 0.7.0.
- [Release notes](https://github.com/golang/sys/releases)
- [Commits](golang/sys@v0.6.0...v0.7.0)

---
updated-dependencies:
- dependency-name: golang.org/x/sys
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…/go_modules/golang.org/x/sys-0.7.0

build(deps): bump golang.org/x/sys from 0.6.0 to 0.7.0
This version is already used by Cirrus CI Fedora 37 job, but other CI
jobs are still using 1.3.0.

Bump it everywhere so we can enjoy new version features and fixes.

For one thing, I noticed that new bats is reporting error location
correctly.

We will also be able to use "run !" and "run -N" commands.

Signed-off-by: Kir Kolyshkin <[email protected]>
Apparently, bash with set -e deliberately ignores non-zero return codes
from ! cmd, unless this is the last command. The workaround is to either
use "! cmd || false', "or run ! cmd". Choose the latter, and require
bash-core 1.5.0 (since this is when "run !" was added), replacing the
older check.

Alas I only learned this recently from the bash-core documentation.

Signed-off-by: Kir Kolyshkin <[email protected]>
This is just so that the container can join the misc controller.

Signed-off-by: Kir Kolyshkin <[email protected]>
This variable is used in curl to download a go release, so we are using
the initial Go 1.19 release in Cirrus CI, not the latest Go 1.19.x
release.

From the CI perspective, it makes more sense to use the latest release.

Add some jq magic to extract the latest minor release information
from the download page, and use it.

This brings Cirrus CI jobs logic in line with all the others (GHA,
Dockerfile), where by 1.20 we actually mean "latest 1.20.x".

Signed-off-by: Kir Kolyshkin <[email protected]>
1. Do not use echo, as this results in lines like this:

	...
	echo "-----"
	-----
	...

2. Move "cat /proc/cpuinfo" to be the last one, as the output is usually
   very long.

3. Add "go version" to CentOS jobs.

Signed-off-by: Kir Kolyshkin <[email protected]>
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.8.0 to 0.9.0.
- [Release notes](https://github.com/golang/net/releases)
- [Commits](golang/net@v0.8.0...v0.9.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…/go_modules/golang.org/x/net-0.9.0

build(deps): bump golang.org/x/net from 0.8.0 to 0.9.0
bump bats-core, fix some tests, use new features
For test jobs, add ubuntu 22.04 into the matrix, so we can test of both
cgroup v1 and v2.

For validate jobs, just switch to ubuntu 22.04

Signed-off-by: Kir Kolyshkin <[email protected]>
Do not accept setjmp return value as variable.

Signed-off-by: Kazuki Hasegawa <[email protected]>
Kazuki Hasegawa (1):
  Fix undefined behavior. Do not accept setjmp return value as variable.

LGTMs: AkihiroSuda cyphar
Closes opencontainers#3790
@Creatone Creatone force-pushed the resctrl-mon-group branch from e5ce002 to 775a366 Compare April 17, 2023 15:11
Signed-off-by: Paweł Szulik <[email protected]>
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.