Skip to content

Fix regression in handling of NotFound err during startup ENGCORE-929#39703

Merged
thaJeztah merged 1 commit intomoby:masterfrom
ddebroy:fix-39623
Aug 9, 2019
Merged

Fix regression in handling of NotFound err during startup ENGCORE-929#39703
thaJeztah merged 1 commit intomoby:masterfrom
ddebroy:fix-39623

Conversation

@ddebroy
Copy link
Contributor

@ddebroy ddebroy commented Aug 8, 2019

- What I did
Address a regression during daemon startup introduced in #38931.

fixes #39623
fixes #39742
fixes docker/for-win#4463
fixes microsoft/navcontainerhelper#548

- How I did it
Added (back) proper handling of NotFound err

- How to verify it
Start a container in Windows like:

docker run -d --restart always --name testcontainer --network nat -p 8080:80 mcr.microsoft.com/windows/servercore:ltsc2019 ping -t 127.0.0.1

and Restart-Computer.
After reboot, make sure container comes back up properly and is not a zombie.

- Description for the changelog
Fix regression in reboot of Windows daemon

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Deep Debroy <ddebroy@docker.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change looks good but linting is failing;

23:55:13 Congratulations!  All commits are properly signed with the DCO!
23:56:27 daemon/daemon.go:1::warning: file is not gofmted with -s (gofmt)
23:56:27 daemon/daemon.go:1::warning: file is not goimported (goimports)

daemon/daemon.go Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err != nil && !errdefs.IsNotFound(err) {
if err != nil && !errdefs.IsNotFound(err) {

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

backport: docker-archive#317

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@StefanScherer StefanScherer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
retested with the NAV container example on Windows 10 with Docker Desktop.

@thaJeztah
Copy link
Member

all green 💚 - merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

5 participants