provider/maas: make bridge script idempotent

If an interface was already bridged the script would create a stuttered
iface name and an additional bridge_ports option which is invalid. For
example:

  iface eth0 inet manual

  iface test-br-eth0 inet manual

  auto test-br-test-br-eth0
  iface test-br-test-br-eth0 inet static
    address 1.2.3.4
    netmask 255.255.255.0
    gateway 4.3.2.1
    bridge_ports eth0
    bridge_ports test-br-eth0

This is fixed by only adding a bridge if the iface does not have an
existing 'bridge_ports' option.

Fixes [LP:#1553915](https://bugs.launchpad.net/juju-core/+bug/1553915)

provider/maas/add-juju-bridge.py

@@ -68,6 +68,7 @@ def __init__(self, definition, options=None):
         self.is_alias = ":" in self.name
         self.is_vlan = [x for x in self.options if x.startswith("vlan-raw-device")]
         self.is_active = self.method == "dhcp" or self.method == "static"
+        self.is_bridged = [x for x in self.options if x.startswith("bridge_ports ")]
 
     def __str__(self):
         return self.name
@@ -77,8 +78,8 @@ def bridge(self, prefix, bridge_name, add_auto_stanza):
         if bridge_name is None:
             bridge_name = prefix + self.name
         # Note: the testing order here is significant.
-        if not self.is_active:
-            return self._bridge_inactive(add_auto_stanza)
+        if not self.is_active or self.is_bridged:
+            return self._bridge_unchanged(add_auto_stanza)
         elif self.is_alias:
             return self._bridge_alias(add_auto_stanza)
         elif self.is_vlan:
@@ -128,7 +129,7 @@ def _bridge_bond(self, prefix, bridge_name, add_auto_stanza):
         stanzas.extend([s1, s2, s3])
         return stanzas
 
-    def _bridge_inactive(self, add_auto_stanza):
+    def _bridge_unchanged(self, add_auto_stanza):
         stanzas = []
         if add_auto_stanza:
             stanzas.append(AutoStanza(self.name))

provider/maas/bridgescript.go

@@ -80,6 +80,7 @@ class LogicalInterface(object):
         self.is_alias = ":" in self.name
         self.is_vlan = [x for x in self.options if x.startswith("vlan-raw-device")]
         self.is_active = self.method == "dhcp" or self.method == "static"
+        self.is_bridged = [x for x in self.options if x.startswith("bridge_ports ")]
 
     def __str__(self):
         return self.name
@@ -89,8 +90,8 @@ class LogicalInterface(object):
         if bridge_name is None:
             bridge_name = prefix + self.name
         # Note: the testing order here is significant.
-        if not self.is_active:
-            return self._bridge_inactive(add_auto_stanza)
+        if not self.is_active or self.is_bridged:
+            return self._bridge_unchanged(add_auto_stanza)
         elif self.is_alias:
             return self._bridge_alias(add_auto_stanza)
         elif self.is_vlan:
@@ -140,7 +141,7 @@ class LogicalInterface(object):
         stanzas.extend([s1, s2, s3])
         return stanzas
 
-    def _bridge_inactive(self, add_auto_stanza):
+    def _bridge_unchanged(self, add_auto_stanza):
         stanzas = []
         if add_auto_stanza:
             stanzas.append(AutoStanza(self.name))

provider/maas/bridgescript_test.go

@@ -94,76 +94,49 @@ func (s *bridgeConfigSuite) TestBridgeScriptWithUndefinedArgs(c *gc.C) {
 	}
 }
 
