From 8cf7270d0684929a60371b0d1b53ffaa83bbd128 Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Mon, 7 Mar 2016 22:21:17 -0800 Subject: [PATCH] Miscellaneous fixes - Fix npe in sbJoin error path - Fail again endpoint Join in case of failure in programming the external connectivity - In bridge, look for parent and child container configs in the generic data - iptables.Exists() might be called before any other call to iptables.raw(). We need to call checkInit() then. Introduced by 1638fbdf279b1646 Signed-off-by: Alessandro Boch --- libnetwork/drivers/bridge/bridge.go | 30 ++++++++++-------------- libnetwork/drivers/bridge/bridge_test.go | 9 +++++-- libnetwork/drivers/bridge/labels.go | 6 ----- libnetwork/endpoint.go | 22 ++++++++++++----- libnetwork/iptables/iptables.go | 2 ++ 5 files changed, 38 insertions(+), 31 deletions(-) diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go index 5e30225535..00e16e1e5b 100644 --- a/libnetwork/drivers/bridge/bridge.go +++ b/libnetwork/drivers/bridge/bridge.go @@ -1362,26 +1362,22 @@ func parseContainerOptions(cOptions map[string]interface{}) (*containerConfigura if cOptions == nil { return nil, nil } - - cc := &containerConfiguration{} - - if opt, ok := cOptions[ParentEndpoints]; ok { - if pe, ok := opt.([]string); ok { - cc.ParentEndpoints = pe - } else { - return nil, types.BadRequestErrorf("Invalid parent endpoints data in sandbox configuration: %v", opt) - } + genericData := cOptions[netlabel.GenericData] + if genericData == nil { + return nil, nil } - - if opt, ok := cOptions[ChildEndpoints]; ok { - if ce, ok := opt.([]string); ok { - cc.ChildEndpoints = ce - } else { - return nil, types.BadRequestErrorf("Invalid child endpoints data in sandbox configuration: %v", opt) + switch opt := genericData.(type) { + case options.Generic: + opaqueConfig, err := options.GenerateFromModel(opt, &containerConfiguration{}) + if err != nil { + return nil, err } + return opaqueConfig.(*containerConfiguration), nil + case *containerConfiguration: + return opt, nil + default: + return nil, nil } - - return cc, nil } func parseConnectivityOptions(cOptions map[string]interface{}) (*connectivityConfiguration, error) { diff --git a/libnetwork/drivers/bridge/bridge_test.go b/libnetwork/drivers/bridge/bridge_test.go index c8fef93bca..2776ade1ca 100644 --- a/libnetwork/drivers/bridge/bridge_test.go +++ b/libnetwork/drivers/bridge/bridge_test.go @@ -10,6 +10,7 @@ import ( "github.com/docker/libnetwork/ipamutils" "github.com/docker/libnetwork/iptables" "github.com/docker/libnetwork/netlabel" + "github.com/docker/libnetwork/options" "github.com/docker/libnetwork/testutils" "github.com/docker/libnetwork/types" ) @@ -599,7 +600,9 @@ func TestLinkContainers(t *testing.T) { } sbOptions = make(map[string]interface{}) - sbOptions[ChildEndpoints] = []string{"ep1"} + sbOptions[netlabel.GenericData] = options.Generic{ + "ChildEndpoints": []string{"ep1"}, + } err = d.Join("net1", "ep2", "", te2, sbOptions) if err != nil { @@ -655,7 +658,9 @@ func TestLinkContainers(t *testing.T) { // Error condition test with an invalid endpoint-id "ep4" sbOptions = make(map[string]interface{}) - sbOptions[ChildEndpoints] = []string{"ep1", "ep4"} + sbOptions[netlabel.GenericData] = options.Generic{ + "ChildEndpoints": []string{"ep1", "ep4"}, + } err = d.Join("net1", "ep2", "", te2, sbOptions) if err != nil { diff --git a/libnetwork/drivers/bridge/labels.go b/libnetwork/drivers/bridge/labels.go index ebdb20d720..7447bd3f93 100644 --- a/libnetwork/drivers/bridge/labels.go +++ b/libnetwork/drivers/bridge/labels.go @@ -15,10 +15,4 @@ const ( // DefaultBridge label DefaultBridge = "com.docker.network.bridge.default_bridge" - - // ChildEndpoints for links - ChildEndpoints = "com.docker.network.bridge.child_endpoints" - - // ParentEndpoints for links - ParentEndpoints = "com.docker.network.bridge.parent_endpoints" ) diff --git a/libnetwork/endpoint.go b/libnetwork/endpoint.go index f0af1e7538..b0aacb9f44 100644 --- a/libnetwork/endpoint.go +++ b/libnetwork/endpoint.go @@ -455,16 +455,26 @@ func (ep *endpoint) sbJoin(sb *sandbox, options ...EndpointOption) error { if moveExtConn { if extEp != nil { log.Debugf("Revoking external connectivity on endpoint %s (%s)", extEp.Name(), extEp.ID()) - if err := d.RevokeExternalConnectivity(extEp.network.ID(), extEp.ID()); err != nil { - log.Warnf("driver failed revoking external connectivity on endpoint %s (%s): %v", + if err = d.RevokeExternalConnectivity(extEp.network.ID(), extEp.ID()); err != nil { + return types.InternalErrorf( + "driver failed revoking external connectivity on endpoint %s (%s): %v", extEp.Name(), extEp.ID(), err) } + defer func() { + if err != nil { + if e := d.ProgramExternalConnectivity(extEp.network.ID(), extEp.ID(), sb.Labels()); e != nil { + log.Warnf("Failed to roll-back external connectivity on endpoint %s (%s): %v", + extEp.Name(), extEp.ID(), e) + } + } + }() } if !n.internal { log.Debugf("Programming external connectivity on endpoint %s (%s)", ep.Name(), ep.ID()) - if err := d.ProgramExternalConnectivity(n.ID(), ep.ID(), sb.Labels()); err != nil { - log.Warnf("driver failed programming external connectivity on endpoint %s (%s): %v", - extEp.Name(), extEp.ID(), err) + if err = d.ProgramExternalConnectivity(n.ID(), ep.ID(), sb.Labels()); err != nil { + return types.InternalErrorf( + "driver failed programming external connectivity on endpoint %s (%s): %v", + ep.Name(), ep.ID(), err) } } } @@ -582,7 +592,7 @@ func (ep *endpoint) sbLeave(sb *sandbox, force bool, options ...EndpointOption) if moveExtConn { log.Debugf("Revoking external connectivity on endpoint %s (%s)", ep.Name(), ep.ID()) if err := d.RevokeExternalConnectivity(n.id, ep.id); err != nil { - log.Warnf("driver failed removing external connectivity on endpoint %s (%s): %v", + log.Warnf("driver failed revoking external connectivity on endpoint %s (%s): %v", ep.Name(), ep.ID(), err) } } diff --git a/libnetwork/iptables/iptables.go b/libnetwork/iptables/iptables.go index 908a04dd7a..298c5bf472 100644 --- a/libnetwork/iptables/iptables.go +++ b/libnetwork/iptables/iptables.go @@ -306,6 +306,8 @@ func Exists(table Table, chain string, rule ...string) bool { table = Filter } + initCheck() + if supportsCOpt { // if exit status is 0 then return true, the rule exists _, err := Raw(append([]string{"-t", string(table), "-C", chain}, rule...)...)