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] Default values for optional attributes being ignored #74

Open RavinderReddyF5 opened 4 years ago

RavinderReddyF5 commented 4 years ago

Issue by dpeachey Tuesday Jan 08, 2019 at 08:58 GMT Originally opened as https://github.com/terraform-providers/terraform-provider-bigip/issues/43


Hi,

I just started testing the F5 BIG-IP Terraform provider and I have quickly run into what appears to be an issue, unless I am misinterpreting the behaviour.

I have used the following Terraform config to deploy a TCP profile:

resource "bigip_ltm_profile_tcp" "PFL_DP_EVAL_TCP" {
  name                  = "/Common/PFL_DP_EVAL_TCP"
  defaults_from         = "/Common/tcp-lan-optimized"
  partition             = "Common"
}

This applies correctly and sets up a TCP profile with the default values.

However when I run a Terraform plan without changing any of the configuration, I get the following:

Terraform will perform the following actions:

bigip_ltm_profile_tcp.PFL_DP_EVAL_TCP
      close_wait_timeout: "5" => "0"
      deferred_accept:    "disabled" => ""
      fast_open:          "disabled" => ""
      finwait_2timeout:   "300" => "0"
      finwait_timeout:    "5" => "0"
      idle_timeout:       "300" => "0"
      keepalive_interval: "1800" => "0"
Plan: 0 to add, 1 to change, 0 to destroy.

It appears to want to make an update to reset all the optional attributes to either 0 or "". I think it does not recognise that I am using the default values. I would expect that running at 'terraform plan' would report 'no changes'. Is there an issue here?

Regards,

Dan

RavinderReddyF5 commented 4 years ago

Comment by dannyk81 Tuesday Jan 08, 2019 at 13:52 GMT


Hi Dan,

This is an issue I'm afraid, I have similar trouble with monitor resource. It seems that the state of the profile resource is not being correctly recorded for all configurable attributes when it's first created so subsequent applies triggers changes.

/cc @scshitole

RavinderReddyF5 commented 4 years ago

Comment by dpeachey Wednesday Jan 09, 2019 at 10:29 GMT


Hi Danny,

I looked a bit further, and it appears in my case that the state is being written with the correct default values when the resource is provisioned:

root@dev:~/dev/obo-terraform-release/sandbox/f5# terraform state show bigip_ltm_profile_tcp.PFL_DP_EVAL_TEST
id                 = /Common/PFL_DP_EVAL_TEST
close_wait_timeout = 5
defaults_from      = /Common/tcp-lan-optimized
deferred_accept    = disabled
fast_open          = disabled
finwait_2timeout   = 300
finwait_timeout    = 5
idle_timeout       = 300
keepalive_interval = 1800
name               = /Common/PFL_DP_EVAL_TEST
partition          = Common

...so I don't know why a subsequent apply would trigger an update, as local state should equal device state.

Thanks,

Dan

RavinderReddyF5 commented 4 years ago

Comment by dpeachey Thursday Jan 10, 2019 at 08:44 GMT


I also tested with the monitor resource and the fastl4 profile resource and experience the same issue.

RavinderReddyF5 commented 4 years ago

Comment by scshitole Monday Jan 14, 2019 at 16:49 GMT


@dpeachey @dannyk81 thanks let me look at that.

RavinderReddyF5 commented 4 years ago

Comment by dpeachey Tuesday Jan 22, 2019 at 16:10 GMT


Hi @scshitole, did you have a chance to look at this one?

Thanks.

RavinderReddyF5 commented 4 years ago

Comment by scshitole Tuesday Jan 22, 2019 at 17:33 GMT


@dpeachey will look today

RavinderReddyF5 commented 4 years ago

Comment by scshitole Tuesday Jan 22, 2019 at 18:55 GMT


Just fixed it and tested the fix

bigip_ltm_profile_tcp.PFL_DP_EVAL_TCP: Creating...
  close_wait_timeout: "" => "5"
  defaults_from:      "" => "/Common/tcp-lan-optimized"
  deferred_accept:    "" => "disabled"
  fast_open:          "" => "disabled"
  finwait_2timeout:   "" => "300"
  finwait_timeout:    "" => "5"
  idle_timeout:       "" => "300"
  keepalive_interval: "" => "1800"
  name:               "" => "/Common/PFL_DP_EVAL_TCP"
  partition:          "" => "Common"
