Re: [RFC PATCH] terraform/OCI: Create a set of multiple generic block devices

[Chandan Babu R <chandanbabu@kernel.org>]

On Wed, Mar 05, 2025 at 04:12:07 PM -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
>
> When provisioning on OCI, terraform creates one block device for
> the /data file system, and one for sparse files. This is unlike
> other provisioning methods (guestfs and AWS being the primary
> examples) which instead create a set of generic block devices and
> then set up the sparse files or /data file system on one of those.
>
> Luis and Chandan have agreed to changing OCI to work like the
> other terraform providers and guestfs.
>

Apart from the nit mentioned below, the remaining changes look good to me,

Reviewed-by: Chandan Babu R <chandanbabu@kernel.org>

> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  .../templates/oci/terraform.tfvars.j2         |   6 +
>  terraform/oci/Kconfig                         | 160 ++++++++++++++++++
>  terraform/oci/main.tf                         |  23 ++-
>  terraform/oci/vars.tf                         |  17 ++
>  4 files changed, 202 insertions(+), 4 deletions(-)
>
> This seems to do what we want. For the moment, this facility is
> gated behind a Kconfig boolean so you can revert back to the old
> data/sparse configuratino quickly. Testing will confirm if we need
> to keep this revert setting.
>
>
> diff --git a/playbooks/roles/gen_tfvars/templates/oci/terraform.tfvars.j2 b/playbooks/roles/gen_tfvars/templates/oci/terraform.tfvars.j2
> index 6429c7289f52..2d45fd77d510 100644
> --- a/playbooks/roles/gen_tfvars/templates/oci/terraform.tfvars.j2
> +++ b/playbooks/roles/gen_tfvars/templates/oci/terraform.tfvars.j2
> @@ -12,10 +12,16 @@ oci_os_image_ocid = "{{ terraform_oci_os_image_ocid }}"
>  oci_assign_public_ip = "{{ terraform_oci_assign_public_ip | lower }}"
>  oci_instance_display_name = "{{ terraform_oci_instance_display_name }}"
>  oci_subnet_ocid = "{{ terraform_oci_subnet_ocid }}"
> +oci_volumes_enable_extra = "{{ terraform_oci_volumes_enable_extra | lower }}"
> +{% if terraform_oci_volumes_enable_extra %}
> +oci_volumes_per_instance = {{ terraform_oci_volumes_per_instance }}
> +oci_volumes_size = {{ terraform_oci_volumes_size }}
> +{% else %}
>  oci_data_volume_display_name = "{{ terraform_oci_data_volume_display_name }}"
>  oci_data_volume_device_file_name = "{{ terraform_oci_data_volume_device_file_name }}"
>  oci_sparse_volume_display_name = "{{ terraform_oci_sparse_volume_display_name }}"
>  oci_sparse_volume_device_file_name = "{{ terraform_oci_sparse_volume_device_file_name }}"
> +{% endif %}
>  
>  ssh_config_pubkey_file = "{{ kdevops_terraform_ssh_config_pubkey_file }}"
>  ssh_config_user = "{{ kdevops_terraform_ssh_config_user }}"
> diff --git a/terraform/oci/Kconfig b/terraform/oci/Kconfig
> index 4b37ad91d4b9..08eb239fcc5c 100644
> --- a/terraform/oci/Kconfig
> +++ b/terraform/oci/Kconfig
> @@ -90,6 +90,160 @@ config TERRAFORM_OCI_SUBNET_OCID
>  	  Read this:
>  	  https://docs.oracle.com/en-us/iaas/Content/API/SDKDocs/terraformproviderconfiguration.htm
>  
> +config TERRAFORM_OCI_VOLUMES_ENABLE_EXTRA
> +	bool "Enable additional block devices"
> +	output yaml
> +	default n
> +	help
> +	  Enable this to provision up to 10 extra block devices
> +	  on each target node.
> +
> +if TERRAFORM_OCI_VOLUMES_ENABLE_EXTRA
> +
> +choice
> +	prompt "Count of extra block volumes"
> +	default TERRAFORM_OCI_VOLUMES_PER_INSTANCE_4
> +	help
> +	  The count of extra block devices attached to each target
> +	  node.
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE_1
> +	bool "1"
> +	help
> +	  Provision 1 extra volume per target node.
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE_2
> +	bool "2"
> +	help
> +	  Provision 2 extra volumes per target node.
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE_3
> +	bool "3"
> +	help
> +	  Provision 3 extra volumes per target node.
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE_4
> +	bool "4"
> +	help
> +	  Provision 4 extra volumes per target node.
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE_5
> +	bool "5"
> +	help
> +	  Provision 5 extra volumes per target node.
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE_6
> +	bool "6"
> +	help
> +	  Provision 6 extra volumes per target node.
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE_7
> +	bool "7"
> +	help
> +	  Provision 7 extra volumes per target node.
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE_8
> +	bool "8"
> +	help
> +	  Provision 8 extra volumes per target node.
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE_9
> +	bool "9"
> +	help
> +	  Provision 9 extra volumes per target node.
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE_10
> +	bool "10"
> +	help
> +	  Provision 10 extra volumes per target node.
> +
> +endchoice
> +
> +config TERRAFORM_OCI_VOLUMES_PER_INSTANCE
> +	int
> +	output yaml
> +	default 1 if TERRAFORM_OCI_VOLUMES_PER_INSTANCE_1
> +	default 2 if TERRAFORM_OCI_VOLUMES_PER_INSTANCE_2
> +	default 3 if TERRAFORM_OCI_VOLUMES_PER_INSTANCE_3
> +	default 4 if TERRAFORM_OCI_VOLUMES_PER_INSTANCE_4
> +	default 5 if TERRAFORM_OCI_VOLUMES_PER_INSTANCE_5
> +	default 6 if TERRAFORM_OCI_VOLUMES_PER_INSTANCE_6
> +	default 7 if TERRAFORM_OCI_VOLUMES_PER_INSTANCE_7
> +	default 8 if TERRAFORM_OCI_VOLUMES_PER_INSTANCE_8
> +	default 9 if TERRAFORM_OCI_VOLUMES_PER_INSTANCE_9
> +	default 10 if TERRAFORM_OCI_VOLUMES_PER_INSTANCE_10
> +
> +choice
> +	prompt "Volume size for each additional volume"
> +	default TERRAFORM_OCI_VOLUMES_SIZE_50G
> +	help
> +	  OCI implements volume sizes between 50G and 32T. In some
> +	  cases, 50G volumes are in the free tier.
> +
> +config TERRAFORM_OCI_VOLUMES_SIZE_50G
> +	bool "50G"
> +	help
> +	  Extra block volumes are 50 GiB in size.
> +
> +config TERRAFORM_OCI_VOLUMES_SIZE_64G
> +	bool "64G"
> +	help
> +	  Extra block volumes are 64 GiB in size.
> +
> +config TERRAFORM_OCI_VOLUMES_SIZE_128G
> +	bool "128G"
> +	help
> +	  Extra block volumes are 128 GiB in size.
> +
> +config TERRAFORM_OCI_VOLUMES_SIZE_256G
> +	bool "256G"
> +	help
> +	  Extra block volumes are 256 GiB in size.
> +
> +config TERRAFORM_OCI_VOLUMES_SIZE_512G
> +	bool "512G"
> +	help
> +	  Extra block volumes are 512 GiB in size.
> +
> +config TERRAFORM_OCI_VOLUMES_SIZE_1024G
> +	bool "1024G"
> +	help
> +	  Extra block volumes are 1024 GiB in size.
> +
> +config TERRAFORM_OCI_VOLUMES_SIZE_2048G
> +	bool "2048G"
> +	help
> +	  Extra block volumes are 2048 GiB in size.
> +
> +config TERRAFORM_OCI_VOLUMES_SIZE_4096G
> +	bool "4096G"
> +	help
> +	  Extra block volumes are 4096 GiB in size.
> +
> +config TERRAFORM_OCI_VOLUMES_SIZE_8192G
> +	bool "8192G"
> +	help
> +	  Extra block volumes are 8192 GiB in size.
> +
> +endchoice
> +
> +config TERRAFORM_OCI_VOLUMES_SIZE
> +	int
> +	output yaml
> +	default 50 if TERRAFORM_OCI_VOLUMES_SIZE_50G
> +	default 64 if TERRAFORM_OCI_VOLUMES_SIZE_64G
> +	default 128 if TERRAFORM_OCI_VOLUMES_SIZE_128G
> +	default 256 if TERRAFORM_OCI_VOLUMES_SIZE_256G
> +	default 512 if TERRAFORM_OCI_VOLUMES_SIZE_512G
> +	default 1024 if TERRAFORM_OCI_VOLUMES_SIZE_1024G
> +	default 2048 if TERRAFORM_OCI_VOLUMES_SIZE_2048G
> +	default 4096 if TERRAFORM_OCI_VOLUMES_SIZE_4096G
> +	default 8192 if TERRAFORM_OCI_VOLUMES_SIZE_8192G
> +
> +endif # TERRAFORM_OCI_VOLUMES_ENABLE_EXTRA
> +
> +if !TERRAFORM_OCI_VOLUMES_ENABLE_EXTRA
> +
>  config TERRAFORM_OCI_DATA_VOLUME_DISPLAY_NAME
>  	string "Display name to use for the data volume"
>  	default "data"
> @@ -98,6 +252,8 @@ config TERRAFORM_OCI_DATA_VOLUME_DISPLAY_NAME
>  	  Read this:
>  	  https://docs.oracle.com/en-us/iaas/Content/API/SDKDocs/terraformproviderconfiguration.htm
>  
> +endif # !TERRAFORM_OCI_VOLUMES_ENABLE_EXTRA
> +
>  config TERRAFORM_OCI_DATA_VOLUME_DEVICE_FILE_NAME
>  	string "Data volume's device file name"
>  	default "/dev/oracleoci/oraclevdb"
> @@ -106,6 +262,8 @@ config TERRAFORM_OCI_DATA_VOLUME_DEVICE_FILE_NAME
>  	  Read this:
>  	  https://docs.oracle.com/en-us/iaas/Content/API/SDKDocs/terraformproviderconfiguration.htm
>  
> +if !TERRAFORM_OCI_VOLUMES_ENABLE_EXTRA
> +
>  config TERRAFORM_OCI_SPARSE_VOLUME_DISPLAY_NAME
>  	string "Display name to use for the sparse volume"
>  	default "sparse"
> @@ -114,6 +272,8 @@ config TERRAFORM_OCI_SPARSE_VOLUME_DISPLAY_NAME
>  	  Read this:
>  	  https://docs.oracle.com/en-us/iaas/Content/API/SDKDocs/terraformproviderconfiguration.htm
>  
> +endif # !TERRAFORM_OCI_VOLUMES_ENABLE_EXTRA
> +
>  config TERRAFORM_OCI_SPARSE_VOLUME_DEVICE_FILE_NAME
>  	string "Sparse volume's device file name"
>  	default "/dev/oracleoci/oraclevdc"
> diff --git a/terraform/oci/main.tf b/terraform/oci/main.tf
> index 033f821d9502..05e51d6bef89 100644
> --- a/terraform/oci/main.tf
> +++ b/terraform/oci/main.tf
> @@ -28,7 +28,7 @@ resource "oci_core_instance" "kdevops_instance" {
>  }
>  
>  resource "oci_core_volume" "kdevops_data_disk" {
> -  count = local.kdevops_num_boxes
> +  count = var.oci_volumes_enable_extra == "true" ? 0 : local.kdevops_num_boxes
>  
>    compartment_id = var.oci_compartment_ocid
>  
> @@ -38,7 +38,7 @@ resource "oci_core_volume" "kdevops_data_disk" {
>  }
>  
>  resource "oci_core_volume" "kdevops_sparse_disk" {
> -  count = local.kdevops_num_boxes
> +  count = var.oci_volumes_enable_extra == "true" ? 0 : local.kdevops_num_boxes
>  
>    compartment_id = var.oci_compartment_ocid
>  
> @@ -48,7 +48,7 @@ resource "oci_core_volume" "kdevops_sparse_disk" {
>  }
>  
>  resource "oci_core_volume_attachment" "kdevops_data_volume_attachment" {
> -  count = local.kdevops_num_boxes
> +  count = var.oci_volumes_enable_extra == "true" ? 0 : local.kdevops_num_boxes
>  
>    attachment_type = "paravirtualized"
>    instance_id = element(oci_core_instance.kdevops_instance.*.id, count.index)
> @@ -58,7 +58,7 @@ resource "oci_core_volume_attachment" "kdevops_data_volume_attachment" {
>  }
>  
>  resource "oci_core_volume_attachment" "kdevops_sparse_disk_attachment" {
> -  count = local.kdevops_num_boxes
> +  count = var.oci_volumes_enable_extra == "true" ? 0 : local.kdevops_num_boxes
>  
>    attachment_type = "paravirtualized"
>    instance_id = element(oci_core_instance.kdevops_instance.*.id, count.index)
> @@ -66,3 +66,18 @@ resource "oci_core_volume_attachment" "kdevops_sparse_disk_attachment" {
>  
>    device = var.oci_sparse_volume_device_file_name
>  }
> +
> +resource "oci_core_volume" "kdevops_volume_extra" {
> +  count               = var.oci_volumes_enable_extra == "false" ? 0 : local.kdevops_num_boxes * var.oci_volumes_per_instance
> +  availability_domain = var.oci_availablity_domain
> +  display_name        = format("kdevops_volume-%02d", count.index + 1)
> +  compartment_id      = var.oci_compartment_ocid
> +  size_in_gbs         = var.oci_volumes_size
> +}
> +
> +resource "oci_core_volume_attachment" "kdevops_volume_extra_att" {
> +  count           = var.oci_volumes_enable_extra == "false" ? 0 : local.kdevops_num_boxes * var.oci_volumes_per_instance
> +  attachment_type = "paravirtualized"
> +  instance_id     = element(oci_core_instance.kdevops_instance.*.id, count.index)
> +  volume_id       = element(oci_core_volume.kdevops_volume_extra.*.id, count.index)
> +}

Terraform generates the following error message when
TERRAFORM_OCI_VOLUMES_ENABLE_EXTRA is disabled,

Error during operation: argument must not be null

So maybe we should set the default value of oci_volumes_per_instance to zero.

> diff --git a/terraform/oci/vars.tf b/terraform/oci/vars.tf
> index b02e79c597ec..5642bd7d5f63 100644
> --- a/terraform/oci/vars.tf
> +++ b/terraform/oci/vars.tf
> @@ -70,6 +70,23 @@ variable "oci_subnet_ocid" {
>    default = ""
>  }
>  
> +variable "oci_volumes_enable_extra" {
> +  description = "Create additional block volumes per instance"
> +  default     = false
> +}
> +
> +variable "oci_volumes_per_instance" {
> +  description = "The count of additional block volumes per instance"
> +  type        = number
> +  default     = null
> +}
> +
> +variable "oci_volumes_size" {
> +  description = "The size of additional block volumes, in gibibytes"
> +  type        = number
> +  default     = null
> +}
> +
>  variable "oci_data_volume_display_name" {
>    description = "Display name to use for the data volume"
>    default = "data"

-- 
Chandan