Fix system df negative reclaimable size bug #25506
Conversation
openshift-ci
bot
added
release-note
do-not-merge/work-in-progress
Luap99
force-pushed
the
disk-usage
branch
2 times, most recently
from
cb2969f to
897ad6a
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
test/system/320-system-df.bats
Outdated
| is "$output" ".*1[0-9].[0-9]\\+MB.*3.[0-9]\\+kB\\s\\+2.*" "Shared and Unique Size" | ||
| is "$output" ".*1[0-9].[0-9]\\+MB.*6.[0-9]\\+kB\\s\\+0.*" "Shared and Unique Size" |
There was a problem hiding this comment.
What do these two regexes check? Wouldn't it be better to use assert instead of is?
There was a problem hiding this comment.
assert or is should be the same here, because the test is using is overall I have opted to keep using is for consistency.
The sizes are non stable (metadat, different storage drivers, etc...) so I have opted to use a regex range that makes sense.
There was a problem hiding this comment.
| is "$output" ".*1[0-9].[0-9]\\+MB.*3.[0-9]\\+kB\\s\\+2.*" "Shared and Unique Size" | |
| is "$output" ".*1[0-9].[0-9]\\+MB.*6.[0-9]\\+kB\\s\\+0.*" "Shared and Unique Size" | |
| # regex match: SHARED SIZE | UNIQUE SIZE | CONTAINERS | |
| is "$output" ".*[1-9]\\d*\\.[0-9]\\+MB\\s\\+[1-9]\\d*\\.[0-9]\\+kB\\s\\+2.*" "Shared and Unique Size" | |
| is "$output" ".*[1-9]\\d*\\.[0-9]\\+MB\\s\\+[1-9]\\d*\\.[0-9]\\+kB\\s\\+0.*" "Shared and Unique Size" |
I would use this regex, which is preserved even when updating and resizing the image.
Maybe only one line could be checked. Because there is only a difference between the container numbers.
There was a problem hiding this comment.
Yes that is an option if we want to allow all numbers, it certainly should make it more future proof on image changes.
There was a problem hiding this comment.
ok used assert and single quotes which hopefully should make the regex more readable, also I did not add 1-9 extra check as this seems like more complications that don't add any test value.
Includes my DiskUsage() changes. Signed-off-by: Paul Holzinger <[email protected]>
Add a test for containers#24452 Signed-off-by: Paul Holzinger <[email protected]>
Our calculation is just wrong and the way the entire API is designed it cannot work. This is the same interface as docker is using and they have the same bug there. So simply document this as known problem, in case users complain we at least have something to point to. An actual fix might be possible but not without reworking the full API and because this is exposed in the docker compat and libpod REST API we cannot really change it. Signed-off-by: Paul Holzinger <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Honny1, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
LGTM |
|
/lgtm |
Does this PR introduce a user-facing change?