-func (s *bridgeConfigSuite) TestBridgeScriptDHCP(c *gc.C) {
-	s.assertScriptWithPrefix(c, networkDHCPInitial, networkDHCPExpected, "test-br-")
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptStatic(c *gc.C) {
-	s.assertScriptWithPrefix(c, networkStaticInitial, networkStaticExpected, "test-br-")
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptDualNIC(c *gc.C) {
-	s.assertScriptWithPrefix(c, networkDualNICInitial, networkDualNICExpected, "test-br-")
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptWithAlias(c *gc.C) {
-	s.assertScriptWithPrefix(c, networkWithAliasInitial, networkWithAliasExpected, "test-br-")
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptDHCPWithAlias(c *gc.C) {
-	s.assertScriptWithPrefix(c, networkDHCPWithAliasInitial, networkDHCPWithAliasExpected, "test-br-")
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptMultipleStaticWithAliases(c *gc.C) {
-	s.assertScriptWithPrefix(c, networkMultipleStaticWithAliasesInitial, networkMultipleStaticWithAliasesExpected, "test-br-")
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptDHCPWithBond(c *gc.C) {
-	s.assertScriptWithPrefix(c, networkDHCPWithBondInitial, networkDHCPWithBondExpected, "test-br-")
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptMultipleAliases(c *gc.C) {
-	s.assertScriptWithPrefix(c, networkMultipleAliasesInitial, networkMultipleAliasesExpected, "test-br-")
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptSmorgasboard(c *gc.C) {
-	s.assertScriptWithPrefix(c, networkSmorgasboardInitial, networkSmorgasboardExpected, "juju-br-")
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptWithVLANs(c *gc.C) {
-	s.assertScriptWithPrefix(c, networkVLANInitial, networkVLANExpected, "vlan-br-")
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptWithMultipleNameservers(c *gc.C) {
-	s.assertScriptWithDefaultPrefix(c, networkVLANWithMultipleNameserversInitial, networkVLANWithMultipleNameserversExpected)
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptWithLoopbackOnly(c *gc.C) {
-	s.assertScriptWithDefaultPrefix(c, networkLoopbackOnlyInitial, networkLoopbackOnlyExpected)
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptBondWithVLANs(c *gc.C) {
-	s.assertScriptWithDefaultPrefix(c, networkStaticBondWithVLANsInitial, networkStaticBondWithVLANsExpected)
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptVLANWithInactive(c *gc.C) {
-	s.assertScriptWithDefaultPrefix(c, networkVLANWithInactiveDeviceInitial, networkVLANWithInactiveDeviceExpected)
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptVLANWithActiveDHCPDevice(c *gc.C) {
-	s.assertScriptWithDefaultPrefix(c, networkVLANWithActiveDHCPDeviceInitial, networkVLANWithActiveDHCPDeviceExpected)
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptMultipleDNSValues(c *gc.C) {
-	s.assertScriptWithDefaultPrefix(c, networkWithMultipleDNSValuesInitial, networkWithMultipleDNSValuesExpected)
-}
-
-func (s *bridgeConfigSuite) TestBridgeScriptEmptyDNSValues(c *gc.C) {
-	s.assertScriptWithDefaultPrefix(c, networkWithEmptyDNSValuesInitial, networkWithEmptyDNSValuesExpected)
+func (s *bridgeConfigSuite) TestBridgeScriptWithPrefixTransformation(c *gc.C) {
+	for i, v := range []struct {
+		initial  string
+		expected string
+		prefix   string
+	}{
+		{networkDHCPInitial, networkDHCPExpected, "test-br-"},
+		{networkDHCPWithAliasInitial, networkDHCPWithAliasExpected, "test-br-"},
+		{networkDHCPWithBondInitial, networkDHCPWithBondExpected, "test-br-"},
+		{networkDualNICInitial, networkDualNICExpected, "test-br-"},
+		{networkMultipleAliasesInitial, networkMultipleAliasesExpected, "test-br-"},
+		{networkMultipleStaticWithAliasesInitial, networkMultipleStaticWithAliasesExpected, "test-br-"},
+		{networkSmorgasboardInitial, networkSmorgasboardExpected, "juju-br-"},
+		{networkStaticInitial, networkStaticExpected, "test-br-"},
+		{networkVLANInitial, networkVLANExpected, "vlan-br-"},
+		{networkWithAliasInitial, networkWithAliasExpected, "test-br-"},
+	} {
+		c.Logf("test #%v - expected transformation", i)
+		s.assertScriptWithPrefix(c, v.initial, v.expected, v.prefix)
+		c.Logf("test #%v - idempotent transformation", i)
+		s.assertScriptWithPrefix(c, v.expected, v.expected, v.prefix)
+	}
 }
 
-func (s *bridgeConfigSuite) TestBridgeScriptMismatchedBridgeNameAndInterfaceArgs(c *gc.C) {
-	s.assertScriptWithDefaultPrefix(c, networkWithEmptyDNSValuesInitial, networkWithEmptyDNSValuesExpected)
+func (s *bridgeConfigSuite) TestBridgeScriptWithDefaultPrefixTransformation(c *gc.C) {
+	for i, v := range []struct {
+		initial  string
+		expected string
+	}{
+		{networkLoopbackOnlyInitial, networkLoopbackOnlyExpected},
+		{networkStaticBondWithVLANsInitial, networkStaticBondWithVLANsExpected},
+		{networkVLANWithActiveDHCPDeviceInitial, networkVLANWithActiveDHCPDeviceExpected},
+		{networkVLANWithInactiveDeviceInitial, networkVLANWithInactiveDeviceExpected},
+		{networkVLANWithMultipleNameserversInitial, networkVLANWithMultipleNameserversExpected},
+		{networkWithEmptyDNSValuesInitial, networkWithEmptyDNSValuesExpected},
+		{networkWithMultipleDNSValuesInitial, networkWithMultipleDNSValuesExpected},
+		{networkPartiallyBridgedInitial, networkPartiallyBridgedExpected},
+	} {
+		c.Logf("test #%v - expected transformation", i)
+		s.assertScriptWithDefaultPrefix(c, v.initial, v.expected)
+		c.Logf("test #%v - idempotent transformation", i)
+		s.assertScriptWithDefaultPrefix(c, v.expected, v.expected)
+	}
 }
 
 func (s *bridgeConfigSuite) TestBridgeScriptInterfaceNameArgumentRequired(c *gc.C) {
@@ -1548,3 +1521,42 @@ iface juju-br0 inet static
     netmask 255.255.255.0
     gateway 4.3.2.1
     bridge_ports eth1`
+
+const networkPartiallyBridgedInitial = `auto lo
+iface lo inet loopback
+
+iface eth0 inet manual
+
+auto br-eth0
+iface br-eth0 inet static
+    address 1.2.3.4
+    netmask 255.255.255.0
+    gateway 4.3.2.1
+    bridge_ports eth0
+
+auto eth1
+iface eth1 inet static
+    address 1.2.3.5
+    netmask 255.255.255.0
+    gateway 4.3.2.1`
+
+const networkPartiallyBridgedExpected = `auto lo
+iface lo inet loopback
+
+iface eth0 inet manual
+
+auto br-eth0
+iface br-eth0 inet static
+    address 1.2.3.4
+    netmask 255.255.255.0
+    gateway 4.3.2.1
+    bridge_ports eth0
+
+iface eth1 inet manual
+
+auto br-eth1
+iface br-eth1 inet static
+    address 1.2.3.5
+    netmask 255.255.255.0
+    gateway 4.3.2.1
+    bridge_ports eth1`