jeremmfr / terraform-provider-junos

Terraform provider for Junos devices
https://registry.terraform.io/providers/jeremmfr/junos
MIT License
62 stars 22 forks source link

Performance: ressource junos_applications quite slow #709

Open lotusnoir opened 2 months ago

lotusnoir commented 2 months ago

Hello You just created this new ressource junos_applications in order to manage all applications object in a single ressource. Its working very well but its a bit slow i think

$ time terraform plan --target=module.test.junos_applications.global
data.vault_generic_secret.vsrx: Reading...
data.vault_generic_secret.vsrx: Read complete after 0s [id=005180b9-def6-829a-df4b-d6e81678cb7b]
module.test.junos_applications.global[0]: Refreshing state... [id=applications]

real    2m22.089s
user    2m53.659s
sys 0m6.024s

There is 428 objects in this ressource. 380 have no terms, the others have just 2 for udp / tcp

If i have to compare with the address-book for exemple:

$ time terraform plan --target=module.test.junos_security_address_book.global
data.vault_generic_secret.vsrx: Reading...
data.vault_generic_secret.vsrx: Read complete after 0s [id=403b9e27-9195-04c8-d214-1f6967baef11]
module.test.junos_security_address_book.global[0]: Refreshing state... [id=global]

real    0m55.481s
user    1m2.576s
sys 0m2.476s

my junos_security_address_book ressource contains 1582 entries

Do you think there is a way to optimize this junos_applications ressource ?

jeremmfr commented 2 months ago

Hi :wave:

This issue seems to be same as #498.

It's a performance issue in terraform-plugin-framework on plan with block set (unordered block list) thats I already notify to hashicorp with this issue https://github.com/hashicorp/terraform-plugin-framework/issues/775

When I created junos_applications resource, I defined application and application_set with set (unordered) instead of list (ordered) for 2 reasons:

Example of plan output when remove application:

With this Terraform code :

resource "junos_applications" "app" {
  application {
    name     = "app_1"
    protocol = "icmp"
  }
  application {
    name     = "app_2"
    protocol = "tcp"
  }
  application {
    name     = "app_3"
    protocol = "udp"
  }
  application {
    name     = "app_4"
    protocol = "esp"
  }
}

Plan output with block set and removing app_3:

Terraform will perform the following actions:

  # junos_applications.app will be updated in-place
  ~ resource "junos_applications" "app" {
        id = "applications"

      - application {
          - name     = "app_3" -> null
          - protocol = "udp" -> null
        }

        # (3 unchanged blocks hidden)
    }

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

Plan output with block list and removing app_3:

Terraform will perform the following actions:

  # junos_applications.app will be updated in-place
  ~ resource "junos_applications" "app" {
        id = "applications"

      ~ application {
          ~ name     = "app_3" -> "app_4"
          ~ protocol = "udp" -> "esp"
        }
      - application {
          - name     = "app_4" -> null
          - protocol = "esp" -> null
        }

        # (2 unchanged blocks hidden)
    }

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

As the issue at Hashicorp looks like stuck and applications definitions in Junos device are ordered, I will take care of adding a second version of junos_applictions resource (junos_applications_ordered for example) with blocks in list mode (ordered) instead of set (unordered), so with the disadvantages described above but with much better performance.

This will be a workaround and the new resource will be depreciated once Hashicorp fixes the issue.

lotusnoir commented 2 months ago

hello thanks for your clear answer, actually i think its better to preserve the actual code with block set than block list I prefer a lot the clear plan than the 2 min saving. Because sometimes i'm losing far more time to decrypt the plan. There is also this issue in the policies, and its a nightmare to read the plan #617 Do you this its possible to set a variable on the ressource to let the user choose between one mode or another one depending he seek performance or visibility ?

Edit: i just realize that you were no going to replace the actual ressource but to create a second one _ordered and its perfect, do you think you could do the same for the policies with _unordered ? ^^

jeremmfr commented 2 months ago

For policies resources, policy blocks is in list mode (ordered) because Junos specifies that the action of the first policy that the traffic matches is applied to the packet. The order is therefore important except in certain cases with basic policies and permit action on all policies.

For more visibility when unordered polices is not a problem, I can add _unordered version resources for policy resources.