diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index ba721e536..1c07b23ef 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -16,6 +16,7 @@ - OSD on PVC now supports multipath device. - OSDs can now be provisioned using Ceph's Drive Groups definitions for Ceph Octopus v15.2.5+. [See docs for more](Documentation/ceph-cluster-crd.md#storage-selection-via-ceph-drive-groups) + - OSDs can be provisioned on support /dev/disk/by-path/pci-HHHH:HH:HH.H devices with colons (`:`) - Added [admission controller](Documentation/admission-controller-usage.md) support for CRD validations. - Support for Ceph CRDs is provided. Some validations for CephClusters are included and additional validations can be added for other CRDs - Can be extended to add support for other providers diff --git a/cmd/rook/ceph/osd.go b/cmd/rook/ceph/osd.go index b46970fda..fb25e7353 100644 --- a/cmd/rook/ceph/osd.go +++ b/cmd/rook/ceph/osd.go @@ -19,8 +19,6 @@ package ceph import ( "encoding/json" "os" - "strconv" - "strings" "k8s.io/client-go/kubernetes" @@ -264,41 +262,28 @@ func updateLocationWithNodeLabels(location *[]string, nodeLabels map[string]stri oposd.UpdateLocationWithNodeLabels(location, nodeLabels) } -// Parse the devices, which are comma separated. A colon indicates a non-default number of osds per device -// or a non collocated metadata device. -// For example, one osd will be created on each of sda and sdb, with 5 osds on the nvme01 device. -// sda:1:::,sdb:1:::,nvme01:5::: -// For example, 3 osds will use sdb SSD for db and 3 osds will use sdc SSD for db. -// sdd:1:::sdb,sde:1:::sdb,sdf:1:::sdb,sdg:1:::sdc,sdh:1:::sdc,sdi:1:::sdc +// Parse the devices, which are sent as a JSON-marshalled list of device IDs with a StorageConfig spec func parseDevices(devices string) ([]osddaemon.DesiredDevice, error) { + configuredDevices := []osdcfg.ConfiguredDevice{} + err := json.Unmarshal([]byte(devices), &configuredDevices) + if err != nil { + return nil, errors.Wrapf(err, "failed to JSON unmarshal configured devices (%q)", devices) + } + var result []osddaemon.DesiredDevice - parsed := strings.Split(devices, ",") - for _, device := range parsed { - parts := strings.Split(device, ":") - d := osddaemon.DesiredDevice{Name: parts[0], OSDsPerDevice: 1} - if len(parts) > 1 { - count, err := strconv.Atoi(parts[1]) - if err != nil { - return nil, errors.Wrapf(err, "error parsing count from devices (%q)", devices) - } - if count < 1 { - return nil, errors.Errorf("osds per device should be greater than 0 (%q)", parts[1]) - } - d.OSDsPerDevice = count + for _, cd := range configuredDevices { + d := osddaemon.DesiredDevice{ + Name: cd.ID, } - if len(parts) > 2 && parts[2] != "" { - size, err := strconv.Atoi(parts[2]) - if err != nil { - return nil, errors.Wrapf(err, "error DatabaseSizeMB (%q) to int", parts[2]) - } - d.DatabaseSizeMB = size - } - if len(parts) > 3 && parts[3] != "" { - d.DeviceClass = parts[3] - } - if len(parts) > 4 { - d.MetadataDevice = parts[4] + d.OSDsPerDevice = cd.StoreConfig.OSDsPerDevice + d.DatabaseSizeMB = cd.StoreConfig.DatabaseSizeMB + d.DeviceClass = cd.StoreConfig.DeviceClass + d.MetadataDevice = cd.StoreConfig.MetadataDevice + + if d.OSDsPerDevice < 1 { + return nil, errors.Errorf("osds per device should be greater than 0 (%q)", d.OSDsPerDevice) } + result = append(result, d) } diff --git a/cmd/rook/ceph/osd_test.go b/cmd/rook/ceph/osd_test.go index ca6f13bad..d280d2c98 100644 --- a/cmd/rook/ceph/osd_test.go +++ b/cmd/rook/ceph/osd_test.go @@ -17,15 +17,40 @@ limitations under the License. package ceph import ( + "encoding/json" "testing" + osdcfg "github.com/rook/rook/pkg/operator/ceph/cluster/osd/config" "github.com/stretchr/testify/assert" ) func TestParseDesiredDevices(t *testing.T) { - devices := "sda,sdb,nvme01:5" + configuredDevices := []osdcfg.ConfiguredDevice{ + { + ID: "sda", + StoreConfig: osdcfg.StoreConfig{ + OSDsPerDevice: 1, + }, + }, + { + ID: "sdb", + StoreConfig: osdcfg.StoreConfig{ + OSDsPerDevice: 1, + }, + }, + { + ID: "nvme01", + StoreConfig: osdcfg.StoreConfig{ + OSDsPerDevice: 5, + }, + }, + } + marshalledDevices, err := json.Marshal(configuredDevices) + assert.NoError(t, err) + devices := string(marshalledDevices) + result, err := parseDevices(devices) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 3, len(result)) assert.Equal(t, "sda", result[0].Name) assert.Equal(t, "sdb", result[1].Name) @@ -41,19 +66,76 @@ func TestParseDesiredDevices(t *testing.T) { assert.False(t, result[2].IsDevicePathFilter) // negative osd count is not allowed - devices = "nvme01:-5" + configuredDevices = []osdcfg.ConfiguredDevice{ + { + ID: "nvme01", + StoreConfig: osdcfg.StoreConfig{ + OSDsPerDevice: -5, + }, + }, + } + marshalledDevices, err = json.Marshal(configuredDevices) + assert.NoError(t, err) + devices = string(marshalledDevices) + result, err = parseDevices(devices) assert.Nil(t, result) - assert.NotNil(t, err) + assert.Error(t, err) // 0 osd count is not allowed - devices = "nvme01:0" + configuredDevices = []osdcfg.ConfiguredDevice{ + { + ID: "nvme01", + StoreConfig: osdcfg.StoreConfig{ + OSDsPerDevice: 0, + }, + }, + } + marshalledDevices, err = json.Marshal(configuredDevices) + assert.NoError(t, err) + devices = string(marshalledDevices) + result, err = parseDevices(devices) assert.Nil(t, result) - assert.NotNil(t, err) + assert.Error(t, err) // OSDsPerDevice, metadataDevice, databaseSizeMB and deviceClass - devices = "sdd:1:2048::sdb,sde:1:::sdb,sdf:1:::sdc,sdg:1::tst:sdc" + configuredDevices = []osdcfg.ConfiguredDevice{ + { + ID: "sdd", + StoreConfig: osdcfg.StoreConfig{ + OSDsPerDevice: 1, + DatabaseSizeMB: 2048, + MetadataDevice: "sdb", + }, + }, + { + ID: "sde", + StoreConfig: osdcfg.StoreConfig{ + OSDsPerDevice: 1, + MetadataDevice: "sdb", + }, + }, + { + ID: "sdf", + StoreConfig: osdcfg.StoreConfig{ + OSDsPerDevice: 1, + MetadataDevice: "sdc", + }, + }, + { + ID: "sdg", + StoreConfig: osdcfg.StoreConfig{ + OSDsPerDevice: 1, + DeviceClass: "tst", + MetadataDevice: "sdc", + }, + }, + } + marshalledDevices, err = json.Marshal(configuredDevices) + assert.NoError(t, err) + devices = string(marshalledDevices) + result, err = parseDevices(devices) assert.Equal(t, "sdd", result[0].Name) assert.Equal(t, "sde", result[1].Name) diff --git a/pkg/operator/ceph/cluster/osd/config/config.go b/pkg/operator/ceph/cluster/osd/config/config.go index a3cdcf297..4bcadb333 100644 --- a/pkg/operator/ceph/cluster/osd/config/config.go +++ b/pkg/operator/ceph/cluster/osd/config/config.go @@ -47,17 +47,27 @@ const ( DeviceClassKey = "deviceClass" ) +// StoreConfig represents the configuration of an OSD on a device. type StoreConfig struct { StoreType string `json:"storeType,omitempty"` WalSizeMB int `json:"walSizeMB,omitempty"` DatabaseSizeMB int `json:"databaseSizeMB,omitempty"` OSDsPerDevice int `json:"osdsPerDevice,omitempty"` EncryptedDevice bool `json:"encryptedDevice,omitempty"` + MetadataDevice string `json:"metadataDevice,omitempty"` DeviceClass string `json:"deviceClass,omitempty"` } +// NewStoreConfig returns a StoreConfig with proper defaults set. +func NewStoreConfig() StoreConfig { + return StoreConfig{ + OSDsPerDevice: 1, + } +} + +// ToStoreConfig converts a config string-string map to a StoreConfig. func ToStoreConfig(config map[string]string) StoreConfig { - storeConfig := StoreConfig{} + storeConfig := NewStoreConfig() for k, v := range config { switch k { case StoreTypeKey: @@ -67,9 +77,14 @@ func ToStoreConfig(config map[string]string) StoreConfig { case DatabaseSizeMBKey: storeConfig.DatabaseSizeMB = convertToIntIgnoreErr(v) case OSDsPerDeviceKey: - storeConfig.OSDsPerDevice = convertToIntIgnoreErr(v) + i := convertToIntIgnoreErr(v) + if i > 0 { // only allow values 1 or more to be set + storeConfig.OSDsPerDevice = i + } case EncryptedDeviceKey: storeConfig.EncryptedDevice = (v == "true") + case MetadataDeviceKey: + storeConfig.MetadataDevice = v case DeviceClassKey: storeConfig.DeviceClass = v } @@ -104,3 +119,9 @@ type DriveGroupBlob string // DriveGroupBlobs is a mapping from Ceph Drive Group names to JSON blobs of Drive Group specs. type DriveGroupBlobs map[string]string + +// ConfiguredDevice is a device with a corresponding configuration. +type ConfiguredDevice struct { + ID string `json:"id"` + StoreConfig StoreConfig `json:"storeConfig"` +} diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index a4c952673..3e61cc51e 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -18,6 +18,7 @@ limitations under the License. package osd import ( + "encoding/json" "fmt" "path" "strconv" @@ -845,40 +846,23 @@ func (c *Cluster) provisionOSDContainer(osdProps osdProperties, copyBinariesMoun // only 1 of device list, device filter, device path filter and use all devices can be specified. We prioritize in that order. if len(osdProps.devices) > 0 { - deviceNames := make([]string, len(osdProps.devices)) - for i, device := range osdProps.devices { - devSuffix := "" - if count, ok := device.Config[config.OSDsPerDeviceKey]; ok { - logger.Infof("%s osds requested on device %s (node %s)", count, device.Name, osdProps.crushHostname) - devSuffix += ":" + count - } else { - devSuffix += ":1" - } - if databaseSizeMB, ok := device.Config[config.DatabaseSizeMBKey]; ok { - logger.Infof("osd %s requested with DB size %sMB (node %s)", device.Name, databaseSizeMB, osdProps.crushHostname) - devSuffix += ":" + databaseSizeMB - } else { - devSuffix += ":" - } - if deviceClass, ok := device.Config[config.DeviceClassKey]; ok { - logger.Infof("osd %s requested with deviceClass %s (node %s)", device.Name, deviceClass, osdProps.crushHostname) - devSuffix += ":" + deviceClass - } else { - devSuffix += ":" - } - if md, ok := device.Config[config.MetadataDeviceKey]; ok { - logger.Infof("osd %s requested with metadataDevice %s (node %s)", device.Name, md, osdProps.crushHostname) - devSuffix += ":" + md - } else { - devSuffix += ":" - } - deviceID := device.Name + configuredDevices := []config.ConfiguredDevice{} + for _, device := range osdProps.devices { + id := device.Name if device.FullPath != "" { - deviceID = device.FullPath + id = device.FullPath } - deviceNames[i] = deviceID + devSuffix + cd := config.ConfiguredDevice{ + ID: id, + StoreConfig: config.ToStoreConfig(device.Config), + } + configuredDevices = append(configuredDevices, cd) } - envVars = append(envVars, dataDevicesEnvVar(strings.Join(deviceNames, ","))) + marshalledDevices, err := json.Marshal(configuredDevices) + if err != nil { + return v1.Container{}, errors.Wrapf(err, "failed to JSON marshal configured devices for node %q", osdProps.crushHostname) + } + envVars = append(envVars, dataDevicesEnvVar(string(marshalledDevices))) } else if osdProps.selection.DeviceFilter != "" { envVars = append(envVars, deviceFilterEnvVar(osdProps.selection.DeviceFilter)) } else if osdProps.selection.DevicePathFilter != "" { @@ -902,12 +886,25 @@ func (c *Cluster) provisionOSDContainer(osdProps osdProperties, copyBinariesMoun if osdProps.onPVC() { volumeMounts = append(volumeMounts, getPvcOSDBridgeMount(osdProps.pvc.ClaimName)) // The device list is read by the Rook CLI via environment variables so let's add them - dev := []string{fmt.Sprintf("/mnt/%s", osdProps.pvc.ClaimName)} + configuredDevices := []config.ConfiguredDevice{ + { + ID: fmt.Sprintf("/mnt/%s", osdProps.pvc.ClaimName), + StoreConfig: config.NewStoreConfig(), + }, + } if osdProps.onPVCWithMetadata() { volumeMounts = append(volumeMounts, getPvcMetadataOSDBridgeMount(osdProps.metadataPVC.ClaimName)) - dev = append(dev, fmt.Sprintf("/srv/%s", osdProps.metadataPVC.ClaimName)) + configuredDevices = append(configuredDevices, + config.ConfiguredDevice{ + ID: fmt.Sprintf("/srv/%s", osdProps.metadataPVC.ClaimName), + StoreConfig: config.NewStoreConfig(), + }) } - envVars = append(envVars, dataDevicesEnvVar(strings.Join(dev, ","))) + marshalledDevices, err := json.Marshal(configuredDevices) + if err != nil { + return v1.Container{}, errors.Wrapf(err, "failed to JSON marshal configured devices for PVC %q", osdProps.crushHostname) + } + envVars = append(envVars, dataDevicesEnvVar(string(marshalledDevices))) envVars = append(envVars, pvcBackedOSDEnvVar("true")) envVars = append(envVars, crushDeviceClassEnvVar(osdProps.crushDeviceClass)) }