Windows: Add service dependency ConDrv#30111
Conversation
|
Tentative LGTM - need to ensure this is on nanoserver without the EMS too. Will have an answer soon. @thaJeztah this is probably also one which should be strongly considered for 1.13 to ensure containers configured for restart always do indeed restart across reboots of the host |
|
Can we make these service-definitions external, like (I know Linux is different), https://github.com/docker/docker/tree/master/contrib/init ? It feels "odd" to define the service in the binary itself, and something that should be related to packaging. /cc @johnstep @andrewhsu |
|
I think that having a separate script doing service registration would be preferable. It could even be integrated to the provided installation script. See : https://msdn.microsoft.com/en-us/powershell/reference/5.1/microsoft.powershell.management/new-service for documentation on how to install a service with Powershell. |
|
Honestly there is no need or reason to have this external |
|
ping @johnstep |
johnstep
left a comment
There was a problem hiding this comment.
LGTM
While I understand the concerns about internal registration, I think there is no cause for concern here. Logically, the service configuration, especially dependencies, belongs with the service itself. I do most definitely prefer a declarative approach, but a standardized one, not a script. In the future, I sure hope this can be achieved through a standardized Windows app package.
thaJeztah
left a comment
There was a problem hiding this comment.
Thanks for looking @simonferquel @johnstep
Let me LGTM
|
ping @jhowardmsft can you confirm if this is ready to go (re: #30111 (comment))? |
|
Yup, just confirmed. But there should be a minor change to this to remove the dependency for builds >14393 as the dependency is in the platform post-RS1. |
Signed-off-by: Darren Stahl <darst@microsoft.com>
0ec43ed to
b2a7f6a
Compare
|
Updated per latest comment RE only adding this dependency in RS1 |
|
Thanks @darrenstahlmsft. LGTM. |
|
LGTM |
|
All green, merging |
Signed-off-by: Darren Stahl darst@microsoft.com
Fixes #27544
Added a service dependency on the Console Driver, which is necessary for using Windows Server Containers (The missing dependency was causing failure to launch processes in containers shortly after a host reboot).
cc @jhowardmsft @jstarks