Conversation
| return nil | ||
| } | ||
|
|
||
| // Shutdown requests a container shutdown, if IsPending() on the error returned is true, |
There was a problem hiding this comment.
This IsPending is different than PendingUpdates. It is about the HCS doing the shutdown/terminate on a different thread as an async task.
There was a problem hiding this comment.
Ah, yes, you're right. I was over-zealous on removing. Reinstated
| return nil | ||
| } | ||
|
|
||
| // Terminate requests a container terminate, if IsPending() on the error returned is true, |
|
When we merge this, it will need to be a major version update as it breaks back-compat. We should be able to merge this here and purge the servicing code in same PR as the revendor, no? We'll also need to pull this from containerd as well as any other consumers. @sunjayBhatia FYI something you might need to update in winc. |
|
@darrenstahlmsft we do not use that container method or property currently but thanks for letting us know! |
|
LGTM other than nits |
Signed-off-by: John Howard <jhoward@microsoft.com>
48b159b to
f83ea5e
Compare
|
@darrenstahlmsft I didn't want another change to HCSShim being merged and a revendor going into moby/moby accidentally breaking down-level daemons. Hence the ordering in the description. As a breaking change, this would probably bump the release to v1.0.0 |
|
LGTM. Holding merge for the breaking change, so we can hopefully get #160 in and backported to older Docker versions without taking the breaking change with it. |
|
@jhowardmsft we haven't been using real semantic version numbering for previous releases. For breaking changes we've just been updating the minor version (v0.7.0). We should eventually tag something as v1.0.0 and use proper semantic versioning eventually though... Maybe the v2 schema can be the 1.0 (or 2.0)? We can discuss later though, as it is not relevant to this PR. |
|
@darrenstahlmsft Still hold off merging? I saw you closed #160. |
|
Yes, please hold off on merging breaking changes for now. #160 was closed for different reasons, but if we can hold off on this as well for a bit, it will make things easier. |
|
This is fine to merge now from my end. |
|
Actually, going to close this - I have a very slightly different version in the v2 work I'm doing. |
Signed-off-by: John Howard jhoward@microsoft.com
@darrenstahlmsft PTAL. Removes support for functionality that didn't work in RS1..RS3 and was removed in RS4 (cupdate no longer exists)
Don't merge until moby/moby#36267 has been merged. Steps are: