From b5dc00f8da4338d49de7a6c205aeb5e157eaf09a Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Mon, 20 Mar 2023 17:16:55 +0100 Subject: [PATCH] Cinder NVMe-oF fixes The Cinder NVMeoF deployment YAML has a couple of issues addressed in this patch: - Incorrect port is being used. According to the NVM Express specs: "TCP port 4420 has been assigned for use by NVMe over Fabrics". And that's the port default in Cinder as well. - Instead of loading ``nvme-rdma`` we should load ``nvme-fabrics`` so it will automatically load the right module when connecting (``nvme-tcp`` or ``nvme-rdma``). - There is no need to load ``nvme`` module, since that's only used for local nvme volumes, and we care about remote modules. - Controller nodes also connect to storage (for example for backups), so we need to load ``nvme-fabrics`` there as well. - An iptables rule to allow port 4420 is needed, just like the one we have for iscsi (port 3260). - Add a new CinderEnableNVMeOFBackend parameter, which triggers the configuration of the LVM volumes, mirroring the behavior of CinderEnableIscsiBackend. Change-Id: I619914a37450dae3dcccbd28e898ca81009bb2bb --- .../cinder/cinder-backend-nvmeof-puppet.yaml | 18 +++++++++++++++--- .../cinder/cinder-common-container-puppet.yaml | 15 ++++++++++++--- environments/cinder-nvmeof-config.yaml | 8 ++++---- .../cinder-nvmeof-fixes-d3f53887e0dde454.yaml | 8 ++++++++ 4 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/cinder-nvmeof-fixes-d3f53887e0dde454.yaml diff --git a/deployment/cinder/cinder-backend-nvmeof-puppet.yaml b/deployment/cinder/cinder-backend-nvmeof-puppet.yaml index 9c27ec6079..04df97151e 100644 --- a/deployment/cinder/cinder-backend-nvmeof-puppet.yaml +++ b/deployment/cinder/cinder-backend-nvmeof-puppet.yaml @@ -4,6 +4,10 @@ description: > Openstack Cinder NVMeOF backend parameters: + CinderEnableNVMeOFBackend: + default: false + description: Whether to enable or not the NVMeOF backend for Cinder + type: boolean CinderNVMeOFBackendName: type: string default: 'tripleo_nvmeof' @@ -15,13 +19,16 @@ parameters: type: string CinderNVMeOFTargetPort: type: number - default: 4460 + default: 4420 CinderNVMeOFTargetHelper: type: string default: 'nvmet' CinderNVMeOFTargetProtocol: - type: string default: 'nvmet_rdma' + description: > + The target protocol, supported values are nvmet_rdma and nvmet_tcp. + constraints: + - allowed_values: ['nvmet_rdma', 'nvmet_tcp'] CinderNVMeOFTargetPrefix: type: string default: 'nvme-subsystem' @@ -60,9 +67,12 @@ outputs: description: Role data for the Cinder NVMeOF backend. value: service_name: cinder_backend_nvmeof + firewall_rules: + '120 LVM nvmet target': + dport: {get_param: CinderNVMeOFTargetPort} config_settings: map_merge: - - tripleo::profile::base::cinder::volume::cinder_enable_nvmeof_backend: true + - tripleo::profile::base::cinder::volume::cinder_enable_nvmeof_backend: {get_param: CinderEnableNVMeOFBackend} tripleo::profile::base::cinder::volume::nvmeof::volume_backend_name: {get_param: CinderNVMeOFBackendName} tripleo::profile::base::cinder::volume::nvmeof::target_port: {get_param: CinderNVMeOFTargetPort} tripleo::profile::base::cinder::volume::nvmeof::target_helper: {get_param: CinderNVMeOFTargetHelper} @@ -76,6 +86,8 @@ outputs: "%{lookup('$NETWORK')}" params: $NETWORK: {get_param: [ServiceNetMap, CinderIscsiNetwork]} + # This is also needed (in addition to tripleo::profile::base::cinder::volume::nvmeof::volume_backend_name) + cinder::backend::nvmeof::volume_backend_name: {get_param: CinderNVMeOFBackendName} - if: - not: {equals : [{get_param: CinderNVMeOFAvailabilityZone}, '']} - tripleo::profile::base::cinder::volume::nvmeof::backend_availability_zone: {get_param: CinderNVMeOFAvailabilityZone} diff --git a/deployment/cinder/cinder-common-container-puppet.yaml b/deployment/cinder/cinder-common-container-puppet.yaml index da97901b6a..35736af2cd 100644 --- a/deployment/cinder/cinder-common-container-puppet.yaml +++ b/deployment/cinder/cinder-common-container-puppet.yaml @@ -44,6 +44,10 @@ parameters: default: true description: Whether to enable or not the Iscsi backend for Cinder type: boolean + CinderEnableNVMeOFBackend: + default: false + description: Whether to enable or not the NVMeOF backend for Cinder + type: boolean CinderLVMLoopDeviceSize: default: 10280 description: The size of the loopback file used by the cinder LVM driver. @@ -139,6 +143,11 @@ conditions: - {get_param: CinderImageConversionNfsShare} - '' + cinder_lvm_driver_enabled: + or: + - {get_param: CinderEnableIscsiBackend} + - {get_param: CinderEnableNVMeOFBackend} + resources: ContainersCommon: type: ../containers-common.yaml @@ -263,10 +272,10 @@ outputs: fstype: nfs4 src: "{{ image_conversion_nfs_share }}" opts: "{{ image_conversion_nfs_options }}" - - name: cinder_enable_iscsi_backend fact + - name: cinder_configure_lvm fact set_fact: - cinder_enable_iscsi_backend: {get_param: CinderEnableIscsiBackend} - - when: cinder_enable_iscsi_backend|bool + cinder_configure_lvm: {if: [cinder_lvm_driver_enabled, true, false]} + - when: cinder_configure_lvm|bool block: - name: ensure LVM rpm dependencies are installed package: diff --git a/environments/cinder-nvmeof-config.yaml b/environments/cinder-nvmeof-config.yaml index a14e2b6fd3..71cd6c8582 100644 --- a/environments/cinder-nvmeof-config.yaml +++ b/environments/cinder-nvmeof-config.yaml @@ -4,8 +4,9 @@ resource_registry: OS::TripleO::Services::CinderBackendNVMeOF: ../deployment/cinder/cinder-backend-nvmeof-puppet.yaml parameter_defaults: + CinderEnableNVMeOFBackend: true CinderNVMeOFBackendName: 'tripleo_nvmeof' - CinderNVMeOFTargetPort: 4460 + CinderNVMeOFTargetPort: 4420 CinderNVMeOFTargetHelper: 'nvmet' CinderNVMeOFTargetProtocol: 'nvmet_rdma' CinderNVMeOFTargetPrefix: 'nvme-subsystem' @@ -15,8 +16,7 @@ parameter_defaults: ControllerParameters: ExtraKernelModules: nvmet: {} - nvmet-rdma: {} + nvme-fabrics: {} ComputeParameters: ExtraKernelModules: - nvme: {} - nvme-rdma: {} + nvme-fabrics: {} diff --git a/releasenotes/notes/cinder-nvmeof-fixes-d3f53887e0dde454.yaml b/releasenotes/notes/cinder-nvmeof-fixes-d3f53887e0dde454.yaml new file mode 100644 index 0000000000..cd898d2c81 --- /dev/null +++ b/releasenotes/notes/cinder-nvmeof-fixes-d3f53887e0dde454.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Cinder NVMe-oF: Use the right port ``4420`` instead of ``4460`` and add the + appropriate iptables rule for LVM+nvmet to work. + - | + Cinder NVMe-oF: Cinder nodes where not loading ``nvme-fabrics`` kernel + module, so nvme-of would not work correctly on controller nodes.