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

Jump to bottom

[release/1.5] Setup pod network after creating the sandbox container #7464

Merged
merged 3 commits into from Nov 16, 2022
Merged

[release/1.5] Setup pod network after creating the sandbox container #7464

merged 3 commits into from Nov 16, 2022

Conversation

qiutongs
Copy link
Contributor

@qiutongs qiutongs commented Oct 3, 2022 •
edited

Backport #7456 and #7505 to 1.5.

$ script/setup/install-failpoint-binaries
$ CONTAINERD_RUNTIME=runc FOCUS=TestRunPodSandboxWithShimStartAndTeardownCNIFailure make cri-integration
$ CONTAINERD_RUNTIME=runc FOCUS=TestRunPodSandboxWithShimDeleteFailure make cri-integration
$ CONTAINERD_RUNTIME=runc FOCUS=TestRunPodSandboxWithShimStartAndTeardownCNISlow make cri-integration

@k8s-ci-robot
Copy link

Hi @qiutongs. 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.

@qiutongs
Copy link
Contributor Author

qiutongs commented Oct 3, 2022

/cc @samuelkarp @fuweid @dmcgowan

@qiutongs qiutongs force-pushed the backport-ip-leakage-1.5 branch 5 times, most recently from 0307ab8 to 3cf005e Compare October 3, 2022 13:29
@samuelkarp samuelkarp added impact/changelog area/cri Container Runtime Interface (CRI) labels Oct 4, 2022
@samuelkarp samuelkarp added this to the 1.5.14 milestone Oct 4, 2022
@samuelkarp
Copy link
Member

/ok-to-test

@samuelkarp samuelkarp modified the milestones: 1.5.14, 1.5.15 Oct 21, 2022
@samuelkarp
Copy link
Member

With #7578 merged it'd be good to bring the failpoint-based tests over to this PR.

@qiutongs qiutongs changed the title [Draft] [release/1.5] Setup pod network after creating the sandbox container [release/1.5] Setup pod network after creating the sandbox container Oct 26, 2022
@qiutongs qiutongs force-pushed the backport-ip-leakage-1.5 branch 4 times, most recently from f136e13 to 5fe264a Compare October 26, 2022 12:37
fuweid
fuweid reviewed Oct 26, 2022
View reviewed changes
integration/main_test.go Outdated Show resolved Hide resolved
integration/sandbox_run_rollback_linux_test.go Outdated Show resolved Hide resolved
integration/sandbox_run_rollback_linux_test.go Outdated Show resolved Hide resolved
integration/sandbox_run_rollback_linux_test.go Outdated Show resolved Hide resolved
integration/sandbox_run_rollback_linux_test.go Outdated Show resolved Hide resolved
@qiutongs qiutongs requested a review from fuweid October 27, 2022 13:01
fuweid
fuweid approved these changes Oct 27, 2022
View reviewed changes
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

We should merge it after rollout 1.5.14 release

@estesp
Copy link
Member

estesp commented Oct 27, 2022

@fuweid 1.5.14 went out earlier this week so I assume this is safe to merge?

@fuweid
Copy link
Member

fuweid commented Oct 27, 2022

@fuweid 1.5.14 went out earlier this week so I assume this is safe to merge?

Oops! I miss the release. Yes! It is safe to merge.

samuelkarp
samuelkarp reviewed Oct 27, 2022
View reviewed changes
Copy link
Member

@samuelkarp samuelkarp left a comment

Choose a reason for hiding this comment

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

LGTM other than the nit about commit history and the errors.Wrap/fmt.Errorf change.

@dmcgowan dmcgowan added this to Ready For Review in Code Review Oct 27, 2022
@estesp estesp mentioned this pull request Oct 28, 2022
Closed
@qiutongs
Persist container and sandbox if resource cleanup fails, like teardow…
b4932cd 
…nPodNetwork

Signed-off-by: Qiutong Song <songqt01@gmail.com>
@qiutongs
Add integration tests with failpoint
b40d1ef 
Signed-off-by: Qiutong Song <songqt01@gmail.com>
(cherry picked from commit 4196fad)
Signed-off-by: Qiutong Song <songqt01@gmail.com>
(cherry picked from commit b9a35c6)
Signed-off-by: Qiutong Song <songqt01@gmail.com>
@qiutongs
Update container with sandbox metadata after NetNS is created
0db865d 
Signed-off-by: Qiutong Song <songqt01@gmail.com>
(cherry picked from commit b41d6f4)
Signed-off-by: Qiutong Song <songqt01@gmail.com>
(cherry picked from commit f2376e6)
Signed-off-by: Qiutong Song <songqt01@gmail.com>
@qiutongs
Copy link
Contributor Author

qiutongs commented Nov 7, 2022

pull-containerd-node-e2e

validate service connection: CRI v1 runtime API is not implemented for endpoint "unix:///run/containerd/containerd.sock": rpc error: code = Unimplemented desc = unknown service runtime.v1.RuntimeService

v1 is expected to be not implemented.

@qiutongs
Copy link
Contributor Author

qiutongs commented Nov 7, 2022

/retest

@estesp
Copy link
Member

estesp commented Nov 7, 2022

@qiutongs: FYI: kubernetes/test-infra#27912

@samuelkarp
Copy link
Member

/retest

@qiutongs
Copy link
Contributor Author

The tests still fail in the https://testgrid.k8s.io/sig-node-containerd#pull-containerd-node-e2e-1-5

@qiutongs
Copy link
Contributor Author

/retest

@k8s-ci-robot
Copy link

k8s-ci-robot commented Nov 14, 2022 •
edited

@qiutongs: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-containerd-node-e2e 0db865d link true /test pull-containerd-node-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@akhilerm
Copy link
Member

Hi @qiutongs . I am looking into the failure of the new job. Seems like the conformance tests are failing.

@bobbypage
Copy link
Contributor

/retest

@akhilerm
Copy link
Member

@qiutongs All the tests has passed . The pull-containerd-node-e2e failure can be ignored as that test is no longer applicable for 1.5 branch.

@samuelkarp samuelkarp merged commit d79db1f into containerd:release/1.5 Nov 16, 2022
Code Review automation moved this from Ready For Review to Done Nov 16, 2022
@dmcgowan dmcgowan mentioned this pull request Dec 6, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samuelkarp samuelkarp samuelkarp approved these changes

@fuweid fuweid fuweid approved these changes

Assignees
No one assigned
Labels
area/cri Container Runtime Interface (CRI) impact/changelog ok-to-test
Projects
Code Review
Done
Milestone
1.5.15
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@qiutongs @k8s-ci-robot @samuelkarp @estesp @fuweid @akhilerm @bobbypage