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

CI fixes #4085

Merged
merged 3 commits into from
Oct 24, 2023
Merged

CI fixes #4085

merged 3 commits into from
Oct 24, 2023

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Oct 20, 2023

Fixups after PR #3205

tests/int: fix cgroup tests

Commit e158483 did two modifications to check_cgroup_value():

  1. It now skips the test if the file is not found.

  2. If the comparison failed, a second comparison, with value divided by 1000,
    is performed.

These modifications were only needed for cpu.burst, but instead were done
in a generic function used from many cgroup tests. As a result, we can
no longer be sure about the test coverage (item 1) and the check being
correct (item 2) anymore. In fact, part of "update cgroup cpu limits"
test is currently skipped on CentOS 7 and 8 because of item 1.

To fix:

  • replace item 1 with a new "cgroups_cpu_burst" argument for "requires",
    and move the test to a separate case;
  • replace item 2 with a local change in check_cpu_burst.

libct/cg/fs.Set: fix error message

There is no point in showing the underlying error when path == "",
because it is inevitably ENOENT.

Revert the change done in commit e158483.

Fixup after PR #4073

tests/int: fix "runc run (hugetlb limits)"

Recent commit 4a7d3ae had a bug.

@kolyshkin kolyshkin force-pushed the fix-check_cgroup_value branch 4 times, most recently from f90e238 to e9b9301 Compare October 20, 2023 23:19
@kolyshkin kolyshkin changed the title [debug] see what breaks Fixups after PR #3205 Oct 20, 2023
@kolyshkin kolyshkin force-pushed the fix-check_cgroup_value branch 2 times, most recently from 824feb6 to 93f28e4 Compare October 23, 2023 17:38
@kolyshkin kolyshkin changed the title Fixups after PR #3205 CI fixes Oct 23, 2023
@kolyshkin kolyshkin marked this pull request as ready for review October 23, 2023 18:24
Recent commit 4a7d3ae had a bug (extra period).

Fixes: 4a7d3ae
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit e158483 did two modifications to check_cgroup_value():

1. It now skips the test if the file is not found.

2. If the comparison failed, a second comparison, with value divided by 1000,
   is performed.

These modifications were only needed for cpu.burst, but instead were done
in a generic function used from many cgroup tests. As a result, we can
no longer be sure about the test coverage (item 1) and the check being
correct (item 2) anymore. In fact, part of "update cgroup cpu limits"
test is currently skipped on CentOS 7 and 8 because of item 1.

To fix:
 - replace item 1 with a new "cgroups_cpu_burst" argument for "requires",
   and move the test to a separate case;
 - replace item 2 with a local change in check_cpu_burst.

Fixes: e158483
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
There is no point in showing the underlying error when path == "",
because it is ENOENT.

Revert the change done in commit e158483.

Fixes: e158483
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Copy link
Member

@lifubang lifubang left a comment

Choose a reason for hiding this comment

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

Sorry, I made a mistake, #4086 should be merged first if you have not cherry-picked it to here.

@kolyshkin
Copy link
Contributor Author

Sorry, I made a mistake, #4086 should be merged first if you have not cherry-picked it to here.

For simplicity, I did just that (#4086 is included).

@kolyshkin kolyshkin merged commit 3272edf into opencontainers:main Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants