-
Notifications
You must be signed in to change notification settings - Fork 882
log error instead if disabling IPv6 router advertisement failed #2563
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
Conversation
return fmt.Errorf("libnetwork: Unable to disable IPv6 router advertisement: %v", err) | ||
logrus.WithError(err).Info("unable to disable IPv6 router advertisement") | ||
} | ||
return nil |
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.
This function can no longer return anything except nil
so you should change the type to no longer return an 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.
We can't change it because it has to match the setupStep
type
libnetwork/drivers/bridge/setup.go
Line 3 in 13a4da0
type setupStep func(*networkConfiguration, *bridgeInterface) error |
@thaJeztah now that we have incorporated the security patch, can we please utilize this library into libnetwork/osl/namespace_linux.go Line 40 in 13a4da0
its best effort and logs errors |
drivers/bridge/setup_device.go
Outdated
} | ||
if err := ioutil.WriteFile(sysPath, []byte{'0', '\n'}, 0644); err != nil { | ||
return fmt.Errorf("libnetwork: Unable to disable IPv6 router advertisement: %v", err) | ||
logrus.WithError(err).Info("unable to disable IPv6 router advertisement") |
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'd prefer to see this as a Warn rather than Info. If we switch this patch to use https://github.com/moby/libnetwork/blob/master/osl/kernel/knobs_linux.go as suggested by @arkodg, failure to write will get logged as Error.
Ok, let me update the Info to a Warn @arkodg I think I prefer to do the refactor in a follow-up directly after, as this was a regression, and want to keep the diff small for the 19.03 branch (I agree with looking at refactoring though) |
Previously, failing to disable IPv6 router advertisement prevented the daemon to start. An issue was reported by a user that started docker using `systemd-nspawn "machine"`, which produced an error; failed to start daemon: Error initializing network controller: Error creating default "bridge" network: libnetwork: Unable to disable IPv6 router advertisement: open /proc/sys/net/ipv6/conf/docker0/accept_ra: read-only file system This patch changes the error to a log-message instead. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@arkodg @samuelkarp @cpuguy83 updated to print a warning, I'll have a look at the refactor later |
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
full diff: moby/libnetwork@2e24aed...9e99af2 - moby/libnetwork#2548 Add docker interfaces to firewalld docker zone - fixes docker/for-linux#957 DNS Not Resolving under Network [CentOS8] - fixes moby/libnetwork#2496 Port Forwarding does not work on RHEL 8 with Firewalld running with FirewallBackend=nftables - store.getNetworksFromStore() remove unused error return - moby/libnetwork#2554 Fix 'failed to get network during CreateEndpoint' - fixes/addresses docker/for-linux#888 failed to get network during CreateEndpoint - moby/libnetwork#2558 [master] bridge: disable IPv6 router advertisements - moby/libnetwork#2563 log error instead if disabling IPv6 router advertisement failed - fixes docker/for-linux#1033 Shouldn't be fatal: Unable to disable IPv6 router advertisement: open /proc/sys/net/ipv6/conf/docker0/accept_ra: read-only file system Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: moby/libnetwork@2e24aed...9e99af2 - moby/libnetwork#2548 Add docker interfaces to firewalld docker zone - fixes docker/for-linux#957 DNS Not Resolving under Network [CentOS8] - fixes moby/libnetwork#2496 Port Forwarding does not work on RHEL 8 with Firewalld running with FirewallBackend=nftables - store.getNetworksFromStore() remove unused error return - moby/libnetwork#2554 Fix 'failed to get network during CreateEndpoint' - fixes/addresses docker/for-linux#888 failed to get network during CreateEndpoint - moby/libnetwork#2558 [master] bridge: disable IPv6 router advertisements - moby/libnetwork#2563 log error instead if disabling IPv6 router advertisement failed - fixes docker/for-linux#1033 Shouldn't be fatal: Unable to disable IPv6 router advertisement: open /proc/sys/net/ipv6/conf/docker0/accept_ra: read-only file system Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 219e7e7ddcf5f0314578d2a517fc0832f03622c1 Component: engine
addresses docker/for-linux#1033
Previously, failing to disable IPv6 router advertisement prevented the daemon to start.
An issue was reported by a user that started docker using
systemd-nspawn "machine"
,which produced an error;
This patch changes the error to a log-message instead.