Merge pull request #47318 from andrewbaxter/47317-allow-macvlan-dup-parent

Allow multiple macvlan networks to share a parent

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) {

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 <link_name>.
+	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 {

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"

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)
 			}
 		}
 	}