-
Notifications
You must be signed in to change notification settings - Fork 52
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
Update dependencies to latest #135
Conversation
Signed-off-by: Balaji Vijayakumar <[email protected]>
Performance report System Info Log
|
@balajiv113 @cfergeau thanks a lot for this PR. The gvisor dependency bump is a requirement for us to unblock podman rpm builds on Fedora, so the sooner we have it, the better. Please let me know if I can help out in anyway with speeding up this PR merge. |
Signed-off-by: Balaji Vijayakumar <[email protected]>
Fixed the lint issue. Not sure if it has to do something with the CI run. |
/approve |
@guillaumerose @gbraad can one of you please merge this? |
@anjannath can you verify? @baude, Guillaume does not work on this anymore. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: balajiv113, baude, gbraad The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@balajiv113 @baude Shall I also create a new release? |
@gbraad - yes we can. But the tests failed again with unexpected EOF on main. But, This is not happening for me locally. Even in CI, after retry it was successful before. |
I think this is the issue, https://github.com/containers/gvisor-tap-vsock/blob/main/test/suite_test.go#L224 Once we are able to run |
at the moment unable to verify or check this (PTO-mode for most of the week) |
@balajiv113 any update on this? |
@gbraad Around next week. I will try to put down a alternate solution than reading 500 character and see if CI is happy with that |
@gbraad |
@balajiv113 if you check here (https://github.com/containers/gvisor-tap-vsock/blob/main/test/suite_test.go#L121) , it’s the reverse. Only if the ssh connection fails it checks the qemu console log for a panic. There is a spurious panic issue that can occur on Mac when the process is run from a background process outside of the UI process tree (unless you set a launchd config that confers similar process priority) The former is the case with the GitHub agent, (which we don’t control the launchd config of) so we need a mechanism to retry a kernel boot. Otherwise CI will intermittently fail. |
@n1hility this might need additional work. Either we choose to disable this specific test and allow the CI to be green (fix soon after), fix the issue and hold further merges until fixed, or choose to ignore this for 'now'. |
@gbraad its likely just a race in the suite, so probably fine to ignore. Alternatively my suggestion would be to just have the code ignore ErrUnexpectedEOF. I just wanted to point out that if the whole section is disabled CI we might see panics come back which result tin CI hangs. |
In that case, the testcase can be dropped or adjusted for this. |
Signed-off-by: Balaji Vijayakumar [email protected]
Incorporates changes from #105