bigip_ltm_profile_tcp.PFL_DP_EVAL_TCP: Creation complete after 0s (ID: /Common/PFL_DP_EVAL_TCP)

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

bigip_ltm_profile_tcp.PFL_DP_EVAL_TCP: Refreshing state... (ID: /Common/PFL_DP_EVAL_TCP)

------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.

This means that Terraform did not detect any differences between your
configuration and real physical resources that exist. As a result, no
actions need to be performed.
RavinderReddyF5 commented 4 years ago

Comment by dpeachey Wednesday Jan 23, 2019 at 09:27 GMT


Thanks @scshitole, I will test. Could you also update the oneconnect profile? Same issue with that one.

RavinderReddyF5 commented 4 years ago

Comment by scshitole Wednesday Jan 23, 2019 at 18:27 GMT


@dpeachey here you go

bigip_ltm_profile_oneconnect.test-oneconnect: Creating...
  defaults_from:         "" => "/Common/oneconnect"
  idle_timeout_override: "" => "disabled"
  max_age:               "" => "3600"
  max_reuse:             "" => "1000"
  max_size:              "" => "1000"
  name:                  "" => "/Common/test-oneconnect"
  partition:             "" => "Common"
  share_pools:           "" => "disabled"
  source_mask:           "" => "255.255.255.255"
bigip_ltm_profile_oneconnect.test-oneconnect: Creation complete after 0s (ID: /Common/test-oneconnect)

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

bigip_ltm_profile_oneconnect.test-oneconnect: Refreshing state... (ID: /Common/test-oneconnect)

------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.
RavinderReddyF5 commented 4 years ago

Comment by dpeachey Wednesday Jan 23, 2019 at 18:57 GMT


great, thank you. I will test.

RavinderReddyF5 commented 4 years ago

Comment by dpeachey Thursday Jan 24, 2019 at 09:52 GMT


Hi @scshitole , I see a couple of issues still.

With the oneconnect profile, the default values seem to be wrong. I get:

* bigip_ltm_profile_oneconnect.PFL_DP_EVAL_PARENT_ONECONNECT: Error create profile oneConnect (/Common/PFL_DP_EVAL_PARENT_ONECONNECT): "max-age":"-1" integer value is out of range

Also with the fastl4, some of the default values are incorrect:

  ~ bigip_ltm_profile_fastl4.PFL_DP_EVAL_CHILD_FASTL4
      iptos_toclient:     "pass-through" => "65535"
      iptos_toserver:     "pass-through" => "65535"
      keepalive_interval: "disabled" => "0"

Thanks,

Dan

RavinderReddyF5 commented 4 years ago

Comment by scshitole Thursday Jan 24, 2019 at 15:37 GMT


@dpeachey I dont see this issue ?


  + bigip_ltm_profile_fastl4.test-fastl4
      id:                     <computed>
      client_timeout:         "40"
      defaults_from:          "/Common/fastL4"
      explicitflow_migration: "enabled"
      hardware_syncookie:     "enabled"
      idle_timeout:           "200"
      iptos_toclient:         "pass-through"
      iptos_toserver:         "pass-through"
      keepalive_interval:     "disabled"
      name:                   "/Common/Fastl4sanjay"
      partition:              "Common"

Plan: 1 to add, 0 to change, 0 to destroy.

------------------------------------------------------------------------

Note: You didn't specify an "-out" parameter to save this plan, so Terraform
can't guarantee that exactly these actions will be performed if
"terraform apply" is subsequently run.

SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ terraform apply

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  + bigip_ltm_profile_fastl4.test-fastl4
      id:                     <computed>
      client_timeout:         "40"
      defaults_from:          "/Common/fastL4"
      explicitflow_migration: "enabled"
      hardware_syncookie:     "enabled"
      idle_timeout:           "200"
      iptos_toclient:         "pass-through"
      iptos_toserver:         "pass-through"
      keepalive_interval:     "disabled"
      name:                   "/Common/Fastl4sanjay"
      partition:              "Common"

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

