-
Notifications
You must be signed in to change notification settings - Fork 654
fix: nerdctl ps filter --status=created #4196
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: nerdctl ps filter --status=created #4196
Conversation
@apostasie |
828ce70
to
31633cc
Compare
ab81f1e
to
58d90e5
Compare
@AkihiroSuda @apostasie
Seems CI/CD is somewhat stable, the gomodjail i dont understand atm. |
The logs after start with -a seems to not get handled correctly so changed the test as it was testing the create functionality. |
2967ba5
to
1368b12
Compare
Thanks a lot for engaging with these @Shubhranshu153 - really appreciated. Gomodjail issue is likely AkihiroSuda/gomodjail#50 - gomodjail does self-unpack (the actual executable and go.mod), but this is not currently done in a safe way (needs to use flock to prevent concurrent access). On your last run, I see an unrelated rekor error, some windows non-sense - and the event filter issue. I think we should have this patch here asap (cc @AkihiroSuda). |
The current CI failures do not seem related to gomodjail |
They are not on the last run (they are a rekor issue, windows, and the event one on EL) |
1368b12
to
4b50d62
Compare
@AkihiroSuda @apostasie Refactored the events test a bit lets see |
4b50d62
to
cf7781b
Compare
@@ -47,23 +48,16 @@ func TestCreate(t *testing.T) { | |||
{ |
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.
@Shubhranshu153 we should remove testCase.Require = nerdtest.IsFlaky("https://github.com/containerd/nerdctl/issues/3717")
just above.
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.
yes we should i want to see it a few more runs and would remove
I still think gomodjail has issues. and is flaky.
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.
Commit 4 in this PR should workaround gomodjail unpack issue: #4199
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.
@Shubhranshu153 so, the presence of testCase.Require = nerdtest.IsFlaky
will make it automatically retry in case of failure, which will "hide" them... What I am suggesting is that we remove it now so that you get accurate feedback from the CI on whether this is fixed or not.
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.
Sorry, wrong PR linked. gomodjail workaround is here: #4198
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.
The other (wrongly) mentioned PR would alleviate the Windows failure you are hitting on your last run.
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.
yeah i got confused as it seems unrelated.
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.
Great, lot of good fixes are going in to make the CI more resiliant.
have few PR's approved but pending successful runs so would like the fixes in to have a successful run.
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.
Agreed!
And thanks again for engaging on that.
@AkihiroSuda pending green and review of course, it would be really nice to have #4198, #4199, and this here, at your convenience - that should reduce a good chunk of flakyness and frustration :-).
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.
This looks good now.
Signed-off-by: Shubharanshu Mahapatra <[email protected]>
cf7781b
to
65f4412
Compare
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.
Thanks
Issue:
Fix: