Use separate matching functions for block and filesystem volume devices

jangocheng · Jul 29, 2021 · be5c89d · be5c89d
1 parent 8298edd
commit be5c89d
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 17 deletions.
diff --git a/apiserver/common/storagecommon/blockdevices.go b/apiserver/common/storagecommon/blockdevices.go
@@ -33,13 +33,35 @@ func BlockDeviceFromState(in state.BlockDeviceInfo) storage.BlockDevice {
 	}
 }
 
-// MatchingBlockDevice finds the block device that matches the
+// MatchingVolumeBlockDevice finds the block device that matches the
 // provided volume info and volume attachment info.
-func MatchingBlockDevice(
+func MatchingVolumeBlockDevice(
 	blockDevices []state.BlockDeviceInfo,
 	volumeInfo state.VolumeInfo,
 	attachmentInfo state.VolumeAttachmentInfo,
 	planBlockInfo state.BlockDeviceInfo,
+) (*state.BlockDeviceInfo, bool) {
+	return matchingBlockDevice(blockDevices, volumeInfo, attachmentInfo, planBlockInfo, false)
+}
+
+// MatchingFilesystemBlockDevice finds the block device that matches the
+// provided volume info and volume attachment info, preferring a matching
+// device of type partition.
+func MatchingFilesystemBlockDevice(
+	blockDevices []state.BlockDeviceInfo,
+	volumeInfo state.VolumeInfo,
+	attachmentInfo state.VolumeAttachmentInfo,
+	planBlockInfo state.BlockDeviceInfo,
+) (*state.BlockDeviceInfo, bool) {
+	return matchingBlockDevice(blockDevices, volumeInfo, attachmentInfo, planBlockInfo, true)
+}
+
+func matchingBlockDevice(
+	blockDevices []state.BlockDeviceInfo,
+	volumeInfo state.VolumeInfo,
+	attachmentInfo state.VolumeAttachmentInfo,
+	planBlockInfo state.BlockDeviceInfo,
+	allowPartitions bool,
 ) (*state.BlockDeviceInfo, bool) {
 	logger.Tracef("looking for block device to match one of planBlockInfo %#v volumeInfo %#v attachmentInfo %#v",
 		planBlockInfo, volumeInfo, attachmentInfo)
@@ -121,9 +143,9 @@ func MatchingBlockDevice(
 	devMatch:
 		for _, dev := range blockDevices {
 			for _, link := range dev.DeviceLinks {
-				if attachmentInfo.DeviceLink == link || attachmentInfo.DeviceLink+"-part1" == link {
+				if attachmentInfo.DeviceLink == link || (allowPartitions && attachmentInfo.DeviceLink+"-part1" == link) {
 					devCopy := dev
-					if dev.UUID != "" {
+					if dev.UUID != "" && allowPartitions {
 						devWithUUID = &devCopy
 					} else {
 						parentDev = &devCopy

diff --git a/apiserver/common/storagecommon/blockdevices_test.go b/apiserver/common/storagecommon/blockdevices_test.go
@@ -32,7 +32,7 @@ func (s *BlockDeviceSuite) TestBlockDeviceMatchingSerialID(c *gc.C) {
 	}
 	atachmentInfo := state.VolumeAttachmentInfo{}
 	planBlockInfo := state.BlockDeviceInfo{}
-	blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(blockDevices, volumeInfo, atachmentInfo, planBlockInfo)
+	blockDeviceInfo, ok := storagecommon.MatchingVolumeBlockDevice(blockDevices, volumeInfo, atachmentInfo, planBlockInfo)
 	c.Assert(ok, jc.IsTrue)
 	c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
 		DeviceName: "sdb",
@@ -55,7 +55,7 @@ func (s *BlockDeviceSuite) TestBlockDeviceMatchingHardwareID(c *gc.C) {
 	}
 	atachmentInfo := state.VolumeAttachmentInfo{}
 	planBlockInfo := state.BlockDeviceInfo{}
-	blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(blockDevices, volumeInfo, atachmentInfo, planBlockInfo)
+	blockDeviceInfo, ok := storagecommon.MatchingVolumeBlockDevice(blockDevices, volumeInfo, atachmentInfo, planBlockInfo)
 	c.Assert(ok, jc.IsTrue)
 	c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
 		DeviceName: "sdb",
@@ -64,7 +64,7 @@ func (s *BlockDeviceSuite) TestBlockDeviceMatchingHardwareID(c *gc.C) {
 }
 
 func (s *BlockDeviceSuite) TestBlockDevicesAWS(c *gc.C) {
-	blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(awsTestBlockDevices, awsTestVolumeInfo, awsTestAttachmentInfo, awsTestPlanBlockInfo)
+	blockDeviceInfo, ok := storagecommon.MatchingVolumeBlockDevice(awsTestBlockDevices, awsTestVolumeInfo, awsTestAttachmentInfo, awsTestPlanBlockInfo)
 	c.Assert(ok, jc.IsTrue)
 	c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
 		DeviceName: "nvme0n1",
@@ -87,7 +87,7 @@ var (
 )
 
 func (s *BlockDeviceSuite) TestBlockDevicesGCE(c *gc.C) {
-	blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(gceTestBlockDevices, gceTestVolumeInfo, gceTestAttachmentInfo, gceTestPlanBlockInfo)
+	blockDeviceInfo, ok := storagecommon.MatchingVolumeBlockDevice(gceTestBlockDevices, gceTestVolumeInfo, gceTestAttachmentInfo, gceTestPlanBlockInfo)
 	c.Assert(ok, jc.IsTrue)
 	c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
 		DeviceName: "sdd",
@@ -104,7 +104,7 @@ func (s *BlockDeviceSuite) TestBlockDevicesGCE(c *gc.C) {
 }
 
 func (s *BlockDeviceSuite) TestBlockDevicesGCEPreferUUID(c *gc.C) {
-	blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(gceTestBlockDevices, gceTestVolumeInfo, gceTestAttachmentInfoForUUID, gceTestPlanBlockInfo)
+	blockDeviceInfo, ok := storagecommon.MatchingFilesystemBlockDevice(gceTestBlockDevices, gceTestVolumeInfo, gceTestAttachmentInfoForUUID, gceTestPlanBlockInfo)
 	c.Assert(ok, jc.IsTrue)
 	c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
 		DeviceName: "sda1",
@@ -137,7 +137,7 @@ var (
 )
 
 func (s *BlockDeviceSuite) TestBlockDevicesOpenStack(c *gc.C) {
-	blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(osTestBlockDevices, osTestVolumeInfo, osTestAttachmentInfo, osTestPlanBlockInfo)
+	blockDeviceInfo, ok := storagecommon.MatchingVolumeBlockDevice(osTestBlockDevices, osTestVolumeInfo, osTestAttachmentInfo, osTestPlanBlockInfo)
 	c.Assert(ok, jc.IsTrue)
 	c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
 		DeviceName: "vdd",
@@ -159,7 +159,7 @@ var (
 )
 
 func (s *BlockDeviceSuite) TestBlockDevicesOCI(c *gc.C) {
-	blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(ociTestBlockDevices, ociTestVolumeInfo, ociTestAttachmentInfo, ociTestPlanBlockInfo)
+	blockDeviceInfo, ok := storagecommon.MatchingVolumeBlockDevice(ociTestBlockDevices, ociTestVolumeInfo, ociTestAttachmentInfo, ociTestPlanBlockInfo)
 	c.Assert(ok, jc.IsTrue)
 	c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
 		DeviceName: "loop2",
@@ -175,7 +175,7 @@ var (
 )
 
 func (s *BlockDeviceSuite) TestBlockDevicesVSphere(c *gc.C) {
-	blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(vsphereTestBlockDevices, vsphereTestVolumeInfo, vsphereTestAttachmentInfo, vsphereTestPlanBlockInfo)
+	blockDeviceInfo, ok := storagecommon.MatchingVolumeBlockDevice(vsphereTestBlockDevices, vsphereTestVolumeInfo, vsphereTestAttachmentInfo, vsphereTestPlanBlockInfo)
 	c.Assert(ok, jc.IsTrue)
 	c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
 		DeviceName: "loop0",

diff --git a/apiserver/common/storagecommon/storage.go b/apiserver/common/storagecommon/storage.go
@@ -147,7 +147,7 @@ func volumeStorageAttachmentInfo(
 	if err != nil {
 		return nil, errors.Annotate(err, "getting block devices")
 	}
-	blockDevice, ok := MatchingBlockDevice(
+	blockDevice, ok := MatchingVolumeBlockDevice(
 		blockDevices,
 		volumeInfo,
 		volumeAttachmentInfo,

diff --git a/apiserver/facades/agent/storageprovisioner/storageprovisioner.go b/apiserver/facades/agent/storageprovisioner/storageprovisioner.go
@@ -1261,7 +1261,7 @@ func (s *StorageProvisionerAPIv3) oneVolumeBlockDevice(
 	if err != nil {
 		return state.BlockDeviceInfo{}, err
 	}
-	blockDevice, ok := storagecommon.MatchingBlockDevice(
+	blockDevice, ok := storagecommon.MatchingFilesystemBlockDevice(
 		blockDevices,
 		volumeInfo,
 		volumeAttachmentInfo,

diff --git a/storage/path.go b/storage/path.go
@@ -27,13 +27,13 @@ func BlockDevicePath(device BlockDevice) (string, error) {
 	if device.HardwareId != "" {
 		return path.Join(diskByID, device.HardwareId), nil
 	}
-	if device.UUID != "" {
-		return path.Join(diskByUUID, device.UUID), nil
-	}
 	if len(device.DeviceLinks) > 0 {
 		// return the first device link in the list
 		return device.DeviceLinks[0], nil
 	}
+	if device.UUID != "" {
+		return path.Join(diskByUUID, device.UUID), nil
+	}
 	if device.DeviceName != "" {
 		return path.Join(diskByDeviceName, device.DeviceName), nil
 	}