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

fix recreate sandbox when a job is done #99343

Closed
wants to merge 1 commit into from
Closed

fix recreate sandbox when a job is done #99343

wants to merge 1 commit into from

Conversation

liuxu623
Copy link
Contributor

@liuxu623 liuxu623 commented Feb 23, 2021 •
edited
Loading

What type of PR is this?

/kind bug

What this PR does / why we need it:

If a job is done, we shouldn't recreate sandbox.

Which issue(s) this PR fixes:

awslabs/amazon-eks-ami#592
#95916

How to reproduce it (as minimally and precisely as possible):

apiVersion: batch/v1beta1
kind: CronJob
metadata:
  name: hello
spec:
  schedule: "*/1 * * * *"
  jobTemplate:
    spec:
      template:
        spec:
          containers:
          - name: hello
            image: busybox
            args:
            - /bin/sh
            - -c
            - date; echo Hello from the Kubernetes cluster
          restartPolicy: OnFailure

kubelet log

I0223 13:20:03.238964  285333 kuberuntime_manager.go:659] SyncPod received new pod "hello-1614057600-9mtz9_default(5c87e898-2dee-4f06-8036-8765a93b06d8)", will create a sandbox for it
I0223 13:20:03.238973  285333 kuberuntime_manager.go:666] Stopping PodSandbox for "hello-1614057600-9mtz9_default(5c87e898-2dee-4f06-8036-8765a93b06d8)", will start new one
I0223 13:20:03.238990  285333 kuberuntime_manager.go:721] Creating sandbox for pod "hello-1614057600-9mtz9_default(5c87e898-2dee-4f06-8036-8765a93b06d8)"
...
I0223 13:20:06.882342  285333 kuberuntime_manager.go:668] Stopping PodSandbox for "hello-1614057600-9mtz9_default(5c87e898-2dee-4f06-8036-8765a93b06d8)" because all other containers are dead.
I0223 13:20:07.894401  285333 kuberuntime_manager.go:440] No ready sandbox for pod "hello-1614057600-9mtz9_default(5c87e898-2dee-4f06-8036-8765a93b06d8)" can be found. Need to start a new one

event

55m         Normal    Scheduled                pod/hello-1614057600-9mtz9   Successfully assigned default/hello-1614057600-9mtz9 to 192.168.0.96
55m         Normal    Pulling                  pod/hello-1614057600-9mtz9   Pulling image "busybox"
55m         Normal    Pulled                   pod/hello-1614057600-9mtz9   Successfully pulled image "busybox"
55m         Normal    Created                  pod/hello-1614057600-9mtz9   Created container hello
55m         Normal    Started                  pod/hello-1614057600-9mtz9   Started container hello
55m         Normal    SandboxChanged           pod/hello-1614057600-9mtz9   Pod sandbox changed, it will be killed and re-created.
55m         Warning   FailedCreatePodSandBox   pod/hello-1614057600-9mtz9   Failed to create pod sandbox: rpc error: code = Unknown desc = failed to start sandbox container for pod "hello-1614057600-9mtz9": Error response from daemon: OCI runtime create failed: container_linux.go:349: starting container process caused "process_linux.go:319: getting the final child's pid from pipe caused \"EOF\"": unknown
55m         Normal    SuccessfulCreate         job/hello-1614057600         Created pod: hello-1614057600-9mtz9
55m         Normal    Completed                job/hello-1614061020         Job completed

@k8s-ci-robot
Copy link
Contributor

@liuxu623: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Feb 23, 2021
@k8s-ci-robot
Copy link
Contributor

@liuxu623: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 23, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @liuxu623. Thanks for your PR.

I'm waiting for a kubernetes 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.

@k8s-ci-robot k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 23, 2021
@liuxu623
Copy link
Contributor Author

/sig node

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 23, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: liuxu623
To complete the pull request process, please assign random-liu after the PR has been reviewed.
You can assign the PR to them by writing /assign @random-liu in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@liuxu623
Copy link
Contributor Author

/assign @Random-Liu

@wzshiming
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 23, 2021
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 23, 2021
@liuxu623
Copy link
Contributor Author

/retest

@k8s-ci-robot
Copy link
Contributor

@liuxu623: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-kubernetes-bazel-test 341d7d0 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-kind 341d7d0 link /test pull-kubernetes-e2e-kind

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.

@liuxu623
Copy link
Contributor Author

sorry, I found #92614 already fix this.

@liuxu623 liuxu623 closed this Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

@matthyx matthyx Awaiting requested review from matthyx

Assignees

@Random-Liu Random-Liu

Labels
area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. kind/bug Categorizes issue or PR as related to a bug. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. sig/node Categorizes an issue or PR as relevant to SIG Node. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
SIG Node: code and documentation PRs
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@liuxu623 @k8s-ci-robot @wzshiming @Random-Liu