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
Windows: Wait for OOBE to prevent crashing during host update #31054
Conversation
@@ -601,3 +606,45 @@ func (daemon *Daemon) verifyVolumesInfo(container *container.Container) error { | |||
func (daemon *Daemon) setupSeccompProfile() error { | |||
return nil | |||
} | |||
|
|||
func waitOOBEComplete() error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if these two OOBE functions should be in pkg\system? (Nit, not essential)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think pkg/system is generally for platform agnostic versions of things. Not to say I would be against moving it makes sense there though.
LGTM with one minor comment. |
@johnstep FYI |
daemon/daemon_windows.go
Outdated
return nil | ||
} | ||
|
||
func isOOBEComplete() (OOBEComplete bool, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like isOOBEComplete
is unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yep, removed.
cmd/dockerd/daemon_windows.go
Outdated
func notifySystem() { | ||
// preNotifySystem sends a message to the host when the API is active, but before the daemon is | ||
func preNotifySystem() { | ||
// start the service now to prevent timeouts waiting for daemon to started |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: s/started/start
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Signed-off-by: Darren Stahl <darst@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@thaJeztah If #32152 is merged, the changelog from this PR is no longer needed. |
Signed-off-by: Darren Stahl darst@microsoft.com
- What I did
Prevent a Daemon crash during host update.
- How I did it
Some services Docker depends on are not active during a host update. This PR holds daemon initialization until after a host update is completed.
It also notifies the Windows Service Control Manager that the service has started as soon as the API is active to prevent service start timeouts waiting for OOBE or daemon initialization.
- How to verify it
Install Docker service on a machine that receives manual OS updates, such as a Windows Insider machine. After an update is installed, the daemon was not running, with an SCM log stating that it terminated unexpectedly. With this update, the daemon will be running.
- Description for the changelog
Prevent crash during Windows host update.
/cc @jhowardmsft @jstarks