bigip_ltm_profile_fastl4.test-fastl4: Creating...
  client_timeout:         "" => "40"
  defaults_from:          "" => "/Common/fastL4"
  explicitflow_migration: "" => "enabled"
  hardware_syncookie:     "" => "enabled"
  idle_timeout:           "" => "200"
  iptos_toclient:         "" => "pass-through"
  iptos_toserver:         "" => "pass-through"
  keepalive_interval:     "" => "disabled"
  name:                   "" => "/Common/Fastl4sanjay"
  partition:              "" => "Common"
bigip_ltm_profile_fastl4.test-fastl4: Creation complete after 0s (ID: /Common/Fastl4sanjay)

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ 
SJC-ML-00028512:terraform-provider-bigip shitole$ sh plan
sh: plan: No such file or directory
SJC-ML-00028512:terraform-provider-bigip shitole$ terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

bigip_ltm_profile_fastl4.test-fastl4: Refreshing state... (ID: /Common/Fastl4sanjay)

------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.

This means that Terraform did not detect any differences between your
configuration and real physical resources that exist. As a result, no
actions need to be performed.
SJC-ML-00028512:terraform-provider-bigip shitole$ 
RavinderReddyF5 commented 4 years ago

Comment by dpeachey Friday Jan 25, 2019 at 09:30 GMT


@scshitole in the resource_bigip_ltm_profile_fastl4.go file I see the following which I think is causing the issue:

            "iptos_toclient": {
                Type:        schema.TypeString,
                Optional:    true,
                Default:     "65535",
                Description: "Use the parent Fastl4 profile",
            },
            "iptos_toserver": {
                Type:        schema.TypeString,
                Optional:    true,
                Default:     "65535",
                Description: "Use the parent Fastl4 profile",
            },
            "keepalive_interval": {
                Type:        schema.TypeString,
                Optional:    true,
                Default:     0,
                Description: "Use the parent Fastl4 profile",
            },

Thanks,

Dan

RavinderReddyF5 commented 4 years ago

Comment by scshitole Friday Jan 25, 2019 at 23:41 GMT


@dpeachey what is the bigip image you are using ? I am using 12.X release and my TF file is below, I don't see the issue ?

resource "bigip_ltm_profile_fastl4" "test-fastl4" {
            name = "/Common/testfastl4"
            partition = "Common"
            defaults_from = "/Common/fastL4"
            client_timeout = 40
            idle_timeout = "200"
            explicitflow_migration = "enabled"
            hardware_syncookie = "enabled"
            iptos_toclient = "pass-through"
            iptos_toserver = "pass-through"
            keepalive_interval = "disabled"
 }
RavinderReddyF5 commented 4 years ago

Comment by dpeachey Sunday Jan 27, 2019 at 11:27 GMT


@scshitole I am using 13.1.1. I see in your config you are specifying the values for those variables, but I'm not in mine, so mine is using the defaults from the schema, which seems to be incorrect.

RavinderReddyF5 commented 4 years ago

Comment by dannyk81 Sunday Jan 27, 2019 at 13:17 GMT


@scshitole, I'm not sure your recent fixes are correct.

The resource needs to read all the attributes values from F5 after Create and update their values in state.

the Defaults are just a way to specify some values when creating a resource, so that the user won't have to specify them, however since they are optional these defaults are not really needed.

Also, defaults change from version to version, so need to be carefull as you may override them. by setting a single value in the provider.

RavinderReddyF5 commented 4 years ago

Comment by scshitole Monday Jan 28, 2019 at 22:44 GMT


@dannyk81 let me check

RavinderReddyF5 commented 4 years ago

Comment by scshitole Monday Jan 28, 2019 at 22:50 GMT


@dpeachey I removed the configs and tried same results, can you please share the .tf file config ?

terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  + bigip_ltm_profile_fastl4.test-l4
      id:                     <computed>
      client_timeout:         "40"
      defaults_from:          "/Common/fastL4"
      explicitflow_migration: "enabled"
      hardware_syncookie:     "enabled"
      idle_timeout:           "200"
      iptos_toclient:         "65535"
      iptos_toserver:         "65535"
      keepalive_interval:     "0"
      name:                   "/Common/Fastjay"
      partition:              "Common"

Plan: 1 to add, 0 to change, 0 to destroy.

------------------------------------------------------------------------

