diff --git a/integration/internal/network/ops.go b/integration/internal/network/ops.go index cc07e842fee1a..49f762ecae02a 100644 --- a/integration/internal/network/ops.go +++ b/integration/internal/network/ops.go @@ -45,6 +45,19 @@ func WithMacvlan(parent string) func(*network.CreateOptions) { } } +// WithMacvlanPassthru sets the network as macvlan with the specified parent in passthru mode +func WithMacvlanPassthru(parent string) func(options *network.CreateOptions) { + return func(n *network.CreateOptions) { + n.Driver = "macvlan" + n.Options = map[string]string{ + "macvlan_mode": "passthru", + } + if parent != "" { + n.Options["parent"] = parent + } + } +} + // WithIPvlan sets the network as ipvlan with the specified parent and mode func WithIPvlan(parent, mode string) func(*network.CreateOptions) { return func(n *network.CreateOptions) { diff --git a/integration/network/helpers.go b/integration/network/helpers.go index 771a4b4cd87aa..cf823cf86de38 100644 --- a/integration/network/helpers.go +++ b/integration/network/helpers.go @@ -42,6 +42,15 @@ func LinkExists(ctx context.Context, t *testing.T, master string) { testutil.RunCommand(ctx, "ip", "link", "show", master).Assert(t, icmd.Success) } +// LinkDoesntExist verifies that a link doesn't exist +func LinkDoesntExist(ctx context.Context, t *testing.T, master string) { + // verify the specified link doesn't exist, ip link show . + testutil.RunCommand(ctx, "ip", "link", "show", master).Assert(t, icmd.Expected{ + ExitCode: 1, + Err: "does not exist", + }) +} + // IsNetworkAvailable provides a comparison to check if a docker network is available func IsNetworkAvailable(ctx context.Context, c client.NetworkAPIClient, name string) cmp.Comparison { return func() cmp.Result { diff --git a/integration/network/macvlan/macvlan_test.go b/integration/network/macvlan/macvlan_test.go index 538cdf0fb188c..929f785505007 100644 --- a/integration/network/macvlan/macvlan_test.go +++ b/integration/network/macvlan/macvlan_test.go @@ -62,6 +62,18 @@ func TestDockerNetworkMacvlan(t *testing.T) { }, { name: "OverlapParent", test: testMacvlanOverlapParent, + }, { + name: "OverlapParentPassthruFirst", + test: testMacvlanOverlapParentPassthruFirst, + }, { + name: "OverlapParentPassthruSecond", + test: testMacvlanOverlapParentPassthruSecond, + }, { + name: "OverlapDeleteCreatedSecond", + test: testMacvlanOverlapDeleteCreatedSecond, + }, { + name: "OverlapKeepExistingParent", + test: testMacvlanOverlapKeepExisting, }, { name: "NilParent", test: testMacvlanNilParent, @@ -99,7 +111,8 @@ func TestDockerNetworkMacvlan(t *testing.T) { } func testMacvlanOverlapParent(t *testing.T, ctx context.Context, client client.APIClient) { - // verify the same parent interface cannot be used if already in use by an existing network + // verify the same parent interface can be used if already in use by an existing network + // as long as neither are passthru master := "dm-dummy0" n.CreateMasterDummy(ctx, t, master) defer n.DeleteInterface(ctx, t, master) @@ -110,6 +123,42 @@ func testMacvlanOverlapParent(t *testing.T, ctx context.Context, client client.A net.WithMacvlan(parentName), ) assert.Check(t, n.IsNetworkAvailable(ctx, client, netName)) + n.LinkExists(ctx, t, parentName) + + overlapNetName := "dm-parent-net-overlap" + _, err := net.Create(ctx, client, overlapNetName, + net.WithMacvlan(parentName), + ) + assert.Check(t, err == nil) + + // delete the second network while preserving the parent link + err = client.NetworkRemove(ctx, overlapNetName) + assert.NilError(t, err) + assert.Check(t, n.IsNetworkNotAvailable(ctx, client, overlapNetName)) + n.LinkExists(ctx, t, parentName) + + // delete the first network + err = client.NetworkRemove(ctx, netName) + assert.NilError(t, err) + assert.Check(t, n.IsNetworkNotAvailable(ctx, client, netName)) + n.LinkDoesntExist(ctx, t, parentName) + + // verify the network delete did not delete the root link + n.LinkExists(ctx, t, master) +} + +func testMacvlanOverlapParentPassthruFirst(t *testing.T, ctx context.Context, client client.APIClient) { + // verify creating a second interface sharing a parent with another passthru interface is rejected + master := "dm-dummy0" + n.CreateMasterDummy(ctx, t, master) + defer n.DeleteInterface(ctx, t, master) + + netName := "dm-subinterface" + parentName := "dm-dummy0.40" + net.CreateNoError(ctx, t, client, netName, + net.WithMacvlanPassthru(parentName), + ) + assert.Check(t, n.IsNetworkAvailable(ctx, client, netName)) _, err := net.Create(ctx, client, "dm-parent-net-overlap", net.WithMacvlan(parentName), @@ -125,6 +174,96 @@ func testMacvlanOverlapParent(t *testing.T, ctx context.Context, client client.A n.LinkExists(ctx, t, master) } +func testMacvlanOverlapParentPassthruSecond(t *testing.T, ctx context.Context, client client.APIClient) { + // verify creating a passthru interface sharing a parent with another interface is rejected + master := "dm-dummy0" + n.CreateMasterDummy(ctx, t, master) + defer n.DeleteInterface(ctx, t, master) + + netName := "dm-subinterface" + parentName := "dm-dummy0.40" + net.CreateNoError(ctx, t, client, netName, + net.WithMacvlan(parentName), + ) + assert.Check(t, n.IsNetworkAvailable(ctx, client, netName)) + + _, err := net.Create(ctx, client, "dm-parent-net-overlap", + net.WithMacvlanPassthru(parentName), + ) + assert.Check(t, err != nil) + + // delete the network while preserving the parent link + err = client.NetworkRemove(ctx, netName) + assert.NilError(t, err) + + assert.Check(t, n.IsNetworkNotAvailable(ctx, client, netName)) + // verify the network delete did not delete the predefined link + n.LinkExists(ctx, t, master) +} + +func testMacvlanOverlapDeleteCreatedSecond(t *testing.T, ctx context.Context, client client.APIClient) { + // verify that a shared created parent interface is kept when the original interface is deleted first + master := "dm-dummy0" + n.CreateMasterDummy(ctx, t, master) + defer n.DeleteInterface(ctx, t, master) + + netName := "dm-subinterface" + parentName := "dm-dummy0.40" + net.CreateNoError(ctx, t, client, netName, + net.WithMacvlan(parentName), + ) + assert.Check(t, n.IsNetworkAvailable(ctx, client, netName)) + + overlapNetName := "dm-parent-net-overlap" + _, err := net.Create(ctx, client, overlapNetName, + net.WithMacvlan(parentName), + ) + assert.Check(t, err == nil) + + // delete the original network while preserving the parent link + err = client.NetworkRemove(ctx, netName) + assert.NilError(t, err) + assert.Check(t, n.IsNetworkNotAvailable(ctx, client, netName)) + n.LinkExists(ctx, t, parentName) + + // delete the second network + err = client.NetworkRemove(ctx, overlapNetName) + assert.NilError(t, err) + assert.Check(t, n.IsNetworkNotAvailable(ctx, client, overlapNetName)) + n.LinkDoesntExist(ctx, t, parentName) + + // verify the network delete did not delete the root link + n.LinkExists(ctx, t, master) +} + +func testMacvlanOverlapKeepExisting(t *testing.T, ctx context.Context, client client.APIClient) { + // verify that deleting interfaces sharing a previously existing parent doesn't delete the + // parent + master := "dm-dummy0" + n.CreateMasterDummy(ctx, t, master) + defer n.DeleteInterface(ctx, t, master) + + netName := "dm-subinterface" + net.CreateNoError(ctx, t, client, netName, + net.WithMacvlan(master), + ) + assert.Check(t, n.IsNetworkAvailable(ctx, client, netName)) + + overlapNetName := "dm-parent-net-overlap" + _, err := net.Create(ctx, client, overlapNetName, + net.WithMacvlan(master), + ) + assert.Check(t, err == nil) + + err = client.NetworkRemove(ctx, overlapNetName) + assert.NilError(t, err) + err = client.NetworkRemove(ctx, netName) + assert.NilError(t, err) + + // verify the network delete did not delete the root link + n.LinkExists(ctx, t, master) +} + func testMacvlanSubinterface(t *testing.T, ctx context.Context, client client.APIClient) { // verify the same parent interface cannot be used if already in use by an existing network master := "dm-dummy0" diff --git a/libnetwork/drivers/macvlan/macvlan_network.go b/libnetwork/drivers/macvlan/macvlan_network.go index 75313dd834563..7413b3fb20bc9 100644 --- a/libnetwork/drivers/macvlan/macvlan_network.go +++ b/libnetwork/drivers/macvlan/macvlan_network.go @@ -60,8 +60,20 @@ func (d *driver) createNetwork(config *configuration) (bool, error) { for _, nw := range networkList { if config.Parent == nw.config.Parent { if config.ID != nw.config.ID { - return false, fmt.Errorf("network %s is already using parent interface %s", - getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + if config.MacvlanMode == modePassthru { + return false, fmt.Errorf( + "cannot use mode passthru, macvlan network %s is already using parent interface %s", + stringid.TruncateID(nw.config.ID), + config.Parent, + ) + } else if nw.config.MacvlanMode == modePassthru { + return false, fmt.Errorf( + "macvlan network %s is already using parent interface %s in mode passthru", + stringid.TruncateID(nw.config.ID), + config.Parent, + ) + } + continue } log.G(context.TODO()).Debugf("Create Network for the same ID %s\n", config.ID) foundExisting = true @@ -90,6 +102,15 @@ func (d *driver) createNetwork(config *configuration) (bool, error) { // if driver created the networks slave link, record it for future deletion config.CreatedSlaveLink = true } + } else { + // Check and mark this network if the interface was created for another network + networkList := d.getNetworks() + for _, testN := range networkList { + if config.Parent == testN.config.Parent && testN.config.CreatedSlaveLink { + config.CreatedSlaveLink = true + break + } + } } if !foundExisting { n := &network{ @@ -105,30 +126,38 @@ func (d *driver) createNetwork(config *configuration) (bool, error) { return foundExisting, nil } +func (d *driver) parentHasSingleUser(n *network) bool { + users := 0 + networkList := d.getNetworks() + for _, testN := range networkList { + if n.config.Parent == testN.config.Parent { + users += 1 + } + } + return users == 1 +} + // DeleteNetwork deletes the network for the specified driver type func (d *driver) DeleteNetwork(nid string) error { n := d.network(nid) if n == nil { return fmt.Errorf("network id %s not found", nid) } - // if the driver created the slave interface, delete it, otherwise leave it - if ok := n.config.CreatedSlaveLink; ok { - // if the interface exists, only delete if it matches iface.vlan or dummy.net_id naming - if ok := parentExists(n.config.Parent); ok { - // only delete the link if it is named the net_id - if n.config.Parent == getDummyName(stringid.TruncateID(nid)) { - err := delDummyLink(n.config.Parent) - if err != nil { - log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v", - n.config.Parent, err) - } - } else { - // only delete the link if it matches iface.vlan naming - err := delVlanLink(n.config.Parent) - if err != nil { - log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v", - n.config.Parent, err) - } + // if the driver created the slave interface and this network is the last user, delete it, otherwise leave it + if n.config.CreatedSlaveLink && parentExists(n.config.Parent) && d.parentHasSingleUser(n) { + // only delete the link if it is named the net_id + if n.config.Parent == getDummyName(stringid.TruncateID(nid)) { + err := delDummyLink(n.config.Parent) + if err != nil { + log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v", + n.config.Parent, err) + } + } else { + // only delete the link if it matches iface.vlan naming + err := delVlanLink(n.config.Parent) + if err != nil { + log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v", + n.config.Parent, err) } } }