RavinderReddyF5 / terraform-provider-bigip-version0.12

Terraform resources that can configure F5 BIGIP products
Mozilla Public License 2.0
0 stars 0 forks source link

[CLOSED] Feature request: support Node.FQDN.Interval #40

Open RavinderReddyF5 opened 4 years ago

RavinderReddyF5 commented 4 years ago

Issue by thomamas Monday Oct 08, 2018 at 19:55 GMT Originally opened as https://github.com/terraform-providers/terraform-provider-bigip/issues/9


I would like to create a virtual servers using the vsphere provider, and then create a node and add it to a LTM pool by hostname.

Unfortunately, there is race where the new server's hostname might not be resolvable when I create these objects. When this happens, I need to wait for 3600 seconds until the LTM retries DNS. The suggestion from F5 is to adjust the FQDN Interval field (see https://support.f5.com/csp/article/K47726919).

This field is available in the Go API (https://github.com/f5devcentral/go-bigip/blob/master/ltm.go), so would it be possible to make it available in the Terraform provider?

Thanks for your help.

RavinderReddyF5 commented 4 years ago

Comment by dannyk81 Wednesday Oct 10, 2018 at 22:52 GMT


@thomamas this field is indeed defined in the Node struct in go-bigip, however it is not implemented in the CreateFQDNNode method.

I think what we can do, is to configure this attribute in the Update method and call it from Create (instead of calling Read), since we pass the Node config directly to ModifyNode method in Update.

@scshitole WDYT?

RavinderReddyF5 commented 4 years ago

Comment by scshitole Thursday Oct 11, 2018 at 02:54 GMT


@thomamas @dannyk81 We need to add the field in bigip_ltm_node.go also just doing d.Get("interval") won't work as fqdn is struct ?

    "fqdn": {
                Type:     schema.TypeList,
                Optional: true,
                Elem: &schema.Resource{
                    Schema: map[string]*schema.Schema{
                        "address_family": {
                            Type:        schema.TypeString,
                            Optional:    true,
                            Description: "Specifies the node's address family. The default is 'unspecified', or IP-agnostic",
                        },
                        "name": {
                            Type:        schema.TypeString,
                            Optional:    true,
                            Description: "Specifies the fully qualified domain name of the node.",
                        },
                        "interval": {
                            Type:        schema.TypeString,
                            Optional:    true,
                            Default:     "3600",
                            Description: "Marks the node up or down. The default value is user-up.",
                        },
                    },
RavinderReddyF5 commented 4 years ago

Comment by dannyk81 Thursday Oct 11, 2018 at 03:21 GMT


Not sure what you mean "won't work", but yes - we need to add the interval field attribute to the resource schema.

RavinderReddyF5 commented 4 years ago

Comment by scshitole Thursday Oct 11, 2018 at 06:50 GMT


@dannyk81 let me make the changes and test it

RavinderReddyF5 commented 4 years ago

Comment by scshitole Friday Oct 12, 2018 at 20:19 GMT


test results

 cat master.tf 
provider "bigip" {
  address = "10.192.74.68"
  username = "admin"
  password = "admin"
}

resource "bigip_ltm_node" "test-fqdn-node" {
        name = "/Common/somefunnynode"
        address = "f5.com"
        connection_limit = "0"
        dynamic_ratio = "1"
        monitor = "default"
        rate_limit = "disabled"
        fqdn = { interval = "3000"}
}

SJC-ML-00028512:terraform-provider-bigip shitole$ terraform apply -auto-approve
bigip_ltm_node.test-fqdn-node: Creating...
  address:          "" => "f5.com"
  connection_limit: "" => "0"
  dynamic_ratio:    "" => "1"
  fqdn.#:           "" => "1"
  fqdn.0.interval:  "" => "3000"
  monitor:          "" => "default"
  name:             "" => "/Common/somefunnynode"
  rate_limit:       "" => "disabled"
  state:            "" => "user-up"
bigip_ltm_node.test-fqdn-node: Creation complete after 0s (ID: /Common/somefunnynode)

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ terraform destroy -force
bigip_ltm_node.test-fqdn-node: Refreshing state... (ID: /Common/somefunnynode)
bigip_ltm_node.test-fqdn-node: Destroying... (ID: /Common/somefunnynode)
bigip_ltm_node.test-fqdn-node: Destruction complete after 0s

Destroy complete! Resources: 1 destroyed.
RavinderReddyF5 commented 4 years ago

Comment by dannyk81 Friday Oct 12, 2018 at 20:47 GMT


Thanks Sanjay, it would be great if you pushed these changes to a branch and did a PR so we could review and discuss instead of pushing directly to master...

I left some comments, here's a summary:

1) there's a variable used called ifaceCount, what Interface is this referring to? copy-pasta?

2) the Interval now defaults to 3600, this means it will be set on non-fqdn Nodes as well? is that even supported?

3) what if we don't want to use the Interval? i.e. "Use TTL" how would that be accomplished?

4) the fqdn schema is defined as TypeList, but there can't be multiple FQDN names for a single node, this doesn't fit. This should be TypeSet instead I think with MaxItems = 1, or at list a TypeList with MaxItems = 1.

    // MaxItems defines a maximum amount of items that can exist within a
    // TypeSet or TypeList. Specific use cases would be if a TypeSet is being
    // used to wrap a complex structure, however more than one instance would
    // cause instability.

Which means you won't need the for loop anymore.

5) the resource uses the same attribute (address) in both cases (IP or Name), we should consider using two attributes with a ConflictsWith (https://github.com/hashicorp/terraform/blob/ead558261d5e322f1f1e90c8e74834ba9215f24e/helper/schema/schema.go#L166-L170) option to ensure either address or fqdn_name (for example) is configured.

6) return err do not use the proper fmt.Errorf("...", name, err) format

7) log.Printf() is used to report on errors, when it should only report DEBUG messages.

please 🙏 could you revert this commit and let's work on this in a PR? WDYT?

RavinderReddyF5 commented 4 years ago

Comment by scshitole Friday Oct 26, 2018 at 19:34 GMT


@thomamas closed this issue with https://github.com/terraform-providers/terraform-provider-bigip/pull/15