Note: You didn't specify an "-out" parameter to save this plan, so Terraform
can't guarantee that exactly these actions will be performed if
"terraform apply" is subsequently run.

SJC-ML-00028512:terraform-provider-bigip shitole$ cat master.tf 
provider "bigip" {
  address = "10.192.74.68"
  username = "admin"
  password = "admin"
}

resource "bigip_ltm_profile_fastl4" "test-l4" {
            name = "/Common/Fastjay"
            partition = "Common"
            defaults_from = "/Common/fastL4"
            client_timeout = 40
            idle_timeout = "200"
            explicitflow_migration = "enabled"
            hardware_syncookie = "enabled"
 }
SJC-ML-00028512:terraform-provider-bigip shitole$ terraform apply

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  + bigip_ltm_profile_fastl4.test-l4
      id:                     <computed>
      client_timeout:         "40"
      defaults_from:          "/Common/fastL4"
      explicitflow_migration: "enabled"
      hardware_syncookie:     "enabled"
      idle_timeout:           "200"
      iptos_toclient:         "65535"
      iptos_toserver:         "65535"
      keepalive_interval:     "0"
      name:                   "/Common/Fastjay"
      partition:              "Common"

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

bigip_ltm_profile_fastl4.test-l4: Creating...
  client_timeout:         "" => "40"
  defaults_from:          "" => "/Common/fastL4"
  explicitflow_migration: "" => "enabled"
  hardware_syncookie:     "" => "enabled"
  idle_timeout:           "" => "200"
  iptos_toclient:         "" => "65535"
  iptos_toserver:         "" => "65535"
  keepalive_interval:     "" => "0"
  name:                   "" => "/Common/Fastjay"
  partition:              "" => "Common"
bigip_ltm_profile_fastl4.test-l4: Creation complete after 0s (ID: /Common/Fastjay)

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
SJC-ML-00028512:terraform-provider-bigip shitole$ 
RavinderReddyF5 commented 4 years ago

Comment by scshitole Wednesday Jan 30, 2019 at 21:16 GMT


@dpeachey may be you are using the 0.12 binary, in that case you are not using the fix, can you send me terraform init . output it should not say 0.12 tag

RavinderReddyF5 commented 4 years ago

Comment by dpeachey Thursday Jan 31, 2019 at 09:03 GMT


@scshitole I've rebuilt from the latest master branch and still have the same issue. My config looks like this:

resource "bigip_ltm_profile_fastl4" "PFL_DP_EVAL_PARENT_FASTL4" {
  name                      = "/Common/PFL_DP_EVAL_PARENT_FASTL4"
  defaults_from             = "/Common/fastL4"
  partition                 = "Common"

And I get the following when I re-apply:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  ~ bigip_ltm_profile_fastl4.PFL_DP_EVAL_PARENT_FASTL4
      iptos_toclient:     "pass-through" => "65535"
      iptos_toserver:     "pass-through" => "65535"
      keepalive_interval: "disabled" => "0"

I know if I specified those values in the config it would workaround it but they should be optional I would have thought.

Thanks,

Dan

RavinderReddyF5 commented 4 years ago

Comment by kylegentle Tuesday Mar 26, 2019 at 15:21 GMT


Hi @scshitole,

I'm not a user of terraform-provider-bigip, but I came across this issue while looking for a fix to this same bug in a Terraform provider I'm developing.

In my case, I was able to solve the issue by adding Computed: true, to the Optional attributes that were causing problems. This seems necessary when an optional attribute may be computed by the API at creation time. It's possible that might help here; if not, feel free to ignore.

Best, Kyle

RavinderReddyF5 commented 4 years ago

Comment by dannyk81 Thursday Apr 18, 2019 at 13:05 GMT


@scshitole was going over @kylegentle's suggestion above and it makes a lot of sense, we have many attributes that default to various values by the device once the resource is created.

I think hard coding these defaults in the provider is problematic long-term, since defaults may change over time or differ across versions of bigip.

The better approach is to mark these optional fields as Computed: true which ensures their values are retrieved and stored in the state right after the resource is created.

Thanks @kylegentle for the suggestion!

RavinderReddyF5 commented 4 years ago

Comment by focrensh Thursday Jul 09, 2020 at 14:37 GMT


Closing into https://github.com/terraform-providers/terraform-provider-bigip/issues/298