-
Notifications
You must be signed in to change notification settings - Fork 950
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
bugfix: fix endpoints disappear when pouchd restart #1312
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,13 +65,32 @@ type NetworkManager struct { | |
} | ||
|
||
// NewNetworkManager creates a brand new network manager. | ||
func NewNetworkManager(cfg *config.Config, store *meta.Store) (*NetworkManager, error) { | ||
func NewNetworkManager(cfg *config.Config, store *meta.Store, ctrMgr ContainerMgr) (*NetworkManager, error) { | ||
// Create a new controller instance | ||
cfg.NetworkConfg.MetaPath = path.Dir(store.BaseDir) | ||
cfg.NetworkConfg.ExecRoot = network.DefaultExecRoot | ||
|
||
initNetworkLog(cfg) | ||
|
||
// get active sandboxes | ||
ctrs, err := ctrMgr.List(context.Background(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replace |
||
func(c *Container) bool { | ||
return (c.IsRunning() || c.IsPaused()) && !isContainer(c.HostConfig.NetworkMode) | ||
}, &ContainerListOption{All: true}) | ||
if err != nil { | ||
logrus.Errorf("failed to new network manager, can not get container list") | ||
return nil, errors.Wrap(err, "failed to get container list") | ||
} | ||
cfg.NetworkConfg.ActiveSandboxes = make(map[string]interface{}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need to store There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't need to store to disk |
||
for _, c := range ctrs { | ||
endpoint := BuildContainerEndpoint(c) | ||
sbOptions, err := buildSandboxOptions(cfg.NetworkConfg, endpoint) | ||
if err != nil { | ||
return nil, errors.Wrap(err, "failed to build sandbox options") | ||
} | ||
cfg.NetworkConfg.ActiveSandboxes[c.NetworkSettings.SandboxID] = sbOptions | ||
} | ||
|
||
ctlOptions, err := controllerOptions(cfg.NetworkConfg) | ||
if err != nil { | ||
return nil, errors.Wrap(err, "failed to build network options") | ||
|
@@ -280,7 +299,7 @@ func (nm *NetworkManager) EndpointCreate(ctx context.Context, endpoint *types.En | |
// create sandbox | ||
sb := nm.getNetworkSandbox(containerID) | ||
if sb == nil { | ||
sandboxOptions, err := nm.sandboxOptions(endpoint) | ||
sandboxOptions, err := buildSandboxOptions(nm.config, endpoint) | ||
if err != nil { | ||
return "", fmt.Errorf("failed to build sandbox options(%v)", err) | ||
} | ||
|
@@ -422,6 +441,10 @@ func controllerOptions(cfg network.Config) ([]nwconfig.Option, error) { | |
options = append(options, nwconfig.OptionExecRoot(cfg.ExecRoot)) | ||
} | ||
|
||
if len(cfg.ActiveSandboxes) != 0 { | ||
options = append(options, nwconfig.OptionActiveSandboxes(cfg.ActiveSandboxes)) | ||
} | ||
|
||
options = append(options, nwconfig.OptionDefaultDriver("bridge")) | ||
options = append(options, nwconfig.OptionDefaultNetwork("bridge")) | ||
|
||
|
@@ -562,7 +585,7 @@ func endpointOptions(n libnetwork.Network, endpoint *types.Endpoint) ([]libnetwo | |
return createOptions, nil | ||
} | ||
|
||
func (nm *NetworkManager) sandboxOptions(endpoint *types.Endpoint) ([]libnetwork.SandboxOption, error) { | ||
func buildSandboxOptions(config network.Config, endpoint *types.Endpoint) ([]libnetwork.SandboxOption, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think sandbox options should not depend on endpoint config. If we have many endpoints, it means we will lose some information? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. endpoint in pouchd is different from endpoint in libnetwork, next version I will change it to 'NetDevice' |
||
var ( | ||
sandboxOptions []libnetwork.SandboxOption | ||
dns []string | ||
|
@@ -577,9 +600,9 @@ func (nm *NetworkManager) sandboxOptions(endpoint *types.Endpoint) ([]libnetwork | |
if len(endpoint.ExtraHosts) == 0 { | ||
sandboxOptions = append(sandboxOptions, libnetwork.OptionOriginHostsPath("/etc/hosts")) | ||
} | ||
if len(endpoint.DNS) == 0 && len(nm.config.DNS) == 0 && | ||
len(endpoint.DNSSearch) == 0 && len(nm.config.DNSSearch) == 0 && | ||
len(endpoint.DNSOptions) == 0 && len(nm.config.DNSOptions) == 0 { | ||
if len(endpoint.DNS) == 0 && len(config.DNS) == 0 && | ||
len(endpoint.DNSSearch) == 0 && len(config.DNSSearch) == 0 && | ||
len(endpoint.DNSOptions) == 0 && len(config.DNSOptions) == 0 { | ||
sandboxOptions = append(sandboxOptions, libnetwork.OptionOriginResolvConfPath("/etc/resolv.conf")) | ||
} | ||
} else { | ||
|
@@ -592,8 +615,8 @@ func (nm *NetworkManager) sandboxOptions(endpoint *types.Endpoint) ([]libnetwork | |
// parse DNS | ||
if len(endpoint.DNS) > 0 { | ||
dns = endpoint.DNS | ||
} else if len(nm.config.DNS) > 0 { | ||
dns = nm.config.DNS | ||
} else if len(config.DNS) > 0 { | ||
dns = config.DNS | ||
} | ||
for _, d := range dns { | ||
sandboxOptions = append(sandboxOptions, libnetwork.OptionDNS(d)) | ||
|
@@ -602,8 +625,8 @@ func (nm *NetworkManager) sandboxOptions(endpoint *types.Endpoint) ([]libnetwork | |
// parse DNS Search | ||
if len(endpoint.DNSSearch) > 0 { | ||
dnsSearch = endpoint.DNSSearch | ||
} else if len(nm.config.DNSSearch) > 0 { | ||
dnsSearch = nm.config.DNSSearch | ||
} else if len(config.DNSSearch) > 0 { | ||
dnsSearch = config.DNSSearch | ||
} | ||
for _, ds := range dnsSearch { | ||
sandboxOptions = append(sandboxOptions, libnetwork.OptionDNSSearch(ds)) | ||
|
@@ -612,8 +635,8 @@ func (nm *NetworkManager) sandboxOptions(endpoint *types.Endpoint) ([]libnetwork | |
// parse DNS Options | ||
if len(endpoint.DNSOptions) > 0 { | ||
dnsOptions = endpoint.DNSOptions | ||
} else if len(nm.config.DNSOptions) > 0 { | ||
dnsOptions = nm.config.DNSOptions | ||
} else if len(config.DNSOptions) > 0 { | ||
dnsOptions = config.DNSOptions | ||
} | ||
for _, ds := range dnsOptions { | ||
sandboxOptions = append(sandboxOptions, libnetwork.OptionDNSOptions(ds)) | ||
|
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.
can we replace
ctrMgr
tocontainerMgr
, becausectrMgr
may be a little confused.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 both is ok, short name in function paramters