From bec5c873e41e23f6044f4bf8f6fb61ab0d43e625 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Mon, 17 Jul 2023 17:32:31 +0900 Subject: [PATCH] Validate protocols using parameter types Parameters types is better way to enforce specific values because it's implemented at the interface level. This offloads some of the validations we have in manifest logics because we already started using parameter types. Change-Id: I93f1bdae811a999bbbc55861d9e2c7500d347c22 --- manifests/backend/dellemc_powermax.pp | 21 ++++----- manifests/backend/dellemc_powerstore.pp | 22 +++------ manifests/backend/dellemc_sc.pp | 45 +++++++++---------- manifests/backend/dellemc_xtremio.pp | 29 +++++------- manifests/backend/ibm_svf.pp | 29 +++++------- manifests/backend/pure.pp | 37 +++++++-------- .../cinder_backend_dellemc_powermax_spec.rb | 4 +- .../cinder_backend_dellemc_powerstore_spec.rb | 4 +- .../defines/cinder_backend_dellemc_sc_spec.rb | 4 +- .../cinder_backend_dellemc_xtremio_spec.rb | 4 +- spec/defines/cinder_backend_ibm_svf_spec.rb | 4 +- 11 files changed, 80 insertions(+), 123 deletions(-) diff --git a/manifests/backend/dellemc_powermax.pp b/manifests/backend/dellemc_powermax.pp index d1c7bb21..603c45ec 100644 --- a/manifests/backend/dellemc_powermax.pp +++ b/manifests/backend/dellemc_powermax.pp @@ -59,24 +59,19 @@ define cinder::backend::dellemc_powermax ( $powermax_array, $powermax_srp, $powermax_port_groups, - $powermax_storage_protocol = 'iSCSI', - $volume_backend_name = $name, - $backend_availability_zone = $facts['os_service_default'], - Hash $extra_options = {}, - Boolean $manage_volume_type = false, + Enum['iSCSI', 'FC'] $powermax_storage_protocol = 'iSCSI', + $volume_backend_name = $name, + $backend_availability_zone = $facts['os_service_default'], + Hash $extra_options = {}, + Boolean $manage_volume_type = false, ) { include cinder::deps include cinder::params - if $powermax_storage_protocol == 'iSCSI' { - $volume_driver = 'cinder.volume.drivers.dell_emc.powermax.iscsi.PowerMaxISCSIDriver' - } - elsif $powermax_storage_protocol == 'FC' { - $volume_driver = 'cinder.volume.drivers.dell_emc.powermax.fc.PowerMaxFCDriver' - } - else { - fail('The cinder::backend::dellemc_powermax powermax_storage_protocol specified is not valid. It should be iSCSI or FC') + $volume_driver = $powermax_storage_protocol ? { + 'FC' => 'cinder.volume.drivers.dell_emc.powermax.fc.PowerMaxFCDriver', + default => 'cinder.volume.drivers.dell_emc.powermax.iscsi.PowerMaxISCSIDriver', } $_powermax_port_groups = join(any2array($powermax_port_groups), ',') diff --git a/manifests/backend/dellemc_powerstore.pp b/manifests/backend/dellemc_powerstore.pp index 8092a69d..123045e8 100644 --- a/manifests/backend/dellemc_powerstore.pp +++ b/manifests/backend/dellemc_powerstore.pp @@ -46,25 +46,17 @@ define cinder::backend::dellemc_powerstore ( $san_ip, $san_login, $san_password, - $powerstore_ports = $facts['os_service_default'], - $storage_protocol = 'iSCSI', - $volume_backend_name = $name, - $backend_availability_zone = $facts['os_service_default'], - Boolean $manage_volume_type = false, - Hash $extra_options = {}, + $powerstore_ports = $facts['os_service_default'], + Enum['iSCSI', 'FC'] $storage_protocol = 'iSCSI', + $volume_backend_name = $name, + $backend_availability_zone = $facts['os_service_default'], + Boolean $manage_volume_type = false, + Hash $extra_options = {}, ) { include cinder::deps - if $storage_protocol == 'iSCSI' { - $driver = 'dell_emc.powerstore.driver.PowerStoreDriver' - } - elsif $storage_protocol == 'FC' { - $driver = 'dell_emc.powerstore.driver.PowerStoreDriver' - } - else { - fail('The cinder::backend::dellemc_powerstore storage_protocol specified is not valid. It should be iSCSI or FC') - } + $driver = 'dell_emc.powerstore.driver.PowerStoreDriver' cinder_config { "${name}/volume_backend_name": value => $volume_backend_name; diff --git a/manifests/backend/dellemc_sc.pp b/manifests/backend/dellemc_sc.pp index c1fda684..0f6c96e9 100644 --- a/manifests/backend/dellemc_sc.pp +++ b/manifests/backend/dellemc_sc.pp @@ -98,35 +98,30 @@ define cinder::backend::dellemc_sc ( $san_login, $san_password, $dell_sc_ssn, - $target_ip_address = undef, - $volume_backend_name = $name, - $backend_availability_zone = $facts['os_service_default'], - $dell_sc_api_port = $facts['os_service_default'], - $dell_sc_server_folder = 'srv', - $dell_sc_verify_cert = $facts['os_service_default'], - $dell_sc_volume_folder = 'vol', - $target_port = $facts['os_service_default'], - $excluded_domain_ips = $facts['os_service_default'], - $secondary_san_ip = $facts['os_service_default'], - $secondary_san_login = $facts['os_service_default'], - $secondary_san_password = $facts['os_service_default'], - $secondary_sc_api_port = $facts['os_service_default'], - Boolean $manage_volume_type = false, - $use_multipath_for_image_xfer = true, - $sc_storage_protocol = 'iSCSI', - Hash $extra_options = {}, + $target_ip_address = undef, + $volume_backend_name = $name, + $backend_availability_zone = $facts['os_service_default'], + $dell_sc_api_port = $facts['os_service_default'], + $dell_sc_server_folder = 'srv', + $dell_sc_verify_cert = $facts['os_service_default'], + $dell_sc_volume_folder = 'vol', + $target_port = $facts['os_service_default'], + $excluded_domain_ips = $facts['os_service_default'], + $secondary_san_ip = $facts['os_service_default'], + $secondary_san_login = $facts['os_service_default'], + $secondary_san_password = $facts['os_service_default'], + $secondary_sc_api_port = $facts['os_service_default'], + Boolean $manage_volume_type = false, + $use_multipath_for_image_xfer = true, + Enum['iSCSI', 'FC'] $sc_storage_protocol = 'iSCSI', + Hash $extra_options = {}, ) { include cinder::deps - if $sc_storage_protocol == 'iSCSI' { - $volume_driver = 'cinder.volume.drivers.dell_emc.sc.storagecenter_iscsi.SCISCSIDriver' - } - elsif $sc_storage_protocol == 'FC' { - $volume_driver = 'cinder.volume.drivers.dell_emc.sc.storagecenter_fc.SCFCDriver' - } - else { - fail('The cinder::backend::dellemc_sc sc_storage_protocol specified is not valid. It should be iSCSI or FC') + $volume_driver = $sc_storage_protocol ? { + 'FC' => 'cinder.volume.drivers.dell_emc.sc.storagecenter_fc.SCFCDriver', + default => 'cinder.volume.drivers.dell_emc.sc.storagecenter_iscsi.SCISCSIDriver', } cinder_config { diff --git a/manifests/backend/dellemc_xtremio.pp b/manifests/backend/dellemc_xtremio.pp index d14f8a20..f7f23ab2 100644 --- a/manifests/backend/dellemc_xtremio.pp +++ b/manifests/backend/dellemc_xtremio.pp @@ -67,27 +67,22 @@ define cinder::backend::dellemc_xtremio ( $san_login, $san_password, $xtremio_cluster_name, - $volume_backend_name = $name, - $backend_availability_zone = $facts['os_service_default'], - $xtremio_array_busy_retry_count = $facts['os_service_default'], - $xtremio_array_busy_retry_interval = $facts['os_service_default'], - $xtremio_volumes_per_glance_cache = $facts['os_service_default'], - Boolean $manage_volume_type = false, - $xtremio_storage_protocol = 'iSCSI', - $xtremio_ports = $facts['os_service_default'], - Hash $extra_options = {}, + $volume_backend_name = $name, + $backend_availability_zone = $facts['os_service_default'], + $xtremio_array_busy_retry_count = $facts['os_service_default'], + $xtremio_array_busy_retry_interval = $facts['os_service_default'], + $xtremio_volumes_per_glance_cache = $facts['os_service_default'], + Boolean $manage_volume_type = false, + Enum['iSCSI', 'FC'] $xtremio_storage_protocol = 'iSCSI', + $xtremio_ports = $facts['os_service_default'], + Hash $extra_options = {}, ) { include cinder::deps - if $xtremio_storage_protocol == 'iSCSI' { - $driver = 'dell_emc.xtremio.XtremIOISCSIDriver' - } - elsif $xtremio_storage_protocol == 'FC' { - $driver = 'dell_emc.xtremio.XtremIOFCDriver' - } - else { - fail('The cinder::backend::dellemc_xtremio xtremio_storage_protocol specified is not valid. It should be iSCSI or FC') + $driver = $xtremio_storage_protocol ? { + 'FC' => 'dell_emc.xtremio.XtremIOFCDriver', + default => 'dell_emc.xtremio.XtremIOISCSIDriver', } cinder_config { diff --git a/manifests/backend/ibm_svf.pp b/manifests/backend/ibm_svf.pp index f1bf810f..d6e1c05f 100644 --- a/manifests/backend/ibm_svf.pp +++ b/manifests/backend/ibm_svf.pp @@ -70,29 +70,24 @@ define cinder::backend::ibm_svf ( $san_login, $san_password, $storwize_svc_volpool_name, - $storwize_svc_allow_tenant_qos = $facts['os_service_default'], - $storwize_svc_connection_protocol = 'iSCSI', - $storwize_svc_iscsi_chap_enabled = $facts['os_service_default'], - $storwize_svc_retain_aux_volume = $facts['os_service_default'], - $storwize_portset = $facts['os_service_default'], - $volume_backend_name = $name, - $backend_availability_zone = $facts['os_service_default'], - Hash $extra_options = {}, - Boolean $manage_volume_type = false, + $storwize_svc_allow_tenant_qos = $facts['os_service_default'], + Enum['iSCSI', 'FC'] $storwize_svc_connection_protocol = 'iSCSI', + $storwize_svc_iscsi_chap_enabled = $facts['os_service_default'], + $storwize_svc_retain_aux_volume = $facts['os_service_default'], + $storwize_portset = $facts['os_service_default'], + $volume_backend_name = $name, + $backend_availability_zone = $facts['os_service_default'], + Hash $extra_options = {}, + Boolean $manage_volume_type = false, ) { include cinder::deps # NOTE: Svf was earlier called as storwize/svc driver, so the cinder # configuration parameters were named accordingly. - if $storwize_svc_connection_protocol == 'iSCSI' { - $volume_driver = 'cinder.volume.drivers.ibm.storwize_svc.storwize_svc_iscsi.StorwizeSVCISCSIDriver' - } - elsif $storwize_svc_connection_protocol == 'FC' { - $volume_driver = 'cinder.volume.drivers.ibm.storwize_svc.storwize_svc_fc.StorwizeSVCFCDriver' - } - else { - fail('The cinder::backend::ibm_svf storwize_svc_connection_protocol specified is not valid. It should be iSCSI or FC') + $volume_driver = $storwize_svc_connection_protocol ? { + 'FC' => 'cinder.volume.drivers.ibm.storwize_svc.storwize_svc_fc.StorwizeSVCFCDriver', + default => 'cinder.volume.drivers.ibm.storwize_svc.storwize_svc_iscsi.StorwizeSVCISCSIDriver', } cinder_config { diff --git a/manifests/backend/pure.pp b/manifests/backend/pure.pp index 9b892105..10a9a3b8 100644 --- a/manifests/backend/pure.pp +++ b/manifests/backend/pure.pp @@ -95,34 +95,29 @@ define cinder::backend::pure( $san_ip, $pure_api_token, - $volume_backend_name = $name, - $backend_availability_zone = $facts['os_service_default'], - $pure_storage_protocol = 'iSCSI', - $use_chap_auth = false, - $use_multipath_for_image_xfer = true, - Boolean $manage_volume_type = false, - $image_volume_cache_enabled = true, - $pure_host_personality = $facts['os_service_default'], - $pure_eradicate_on_delete = $facts['os_service_default'], - $pure_nvme_transport = $facts['os_service_default'], - $pure_nvme_cidr = $facts['os_service_default'], - $pure_nvme_cidr_list = $facts['os_service_default'], - $pure_iscsi_cidr = $facts['os_service_default'], - $pure_iscsi_cidr_list = $facts['os_service_default'], - Hash $extra_options = {}, + $volume_backend_name = $name, + $backend_availability_zone = $facts['os_service_default'], + Enum['iSCSI', 'FC', 'NVMe'] $pure_storage_protocol = 'iSCSI', + $use_chap_auth = false, + $use_multipath_for_image_xfer = true, + Boolean $manage_volume_type = false, + $image_volume_cache_enabled = true, + $pure_host_personality = $facts['os_service_default'], + $pure_eradicate_on_delete = $facts['os_service_default'], + $pure_nvme_transport = $facts['os_service_default'], + $pure_nvme_cidr = $facts['os_service_default'], + $pure_nvme_cidr_list = $facts['os_service_default'], + $pure_iscsi_cidr = $facts['os_service_default'], + $pure_iscsi_cidr_list = $facts['os_service_default'], + Hash $extra_options = {}, ) { include cinder::deps $volume_driver = $pure_storage_protocol ? { 'FC' => 'cinder.volume.drivers.pure.PureFCDriver', - 'iSCSI' => 'cinder.volume.drivers.pure.PureISCSIDriver', 'NVMe' => 'cinder.volume.drivers.pure.PureNVMEDriver', - default => undef, - } - - if ! $volume_driver { - fail('Invalid pure_storage_protocol. It should be FC, iSCSI or NVMe') + default => 'cinder.volume.drivers.pure.PureISCSIDriver', } cinder_config { diff --git a/spec/defines/cinder_backend_dellemc_powermax_spec.rb b/spec/defines/cinder_backend_dellemc_powermax_spec.rb index b022ab9a..63154b99 100644 --- a/spec/defines/cinder_backend_dellemc_powermax_spec.rb +++ b/spec/defines/cinder_backend_dellemc_powermax_spec.rb @@ -75,9 +75,7 @@ describe 'cinder::backend::dellemc_powermax' do end it 'should raise an error' do - is_expected.to compile.and_raise_error( - /The cinder::backend::dellemc_powermax powermax_storage_protocol specified is not valid. It should be iSCSI or FC/ - ) + is_expected.to compile.and_raise_error(/Evaluation Error/) end end diff --git a/spec/defines/cinder_backend_dellemc_powerstore_spec.rb b/spec/defines/cinder_backend_dellemc_powerstore_spec.rb index a205459f..ed3b729b 100644 --- a/spec/defines/cinder_backend_dellemc_powerstore_spec.rb +++ b/spec/defines/cinder_backend_dellemc_powerstore_spec.rb @@ -66,9 +66,7 @@ describe 'cinder::backend::dellemc_powerstore' do end it 'should raise an error' do - is_expected.to compile.and_raise_error( - /The cinder::backend::dellemc_powerstore storage_protocol specified is not valid. It should be iSCSI or FC/ - ) + is_expected.to compile.and_raise_error(/Evaluation Error/) end end end diff --git a/spec/defines/cinder_backend_dellemc_sc_spec.rb b/spec/defines/cinder_backend_dellemc_sc_spec.rb index 5ce5b75d..32723ccb 100644 --- a/spec/defines/cinder_backend_dellemc_sc_spec.rb +++ b/spec/defines/cinder_backend_dellemc_sc_spec.rb @@ -84,9 +84,7 @@ describe 'cinder::backend::dellemc_sc' do end it 'should raise an error' do - is_expected.to compile.and_raise_error( - /The cinder::backend::dellemc_sc sc_storage_protocol specified is not valid. It should be iSCSI or FC/ - ) + is_expected.to compile.and_raise_error(/Evaluation Error/) end end diff --git a/spec/defines/cinder_backend_dellemc_xtremio_spec.rb b/spec/defines/cinder_backend_dellemc_xtremio_spec.rb index 921e1bf9..745fa2e1 100644 --- a/spec/defines/cinder_backend_dellemc_xtremio_spec.rb +++ b/spec/defines/cinder_backend_dellemc_xtremio_spec.rb @@ -69,9 +69,7 @@ describe 'cinder::backend::dellemc_xtremio' do end it 'should raise an error' do - is_expected.to compile.and_raise_error( - /The cinder::backend::dellemc_xtremio xtremio_storage_protocol specified is not valid. It should be iSCSI or FC/ - ) + is_expected.to compile.and_raise_error(/Evaluation Error/) end end diff --git a/spec/defines/cinder_backend_ibm_svf_spec.rb b/spec/defines/cinder_backend_ibm_svf_spec.rb index 03ed7dd8..7701537b 100644 --- a/spec/defines/cinder_backend_ibm_svf_spec.rb +++ b/spec/defines/cinder_backend_ibm_svf_spec.rb @@ -51,9 +51,7 @@ describe 'cinder::backend::ibm_svf' do end it 'should raise an error' do - is_expected.to compile.and_raise_error( - /The cinder::backend::ibm_svf storwize_svc_connection_protocol specified is not valid. It should be iSCSI or FC/ - ) + is_expected.to compile.and_raise_error(/Evaluation Error/) end end