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

Fix podman tests #470

Merged
merged 1 commit into from
Dec 12, 2023
Merged

Fix podman tests #470

merged 1 commit into from
Dec 12, 2023

Conversation

alexlarsson
Copy link
Contributor

The change in commit 3605a36 broke the podman test/system tests:

 ✗ [075] podman exec - cat from stdin
   (from function `bail-now' in file test/system/helpers.bash, line 227,
    from function `is' in file test/system/helpers.bash, line 956,
    in test file test/system/075-exec.bats, line 84)
     `is "${lines[0]}" "3000+0 records in"  "dd: number of records in"' failed
   [11:10:24.237041104] $ /vcs/other/podman/bin/podman rm -t 0 --all --force --ignore
   [11:10:24.272179180] $ /vcs/other/podman/bin/podman ps --all --external --format {{.ID}} {{.Names}}
   [11:10:24.307984108] $ /vcs/other/podman/bin/podman images --all --format {{.Repository}}:{{.Tag}} {{.ID}}
   [11:10:24.339679735] quay.io/libpod/systemd-image:20230531 9984d4cfd1eb  quay.io/libpod/testimage:20221018 f5a99120db64
   [11:10:24.357682655] $ /vcs/other/podman/bin/podman run -d quay.io/libpod/testimage:20221018 top
   [11:10:24.499542609] 76f963b708fe74814e8e078bbc41af84368beafaad2da932dc4ee7c3d6629409
   [11:10:24.512104004] $ /vcs/other/podman/bin/podman exec -i 76f963b708fe74814e8e078bbc41af84368beafaad2da932dc4ee7c3d6629409 cat
   [11:10:24.648299147] of9AOlHs62yCSNsgYav5
   1500+0 records in
   1500+0 records out
   1536000 bytes (1,5 MB, 1,5 MiB) copied, 0,00696935 s, 220 MB/s
   [11:10:24.670650275] $ /vcs/other/podman/bin/podman exec -i 76f963b708fe74814e8e078bbc41af84368beafaad2da932dc4ee7c3d6629409 dd of=/tmp/bigfile bs=512
   [11:10:24.768570699] 2999+1 records in
   2999+1 records out
   #/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
   #|     FAIL: dd: number of records in
   #| expected: '3000+0 records in'
   #|   actual: '2999+1 records in'
   #\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I'm not sure exactly why the socket buffer read size matters here, but changing it back to reading exactly 32k fixes the test again.

To keep the fix from #467 I added an extra byte to the buffer. However, I wonder if that change was strictly needed, as the sock->buf[num_read] = '\0' code is only used for the notify case, which is not a streaming socket.

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

The change in commit 3605a36 broke
the podman test/system tests:

```
 ✗ [075] podman exec - cat from stdin
   (from function `bail-now' in file test/system/helpers.bash, line 227,
    from function `is' in file test/system/helpers.bash, line 956,
    in test file test/system/075-exec.bats, line 84)
     `is "${lines[0]}" "3000+0 records in"  "dd: number of records in"' failed
   [11:10:24.237041104] $ /vcs/other/podman/bin/podman rm -t 0 --all --force --ignore
   [11:10:24.272179180] $ /vcs/other/podman/bin/podman ps --all --external --format {{.ID}} {{.Names}}
   [11:10:24.307984108] $ /vcs/other/podman/bin/podman images --all --format {{.Repository}}:{{.Tag}} {{.ID}}
   [11:10:24.339679735] quay.io/libpod/systemd-image:20230531 9984d4cfd1eb  quay.io/libpod/testimage:20221018 f5a99120db64
   [11:10:24.357682655] $ /vcs/other/podman/bin/podman run -d quay.io/libpod/testimage:20221018 top
   [11:10:24.499542609] 76f963b708fe74814e8e078bbc41af84368beafaad2da932dc4ee7c3d6629409
   [11:10:24.512104004] $ /vcs/other/podman/bin/podman exec -i 76f963b708fe74814e8e078bbc41af84368beafaad2da932dc4ee7c3d6629409 cat
   [11:10:24.648299147] of9AOlHs62yCSNsgYav5
   1500+0 records in
   1500+0 records out
   1536000 bytes (1,5 MB, 1,5 MiB) copied, 0,00696935 s, 220 MB/s
   [11:10:24.670650275] $ /vcs/other/podman/bin/podman exec -i 76f963b708fe74814e8e078bbc41af84368beafaad2da932dc4ee7c3d6629409 dd of=/tmp/bigfile bs=512
   [11:10:24.768570699] 2999+1 records in
   2999+1 records out
   #/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
   #|     FAIL: dd: number of records in
   #| expected: '3000+0 records in'
   #|   actual: '2999+1 records in'
   #\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

I'm not sure exactly why the socket buffer read size matters here, but
changing it back to reading exactly 32k fixes the test again.

To keep the fix from containers#467 I
added an extra byte to the buffer. However, I wonder if that change
was strictly needed, as the `sock->buf[num_read] = '\0'` code is only
used for the notify case, which is not a streaming socket.

Signed-off-by: Alexander Larsson <[email protected]>
@giuseppe giuseppe merged commit 311c053 into containers:main Dec 12, 2023
16 checks passed
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.

2 participants