Skip to content

Update critools to v1.24#6894

Merged
kzys merged 1 commit intocontainerd:mainfrom
psschwei:update-critools
May 6, 2022
Merged

Update critools to v1.24#6894
kzys merged 1 commit intocontainerd:mainfrom
psschwei:update-critools

Conversation

@psschwei
Copy link
Copy Markdown
Contributor

@psschwei psschwei commented May 5, 2022

Signed-off-by: Paul S. Schweigert [email protected]

Fixes #6886

@k8s-ci-robot
Copy link
Copy Markdown

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.

Details

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.

@estesp
Copy link
Copy Markdown
Member

estesp commented May 5, 2022

Did anything change with the portforward test? Seems it passed in some runs and failed in others:

E0505 14:03:37.868876   17020 portforward.go:406] an error occurred forwarding 12002 -> 12003: error forwarding port 12003 to pod 43c0bad530f5f826257f7e14f653046147445373508f992900d128902e5773e7, uid : failed to execute portforward in network namespace "host": failed to connect to localhost:12003 inside namespace "43c0bad530f5f826257f7e14f653046147445373508f992900d128902e5773e7", IPv4: dial tcp4 127.0.0.1:12003: connect: connection refused IPv6 dial tcp6 [::1]:12003: connect: connection refused 
E0505 14:03:37.868993   17020 portforward.go:234] lost connection to pod

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 5, 2022

Did anything change with the portforward test? Seems it passed in some runs and failed in others:

E0505 14:03:37.868876   17020 portforward.go:406] an error occurred forwarding 12002 -> 12003: error forwarding port 12003 to pod 43c0bad530f5f826257f7e14f653046147445373508f992900d128902e5773e7, uid : failed to execute portforward in network namespace "host": failed to connect to localhost:12003 inside namespace "43c0bad530f5f826257f7e14f653046147445373508f992900d128902e5773e7", IPv4: dial tcp4 127.0.0.1:12003: connect: connection refused IPv6 dial tcp6 [::1]:12003: connect: connection refused 
E0505 14:03:37.868993   17020 portforward.go:234] lost connection to pod

https://github.com/kubernetes-sigs/cri-tools/blame/master/pkg/validate/networking.go#L193 but I don't think that change could be the problem.

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 5, 2022

Comment thread script/setup/critools-version Outdated
@psschwei psschwei force-pushed the update-critools branch from 53ba1ba to ceba74b Compare May 5, 2022 17:49
@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 5, 2022

/ok-to-test

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 5, 2022

hmm.. vagrant (rocky /fedora) not picking up the skip for HostIpc is true

https://github.com/containerd/containerd/blame/main/Vagrantfile#L273

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 5, 2022

ah.. the report dir env variable isn't set so it "ate" the skip

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 5, 2022

@psschwei rebase when you get a chance to pick up #6900

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 5, 2022

i'll cherry 6900 after you verify the fix here...

@psschwei psschwei force-pushed the update-critools branch from ceba74b to 255b6a0 Compare May 5, 2022 23:56
@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 6, 2022

still not skipping ..

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 6, 2022

ok.. critest --ginkgo.focus="HostIpc" --ginkgo.skip="HostIpc is true" works but add --parallel=2

and it fails to focus or skip

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 6, 2022

even with ginkgo v2 --parallel=2 just does not work anymore

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 6, 2022

have to add this: go install github.com/onsi/ginkgo/v2/ginkgo@latest to get critest parallel to work

@psschwei psschwei force-pushed the update-critools branch from 89960d5 to 9348c7c Compare May 6, 2022 14:27
Comment thread script/setup/install-critools Outdated
Copy link
Copy Markdown
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

@psschwei psschwei force-pushed the update-critools branch from a27ed3c to 7d04bba Compare May 6, 2022 17:09
Comment thread script/setup/critools-version Outdated
Signed-off-by: Paul S. Schweigert <[email protected]>
@psschwei psschwei force-pushed the update-critools branch from 7d04bba to bb11c9d Compare May 6, 2022 17:28
Copy link
Copy Markdown
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 Thanks Paul!

@estesp
Copy link
Copy Markdown
Member

estesp commented May 6, 2022

Just FYI, I compared some runs with the parallel flag working against this PR. While it is a "huge" jump for this specific test in isolation (sample: 2m46s - 2m50s vs. 1m51 - 2m01s), those 40-50 seconds are being added to an overall CI run that averages 25-28 minutes, so I don't think this is a huge problem to not be parallelized until we solve the gingko issue.

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp added cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch and removed cherry-pick/1.6.x labels May 6, 2022
@estesp
Copy link
Copy Markdown
Member

estesp commented May 6, 2022

Cherry picked in #6895

@mikebrow
Copy link
Copy Markdown
Member

mikebrow commented May 6, 2022

Just FYI, I compared some runs with the parallel flag working against this PR. While it is a "huge" jump for this specific test in isolation (sample: 2m46s - 2m50s vs. 1m51 - 2m01s), those 40-50 seconds are being added to an overall CI run that averages 25-28 minutes, so I don't think this is a huge problem to not be parallelized until we solve the gingko issue.

nod.. for me running in parallel is more about finding "random" timing issues... we run parallel 8 over in ci.yaml.. and I agree not a big issue.. just don't want to forget about it.. Talked to Sascha will work on it with him next week.

@kzys kzys merged commit 68d9d46 into containerd:main May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch ok-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade to latest cri-tools for k8s v1.24

6 participants