Repository: nanovms/ops
Pull request #901 (2021-02-17 10:57:20)

support multiple bridges (closes 174)
* set default bridge name "br0" if bridged flag is specified and the bridgename it is not


Patch:
diff --git a/cmd/flags_start_image.go b/cmd/flags_start_image.go
index 02c753f..5b809d6 100644
--- a/cmd/flags_start_image.go
+++ b/cmd/flags_start_image.go
@@ -16,6 +16,7 @@ import (
 type StartImageCommandFlags struct {
 	Accel          bool
 	Bridged        bool
+	BridgeName     string
 	Debug          bool
 	Force          bool
 	Gateway        string
@@ -77,6 +78,10 @@ func (flags *StartImageCommandFlags) MergeToConfig(c *config.Config) (err error)
 		c.RunConfig.TapName = flags.TapName
 	}
 
+	if flags.BridgeName != "" {
+		c.RunConfig.BridgeName = flags.BridgeName
+	}
+
 	c.RunConfig.Verbose = flags.Verbose
 	c.RunConfig.Bridged = flags.Bridged
 	c.RunConfig.Accel = flags.Accel
@@ -206,6 +211,11 @@ func NewStartImageCommandFlags(cmdFlags *pflag.FlagSet) (flags *StartImageComman
 		exitWithError(err.Error())
 	}
 
+	flags.BridgeName, err = cmdFlags.GetString("bridgename")
+	if err != nil {
+		exitWithError(err.Error())
+	}
+
 	flags.Trace, err = cmdFlags.GetBool("trace")
 	if err != nil {
 		exitWithError(err.Error())
@@ -229,6 +239,7 @@ func PersistStartImageCommandFlags(cmdFlags *pflag.FlagSet) {
 	cmdFlags.StringArrayP("no-trace", "", nil, "do not trace syscall")
 	cmdFlags.BoolP("verbose", "v", false, "verbose")
 	cmdFlags.BoolP("bridged", "b", false, "bridge networking")
+	cmdFlags.StringP("bridgename", "", "", "bridge name")
 	cmdFlags.StringP("tapname", "t", "", "tap device name")
 	cmdFlags.String("ip-address", "", "static ip address")
 	cmdFlags.String("gateway", "", "network gateway")
diff --git a/cmd/start_unikernel.go b/cmd/start_unikernel.go
index 7f4a2aa..d4f15ec 100644
--- a/cmd/start_unikernel.go
+++ b/cmd/start_unikernel.go
@@ -21,12 +21,16 @@ func StartUnikernel(c *config.Config) (err error) {
 	bridged := c.RunConfig.Bridged
 	ipaddr := c.RunConfig.IPAddr
 	netmask := c.RunConfig.NetMask
-	bridgeName := "br0"
+
+	bridgeName := c.RunConfig.BridgeName
+	if bridged && bridgeName == "" {
+		bridgeName = "br0"
+	}
 
 	networkService := network.NewIprouteNetworkService()
 
 	if tapDeviceName != "" {
-		err = network.SetupNetworkInterfaces(networkService, tapDeviceName, bridged, ipaddr, netmask)
+		err = network.SetupNetworkInterfaces(networkService, tapDeviceName, bridgeName, ipaddr, netmask)
 		if err != nil {
 			return
 		}
@@ -36,7 +40,7 @@ func StartUnikernel(c *config.Config) (err error) {
 	hypervisor.Start(&c.RunConfig)
 
 	if tapDeviceName != "" {
-		err = network.TurnOffNetworkInterfaces(networkService, tapDeviceName, bridged, bridgeName)
+		err = network.TurnOffNetworkInterfaces(networkService, tapDeviceName, bridgeName)
 		if err != nil {
 			return
 		}
diff --git a/config/config.go b/config/config.go
index dad4af6..304dbc7 100644
--- a/config/config.go
+++ b/config/config.go
@@ -124,6 +124,9 @@ type RunConfig struct {
 	// in use. This also enables KVM acceleration.
 	Bridged bool
 
+	// BridgeName
+	BridgeName string
+
 	// CPUs specifies the number of CPU cores to use
 	CPUs int
 
diff --git a/network/setup_network_interfaces.go b/network/setup_network_interfaces.go
index d4fc979..df0712b 100644
--- a/network/setup_network_interfaces.go
+++ b/network/setup_network_interfaces.go
@@ -24,7 +24,7 @@ type Service interface {
 }
 
 // SetupNetworkInterfaces changes network configuration to support requirements
-func SetupNetworkInterfaces(network Service, tapDeviceName string, bridged bool, ipaddr string, netmask string) error {
+func SetupNetworkInterfaces(network Service, tapDeviceName string, bridgeName string, ipaddr string, netmask string) error {
 	tapExists, err := network.CheckNetworkInterfaceExists(tapDeviceName)
 	if err != nil {
 		return errors.New("Not able to check tap exists")
@@ -37,9 +37,7 @@ func SetupNetworkInterfaces(network Service, tapDeviceName string, bridged bool,
 		}
 	}
 
-	bridgeName := "br0"
-
-	if bridged {
+	if bridgeName != "" {
 
 		bridgeExists, err := network.CheckNetworkInterfaceExists(bridgeName)
 		if err != nil {
@@ -118,13 +116,13 @@ func SetupNetworkInterfaces(network Service, tapDeviceName string, bridged bool,
 }
 
 // TurnOffNetworkInterfaces turns network interfaces off if they aren't used
-func TurnOffNetworkInterfaces(network Service, tapDeviceName string, bridged bool, bridgeName string) error {
+func TurnOffNetworkInterfaces(network Service, tapDeviceName string, bridgeName string) error {
 	_, err := network.TurnNIDown(tapDeviceName)
 	if err != nil {
 		return errors.New("Not able to turn tap down")
 	}
 
-	if bridged {
+	if bridgeName != "" {
 		bridgeInterfaces, err := network.GetBridgeInterfacesNames(bridgeName)
 		if err != nil {
 			return errors.New("Not able to get network interfaces in bridge")


Test patch:
diff --git a/cmd/flags_start_image_test.go b/cmd/flags_start_image_test.go
index 9f7a244..ee7b0bd 100644
--- a/cmd/flags_start_image_test.go
+++ b/cmd/flags_start_image_test.go
@@ -20,6 +20,7 @@ func TestCreateStartImageFlags(t *testing.T) {
 	assert.Equal(t, startImageFlags.NoTrace, []string{"a"})
 	assert.Equal(t, startImageFlags.Verbose, true)
 	assert.Equal(t, startImageFlags.Bridged, true)
+	assert.Equal(t, startImageFlags.BridgeName, "br1")
 
 	assert.Equal(t, startImageFlags.TapName, "tap1")
 	assert.Equal(t, startImageFlags.IPAddress, "192.168.0.1")
@@ -48,18 +49,19 @@ func TestStartImageFlagsMergeToConfig(t *testing.T) {
 		Force:      true,
 		NoTrace:    []string{"a"},
 		RunConfig: config.RunConfig{
-			Accel:   true,
-			Bridged: true,
-			CPUs:    2,
-			Debug:   false,
-			Gateway: "192.168.1.254",
-			GdbPort: 1234,
-			IPAddr:  "192.168.0.1",
-			Mounts:  []string(nil),
-			NetMask: "255.255.0.0",
-			Ports:   []string{"80", "81", "82-85"},
-			TapName: "tap1",
-			Verbose: true,
+			Accel:      true,
+			Bridged:    true,
+			BridgeName: "br1",
+			CPUs:       2,
+			Debug:      false,
+			Gateway:    "192.168.1.254",
+			GdbPort:    1234,
+			IPAddr:     "192.168.0.1",
+			Mounts:     []string(nil),
+			NetMask:    "255.255.0.0",
+			Ports:      []string{"80", "81", "82-85"},
+			TapName:    "tap1",
+			Verbose:    true,
 		},
 	}
 
@@ -80,6 +82,7 @@ func newStartImageFlagSet(debug string) *cmd.StartImageCommandFlags {
 	flagSet.Set("no-trace", "a")
 	flagSet.Set("verbose", "true")
 	flagSet.Set("bridged", "true")
+	flagSet.Set("bridgename", "br1")
 	flagSet.Set("tapname", "tap1")
 	flagSet.Set("ip-address", "192.168.0.1")
 	flagSet.Set("gateway", "192.168.1.254")
diff --git a/network/setup_network_interfaces_test.go b/network/setup_network_interfaces_test.go
index 194ed30..2e4bf80 100644
--- a/network/setup_network_interfaces_test.go
+++ b/network/setup_network_interfaces_test.go
@@ -34,7 +34,7 @@ func TestSetupNetworkInterfaces(t *testing.T) {
 			TurnNIUp(tapDeviceName).
 			Return("success", nil)
 
-		network.SetupNetworkInterfaces(networkService, tapDeviceName, false, "", "")
+		network.SetupNetworkInterfaces(networkService, tapDeviceName, "", "", "")
 	})
 
 	t.Run("should not create tap or turn it on if it already exists and is active", func(t *testing.T) {
@@ -51,7 +51,7 @@ func TestSetupNetworkInterfaces(t *testing.T) {
 			IsNIUp(tapDeviceName).
 			Return(true, nil)
 
-		network.SetupNetworkInterfaces(networkService, tapDeviceName, false, "", "")
+		network.SetupNetworkInterfaces(networkService, tapDeviceName, "", "", "")
 	})
 
 	t.Run("should turn the tap up if it exists and is not active", func(t *testing.T) {
@@ -73,13 +73,13 @@ func TestSetupNetworkInterfaces(t *testing.T) {
 			TurnNIUp(tapDeviceName).
 			Return("success", nil)
 
-		network.SetupNetworkInterfaces(networkService, tapDeviceName, false, "", "")
+		network.SetupNetworkInterfaces(networkService, tapDeviceName, "", "", "")
 	})
 
 	t.Run("should create bridge and turn it up if it does not exist and add tap to bridge network", func(t *testing.T) {
 		networkService := NewNetworkService(t)
 		tapDeviceName := "tap0-test"
-		bridgeName := "br0"
+		bridgeName := "br1"
 
 		networkService.
 			EXPECT().
@@ -121,13 +121,13 @@ func TestSetupNetworkInterfaces(t *testing.T) {
 			IsNIUp(tapDeviceName).
 			Return(true, nil)
 
-		network.SetupNetworkInterfaces(networkService, tapDeviceName, true, "", "")
+		network.SetupNetworkInterfaces(networkService, tapDeviceName, bridgeName, "", "")
 	})
 
 	t.Run("should set IP in bridge if it already exists", func(t *testing.T) {
 		networkService := NewNetworkService(t)
 		tapDeviceName := "tap0-test"
-		bridgeName := "br0"
+		bridgeName := "br1"
 
 		networkService.
 			EXPECT().
@@ -169,13 +169,13 @@ func TestSetupNetworkInterfaces(t *testing.T) {
 			IsNIUp(tapDeviceName).
 			Return(true, nil)
 
-		network.SetupNetworkInterfaces(networkService, tapDeviceName, true, "192.168.1.100", "255.255.255.0")
+		network.SetupNetworkInterfaces(networkService, tapDeviceName, bridgeName, "192.168.1.100", "255.255.255.0")
 	})
 
 	t.Run("should not set IP in bridge if it is already set", func(t *testing.T) {
 		networkService := NewNetworkService(t)
 		tapDeviceName := "tap0-test"
-		bridgeName := "br0"
+		bridgeName := "br1"
 
 		networkService.
 			EXPECT().
@@ -207,7 +207,7 @@ func TestSetupNetworkInterfaces(t *testing.T) {
 			IsNIUp(tapDeviceName).
 			Return(true, nil)
 
-		network.SetupNetworkInterfaces(networkService, tapDeviceName, true, "192.168.1.100", "255.255.255.0")
+		network.SetupNetworkInterfaces(networkService, tapDeviceName, bridgeName, "192.168.1.100", "255.255.255.0")
 	})
 
 }