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

Call container.Terminate() on shutdown timeouts #1488

Merged
merged 1 commit into from
Aug 24, 2022

Conversation

dcantah
Copy link
Contributor

@dcantah dcantah commented Aug 24, 2022

We were logging if HcsShutdownComputeSystem failed, but we weren't
trying to force kill the container via Terminate after if we timed
out waiting for it to complete. Shutdown is async and we wait for
a notification for success, so most of the time the call itself will
return nil, but it doesn't indicate indicate success until we can
see that the system exited. So now we will fallback to Terminate for:

  1. Shutdown returning an error that doesn't indicate the result is
    to be waited on.
  2. The async result of shutdown was non-nil
  3. Waiting for the result passed the timeout we set.

We were logging if HcsShutdownComputeSystem failed, but we weren't
trying to force kill the container via Terminate after if we timed
out waiting for it to complete. Shutdown is async and we wait for
a notification for success, so most of the time the call itself will
return nil, but it doesn't indicate indicate success until we can
see that the system exited. So now we will fallback to Terminate for:

1. Shutdown returning an error that doesn't indicate the result is
to be waited on.
2. The async result of shutdown was non-nil
3. Waiting for the result passed the timeout we set.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah dcantah requested a review from a team as a code owner August 24, 2022 02:43
@dcantah dcantah merged commit f12cf48 into microsoft:main Aug 24, 2022
kiashok pushed a commit to kiashok/hcsshim that referenced this pull request Oct 29, 2022
Call container.Terminate() on shutdown timeouts

(cherry picked from commit f12cf48)
Signed-off-by: Kirtana Ashok <Kirtana.Ashok@microsoft.com>
kiashok pushed a commit to kiashok/hcsshim that referenced this pull request Oct 29, 2022
…ate"

This reverts commit f12cf48, reversing
changes made to ab849cf.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants