Merge pull request #6184 from cybozu-go/ceph-raw-mode-on-lv-backed-pvc

ceph: raw mode OSD on LV-backed PVC

Documentation/ceph-common-issues.md

@@ -815,9 +815,11 @@ If you still decide to configure an OSD on LVM, please keep the following in min
 
 - Disable `lvmetad.`
 - Avoid configuration of LVs from the host. In addition, don't touch the VGs and physical volumes that back these LVs.
-- Avoid incrementing the `count` field of `storageClassDeviceSets` and create a new LV that backs a OSD simultaneously.
+- Avoid incrementing the `count` field of `storageClassDeviceSets` and create a new LV that backs an OSD simultaneously.
 
-You can know whether the above-mentioned tag exists tag with the command: `sudo lvs -o lv_name,lv_tags`. If the `lv_tag` field is empty in an LV corresponding to the OSD lv_tags, this OSD encountered the problem. In this case, please [retire this OSD](ceph-osd-mgmt.md#remove-an-osd) or replace with other new OSD before restarting.
+You can know whether the above-mentioned tag exists with the command: `sudo lvs -o lv_name,lv_tags`. If the `lv_tag` field is empty in an LV corresponding to the OSD lv_tags, this OSD encountered the problem. In this case, please [retire this OSD](ceph-osd-mgmt.md#remove-an-osd) or replace with other new OSD before restarting.
+
+This problem doesn't happen in newly created LV-backed PVCs because OSD container doesn't modify LVM metadata anymore. The existing lvm mode OSDs work continuously even thought upgrade your Rook. However, using the raw mode OSDs is recommended because of the above-mentioned problem. You can replace the existing OSDs with raw mode OSDs by retiring them and adding new OSDs one by one. See the documents [Remove an OSD](ceph-osd-mgmt.md#remove-an-osd) and [Add an OSD on a PVC](ceph-osd-mgmt.md#add-an-osd-on-a-pvc).
 
 ## OSD prepare job fails due to low aio-max-nr setting
 

Documentation/ceph-upgrade.md

@@ -532,3 +532,7 @@ k8s.gcr.io/sig-storage/k8scsi/csi-resizer:v1.0.0
 k8s.gcr.io/sig-storage/csi-snapshotter:v3.0.0
 k8s.gcr.io/sig-storage/csi-resizer:v1.0.0
 ```
+
+## Replace lvm mode OSDs with raw mode (if you use LV-backed PVC)
+
+For LV-backed PVC, we recommend replacing lvm mode OSDs with raw mode OSDs. See [common issue](Documentation/ceph-common-issues.md#lvm-metadata-can-be-corrupted-with-osd-on-lv-backed-pvc).

pkg/daemon/ceph/osd/volume.go

@@ -110,6 +110,10 @@ func (a *OsdAgent) configureCVDevices(context *clusterd.Context, devices *Device
 
 			// For LV mode
 			lvPath = getDeviceLVPath(context, fmt.Sprintf("/mnt/%s", a.nodeName))
+			lvBackedPV, err := sys.IsLV(fmt.Sprintf("/mnt/%s", a.nodeName), context.Executor)
+			if err != nil {
+				return nil, errors.Wrap(err, "failed to check device type")
+			}
 
 			// List THE existing OSD configured with ceph-volume lvm mode
 			lvmOsds, err = GetCephVolumeLVMOSDs(context, a.clusterInfo, a.clusterInfo.FSID, lvPath, skipLVRelease, lvBackedPV)
@@ -117,9 +121,13 @@ func (a *OsdAgent) configureCVDevices(context *clusterd.Context, devices *Device
 				logger.Infof("failed to get device already provisioned by ceph-volume lvm. %v", err)
 			}
 			osds = append(osds, lvmOsds...)
+			if len(osds) > 0 {
+				// "ceph-volume raw list" lists the existing OSD even if it is configured with lvm mode, so escape here to avoid dupe.
+				return osds, nil
+			}
 
 			// List THE existing OSD configured with ceph-volume raw mode
-			if a.clusterInfo.CephVersion.IsAtLeast(cephVolumeRawModeMinCephVersion) && !lvBackedPV {
+			if a.clusterInfo.CephVersion.IsAtLeast(cephVolumeRawModeMinCephVersion) {
 				// For block mode
 				block = fmt.Sprintf("/mnt/%s", a.nodeName)
 
@@ -143,7 +151,7 @@ func (a *OsdAgent) configureCVDevices(context *clusterd.Context, devices *Device
 		}
 
 		// List existing OSD(s) configured with ceph-volume lvm mode
-		lvmOsds, err = GetCephVolumeLVMOSDs(context, a.clusterInfo, a.clusterInfo.FSID, lvPath, false, lvBackedPV)
+		lvmOsds, err = GetCephVolumeLVMOSDs(context, a.clusterInfo, a.clusterInfo.FSID, lvPath, false, false)
 		if err != nil {
 			logger.Infof("failed to get devices already provisioned by ceph-volume. %v", err)
 		}
@@ -172,9 +180,7 @@ func (a *OsdAgent) configureCVDevices(context *clusterd.Context, devices *Device
 
 	// Update LVM configuration file
 	// Only do this after Ceph Nautilus 14.2.6 since it will use the ceph-volume raw mode by default and not LVM anymore
-	//
-	// Or keep doing this if the PV is backend by an LV already
-	if !a.clusterInfo.CephVersion.IsAtLeast(cephVolumeRawModeMinCephVersion) || lvBackedPV {
+	if !a.clusterInfo.CephVersion.IsAtLeast(cephVolumeRawModeMinCephVersion) {
 		if err := UpdateLVMConfig(context, a.pvcBacked, lvBackedPV); err != nil {
 			return nil, errors.Wrap(err, "failed to update lvm configuration file")
 		}
@@ -199,7 +205,7 @@ func (a *OsdAgent) configureCVDevices(context *clusterd.Context, devices *Device
 	osds = append(osds, lvmOsds...)
 
 	// List THE configured OSD with ceph-volume raw mode
-	if a.clusterInfo.CephVersion.IsAtLeast(cephVolumeRawModeMinCephVersion) && !lvBackedPV {
+	if a.clusterInfo.CephVersion.IsAtLeast(cephVolumeRawModeMinCephVersion) {
 		// When the block is encrypted we need to list against the encrypted device mapper
 		if !isEncrypted {
 			block = fmt.Sprintf("/mnt/%s", a.nodeName)
@@ -220,8 +226,10 @@ func (a *OsdAgent) initializeBlockPVC(context *clusterd.Context, devices *Device
 	var baseArgs []string
 
 	cephVolumeMode := "lvm"
-	if a.clusterInfo.CephVersion.IsAtLeast(cephVolumeRawModeMinCephVersion) && !lvBackedPV {
+	if a.clusterInfo.CephVersion.IsAtLeast(cephVolumeRawModeMinCephVersion) {
 		cephVolumeMode = "raw"
+	} else if lvBackedPV {
+		return "", "", "", errors.New("OSD on LV-backed PVC requires new Ceph to use raw mode")
 	}
 
 	// Create a specific log directory so that each prepare command will have its own log
@@ -278,20 +286,12 @@ func (a *OsdAgent) initializeBlockPVC(context *clusterd.Context, devices *Device
 			var err error
 			var deviceArg string
 
-			if lvBackedPV {
-				// pass 'vg/lv' to ceph-volume
-				deviceArg, err = sys.GetLVName(context.Executor, device.Config.Name)
-				if err != nil {
-					return "", "", "", errors.Wrapf(err, "failed to get lv name from device path %q", device.Config.Name)
-				}
-			} else {
-				deviceArg = device.Config.Name
-				logger.Info("devlink names:")
-				for _, devlink := range device.PersistentDevicePaths {
-					logger.Info(devlink)
-					if strings.HasPrefix(devlink, "/dev/mapper") {
-						deviceArg = devlink
-					}
+			deviceArg = device.Config.Name
+			logger.Info("devlink names:")
+			for _, devlink := range device.PersistentDevicePaths {
+				logger.Info(devlink)
+				if strings.HasPrefix(devlink, "/dev/mapper") {
+					deviceArg = devlink
 				}
 			}
 
@@ -335,7 +335,7 @@ func (a *OsdAgent) initializeBlockPVC(context *clusterd.Context, devices *Device
 			}
 			logger.Infof("%v", op)
 			// if raw mode is used or PV on LV, let's return the path of the device
-			if lvBackedPV || cephVolumeMode == "raw" && !isEncrypted {
+			if cephVolumeMode == "raw" && !isEncrypted {
 				blockPath = deviceArg
 			} else if cephVolumeMode == "raw" && isEncrypted {
 				blockPath = getEncryptedBlockPath(op, oposd.DmcryptBlockType)

pkg/daemon/ceph/osd/volume_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package osd
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path"
@@ -269,6 +270,271 @@ var cephVolumeRAWTestResult = `{
 }
 `
 
+func createPVCAvailableDevices() *DeviceOsdMapping {
+	devices := &DeviceOsdMapping{
+		Entries: map[string]*DeviceOsdIDEntry{
+			"data": {
+				Data:     -1,
+				Metadata: nil,
+				Config: DesiredDevice{
+					Name:               "/mnt/set1-data-0-rpf2k",
+					OSDsPerDevice:      1,
+					MetadataDevice:     "",
+					DatabaseSizeMB:     0,
+					DeviceClass:        "",
+					IsFilter:           false,
+					IsDevicePathFilter: false,
+				},
+				PersistentDevicePaths: []string{
+					"/dev/rook-vg/rook-lv1",
+					"/dev/mapper/rook--vg-rook--lv1",
+					"/dev/disk/by-id/dm-name-rook--vg-rook--lv1",
+					"/dev/disk/by-id/dm-uuid-LVM-4BOeIsrVP5O2J36cVqMSJNLEcwGIrqSF12RyWdpUaiCuAqOa1hAmD6EUYTO6dyD1",
+				},
+			},
+		},
+	}
+
+	return devices
+}
+
+func TestConfigureCVDevices(t *testing.T) {
+	originalLVMConfPath := lvmConfPath
+	lvmConfPathTemp, err := ioutil.TempFile("", "lvmconf")
+	if err != nil {
+		t.Fatal(err)
+	}
+	lvmConfPath = lvmConfPathTemp.Name()
+	defer func() {
+		os.Remove(lvmConfPath)
+		lvmConfPath = originalLVMConfPath
+	}()
+
+	originalCephConfigDir := cephConfigDir
+	cephConfigDir, err = ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		os.RemoveAll(cephConfigDir)
+		cephConfigDir = originalCephConfigDir
+	}()
+
+	nodeName := "set1-data-0-rpf2k"
+	mountedDev := "/mnt/" + nodeName
+	mapperDev := "/dev/mapper/rook--vg-rook--lv1"
+	clusterFSID := "4bfe8b72-5e69-4330-b6c0-4d914db8ab89"
+	osdUUID := "c03d7353-96e5-4a41-98de-830dfff97d06"
+	lvBlockPath := "/dev/rook-vg/rook-lv1"
+
+	// Test case for creating new raw mode OSD on LV-backed PVC
+	{
+		executor := &exectest.MockExecutor{}
+		executor.MockExecuteCommandWithOutputFile = func(command string, outFileArg string, args ...string) (string, error) {
+			return "{\"key\":\"mysecurekey\"}", nil
+		}
+		executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
+			logger.Infof("[MockExecuteCommandWithOutput] %s %v", command, args)
+			if command == "lsblk" && args[0] == mountedDev {
+				return fmt.Sprintf(`SIZE="17179869184" ROTA="1" RO="0" TYPE="lvm" PKNAME="" NAME="%s" KNAME="/dev/dm-1, a ...interface{})`, mapperDev), nil
+			}
+			if args[1] == "ceph-volume" && args[4] == "lvm" && args[5] == "list" && args[6] == mapperDev {
+				return `{}`, nil
+			}
+			if args[1] == "ceph-volume" && args[4] == "raw" && args[5] == "list" && args[6] == mountedDev {
+				return fmt.Sprintf(`{
+				"0": {
+					"ceph_fsid": "%s",
+					"device": "%s",
+					"osd_id": 0,
+					"osd_uuid": "%s",
+					"type": "bluestore"
+				}
+			}
+			`, clusterFSID, mountedDev, osdUUID), nil
+			}
+			return "", errors.Errorf("unknown command %s %s", command, args)
+		}
+		executor.MockExecuteCommandWithCombinedOutput = func(command string, args ...string) (string, error) {
+			logger.Infof("[MockExecuteCommandWithCombinedOutput] %s %v", command, args)
+			if args[1] == "ceph-volume" && args[2] == "raw" && args[3] == "prepare" && args[4] == "--bluestore" && args[6] == mapperDev {
+				return initializeBlockPVCTestResult, nil
+			}
+			return "", errors.Errorf("unknown command %s %s", command, args)
+		}
+
+		context := &clusterd.Context{Executor: executor, ConfigDir: cephConfigDir}
+		clusterInfo := &cephclient.ClusterInfo{
+			CephVersion: cephver.CephVersion{Major: 14, Minor: 2, Extra: 8},
+			FSID:        clusterFSID,
+		}
+		agent := &OsdAgent{clusterInfo: clusterInfo, nodeName: nodeName, pvcBacked: true}
+		devices := createPVCAvailableDevices()
+
+		deviceOSDs, err := agent.configureCVDevices(context, devices)
+
+		assert.Nil(t, err)
+		assert.Len(t, deviceOSDs, 1)
+		deviceOSD := deviceOSDs[0]
+		logger.Infof("deviceOSDs: %+v", deviceOSDs)
+		assert.Equal(t, osdUUID, deviceOSD.UUID)
+		assert.Equal(t, mountedDev, deviceOSD.BlockPath)
+		assert.Equal(t, true, deviceOSD.SkipLVRelease)
+		assert.Equal(t, true, deviceOSD.LVBackedPV)
+		assert.Equal(t, "raw", deviceOSD.CVMode)
+		assert.Equal(t, "bluestore", deviceOSD.Store)
+	}
+
+	{
+		// Test case for tending to create new lvm mode OSD on LV-backed PVC, but it catches an error
+		executor := &exectest.MockExecutor{}
+		executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
+			logger.Infof("[MockExecuteCommandWithOutput] %s %v", command, args)
+			if command == "lsblk" && args[0] == mountedDev {
+				return fmt.Sprintf(`SIZE="17179869184" ROTA="1" RO="0" TYPE="lvm" PKNAME="" NAME="%s" KNAME="/dev/dm-1, a ...interface{})`, mapperDev), nil
+			}
+			return "", errors.Errorf("unknown command %s %s", command, args)
+		}
+		executor.MockExecuteCommandWithCombinedOutput = func(command string, args ...string) (string, error) {
+			logger.Infof("[MockExecuteCommandWithCombinedOutput] %s %v", command, args)
+			return "", errors.Errorf("unknown command %s %s", command, args)
+		}
+
+		context := &clusterd.Context{Executor: executor, ConfigDir: cephConfigDir}
+		clusterInfo := &cephclient.ClusterInfo{
+			CephVersion: cephver.CephVersion{Major: 14, Minor: 2, Extra: 7}, // It doesn't support raw mode OSD
+			FSID:        clusterFSID,
+		}
+		agent := &OsdAgent{clusterInfo: clusterInfo, nodeName: nodeName, pvcBacked: true}
+		devices := createPVCAvailableDevices()
+
+		_, err := agent.configureCVDevices(context, devices)
+
+		assert.EqualError(t, err, "failed to initialize devices on PVC: OSD on LV-backed PVC requires new Ceph to use raw mode")
+	}
+
+	{
+		// Test case for with no available lvm mode OSD and existing raw mode OSD on LV-backed PVC, it should return info of raw mode OSD
+		executor := &exectest.MockExecutor{}
+		executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
+			logger.Infof("[MockExecuteCommandWithOutput] %s %v", command, args)
+			if command == "lsblk" && args[0] == mountedDev {
+				return fmt.Sprintf(`SIZE="17179869184" ROTA="1" RO="0" TYPE="lvm" PKNAME="" NAME="%s" KNAME="/dev/dm-1, a ...interface{})`, mapperDev), nil
+			}
+			if args[1] == "ceph-volume" && args[4] == "lvm" && args[5] == "list" && args[6] == mapperDev {
+				return `{}`, nil
+			}
+			if args[1] == "ceph-volume" && args[4] == "raw" && args[5] == "list" && args[6] == mountedDev {
+				return fmt.Sprintf(`{
+				"0": {
+					"ceph_fsid": "%s",
+					"device": "%s",
+					"osd_id": 0,
+					"osd_uuid": "%s",
+					"type": "bluestore"
+				}
+			}
+			`, clusterFSID, mountedDev, osdUUID), nil
+			}
+			return "", errors.Errorf("unknown command %s %s", command, args)
+		}
+		executor.MockExecuteCommandWithCombinedOutput = func(command string, args ...string) (string, error) {
+			return "", errors.Errorf("unknown command %s %s", command, args)
+		}
+
+		context := &clusterd.Context{Executor: executor, ConfigDir: cephConfigDir}
+		clusterInfo := &cephclient.ClusterInfo{
+			CephVersion: cephver.CephVersion{Major: 14, Minor: 2, Extra: 8}, // It supports raw mode OSD
+			FSID:        clusterFSID,
+		}
+		agent := &OsdAgent{clusterInfo: clusterInfo, nodeName: nodeName, pvcBacked: true}
+		devices := &DeviceOsdMapping{Entries: map[string]*DeviceOsdIDEntry{}}
+
+		deviceOSDs, err := agent.configureCVDevices(context, devices)
+
+		assert.Nil(t, err)
+		assert.Len(t, deviceOSDs, 1)
+		deviceOSD := deviceOSDs[0]
+		logger.Infof("deviceOSDs: %+v", deviceOSDs)
+		assert.Equal(t, osdUUID, deviceOSD.UUID)
+		assert.Equal(t, mountedDev, deviceOSD.BlockPath)
+		assert.Equal(t, true, deviceOSD.SkipLVRelease)
+		assert.Equal(t, true, deviceOSD.LVBackedPV)
+		assert.Equal(t, "raw", deviceOSD.CVMode)
+		assert.Equal(t, "bluestore", deviceOSD.Store)
+	}
+
+	{
+		// Test case for a lvm mode OSD on LV-backed PVC, it should return info of lvm mode OSD
+		executor := &exectest.MockExecutor{}
+		executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
+			logger.Infof("[MockExecuteCommandWithOutput] %s %v", command, args)
+			if command == "lsblk" && args[0] == mountedDev {
+				return fmt.Sprintf(`SIZE="17179869184" ROTA="1" RO="0" TYPE="lvm" PKNAME="" NAME="%s" KNAME="/dev/dm-1"
+	`, mapperDev), nil
+			}
+			if args[1] == "ceph-volume" && args[4] == "lvm" && args[5] == "list" {
+				return fmt.Sprintf(`{
+					"0": [
+						{
+							"devices": [
+								"/dev/sdb"
+							],
+							"lv_name": "lv1",
+							"lv_path": "%[1]s",
+							"lv_size": "6.00g",
+							"lv_tags": "ceph.block_device=%[1]s,ceph.block_uuid=hO8Hua-3H6B-qEt0-0NNN-ykFF-lsos-rSlmt2,ceph.cephx_lockbox_secret=,ceph.cluster_fsid=%[2]s,ceph.cluster_name=ceph,ceph.crush_device_class=None,ceph.encrypted=0,ceph.osd_fsid=%[3]s,ceph.osd_id=0,ceph.osdspec_affinity=,ceph.type=block,ceph.vdo=0",
+							"lv_uuid": "hO8Hua-3H6B-qEt0-0NNN-ykFF-lsos-rSlmt2",
+							"name": "lv1",
+							"path": "%[1]s",
+							"tags": {
+								"ceph.block_device": "%[1]s",
+								"ceph.block_uuid": "hO8Hua-3H6B-qEt0-0NNN-ykFF-lsos-rSlmt2",
+								"ceph.cephx_lockbox_secret": "",
+								"ceph.cluster_fsid": "%[2]s",
+								"ceph.cluster_name": "ceph",
+								"ceph.crush_device_class": "None",
+								"ceph.encrypted": "0",
+								"ceph.osd_fsid": "%[3]s",
+								"ceph.osd_id": "0",
+								"ceph.osdspec_affinity": "",
+								"ceph.type": "block",
+								"ceph.vdo": "0"
+							},
+							"type": "block",
+							"vg_name": "test-vg"
+						}
+					]
+				}
+				`, lvBlockPath, clusterFSID, osdUUID), nil
+			}
+
+			return "", errors.Errorf("unknown command %s %s", command, args)
+		}
+
+		context := &clusterd.Context{Executor: executor, ConfigDir: cephConfigDir}
+		clusterInfo := &cephclient.ClusterInfo{
+			CephVersion: cephver.CephVersion{Major: 14, Minor: 2, Extra: 8}, // It supports raw mode OSD
+			FSID:        clusterFSID,
+		}
+		agent := &OsdAgent{clusterInfo: clusterInfo, nodeName: nodeName, pvcBacked: true}
+		devices := &DeviceOsdMapping{Entries: map[string]*DeviceOsdIDEntry{}}
+
+		deviceOSDs, err := agent.configureCVDevices(context, devices)
+
+		assert.Nil(t, err)
+		assert.Len(t, deviceOSDs, 1)
+		deviceOSD := deviceOSDs[0]
+		logger.Infof("deviceOSDs: %+v", deviceOSDs)
+		assert.Equal(t, osdUUID, deviceOSD.UUID)
+		assert.Equal(t, lvBlockPath, deviceOSD.BlockPath)
+		assert.Equal(t, true, deviceOSD.SkipLVRelease)
+		assert.Equal(t, true, deviceOSD.LVBackedPV)
+		assert.Equal(t, "lvm", deviceOSD.CVMode)
+		assert.Equal(t, "bluestore", deviceOSD.Store)
+	}
+}
+
 func TestInitializeBlockPVC(t *testing.T) {
 	executor := &exectest.MockExecutor{}
 	executor.MockExecuteCommandWithCombinedOutput = func(command string, args ...string) (string, error) {
@@ -354,6 +620,26 @@ func TestInitializeBlockPVC(t *testing.T) {
 	_, _, _, err = a.initializeBlockPVC(context, devices, false)
 	assert.NotNil(t, err)
 
+	// Test for OSD on LV-backed PVC where Ceph does not support raw mode.
+	// Expect no commands to be used.
+	executor.MockExecuteCommandWithCombinedOutput = func(command string, args ...string) (string, error) {
+		logger.Infof("%s %v", command, args)
+		return "", errors.Errorf("unknown command %s %s", command, args)
+	}
+	// Test with CephVersion{Major: 14, Minor: 2, Extra: 7}
+	clusterInfo = &cephclient.ClusterInfo{
+		CephVersion: cephver.CephVersion{Major: 14, Minor: 2, Extra: 7},
+	}
+	a = &OsdAgent{clusterInfo: clusterInfo, nodeName: "node1"}
+	devices = &DeviceOsdMapping{
+		Entries: map[string]*DeviceOsdIDEntry{
+			"data": {Data: -1, Metadata: nil, Config: DesiredDevice{Name: "/mnt/set1-data-0-rpf2k"}},
+		},
+	}
+	_, _, _, err = a.initializeBlockPVC(context, devices, true)
+	assert.NotNil(t, err)
+	logger.Infof("error message %v", err)
+
 	executor.MockExecuteCommandWithCombinedOutput = func(command string, args ...string) (string, error) {
 		logger.Infof("%s %v", command, args)
 		if args[1] == "ceph-volume" && args[2] == "raw" && args[3] == "prepare" && args[4] == "--bluestore" && args[7] == "--crush-device-class" {