Skip to content

Commit dd9e289

Browse files
robmryrobmry
committed
Don't pass sandbox options to ProgramExternalConnectivity
The same sandbox options are passed to Join. Signed-off-by: Rob Murray <rob.murray@docker.com>
1 parent 4f7afb8 commit dd9e289

File tree

6 files changed

+44
-44
lines changed

6 files changed

+44
-44
lines changed

‎libnetwork/driverapi/driverapi.go‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,22 +81,19 @@ type Driver interface {
8181

8282
// ExtConner is an optional interface for a network driver.
8383
type ExtConner interface {
84-
// ProgramExternalConnectivity tells the driver a container's options (including
85-
// port mapping options), so that it can configure the endpoint eid in network
86-
// nid.
87-
//
88-
// Ids of the endpoints currently acting as the container's default gateway for
89-
// IPv4 and IPv6 are passed as gw4Id/gw6Id. (Those endpoints may be managed by
90-
// different network drivers. If there is no gateway, the id will be the
91-
// empty string.)
84+
// ProgramExternalConnectivity tells the driver the ids of the endpoints
85+
// currently acting as the container's default gateway for IPv4 and IPv6,
86+
// passed as gw4Id/gw6Id. (Those endpoints may be managed by different network
87+
// drivers. If there is no gateway, the id will be the empty string.)
9288
//
9389
// This method is called after Driver.Join, before Driver.Leave, and when eid
94-
// is or was equal to gw4Id or gw6Id, and there's a change.
90+
// is or was equal to gw4Id or gw6Id, and there's a change. It may also be
91+
// called when the gateways have not changed.
9592
//
9693
// When an endpoint acting as a gateway is deleted, this function is called
9794
// with that endpoint's id in eid, and empty gateway ids (even if another
9895
// is present and will shortly be selected as the gateway).
99-
ProgramExternalConnectivity(ctx context.Context, nid, eid string, options map[string]interface{}, gw4Id, gw6Id string) error
96+
ProgramExternalConnectivity(ctx context.Context, nid, eid string, gw4Id, gw6Id string) error
10097
}
10198

10299
// GwAllocChecker is an optional interface for a network driver.

‎libnetwork/drivers/bridge/bridge_linux.go‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1447,6 +1447,10 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf
14471447
if err != nil {
14481448
return err
14491449
}
1450+
endpoint.extConnConfig, err = parseConnectivityOptions(sbOpts)
1451+
if err != nil {
1452+
return err
1453+
}
14501454

14511455
iNames := jinfo.InterfaceName()
14521456
containerVethPrefix := defaultContainerVethPrefix
@@ -1499,7 +1503,7 @@ type portBindingMode struct {
14991503
ipv6 bool
15001504
}
15011505

1502-
func (d *driver) ProgramExternalConnectivity(ctx context.Context, nid, eid string, options map[string]interface{}, gw4Id, gw6Id string) (retErr error) {
1506+
func (d *driver) ProgramExternalConnectivity(ctx context.Context, nid, eid string, gw4Id, gw6Id string) (retErr error) {
15031507
ctx, span := otel.Tracer("").Start(ctx, spanPrefix+".ProgramExternalConnectivity", trace.WithAttributes(
15041508
attribute.String("nid", nid),
15051509
attribute.String("eid", eid),
@@ -1526,15 +1530,6 @@ func (d *driver) ProgramExternalConnectivity(ctx context.Context, nid, eid strin
15261530
return endpointNotFoundError(eid)
15271531
}
15281532

1529-
// Only parse options if given (options may be nil when revoking gateway-ness, but that
1530-
// doesn't mean the endpoint's config has changed).
1531-
if options != nil {
1532-
endpoint.extConnConfig, err = parseConnectivityOptions(options)
1533-
if err != nil {
1534-
return err
1535-
}
1536-
}
1537-
15381533
var pbmReq portBindingMode
15391534
// Act as the IPv4 gateway if explicitly selected.
15401535
if gw4Id == eid {

‎libnetwork/drivers/bridge/bridge_linux_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -843,7 +843,7 @@ func testQueryEndpointInfo(t *testing.T, ulPxyEnabled bool) {
843843
t.Fatalf("Failed to join the endpoint: %v", err)
844844
}
845845

846-
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep1", sbOptions, "ep1", "")
846+
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep1", "ep1", "")
847847
if err != nil {
848848
t.Fatalf("Failed to program external connectivity: %v", err)
849849
}
@@ -874,7 +874,7 @@ func testQueryEndpointInfo(t *testing.T, ulPxyEnabled bool) {
874874
}
875875
}
876876

877-
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep1", nil, "", "")
877+
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep1", "", "")
878878
if err != nil {
879879
t.Fatal(err)
880880
}
@@ -947,7 +947,7 @@ func TestLinkContainers(t *testing.T) {
947947
t.Fatalf("Failed to join the endpoint: %v", err)
948948
}
949949

950-
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep1", sbOptions, "ep1", "")
950+
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep1", "ep1", "")
951951
if err != nil {
952952
t.Fatalf("Failed to program external connectivity: %v", err)
953953
}
@@ -978,7 +978,7 @@ func TestLinkContainers(t *testing.T) {
978978
t.Fatal("Failed to link ep1 and ep2")
979979
}
980980

981-
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep2", sbOptions, "ep2", "ep2")
981+
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep2", "ep2", "ep2")
982982
if err != nil {
983983
t.Fatalf("Failed to program external connectivity: %v", err)
984984
}
@@ -997,7 +997,7 @@ func TestLinkContainers(t *testing.T) {
997997
}
998998
checkLink(true)
999999

1000-
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep2", nil, "", "")
1000+
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep2", "", "")
10011001
if err != nil {
10021002
t.Fatalf("Failed to revoke external connectivity: %v", err)
10031003
}
@@ -1017,7 +1017,7 @@ func TestLinkContainers(t *testing.T) {
10171017
if err != nil {
10181018
t.Fatal(err)
10191019
}
1020-
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep2", sbOptions, "ep2", "ep2")
1020+
err = d.ProgramExternalConnectivity(context.Background(), "net1", "ep2", "ep2", "ep2")
10211021
assert.Check(t, err != nil, "Expected Join to fail given link conditions are not satisfied")
10221022
checkLink(false)
10231023
}

‎libnetwork/drivers/bridge/port_mapping_linux_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func TestPortMappingConfig(t *testing.T) {
7373
t.Fatalf("Failed to join the endpoint: %v", err)
7474
}
7575

76-
if err = d.ProgramExternalConnectivity(context.Background(), "dummy", "ep1", sbOptions, "ep1", ""); err != nil {
76+
if err = d.ProgramExternalConnectivity(context.Background(), "dummy", "ep1", "ep1", ""); err != nil {
7777
t.Fatalf("Failed to program external connectivity: %v", err)
7878
}
7979

@@ -102,7 +102,7 @@ func TestPortMappingConfig(t *testing.T) {
102102
t.Fatal(err)
103103
}
104104

105-
err = d.ProgramExternalConnectivity(context.Background(), "dummy", "ep1", nil, "", "")
105+
err = d.ProgramExternalConnectivity(context.Background(), "dummy", "ep1", "", "")
106106
if err != nil {
107107
t.Fatal(err)
108108
}
@@ -159,7 +159,7 @@ func TestPortMappingV6Config(t *testing.T) {
159159
t.Fatalf("Failed to join the endpoint: %v", err)
160160
}
161161

162-
if err = d.ProgramExternalConnectivity(context.Background(), "dummy", "ep1", sbOptions, "ep1", "ep1"); err != nil {
162+
if err = d.ProgramExternalConnectivity(context.Background(), "dummy", "ep1", "ep1", "ep1"); err != nil {
163163
t.Fatalf("Failed to program external connectivity: %v", err)
164164
}
165165

@@ -178,7 +178,7 @@ func TestPortMappingV6Config(t *testing.T) {
178178
t.Fatal(err)
179179
}
180180

181-
err = d.ProgramExternalConnectivity(context.Background(), "dummy", "ep1", nil, "", "")
181+
err = d.ProgramExternalConnectivity(context.Background(), "dummy", "ep1", "", "")
182182
if err != nil {
183183
t.Fatal(err)
184184
}

‎libnetwork/drivers/remote/driver.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
1+
// FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16:
2+
//go:build go1.23
3+
14
package remote
25

36
import (
47
"context"
58
"fmt"
9+
"maps"
610
"net"
711
"sync"
812

@@ -30,8 +34,10 @@ type driver struct {
3034
nwEndpointsMu sync.Mutex
3135
}
3236

37+
// State info for an endpoint.
3338
type nwEndpoint struct {
34-
isGateway4 bool
39+
sbOptions map[string]any // Sandbox (container) options, from Join.
40+
isGateway4 bool // Whether ProgramExternalConnectivity reported that this ep is a gateway.
3541
isGateway6 bool
3642
}
3743

@@ -352,7 +358,7 @@ func (d *driver) Join(_ context.Context, nid, eid string, sboxKey string, jinfo
352358

353359
d.nwEndpointsMu.Lock()
354360
defer d.nwEndpointsMu.Unlock()
355-
d.nwEndpoints[eid] = &nwEndpoint{}
361+
d.nwEndpoints[eid] = &nwEndpoint{sbOptions: options}
356362
return nil
357363
}
358364

@@ -372,7 +378,7 @@ func (d *driver) Leave(nid, eid string) error {
372378
}
373379

374380
// ProgramExternalConnectivity is invoked to program the rules to allow external connectivity for the endpoint.
375-
func (d *driver) ProgramExternalConnectivity(_ context.Context, nid, eid string, options map[string]interface{}, gw4Id, gw6Id string) error {
381+
func (d *driver) ProgramExternalConnectivity(_ context.Context, nid, eid string, gw4Id, gw6Id string) error {
376382
d.nwEndpointsMu.Lock()
377383
ep, ok := d.nwEndpoints[eid]
378384
d.nwEndpointsMu.Unlock()
@@ -387,6 +393,7 @@ func (d *driver) ProgramExternalConnectivity(_ context.Context, nid, eid string,
387393
return d.revokeExternalConnectivity(nid, eid)
388394
}
389395
ep.isGateway4, ep.isGateway6 = isGw4, isGw6
396+
options := ep.sbOptions
390397
if !isGw6 && gw6Id != "" {
391398
// If there is an IPv6 gateway, but it's not eid, set NoProxy6To4. This label was
392399
// used to tell the bridge driver not to try to use the userland proxy for dual
@@ -395,6 +402,7 @@ func (d *driver) ProgramExternalConnectivity(_ context.Context, nid, eid string,
395402
// remote driver, marked as being for internal use and subject to later removal.
396403
// But, preserve it here for now as there's no other way for a remote driver to
397404
// know it shouldn't try to deal with IPv6 in this case.
405+
options = maps.Clone(ep.sbOptions)
398406
options[netlabel.NoProxy6To4] = true
399407
}
400408
data := &api.ProgramExternalConnectivityRequest{

‎libnetwork/endpoint.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -638,12 +638,12 @@ func (ep *Endpoint) sbJoin(ctx context.Context, sb *Sandbox, options ...Endpoint
638638
// If ep has taken over as a gateway and there were gateways before, update them.
639639
if ep == gwepAfter4 || ep == gwepAfter6 {
640640
if gwepBefore4 != nil {
641-
if err := gwepBefore4.programExternalConnectivity(ctx, sb.Labels(), gwepAfter4, gwepAfter6); err != nil {
641+
if err := gwepBefore4.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
642642
return fmt.Errorf("updating external connectivity for IPv4 endpoint %s: %v", epShortId(gwepBefore4), err)
643643
}
644644
defer func() {
645645
if retErr != nil {
646-
if err := gwepBefore4.programExternalConnectivity(ctx, sb.Labels(), gwepBefore4, gwepBefore6); err != nil {
646+
if err := gwepBefore4.programExternalConnectivity(ctx, gwepBefore4, gwepBefore6); err != nil {
647647
log.G(ctx).WithFields(log.Fields{
648648
"error": err,
649649
"restoreEp": epShortId(gwepBefore4),
@@ -653,12 +653,12 @@ func (ep *Endpoint) sbJoin(ctx context.Context, sb *Sandbox, options ...Endpoint
653653
}()
654654
}
655655
if gwepBefore6 != nil {
656-
if err := gwepBefore6.programExternalConnectivity(ctx, sb.Labels(), gwepAfter4, gwepAfter6); err != nil {
656+
if err := gwepBefore6.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
657657
return fmt.Errorf("updating external connectivity for IPv6 endpoint %s: %v", epShortId(gwepBefore6), err)
658658
}
659659
defer func() {
660660
if retErr != nil {
661-
if err := gwepBefore6.programExternalConnectivity(ctx, sb.Labels(), gwepBefore4, gwepBefore6); err != nil {
661+
if err := gwepBefore6.programExternalConnectivity(ctx, gwepBefore4, gwepBefore6); err != nil {
662662
log.G(ctx).WithFields(log.Fields{
663663
"error": err,
664664
"restoreEp": epShortId(gwepBefore6),
@@ -669,8 +669,8 @@ func (ep *Endpoint) sbJoin(ctx context.Context, sb *Sandbox, options ...Endpoint
669669
}
670670
}
671671

672-
// Tell the new endpoint its port mappings, and whether it's a gateway.
673-
if err := ep.programExternalConnectivity(ctx, sb.Labels(), gwepAfter4, gwepAfter6); err != nil {
672+
// Tell the new endpoint whether it's a gateway.
673+
if err := ep.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
674674
return err
675675
}
676676

@@ -687,7 +687,7 @@ func (ep *Endpoint) sbJoin(ctx context.Context, sb *Sandbox, options ...Endpoint
687687
return nil
688688
}
689689

690-
func (ep *Endpoint) programExternalConnectivity(ctx context.Context, sbLabels map[string]any, gwep4, gwep6 *Endpoint) error {
690+
func (ep *Endpoint) programExternalConnectivity(ctx context.Context, gwep4, gwep6 *Endpoint) error {
691691
n, err := ep.getNetworkFromStore()
692692
if err != nil {
693693
return types.InternalErrorf("failed to get network from store for programming external connectivity: %v", err)
@@ -703,7 +703,7 @@ func (ep *Endpoint) programExternalConnectivity(ctx context.Context, sbLabels ma
703703
"gw4": epShortId(gwep4),
704704
"gw6": epShortId(gwep6),
705705
}).Debug("Programming external connectivity on endpoint")
706-
if err := ecd.ProgramExternalConnectivity(context.WithoutCancel(ctx), n.ID(), ep.ID(), sbLabels, epId(gwep4), epId(gwep6)); err != nil {
706+
if err := ecd.ProgramExternalConnectivity(context.WithoutCancel(ctx), n.ID(), ep.ID(), epId(gwep4), epId(gwep6)); err != nil {
707707
return types.InternalErrorf("driver failed programming external connectivity on endpoint %s (%s): %v",
708708
ep.Name(), ep.ID(), err)
709709
}
@@ -820,7 +820,7 @@ func (ep *Endpoint) sbLeave(ctx context.Context, sb *Sandbox, force bool) error
820820

821821
if d != nil {
822822
if ecd, ok := d.(driverapi.ExtConner); ok {
823-
if err := ecd.ProgramExternalConnectivity(context.WithoutCancel(ctx), n.ID(), ep.ID(), nil, "", ""); err != nil {
823+
if err := ecd.ProgramExternalConnectivity(context.WithoutCancel(ctx), n.ID(), ep.ID(), "", ""); err != nil {
824824
log.G(ctx).WithError(err).Warn("driver failed revoking external connectivity on endpoint")
825825
}
826826
}
@@ -895,12 +895,12 @@ func (ep *Endpoint) sbLeave(ctx context.Context, sb *Sandbox, force bool) error
895895
// Find new endpoint(s) to provide external connectivity for the sandbox.
896896
gwepAfter4, gwepAfter6 := sb.getGatewayEndpoint()
897897
if gwepAfter4 != nil {
898-
if err := gwepAfter4.programExternalConnectivity(ctx, sb.Labels(), gwepAfter4, gwepAfter6); err != nil {
898+
if err := gwepAfter4.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
899899
log.G(ctx).WithError(err).Error("Failed to set IPv4 gateway")
900900
}
901901
}
902902
if gwepAfter6 != nil && gwepAfter6 != gwepAfter4 {
903-
if err := gwepAfter6.programExternalConnectivity(ctx, sb.Labels(), gwepAfter4, gwepAfter6); err != nil {
903+
if err := gwepAfter6.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
904904
log.G(ctx).WithError(err).Error("Failed to set IPv6 gateway")
905905
}
906906
}

0 commit comments

Comments
 (0)