conformance tests: more parallelization - #6851
Conversation
24cc927 to
1879631
Compare
|
Looks like that conformance-using-vfs job really is I/O bound. |
b5404bb to
2963718
Compare
|
LGTM |
|
@nalind looks like you have some conflicts that need to be resolved on this one. |
Honny1
left a comment
There was a problem hiding this comment.
LGTM, I have just a possible cleanup comment.
| var eg errgroup.Group | ||
| eg.Go(func() error { | ||
| if test.contextDir != "" { | ||
| getErr = copier.Get("", testDataDir, copier.GetOptions{}, []string{test.contextDir}, pipeWriter) | ||
| } else if test.dockerfile != "" { | ||
| getErr = copier.Get("", testDataDir, copier.GetOptions{}, []string{test.dockerfile}, pipeWriter) | ||
| } | ||
| pipeWriter.Close() | ||
| wg.Done() | ||
| }() | ||
| wg.Add(1) | ||
| go func() { | ||
| return errors.Join(getErr, pipeWriter.Close()) | ||
| }) | ||
| eg.Go(func() error { | ||
| if test.contextDir != "" || test.dockerfile != "" { | ||
| putErr = copier.Put("", contextDir, copier.PutOptions{}, pipeReader) | ||
| } else { | ||
| putErr = os.Mkdir(contextDir, 0o755) | ||
| } | ||
| pipeReader.Close() | ||
| wg.Done() | ||
| }() | ||
| wg.Wait() | ||
| return errors.Join(putErr, pipeReader.Close()) | ||
| }) | ||
| err = eg.Wait() | ||
| assert.NoErrorf(t, err, "error copying build info from %q", filepath.Join("testdata", test.dockerfile)) | ||
| assert.NoErrorf(t, getErr, "error reading build info from %q", filepath.Join("testdata", test.dockerfile)) | ||
| assert.NoErrorf(t, putErr, "error writing build info to %q", contextDir) |
There was a problem hiding this comment.
Would it be better to assert errors immediately in the go routine?
There was a problem hiding this comment.
Sure. They're safe to be called outside of the goroutine that's running the test (their require counterparts, not so much).
8c8538f to
25b063d
Compare
And they both are in the new setup, the conformance test jobs don't complete within the timeout when they currently finish on main. |
In cases where we _prefetch an image, don't create a working container using "--pull" if we only care about the ID of the new container, i.e., we don't care that we actually tried to pull the image from the registry. Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
The buildah_version and buildah_info tasks run the main binary without making sure that it's been built first. Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Parallelize (most of) the subtests in TestConformance() and TestCommit(). Use t.Cleanup() instead of "defer" for a few cleanups. Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
|
Ephemeral COPR build failed. @containers/packit-build please check. |
What type of PR is this?
/kind failing-test
What this PR does / why we need it:
Parallelize more subtests to try to finish conformance testing faster.
Use t.Cleanup() instead of defer for a few cleanups.
How to verify it
Conformance tests should complete in less time unless they're I/O bound.
Which issue(s) this PR fixes:
Special notes for your reviewer:
The "conformance tests using VFS" job seems to be limited by I/O, as it's not faster when given more memory and more CPU, and actually appears to be slower when we add the parallelization here. The overlay counterpart, however, finishes in less time than it did before.
Does this PR introduce a user-facing change?