Fix system df negative reclaimable size bug#25506
Fix system df negative reclaimable size bug#25506openshift-merge-bot[bot] merged 4 commits intocontainers:mainfrom
Conversation
cb2969f to
897ad6a
Compare
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.
|
LGTM |
|
/lgtm |
Includes my DiskUsage() changes. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Add a test for containers#24452 Signed-off-by: Paul Holzinger <pholzing@redhat.com>
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 <pholzing@redhat.com>
Giuseppe is working on some proper fixes, for now in order to get this moved along skip it so we can merge the disk usage fix. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
|
Ok I skipped the test on vfs for now unit we get the c/storage fix in. I like to get this merged and not wait for c/storage as there are more people with c/common changes and without this the tests will always fail for them. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, Honny1, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
Does this PR introduce a user-facing change?