CRI: set container to unknown if fail to start#7044
CRI: set container to unknown if fail to start#7044Burning1020 wants to merge 1 commit intocontainerd:mainfrom
Conversation
|
please use your real name to sign off. Thanks |
Contianer state should be unknown if fail to start, not exited. Signed-off-by: Zhang Tianyang <[email protected]>
| // Set container to exited if fail to start. | ||
| // Set container state depended on whether task is alive | ||
| if err := cntr.Status.UpdateSync(func(status containerstore.Status) (containerstore.Status, error) { | ||
| if _, err := container.Task(ctx, nil); err == nil { |
There was a problem hiding this comment.
To be honest, I don't want to make RPC call in UpdateSync because the lock in UpdateSync will impact the ListContainer and kubelet's PLEG.
I would like to use a flag named by shouldBeUnknown to let rollback know there is no idea about shim.
| defer func() { | ||
| if retErr != nil { | ||
| // Set container to exited if fail to start. | ||
| // Set container state depended on whether task is alive |
There was a problem hiding this comment.
the original comment is not detailed enough... it not only sets the finished time indicating exited state.. it also sets errorStartExitCode which has further meaning:
// errorStartExitCode is the exit code when fails to start container.
// 128 is the same with Docker's behavior.
// TODO(windows): Figure out what should be used for windows.
errorStartExitCode = 128
thus.. I wonder if this change is needed
|
@Burning1020 could you mind to add the integration test case with failpoint? Thanks |
|
PR needs rebase. DetailsInstructions 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. |
|
This PR is stale because it has been open 90 days with no activity. This PR will be closed in 7 days unless new comments are made or the stale label is removed. |
|
This PR was closed because it has been stalled for 7 days with no activity. |
7 participants
CRI: Contianer state should be unknown if fail to start, not exited.
I believe the exited state indicates that the process has started successfully and exited either by killed or with a completed execution. While the unknown state can be interpreted as this task has returned an error somehow and the reason is unknown.
In addition, if
task.Start()failed and deferedtask.Delete()failed too, the follow-upRemoveContainer()will failed withErrFailedPreconditionforever because task still can be loaded.Signed-off-by: Zhang Tianyang [email protected]