Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for dynamic block on resource "cml2_node" #100

Closed
tiagosil-cisco opened this issue Jan 16, 2024 · 4 comments
Closed

Add support for dynamic block on resource "cml2_node" #100

tiagosil-cisco opened this issue Jan 16, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@tiagosil-cisco
Copy link

tiagosil-cisco commented Jan 16, 2024

It would be good to have the option of using dynamic blocks on "cml2_nodes" as we could differ basic config depending on nodedefinition, for example.

Something similar to below

resource "cml2_node" "nodes" {
  for_each       = { for key, value in local.nodes : key => value if value.nodedefinition == "iosxrv9000" || value.nodedefinition == "cat8000v" }
  lab_id         = cml2_lab.lab.id
  label          = each.value.hostname
  nodedefinition = each.value.nodedefinition
  tags           = each.value.tags
  x              = each.value.x
  y              = each.value.y

  dynamic "configuration" {
    for_each = each.value.nodedefinition == "iosxrv9000" ? [1] : []
    content {
      configuration = <<-EOT
      // XR base config
      EOT
    }
  }

  dynamic "configuration" {
    for_each = each.value.nodedefinition == "cat8000v" ? [1] : []
    content {
      configuration = <<-EOT
      // XE base config
      EOT
    }
  }
}
@rschmied rschmied added the enhancement New feature or request label Feb 2, 2024
@rschmied
Copy link
Member

rschmied commented Feb 2, 2024

I haven't tried this, yet. But "dynamic" is a HCL construct -- so, wouldn't this work already? What would be required on the CML TF provider to make this work?

Also note that the way configurations are provisioned with 2.7 forward allows for multiple configuration files. Basically, the configuration schema allows now either a string or a map, where the key is the filename and the value is the content of the filename. How would that be handled with this "dynamic configuration"?

@rschmied
Copy link
Member

based on this https://blog.boltops.com/2020/10/05/terraform-hcl-loops-with-dynamic-block/ and also on the new schema for node configurations:

configurations	[
List of node configuration file objects.
{
name*	string
minLength: 1
maxLength: 64
The name of the configuration file. Can also use the keyword "Main" to denote the main configuration file for the given definition.

content	string
nullable: true
Node configuration (no more than 20MB).
}]

I think this will be doable when I update the node resource to accept a list of configurations in addition to a single configuration.

@rschmied
Copy link
Member

@tiagosil-cisco after implementing named configurations and playing with this for a bit it seems like this isn't exactly possible... Named configurations are not "blocks" but a list of configurations. Therefore, the dynamic keyword does not apply... One can, however, build something like show below. It would also be possible to have the "baseconfigs" map use the node definition as key and the choose the proper entry per node.

I think it would also be possible to merge a base config (on node definition) and a node config (in a separate list). But all of this does NOT use dynamic blocks as there's "configuration block" as part of a node.

# Define nodes

locals {
  nodes = [
    {
      hostname       = "host1"
      nodedefinition = "ubuntu"
      tags           = toset(["nodes", "bla"])
      x              = 0
      y              = 100
    },
    {
      hostname       = "host2"
      nodedefinition = "ubuntu"
      tags           = toset(["nodes", "bla"])
      x              = 0
      y              = 100
    },
    {
      hostname       = "host3"
      nodedefinition = "ubuntu"
      tags           = toset(["nodes", "bla"])
      x              = 0
      y              = 100
    },
  ]
  configs = {
    "host1" = [{
      name    = "user-data"
      content = "hostname host1"
    }]
    "host2" = [{
      name    = "user-data"
      content = "hostname host2"
    }]
    "host3" = [{
      name    = "user-data"
      content = "hostname host3"
    }]
  }
}

resource "cml2_node" "node" {
  for_each       = { for key, value in local.nodes : key => value if value.nodedefinition == "ubuntu" }
  lab_id         = cml2_lab.lab.id
  label          = each.value.hostname
  nodedefinition = each.value.nodedefinition
  tags           = each.value.tags
  x              = each.value.x
  y              = each.value.y
  configurations = local.configs[each.value.hostname]
}

@rschmied
Copy link
Member

rschmied commented Jun 14, 2024

Fixed by PR #130.

Keeping the nested attribute over the block even though that only blocks allow to use dynamic. configuration. Apparently, with the framework nested attributes are preferred and the desired result can be achieved there as well. Dynamic blocks were only needed because they were NOT nested attributes (afaict).

Prefer NestedAttribute over Block. Blocks should typically be used for configuration compatibility with previously existing schemas from an older Terraform Plugin SDK. Efforts should be made to convert from Block to NestedAttribute as a breaking change for practitioners.

https://pkg.go.dev/github.com/hashicorp/terraform-plugin-framework/provider/schema

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants