cloudstack-terraform-provider icon indicating copy to clipboard operation
cloudstack-terraform-provider copied to clipboard

service offering. sdk framework rewrite

Open poddm opened this issue 1 year ago • 16 comments

Enhanced the following resources to support the full api spec.

  • cloudstack_service_offering_constrained
  • cloudstack_service_offering_unconstrained
  • cloudstack_service_offering_fixed

I split the service offering API up into multiple resources to make it a bit clearer of the different qos types

poddm avatar Oct 21 '24 21:10 poddm

@poddm Can you add the updated docs to this PR as well? I believe you had docs on the other PR you closed, but there doesn't appear to be updated docs for this.

I have pulled down your branch to test, and am still having issues with needing to specify the domain or domainid.

I get the following error:

│ Error: Error creating service offering
│
│   with cloudstack_service_offering_fixed.standard_offering["8-8"],
│   on service_offerings.tf line 21, in resource "cloudstack_service_offering_fixed" "standard_offering":
│   21: resource "cloudstack_service_offering_fixed" "standard_offering" {
│
│ Could not create fixed offering, unexpected error: CloudStack API error 431 (CSExceptionErrorCode: 9999): Unable to execute API command createserviceoffering due to invalid value. Invalid parameter domainid value="79151a13-70ff-4ddc-918e-8d8deab2c289" due to incorrect
│ long value format, or entity does not exist or due to incorrect parameter annotation for the field in api cmd class.

I'm using it like this:

resource "cloudstack_service_offering_fixed" "standard_offering" {
  for_each = local.cpu_ram_map

  name         = "ACE-${each.value.cpu}C${each.value.cpu}-${each.value.ram}R${each.value.ram}-S"
  domain_ids  = [local.domain_id]
  display_text = "ACE-${each.value.cpu}C${each.value.cpu}-${each.value.ram}R${each.value.ram}-S"
  cpu_number   = each.value.cpu
  cpu_speed    = var.standard_cpu_speed
  host_tags    = "standard"
  storage_tags = "standard_storage"
  memory       = each.value.ram * 1024 # Convert GB to MB
  offer_ha     = false
}

my locals

locals {
  cpus = [1, 2, 4, 8, 16, 32, 64]
  rams = [1, 2, 4, 8, 24, 32, 40, 48, 56, 64, 128]

  cpu_ram_combinations = flatten([
    for cpu in local.cpus : [
      for ram in local.rams : {
        cpu = cpu
        ram = ram
      }
    ]
  ])
  # Create a map for for_each
  cpu_ram_map = {
    for combination in local.cpu_ram_combinations :
    "${combination.cpu}-${combination.ram}" => combination
  }
  domain_id = "79151a13-70ff-4ddc-918e-8d8deab2c289"
}

CodeBleu avatar Oct 21 '24 23:10 CodeBleu

@CodeBleu , I fixed the domain ids issue. Try now.

Also, regarding the documentation. I was looking at using the generator but it might be better to scope all that work to its own PR. https://github.com/hashicorp/terraform-plugin-docs

poddm avatar Oct 25 '24 01:10 poddm

@CodeBleu , I fixed the domain ids issue. Try now.

Also, regarding the documentation. I was looking at using the generator but it might be better to scope all that work to its own PR. https://github.com/hashicorp/terraform-plugin-docs

@poddm Thanks!!
I was able to successfully add offering with specifying the UUID for domain_ids in an list, but I have a couple of questions.

  1. Can this also be added with putting the name "ACE" or "/ROOT/ACE" and it automatically get the correct UUID?
  2. I need to specify the storage_type as well, but I need to be able to do this without having to specify a disk_offering Is this feasable? Right now without specifying the disk_offering it defaults to shared, and I'd like to be able to change this to local without having to specify a full disk_offering.

I just did a disk_offering and it applied, however it still shows shared instead of local like I have selected.

-/+ resource "cloudstack_service_offering_fixed" "standard_offering" {
      + disk_offering           = {
          + cache_mode               = "none" # forces replacement
          + disk_offering_strictness = false # forces replacement
          + provisioning_type        = "thin" # forces replacement
          + root_disk_size           = 0 # forces replacement
          + storage_type             = "local" # forces replacement
        }
      ~ id                      = "16ee216c-eba1-4dc0-ab96-5bc6513c6616" -> (known after apply)
        name                    = "ACE-8C8-8R8-S"
        # (11 unchanged attributes hidden)
    }

you can see above that the tags is not shown there

{
  "format_version": "1.0",
  "terraform_version": "1.8.1",
  "values": {
    "root_module": {
      "resources": [
        {
          "address": "cloudstack_service_offering_fixed.standard_offering[\"8-8\"]",
          "mode": "managed",
          "type": "cloudstack_service_offering_fixed",
          "name": "standard_offering",
          "index": "8-8",
          "provider_name": "registry.opentofu.org/cloudstack/cloudstack",
          "schema_version": 0,
          "values": {
            "cpu_number": 8,
            "cpu_speed": 2400,
            "deployment_planner": null,
            "disk_hypervisor": null,
            "disk_offering": {
              "cache_mode": "none",
              "disk_offering_strictness": false,
              "provisioning_type": "thin",
              "root_disk_size": 0,
              "storage_type": "local"
            },
            "disk_offering_id": null,
            "disk_storage": null,
            "display_text": "ACE-8C8-8R8-S",
            "domain_ids": [
              "79151a13-70ff-4ddc-918e-8d8deab2c289"
            ],
            "dynamic_scaling_enabled": false,
            "host_tags": "standard",
            "id": "b7945e90-5432-4a87-b8f8-0711576ed75c",
            "is_volatile": false,
            "limit_cpu_use": false,
            "memory": 8192,
            "name": "ACE-8C8-8R8-S",
            "network_rate": null,
            "offer_ha": false,
            "storage_tags": "standard_storage",
            "zone_ids": null
          },
          "sensitive_values": {
            "disk_offering": {},
            "domain_ids": [
              false
            ]
          }
        }
      ]
    }
  }
}

Also, why is storage_tags avail in both at the 'root' level of the resource and also as tags in the disk_offerings?

resource "cloudstack_service_offering_fixed" "standard_offering" {
  for_each = local.cpu_ram_map

  name                     = "ACE-${each.value.cpu}C${each.value.cpu}-${each.value.ram}R${each.value.ram}-S"
  domain_ids               = [local.domain_id]
  display_text             = "ACE-${each.value.cpu}C${each.value.cpu}-${each.value.ram}R${each.value.ram}-S"
  cpu_number               = each.value.cpu
  cpu_speed                = var.standard_cpu_speed
  host_tags                = "standard"
  storage_tags             = "standard_storage"
  memory                   = each.value.ram * 1024 # Convert GB to MB
  offer_ha                 = false
  disk_offering            =  {
    storage_type             = "local"
    provisioning_type        = "thin"
    cache_mode               = "none"
    root_disk_size           = 0
    tags                     = "standard_storage"
    disk_offering_strictness = false
  }
}

image

CodeBleu avatar Oct 25 '24 17:10 CodeBleu

Can this also be added with putting the name "ACE" or "/ROOT/ACE" and it automatically get the correct UUID?

I think it would be better to add a data resource for domains. The problem I see with this approach is the resource will never become tainted if it was recreated as the same name. Subsequent resources that may modify or depend on the domain would never detect the newly re/created domain.

ex.

data "cloudstack_domain" "ace" {
    name = "/ROOT/ACE" 
}

resource "cloudstack_service_offering_fixed" "standard_offering" {
  ...
  domain_ids = [data.cloudstack_domain.ace.id]
}

I need to specify the storage_type as well, but I need to be able to do this without having to specify a disk_offering Is this feasable? Right now without specifying the disk_offering it defaults to shared, and I'd like to be able to change this to local without having to specify a full disk_offering.

Let me check. I believe storage_type is only applicable when creating a "compute only disk offering"/disk_offering block.

Also, why is storage_tags avail in both at the 'root' level of the resource and also as tags in the disk_offerings?

Thats a bug. storage tags should be in the disk_offering block

poddm avatar Oct 25 '24 19:10 poddm

@poddm Thank you so much for looking into this.

Once this offering stuff is complete, I still need a way to implement this with the cloudstack_instance resource. If I specify an offering that is constrained/dynamic, I need a way in the cloudstack_instance resource to specify the min/max of the CPU, Mem..etc. Otherwise right now it will just fail saying that it's missing values I'm not able to provide currently, and I can't use it.

Is this something you can fix with a new PR for cloudstack_instance

CodeBleu avatar Oct 26 '24 13:10 CodeBleu

@poddm I haven' tested but PR looks good. Left some minor comments.

vishesh92 avatar Oct 27 '24 09:10 vishesh92

@poddm Thank you so much for looking into this.

Once this offering stuff is complete, I still need a way to implement this with the cloudstack_instance resource. If I specify an offering that is constrained/dynamic, I need a way in the cloudstack_instance resource to specify the min/max of the CPU, Mem..etc. Otherwise right now it will just fail saying that it's missing values I'm not able to provide currently, and I can't use it.

Is this something you can fix with a new PR for cloudstack_instance

You can provide the values in the details field.

resource "cloudstack_instance" "this" {
    name = cloud
    display_name  = "cloud details"
    details = {cpuNumber = "4", memory = "2048"}
}

poddm avatar Oct 28 '24 20:10 poddm

@poddm Thank you so much for looking into this. Once this offering stuff is complete, I still need a way to implement this with the cloudstack_instance resource. If I specify an offering that is constrained/dynamic, I need a way in the cloudstack_instance resource to specify the min/max of the CPU, Mem..etc. Otherwise right now it will just fail saying that it's missing values I'm not able to provide currently, and I can't use it. Is this something you can fix with a new PR for cloudstack_instance

You can provide the values in the details field.

resource "cloudstack_instance" "this" {
    name = cloud
    display_name  = "cloud details"
    details = {cpuNumber = "4", memory = "2048"}
}

@poddm This is not currently part of the provider. Do you have this in your own branch? If so, Is there a PR for this too?

https://registry.terraform.io/providers/cloudstack/cloudstack/latest/docs/resources/instance

I tried to use what you show anyway, and I get this:

cloudstack_instance.test_box[0]: Creating...
╷
│ Error: Error creating the new instance ace-test-box-0: invalid character '<' looking for beginning of value
│
│   with cloudstack_instance.test_box[0],
│   on main.tf line 46, in resource "cloudstack_instance" "test_box":
│   46: resource "cloudstack_instance" "test_box" {
│
╵

  # cloudstack_instance.test_box[0] will be created
  + resource "cloudstack_instance" "test_box" {
      + details          = {
          + "cpuNumber" = "8"
          + "memory"    = "4096"
        }

CodeBleu avatar Oct 28 '24 21:10 CodeBleu

@poddm Not sure if you saw that I'm still having issues with this or not. Wondering what I need to do? https://github.com/apache/cloudstack-terraform-provider/pull/138#issuecomment-2442665334

CodeBleu avatar Oct 30 '24 14:10 CodeBleu

@CodeBleu ,

I need to specify the storage_type as well, but I need to be able to do this without having to specify a disk_offering Is this feasable? Right now without specifying the disk_offering it defaults to shared, and I'd like to be able to change this to local without having to specify a full disk_offering.

I fixed the duplicate storage_offering field.

Error: Error creating the new instance ace-test-box-0: invalid character '<' looking for beginning of value

This usually means the management server api returned an html error response and not json like the client library is expecting. I've been using this in 0.4.0 release.

poddm avatar Oct 30 '24 18:10 poddm

@poddm

I fixed the duplicate storage_offering field.

Thanks, I will test this later. My main issue was the storage_type would not change to what I selected. It defaults to shared and I wanted local. This is mentioned above in big comment, so maybe that part got lost.

image

Error: Error creating the new instance ace-test-box-0: invalid character '<' looking for beginning of value

This usually means the management server api returned an html error response and not json like the client library is expecting. I've been using this in 0.4.0 release.

What should I do?

  1. I don't even see the details part of cloudstack_instance even documented.
  2. I went ahead and used what you said to use, but I get that error whenever I add the details part

CodeBleu avatar Oct 30 '24 20:10 CodeBleu

@poddm Was wondering if you could help me with this :point_up: ? I need the storage_type to be what my terraform is set to, but it's not. https://github.com/apache/cloudstack-terraform-provider/pull/138#issuecomment-2448287725

CodeBleu avatar Dec 03 '24 20:12 CodeBleu

@poddm Was wondering if you could help me with this ☝️ ? I need the storage_type to be what my terraform is set to, but it's not. #138 (comment)

fixed.

poddm avatar Dec 05 '24 04:12 poddm

@poddm Was wondering if you could help me with this ☝️ ? I need the storage_type to be what my terraform is set to, but it's not. #138 (comment)

fixed.

@poddm Thanks, I do see that it is showing the storage_type correctly now, however I need the offering to allow 0 as the root_disk_size, so when using the service offering, I can specify whatever disk size I want during deployment.

Could not create fixed offering, unexpected error: CloudStack API error 431 (CSExceptionErrorCode: 4350): The Root disk size is of 0 GB but it must be greater than 0.

CodeBleu avatar Dec 05 '24 16:12 CodeBleu

The error is being thrown from cloudstacks API. Try setting this global var to 0.

Custom diskoffering size min (custom.diskoffering.size.min)
Minimum size in GB for custom disk offering.

poddm avatar Dec 09 '24 15:12 poddm

The error is being thrown from cloudstacks API. Try setting this global var to 0.

Custom diskoffering size min (custom.diskoffering.size.min)
Minimum size in GB for custom disk offering.

Not really sure where you are talking about. Before the last change, this wasn't an issue and would create the offering with a root_disk_size of 0, but the local/shared storage_type was not working. When I updated it with that fix...now is when I get the error about 0 for the size

In my resource "cloudstack_service_offering_fixed" "standard_offering" {

I have this: root_disk_size = 0

CodeBleu avatar Dec 09 '24 16:12 CodeBleu

@poddm Reviewing this PR again, I believe the only issue remaining ( that I had) was the root_disk_size needing to work if not specified or set to 0 You can see above where I was able to resolve that issue. ~~Is this something you can add to your PR and hopefully we can get this across the line?~~ CC: @vishesh92

Update: I went ahead and committed my suggestion for the root_disk_size fix

CodeBleu avatar Jun 10 '25 20:06 CodeBleu

@poddm Can you also update the documentation for this?

vishesh92 avatar Sep 02 '25 08:09 vishesh92

@vishesh92, docs are updated.

poddm avatar Sep 10 '25 23:09 poddm