Updates LXD networking to appropriately detect valid network devices

when using the default profile generated by LXD versions 2.33 and above. Old versions of the profile used the "parent" key under the NIC device to designate the network name, and had "nictype" on the device. New versions designate the network under the "network" device key and eschew the nictype. The type however, is available from the network itself.
hatched · Mar 18, 2020 · 211ef99 · 211ef99
1 parent e19b1cb
commit 211ef99
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 16 deletions.
diff --git a/container/lxd/manager_test.go b/container/lxd/manager_test.go
@@ -406,7 +406,7 @@ func (s *managerSuite) TestNetworkDevicesFromConfigNoInputGetsProfileNICs(c *gc.
 	defer s.setup(c).Finish()
 	s.patch()
 
-	s.cSvr.EXPECT().GetProfile("default").Return(defaultProfileWithNIC(), lxdtesting.ETag, nil)
+	s.cSvr.EXPECT().GetProfile("default").Return(defaultLegacyProfileWithNIC(), lxdtesting.ETag, nil)
 
 	s.makeManager(c)
 	result, _, err := lxd.NetworkDevicesFromConfig(s.manager, &container.NetworkConfig{})

diff --git a/container/lxd/network.go b/container/lxd/network.go
@@ -194,11 +194,13 @@ func (s *Server) verifyNICsWithAPI(nics map[string]device) error {
 	for name, nic := range nics {
 		checked = append(checked, name)
 
-		if !isValidNICType(nic) {
-			continue
+		// Versions of the LXD profile prior to 3.22 have the network name as
+		// "parent" under NIC entries in the "devices" list.
+		// Later versions have it under "network".
+		netName, ok := nic["network"]
+		if !ok {
+			netName = nic["parent"]
 		}
-
-		netName := nic["parent"]
 		if netName == "" {
 			continue
 		}
@@ -207,11 +209,20 @@ func (s *Server) verifyNICsWithAPI(nics map[string]device) error {
 		if err != nil {
 			return errors.Annotatef(err, "retrieving network %q", netName)
 		}
+
 		if err := verifyNoIPv6(net); err != nil {
 			ipV6ErrMsg = err
 			continue
 		}
 
+		// Versions of the LXD profile prior to 3.22 have a "nictype" member
+		// under NIC entries in the "devices" list.
+		// Later versions were observed to have this member absent,
+		// however this information is available from the actual network.
+		if !isValidNetworkType(net) && !isValidNICType(nic) {
+			continue
+		}
+
 		logger.Tracef("found usable network device %q with parent %q", name, netName)
 		s.localBridgeName = netName
 		return nil
@@ -319,6 +330,10 @@ func isValidNICType(nic device) bool {
 	return nic["nictype"] == nicTypeBridged || nic["nictype"] == nicTypeMACVLAN
 }
 
+func isValidNetworkType(net *api.Network) bool {
+	return net.Type == nicTypeBridged || net.Type == nicTypeMACVLAN
+}
+
 const BridgeConfigFile = "/etc/default/lxd-bridge"
 const SnapBridgeConfigFile = "/var/snap/lxd/common/lxd-bridge/config"
 

diff --git a/container/lxd/network_test.go b/container/lxd/network_test.go
@@ -27,6 +27,20 @@ func (s *networkSuite) patch() {
 }
 
 func defaultProfileWithNIC() *lxdapi.Profile {
+	return &lxdapi.Profile{
+		Name: "default",
+		ProfilePut: lxdapi.ProfilePut{
+			Devices: map[string]map[string]string{
+				"eth0": {
+					"network": network.DefaultLXDBridge,
+					"type":    "nic",
+				},
+			},
+		},
+	}
+}
+
+func defaultLegacyProfileWithNIC() *lxdapi.Profile {
 	return &lxdapi.Profile{
 		Name: "default",
 		ProfilePut: lxdapi.ProfilePut{
@@ -94,7 +108,7 @@ func (s *networkSuite) TestGetNICsFromProfile(c *gc.C) {
 	defer ctrl.Finish()
 	cSvr := s.NewMockServer(ctrl)
 
-	cSvr.EXPECT().GetProfile("default").Return(defaultProfileWithNIC(), lxdtesting.ETag, nil)
+	cSvr.EXPECT().GetProfile("default").Return(defaultLegacyProfileWithNIC(), lxdtesting.ETag, nil)
 
 	jujuSvr, err := lxd.NewServer(cSvr)
 	c.Assert(err, jc.ErrorIsNil)
@@ -119,7 +133,12 @@ func (s *networkSuite) TestVerifyNetworkDevicePresentValid(c *gc.C) {
 	defer ctrl.Finish()
 	cSvr := s.NewMockServerWithExtensions(ctrl, "network")
 
-	cSvr.EXPECT().GetNetwork(network.DefaultLXDBridge).Return(&lxdapi.Network{}, "", nil)
+	net := &lxdapi.Network{
+		Name:    network.DefaultLXDBridge,
+		Managed: true,
+		Type:    "bridged",
+	}
+	cSvr.EXPECT().GetNetwork(network.DefaultLXDBridge).Return(net, "", nil)
 
 	jujuSvr, err := lxd.NewServer(cSvr)
 	c.Assert(err, jc.ErrorIsNil)
@@ -128,12 +147,26 @@ func (s *networkSuite) TestVerifyNetworkDevicePresentValid(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 }
 
+func (s *networkSuite) TestVerifyNetworkDevicePresentValidLegacy(c *gc.C) {
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+	cSvr := s.NewMockServerWithExtensions(ctrl, "network")
+
+	cSvr.EXPECT().GetNetwork(network.DefaultLXDBridge).Return(&lxdapi.Network{}, "", nil)
+
+	jujuSvr, err := lxd.NewServer(cSvr)
+	c.Assert(err, jc.ErrorIsNil)
+
+	err = jujuSvr.VerifyNetworkDevice(defaultLegacyProfileWithNIC(), "")
+	c.Assert(err, jc.ErrorIsNil)
+}
+
 func (s *networkSuite) TestVerifyNetworkDeviceMultipleNICsOneValid(c *gc.C) {
 	ctrl := gomock.NewController(c)
 	defer ctrl.Finish()
 	cSvr := s.NewMockServerClustered(ctrl, "cluster-1")
 
-	profile := defaultProfileWithNIC()
+	profile := defaultLegacyProfileWithNIC()
 	profile.Devices["eno1"] = profile.Devices["eth0"]
 	profile.Devices["eno1"]["parent"] = "valid-net"
 
@@ -165,9 +198,16 @@ func (s *networkSuite) TestVerifyNetworkDevicePresentBadNicType(c *gc.C) {
 	defer ctrl.Finish()
 	cSvr := s.NewMockServerWithExtensions(ctrl, "network")
 
-	profile := defaultProfileWithNIC()
+	profile := defaultLegacyProfileWithNIC()
 	profile.Devices["eth0"]["nictype"] = "not-bridge-or-macvlan"
 
+	net := &lxdapi.Network{
+		Name:    network.DefaultLXDBridge,
+		Managed: true,
+		Type:    "not-bridge-or-macvlan",
+	}
+	cSvr.EXPECT().GetNetwork(network.DefaultLXDBridge).Return(net, "", nil)
+
 	jujuSvr, err := lxd.NewServer(cSvr)
 	c.Assert(err, jc.ErrorIsNil)
 
@@ -198,7 +238,7 @@ func (s *networkSuite) TestVerifyNetworkDeviceIPv6Present(c *gc.C) {
 	jujuSvr, err := lxd.NewServer(cSvr)
 	c.Assert(err, jc.ErrorIsNil)
 
-	err = jujuSvr.VerifyNetworkDevice(defaultProfileWithNIC(), "")
+	err = jujuSvr.VerifyNetworkDevice(defaultLegacyProfileWithNIC(), "")
 	c.Assert(err, gc.ErrorMatches,
 		`profile "default": juju does not support IPv6. Disable IPv6 in LXD via:\n`+
 			`\tlxc network set lxdbr0 ipv6.address none\n`+
@@ -234,10 +274,10 @@ func (s *networkSuite) TestVerifyNetworkDeviceNotPresentCreated(c *gc.C) {
 		cSvr.EXPECT().GetNetwork(network.DefaultLXDBridge).Return(nil, "", errors.New("not found")),
 		cSvr.EXPECT().CreateNetwork(netCreateReq).Return(nil),
 		cSvr.EXPECT().GetNetwork(network.DefaultLXDBridge).Return(newNet, "", nil),
-		cSvr.EXPECT().UpdateProfile("default", defaultProfileWithNIC().Writable(), lxdtesting.ETag).Return(nil),
+		cSvr.EXPECT().UpdateProfile("default", defaultLegacyProfileWithNIC().Writable(), lxdtesting.ETag).Return(nil),
 	)
 
-	profile := defaultProfileWithNIC()
+	profile := defaultLegacyProfileWithNIC()
 	delete(profile.Devices, "eth0")
 
 	jujuSvr, err := lxd.NewServer(cSvr)
@@ -284,7 +324,7 @@ func (s *networkSuite) TestVerifyNetworkDeviceNotPresentCreatedWithUnusedName(c
 		cSvr.EXPECT().UpdateProfile("default", devReq, lxdtesting.ETag).Return(nil),
 	)
 
-	profile := defaultProfileWithNIC()
+	profile := defaultLegacyProfileWithNIC()
 	profile.Devices["eth0"] = map[string]string{}
 	profile.Devices["eth1"] = map[string]string{}
 
@@ -300,7 +340,7 @@ func (s *networkSuite) TestVerifyNetworkDeviceNotPresentErrorForCluster(c *gc.C)
 	defer ctrl.Finish()
 	cSvr := s.NewMockServerClustered(ctrl, "cluster-1")
 
-	profile := defaultProfileWithNIC()
+	profile := defaultLegacyProfileWithNIC()
 	delete(profile.Devices, "eth0")
 
 	jujuSvr, err := lxd.NewServer(cSvr)
@@ -445,7 +485,7 @@ func (s *networkSuite) TestVerifyNICsWithConfigFileNICFound(c *gc.C) {
 	jujuSvr, err := lxd.NewServer(cSvr)
 	c.Assert(err, jc.ErrorIsNil)
 
-	err = lxd.VerifyNICsWithConfigFile(jujuSvr, defaultProfileWithNIC().Devices, validBridgeConfig)
+	err = lxd.VerifyNICsWithConfigFile(jujuSvr, defaultLegacyProfileWithNIC().Devices, validBridgeConfig)
 	c.Assert(err, jc.ErrorIsNil)
 	c.Check(jujuSvr.LocalBridgeName(), gc.Equals, "lxdbr0")
 }
@@ -458,7 +498,7 @@ func (s *networkSuite) TestVerifyNICsWithConfigFileNICNotFound(c *gc.C) {
 	jujuSvr, err := lxd.NewServer(cSvr)
 	c.Assert(err, jc.ErrorIsNil)
 
-	nics := defaultProfileWithNIC().Devices
+	nics := defaultLegacyProfileWithNIC().Devices
 	nics["eth0"]["parent"] = "br0"
 
 	err = lxd.VerifyNICsWithConfigFile(jujuSvr, nics, validBridgeConfig)