Merge pull request juju#9491 from jameinel/2.4-bridge-name-1804018

juju#9491 ## Description of change This is rebasing juju#9482 onto 2.4 and fixing up the test suite. It changes how we create bridge names for bridges put on top of devices that have vlans. We essentially never put a '.' into a bridge name, so that it bridges to the proper device, rather than bridging to the underlying device and adding vlan on top of it. ## QA steps See the bug report. I did not test handling of VLAN'd bridges directly, as I don't have an appropriate setup to test that packets actually get routed correctly. I did test that I was still able to create a machine and put containers on it, and it would create a bridge for those containers. ## Documentation changes None. ## Bug reference https://bugs.launchpad.net/juju/+bug/1804018
SimonRichardson · Nov 22, 2018 · bc96a4d · bc96a4d
2 parents 203ac68 + 9c3c2f1
commit bc96a4d
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 7 deletions.
diff --git a/network/containerizer/bridgepolicy.go b/network/containerizer/bridgepolicy.go
@@ -180,7 +180,9 @@ var skippedDeviceNames = set.NewStrings(
 // 2. Add b- to device name, if it doesn't fit in 15 characters then:
 // 3a. For devices starting in 'en' remove 'en' and add 'b-'
 // 3b. For all other devices 'b-' + 6-char hash of name + '-' + last 6 chars of name
+// 4. If using the device name directly always replace '.' with '-', to make sure that bridges from VLANs won't break
 func BridgeNameForDevice(device string) string {
+	device = strings.Replace(device, ".", "-", -1)
 	switch {
 	case len(device) < 13:
 		return fmt.Sprintf("br-%s", device)

diff --git a/network/containerizer/bridgepolicy_test.go b/network/containerizer/bridgepolicy_test.go
@@ -235,7 +235,7 @@ func (s *bridgePolicyStateSuite) TestPopulateContainerLinkLayerDevicesCorrectlyP
 			Type: state.BridgeDevice,
 		},
 		state.LinkLayerDeviceArgs{
-			Name: "br-eth10.100",
+			Name: "br-eth10-100",
 			Type: state.BridgeDevice,
 		},
 		state.LinkLayerDeviceArgs{
@@ -282,7 +282,7 @@ func (s *bridgePolicyStateSuite) TestPopulateContainerLinkLayerDevicesCorrectlyP
 		"br-eth3",
 		"br-eth4",
 		"br-eth10",
-		"br-eth10.100",
+		"br-eth10-100",
 	}
 
 	err = s.machine.SetParentLinkLayerDevicesBeforeTheirChildren(devicesArgs[:])
@@ -865,7 +865,7 @@ func (s *bridgePolicyStateSuite) TestFindMissingBridgesForContainerOneOfTwoBridg
 	// Only the first device (by sort order) should be selected
 	c.Check(missing, gc.DeepEquals, []network.DeviceToBridge{{
 		DeviceName: "ens2.1",
-		BridgeName: "br-ens2.1",
+		BridgeName: "br-ens2-1",
 	}})
 	c.Check(reconfigureDelay, gc.Equals, 0)
 }
@@ -944,7 +944,7 @@ func (s *bridgePolicyStateSuite) TestFindMissingBridgesForContainerMultipleSpace
 		BridgeName: "br-eth0",
 	}, {
 		DeviceName: "eth0.1",
-		BridgeName: "br-eth0.1",
+		BridgeName: "br-eth0-1",
 	}, {
 		DeviceName: "eth1",
 		BridgeName: "br-eth1",
@@ -1056,7 +1056,7 @@ func (s *bridgePolicyStateSuite) TestFindMissingBridgesForContainerVLAN(c *gc.C)
 		BridgeName: "br-eth0",
 	}, {
 		DeviceName: "eth0.100",
-		BridgeName: "br-eth0.100",
+		BridgeName: "br-eth0-100",
 	}})
 	c.Check(reconfigureDelay, gc.Equals, 0)
 }
@@ -1122,7 +1122,7 @@ func (s *bridgePolicyStateSuite) TestFindMissingBridgesForContainerVLANOnBond(c
 		BridgeName: "br-bond0",
 	}, {
 		DeviceName: "bond0.100",
-		BridgeName: "br-bond0.100",
+		BridgeName: "br-bond0-100",
 	}})
 	c.Check(reconfigureDelay, gc.Equals, 13)
 }
@@ -1145,12 +1145,13 @@ func (s *bridgePolicyStateSuite) TestFindMissingBridgesForContainerNetworkingMet
 
 var bridgeNames = map[string]string{
 	"eno0":            "br-eno0",
+	"enovlan.123":     "br-enovlan-123",
 	"twelvechars0":    "br-twelvechars0",
 	"thirteenchars":   "b-thirteenchars",
 	"enfourteenchar":  "b-fourteenchar",
 	"enfifteenchars0": "b-fifteenchars0",
 	"fourteenchars1":  "b-5590a4-chars1",
-	"fifteenchars.12": "b-7e0acf-ars.12",
+	"fifteenchars.12": "b-38b496-ars-12",
 	"zeros0526193032": "b-000000-193032",
 	"enx00e07cc81e1d": "b-x00e07cc81e1d",
 }