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

Restore error type in FindNetwork #35634

Merged
merged 1 commit into from Nov 30, 2017

Conversation

@fcrisciani
Copy link
Contributor

commented Nov 29, 2017

- What I did
Restored libnetwork error type

- Description for the changelog

The error type libnetwork.ErrNoSuchNetwork is used in the controller
to retry the network creation as a managed network though the manager.
The change of the type was breaking the logic causing the network to
not being created anymore so that no new container on that network
was able to be launched

Relates to: docker/swarmkit#2388

@fcrisciani

This comment has been minimized.

Copy link
Contributor Author

commented Nov 29, 2017

@cpuguy83 PTAL

return nil, errors.WithStack(networkNotFound(idName))
// The Error type ErrNoSuchNetwork is also used by the controller to try to create a managed network
// asking to the manager, for this reason the type cannot be changed
return nil, errors.WithStack(libnetwork.ErrNoSuchNetwork(idName))

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Nov 29, 2017

Member

looks like networkNotFound() is no longer used now.

Wondering can we wrap the libnetwork.ErrNoSuchNetwork(idName) so that it's still detected as an libnetwork.ErrNoSuchNetwork() error, but also implements NotFound() to make the API translations work?

return nil, errors.WithStack(networkNotFound(idName))
// The Error type ErrNoSuchNetwork is also used by the controller to try to create a managed network
// asking to the manager, for this reason the type cannot be changed
return nil, errors.WithStack(libnetwork.ErrNoSuchNetwork(idName))

This comment has been minimized.

Copy link
@cpuguy83

cpuguy83 Nov 29, 2017

Contributor

This still needs to implement the NotFound() interface.
If we can wrap it in an error type that implements both NotFound and Cause(), then the caller would just need to do errors.Cause(err).(libnetwork.ErrNoSuchNetwork)

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Nov 29, 2017

Member

👍 looks like you're describing what I tried to phrase above 😄

This comment has been minimized.

Copy link
@fcrisciani

fcrisciani Nov 29, 2017

Author Contributor

k will try to look into this, thanks

@fcrisciani fcrisciani force-pushed the fcrisciani:fix-net-not-found branch 5 times, most recently from 425dbd9 to 2ba2a3c Nov 29, 2017

@cpuguy83
Copy link
Contributor

left a comment

LGTM

Flavio Crisciani
Restore error type in FindNetwork
The error type libnetwork.ErrNoSuchNetwork is used in the controller
to retry the network creation as a managed network though the manager.
The change of the type was breaking the logic causing the network to
not being created anymore so that no new container on that network
was able to be launched
Added unit test

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
@thaJeztah
Copy link
Member

left a comment

LGTM

@yongtang yongtang merged commit e0b3ddd into moby:master Nov 30, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 38102 has succeeded
Details
janky Jenkins build Docker-PRs 46816 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 7224 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 18370 has succeeded
Details
z Jenkins build Docker-PRs-s390x 7044 has succeeded
Details
@jmarcos-cano

This comment has been minimized.

Copy link

commented Jan 9, 2018

quick question guys, which release version contains this fix?

@thaJeztah

This comment has been minimized.

Copy link
Member

commented Jan 9, 2018

This is included in Docker 17.12 (and up), and back-ported to Docker 17.09.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.