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.

Sign up for GitHub

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

Jump to bottom

Reverts removal of parallel run from critest #6938

Merged
merged 1 commit into from
May 12, 2022
Merged

Reverts removal of parallel run from critest #6938

merged 1 commit into from
May 12, 2022

Conversation

psschwei
Copy link
Contributor

Signed-off-by: Paul S. Schweigert paulschw@us.ibm.com

As discussed in #6903,
running ginkgo tests in parallel while trying to skip wasn't working.
However, now that
kubernetes-sigs/cri-tools#930 has fixed the
issue upstream, we can revert back to running our tests in parallel
with the skip.

@k8s-ci-robot
Copy link

Hi @psschwei. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mikebrow
mikebrow reviewed May 12, 2022
View reviewed changes
Vagrantfile Outdated
@@ -271,8 +271,7 @@ EOF
fi
trap cleanup EXIT
ctr version
# TODO run in parallel, see https://github.com/containerd/containerd/issues/6903
critest --ginkgo.skip='HostIpc is true' --report-dir="${REPORT_DIR}"
critest --parallel=$(nproc) --ginkgo.skip='HostIpc is true' --report-dir="${REPORT_DIR}"
Copy link
Member

Choose a reason for hiding this comment

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

maybe nproc + some extra.. we have underutilization issues where wait loops are involved and could use some extra stress

@mikebrow
Copy link
Member

/ok-to-test

mikebrow
mikebrow approved these changes May 12, 2022
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM on green Thanks Paul!

@dmcgowan dmcgowan added this to New in Code Review via automation May 12, 2022
@dmcgowan dmcgowan moved this from New to Ready For Review in Code Review May 12, 2022
@psschwei psschwei force-pushed the fix-parallel branch 2 times, most recently from bdc6cca to 894d689 Compare May 12, 2022 17:36
@psschwei
reverts removal of parallel run from critest
7300296 
Signed-off-by: Paul S. Schweigert <paulschw@us.ibm.com>

As discussed in containerd#6903,
running ginkgo tests in parallel while trying to skip wasn't working.
However, now that
kubernetes-sigs/cri-tools#930 has fixed the
issue upstream, we can revert back to running our tests in parallel
with the skip.
@kzys kzys moved this from Ready For Review to Merge on Green in Code Review May 12, 2022
@mikebrow mikebrow added the cherry-pick/1.6.x Change to be cherry picked to release/1.6 branch label May 12, 2022
@psschwei psschwei mentioned this pull request May 12, 2022
Merged
@dmcgowan dmcgowan merged commit b9bffd1 into containerd:main May 12, 2022
Code Review automation moved this from Merge on Green to Done May 12, 2022
@mikebrow mikebrow added cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch and removed cherry-pick/1.6.x Change to be cherry picked to release/1.6 branch labels May 12, 2022
@fuweid fuweid mentioned this pull request May 13, 2022
Closed
@mikebrow mikebrow mentioned this pull request May 27, 2022
Closed
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kzys kzys kzys approved these changes

@dmcgowan dmcgowan dmcgowan approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@mikebrow mikebrow mikebrow approved these changes

Assignees
No one assigned
Labels
cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch ok-to-test
Projects
Code Review
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@psschwei @k8s-ci-robot @mikebrow @kzys @dmcgowan @AkihiroSuda