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

Workaround flaky save #3179

Merged
merged 1 commit into from
Jul 5, 2024
Merged

Conversation

apostasie
Copy link
Contributor

Let's see what the CI says...

This one has been driving me bonkers for a while.

The issue is obviously still here (and I believe it is a containerd bug), but the workaround seems to work.

@apostasie
Copy link
Contributor Author

Ok, let's not jinx it, but for me locally, it DOES fix #2327 and with four pushes, we never failed once the dreaded TestRunCustomRootfs:
https://github.com/containerd/nerdctl/actions
(other stuff failing though...)

I am convinced the underlying issue is still here though - I also reproduced it with nerdcl/mod-v2 & containerd-v2 - so, it is here to stay.

@AkihiroSuda I suggest we merge this ASAP.

Worse case scenario it does nothing (this is good hygiene anyway to have that in a separate namespace). Best case scenario, it does make our lives significantly better...

Tagging @yankay and @fahedouch - who had some discussion about the underlying issue in #2327 .

For good measure, I am namespacing as well the other save tests.

@apostasie apostasie force-pushed the dev-die-flake-die branch from c8e0204 to 9935520 Compare July 5, 2024 05:16
@AkihiroSuda
Copy link
Member

Please squash the commits, then LGTM

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Jul 5, 2024
@AkihiroSuda AkihiroSuda added the area/ci e.g., CI failure label Jul 5, 2024
@apostasie apostasie force-pushed the dev-die-flake-die branch from 9935520 to ecf7dac Compare July 5, 2024 05:42
@apostasie
Copy link
Contributor Author

@AkihiroSuda done. Thanks.

@apostasie
Copy link
Contributor Author

Wait. Something wrong with the project checks?

@apostasie
Copy link
Contributor Author

apostasie commented Jul 5, 2024

@AkihiroSuda

Ok, so.
Apparently, containerd project-checks github action does bundle and depend on Vincent's https://github.com/vbatts/git-validation which by default calls git show --check which does enforce some form of linting on whitespaces.

Whitespaces "issues" have been flagged in my commit about testing from today (the ones that modified just markdown files).

The reason it did escape us is that we recently disabled running CI test for markdown-only changes...
And clearly we should still run Project Checks for these.

I'll take care of the CI right now (splitting out project checks and re-enabling them for md files changes)

Sorry about this mess.
This testing doc is cursed...

Signed-off-by: apostasie <spam_blackhole@farcloser.world>
@apostasie apostasie force-pushed the dev-die-flake-die branch from ecf7dac to faa28f8 Compare July 5, 2024 06:23
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit bc24aa9 into containerd:main Jul 5, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci e.g., CI failure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants