CiscoDevNet / terraform-provider-aci

Terraform Cisco ACI provider
https://registry.terraform.io/providers/CiscoDevNet/aci/latest/docs
Mozilla Public License 2.0
87 stars 100 forks source link

vzFilter relation to vzRFltP causes vicious circle when creating existing Filter using Terraform #1110

Closed mfr-6 closed 11 months ago

mfr-6 commented 12 months ago

Community Note

Terraform Version

1.0.4

APIC version and APIC Platform

Affected Resource(s)

Problem description

I've got an issue where we deployed new contract to the ACI (along with Subject and Filter which has single entry), and we ended up with having situation where 'terraform plan' is constantly producing output that relations [relation_vz_rs_fwd_r_flt_p_att] and [relation_vz_rs_rev_r_flt_p_att] needs to be removed from the filter.


Terraform will perform the following actions:

  # aci_filter.Test_Filter will be updated in-place
  ~ resource "aci_filter" "Test_Filter" {
        id                             = "uni/tn-test-tenant/flt-icmp"
        name                           = "icmp"
      - relation_vz_rs_fwd_r_flt_p_att = "uni/tn-common/fp-5" -> null
      - relation_vz_rs_rev_r_flt_p_att = "uni/tn-common/fp-5" -> null
        # (1 unchanged attribute hidden)
    }

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

Troubleshooting

I did investigation on this and I want to describe what I've found.

At the beginning it's worth to mention, that our team tried to create Contract and Subject in Tenant X and Filter "icmp" in Tenant common. Final chain looked like this: Contract1 (Tn: X) -> Subject1 (Tn: X) -> Filter icmp (Tn: common) -> entry icmp (Tn: common)

"icmp" is a default one in tenant common that existed on a APIC, but was not managed by terraform, so actually what terraform did is that it tried to deploy "icmp" filter to APIC in tenant common, which already existed.

Besides the fact that Terraform didn't manage to overwrite icmp filter on APIC (no orchestrator:terraform annotations added for example) - entire chain was deployed and terraform at this point started managing it including "icmp" filter. Configuration was applied successfully and the expected result was achieved.

Later on, there was need to add additional changes to the infrastructure. Our team is executing "terraform plan" before every change and they got output as follows (renamed resource name for obvious reason):


Terraform will perform the following actions:

  # aci_filter.Test_Filter will be updated in-place
  ~ resource "aci_filter" "Test_Filter" {
        id                             = "uni/tn-test-tenant/flt-icmp"
        name                           = "icmp"
      - relation_vz_rs_fwd_r_flt_p_att = "uni/tn-common/fp-5" -> null
      - relation_vz_rs_rev_r_flt_p_att = "uni/tn-common/fp-5" -> null
        # (1 unchanged attribute hidden)
    }

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

We didn't know how we should approach this, so I reproduced this issue in lab and tried to apply this change. Terraform output showed that change has been applied, but when next "plan" was prepared - it displayed that the same change needs to be applied. I reviewed terraform statefile and there was no change applied to statefile and to infrastructure. Now we are in a vicious circle where we have terraform that wants to remove something that cannot be removed.

Why is that?

I've discovered that filter "icmp" in statefile is the only one which has defined relations [relation_vz_rs_fwd_r_flt_p_att] and [relation_vz_rs_rev_r_flt_p_att].

    {
      "mode": "managed",
      "type": "aci_filter",
      "name": "Test_Filter",
      "provider": "provider[\"registry.terraform.io/ciscodevnet/aci\"]",
      "instances": [
        {
          "schema_version": 1,
          "attributes": {
            "annotation": "",
            "description": "",
            "id": "uni/tn-test-tenant/flt-icmp",
            "name": "icmp",
            "name_alias": "",
            "relation_vz_rs_filt_graph_att": "",
            "relation_vz_rs_fwd_r_flt_p_att": "uni/tn-common/fp-5", <-----
            "relation_vz_rs_rev_r_flt_p_att": "uni/tn-common/fp-5", <-----
            "tenant_dn": "uni/tn-test-tenant"
          },
          "sensitive_attributes": [],
          "private": "x",
          "dependencies": [
            "aci_tenant.Test_Tenant"
          ]
        }
      ]
    },

Other filters that were built from scratch didn't have those properties filled in. I've found it weird so i started digging.

I discovered that when creating Filter on APIC - WITHOUT any Entry inside of it - objects: vz:RtFwdRfltPAtt and vz:RsRevRFltPAtt and vz:RFltP are not created by APIC. This means that when you are creating filter and entry with respecting dependencies using terraform - in the first step vzFilter is created, then terraform reads created resource's data from the controller and put the data into statefile - at this time Entry is not created, so Filter has no relation to Filter Profile (RFltP) and it's not created itself, so RtFwdRfltPAtt and RsRevRFltPAtt are empty in statefile for this resource. In the next step entry is created under previously created filter - under the scene APIC creates RFltP and creates relation using RtFwdRfltPAtt and RsRevRFltPAtt, but of course at this step - relations under filter are not updated.

The case is completely different when trying to create something that already exists. Since "icmp" Filter (present on APIC already) has entry - it means that RFltP, RtFwdRfltPAtt and RsRevRFltPAtt was present. This means that when Filter was "created" by terraform - it was able to read those relations and put them into statefile including two relations.

Since our Filter HCL declaration has not defined [relation_vz_rs_fwd_r_flt_p_att] and [relation_vz_rs_rev_r_flt_p_att] - terraform thought that this needs to be removed, that's why it tried to remove it as follows:


Terraform will perform the following actions:

  # aci_filter.Test_Filter will be updated in-place
  ~ resource "aci_filter" "Test_Filter" {
        id                             = "uni/tn-test-tenant/flt-icmp"
        name                           = "icmp"
      - relation_vz_rs_fwd_r_flt_p_att = "uni/tn-common/fp-5" -> null
      - relation_vz_rs_rev_r_flt_p_att = "uni/tn-common/fp-5" -> null
        # (1 unchanged attribute hidden)
    }

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

Why those relations was not removed?

Based what I've found in Object MIM docs - RFltP, RtFwdRfltPAtt and RsRevRFltPAtt have status "NOT-CONFIGURABLE" so these are used for internal purposes. This means that API has no access to modify them. When terraform tried to remove them - APIC returned HTTP 400 Bad Request:

2023-09-25T15:42:58.421+0200 [INFO]  provider.terraform-provider-aci_v2.5.2.exe: 2023/09/25 15:42:58 [TRACE] HTTP Response: 400 400 Bad Request &{400 Bad Request 400 HTTP/1.1 1 1 map[Access-Control-Allow-Credentials:[false] Access-Control-Allow-Headers:[Origin, X-Requested-With, Content-Type, Accept, DevCookie, APIC-challenge, Request-Tag] Access-Control-Allow-Methods:[POST,GET,OPTIONS,DELETE] Connection:[keep-alive] Content-Length:[114] Content-Type:[application/json] Date:[Mon, 25 Sep 2023 13:41:12 GMT] Server:[Cisco APIC]] 0xc0002c96a0 114 [] false false map[] 0xc00071f900 0xc000754630}: timestamp=2023-09-25T15:42:58.419+0200
2023-09-25T15:42:58.421+0200 [INFO]  provider.terraform-provider-aci_v2.5.2.exe: 2023/09/25 15:42:58 [DEBUG] HTTP response unique string POST https://XXX/api/node/mo.json {"totalCount":"1","imdata":[{"error":{"attributes":{"code":"170","text":"Invalid access, MO: vzRsFwdRFltPAtt"}}}]}: timestamp=2023-09-25T15:42:58.419+0200

Conclusion

After long description I want to put into consideration one thing - do we really need [relation_vz_rs_fwd_r_flt_p_att] and [relation_vz_rs_rev_r_flt_p_att] as a arguments in aci_filter resource? Seems like those are used internally, they cannot be modified and deleted once created, so I see no point of keeping those as a configuration option in this resource. In my opinion there is also no need to give user possibility to create them if this is something that is created by APIC automatically for internal purposes. Maybe I'm not aware about some unique scenario, but imho we could consider removing it from provider.

I'm aware that creating something using terraform which already exists on APIC is not a good option, but this case leads to the problem, where we need to manually remove this resource from statefile, delete the filter and recreate it from the scratch using terraform so it won't have relations to Filter Profile in terraform state.

Sorry for description that doesn't meet template, but this was quite special and didn't fit into proposed template. Description is a bit chaotic, but I can make this much clear, so don't hesitate to ask questions.

Thanks

Steps to Reproduce

  1. Create filter with one entry using GUI no matter in which tenant.
  2. Declare the same filter using Terraform - deploy it (apply)
  3. Issue "terraform plan" to see for changes - you'll notice that terraform want to remove relations that are not obviously declared in your HCL code.
  4. 'terraform apply' - you'll see that relations haven't been removed from statefile and controller.

References

akinross commented 12 months ago

Hi @mfr-6,

Thank you for raising the issue. I had a quick look at the code and it seems these relational attributes are not set in schema as computed variables which I suspect is causing the terraform plan to output "uni/tn-common/fp-5" -> null. I quickly verified this behaviour (see output below) and setting these attributes to computed would fix this behaviour and allows you to remove the filter accordingly. I am not sure why these relational attributes are not set with computed to true, so this would need to be further investigated.

0777b0e80f3f:/test# terraform plan
data.aci_tenant.abr_test: Reading...
data.aci_tenant.abr_test: Read complete after 1s [id=uni/tn-abr_test]
aci_filter.icmp: Refreshing state... [id=uni/tn-abr_test/flt-icmp]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the
following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # aci_filter.icmp will be updated in-place
  ~ resource "aci_filter" "icmp" {
        id                             = "uni/tn-abr_test/flt-icmp"
        name                           = "icmp"
      - relation_vz_rs_fwd_r_flt_p_att = "uni/tn-common/fp-5" -> null
      - relation_vz_rs_rev_r_flt_p_att = "uni/tn-common/fp-5" -> null
        # (1 unchanged attribute hidden)
    }

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

─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Note: You didn't use the -out option to save this plan, so Terraform can't guarantee to take exactly these actions if you
run "terraform apply" now.
0777b0e80f3f:/test# aci_install 
gofmt -w $(find . -name '*.go' |grep -v vendor)
==> Checking that code complies with gofmt requirements...
go install

Initializing the backend...

Initializing provider plugins...
- Reusing previous version of ciscodevnet/mso from the dependency lock file
- Reusing previous version of ciscodevnet/aci from the dependency lock file
- Using previously-installed ciscodevnet/mso v0.11.1
- Installing ciscodevnet/aci v2.10.1...
- Installed ciscodevnet/aci v2.10.1 (signed by a HashiCorp partner, key ID 433649E2C56309DE)

Partner and community providers are signed by their developers.
If you'd like to know more about provider signing, you can read about it here:
https://www.terraform.io/docs/cli/plugins/signing.html

Terraform has made some changes to the provider dependency selections recorded
in the .terraform.lock.hcl file. Review those changes and commit them to your
version control system if they represent changes you intended to make.

Terraform has been successfully initialized!

You may now begin working with Terraform. Try running "terraform plan" to see
any changes that are required for your infrastructure. All Terraform commands
should now work.

If you ever set or change modules or backend configuration for Terraform,
rerun this command to reinitialize your working directory. If you forget, other
commands will detect it and remind you to do so if necessary.
0777b0e80f3f:/test# terraform plan
data.aci_tenant.abr_test: Reading...
data.aci_tenant.abr_test: Read complete after 1s [id=uni/tn-abr_test]
aci_filter.icmp: Refreshing state... [id=uni/tn-abr_test/flt-icmp]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are
needed.
0777b0e80f3f:/test# terraform destroy -auto-approve
data.aci_tenant.abr_test: Reading...
data.aci_tenant.abr_test: Read complete after 1s [id=uni/tn-abr_test]
aci_filter.icmp: Refreshing state... [id=uni/tn-abr_test/flt-icmp]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the
following symbols:
  - destroy

Terraform will perform the following actions:

  # aci_filter.icmp will be destroyed
  - resource "aci_filter" "icmp" {
      - id                             = "uni/tn-abr_test/flt-icmp" -> null
      - name                           = "icmp" -> null
      - relation_vz_rs_fwd_r_flt_p_att = "uni/tn-common/fp-5" -> null
      - relation_vz_rs_rev_r_flt_p_att = "uni/tn-common/fp-5" -> null
      - tenant_dn                      = "uni/tn-abr_test" -> null
    }

Plan: 0 to add, 0 to change, 1 to destroy.
aci_filter.icmp: Destroying... [id=uni/tn-abr_test/flt-icmp]
aci_filter.icmp: Destruction complete after 2s

Destroy complete! Resources: 1 destroyed.
0777b0e80f3f:/test# 

Regarding your question about, do we really need [relation_vz_rs_fwd_r_flt_p_att] and [relation_vz_rs_rev_r_flt_p_att] as a arguments in aci_filter resource? I am not sure, and we would need to check the reason for these relationship attributes. I will mark this issue as a bug and will add it to the todo issues.

mfr-6 commented 12 months ago

Hi @akinross

Thanks for response.

and allows you to remove the filter accordingly

Actually there was no issue with deleting affected Filter, the only issue was that terraform tried to interact (delete) with relations that are - from user perspective - read-only when preparing tf plan. Just to clarify and be sure we're on the same page.

Nevertheless - changing argument type to Computed sounds like a good plan. From your output I see that after the change - terraform is no longer trying to remove relations since they are now treated as a computed.

Thanks for investigating this issue further.

akinross commented 12 months ago

Hi @mfr-6,

Ok misunderstood there, thought you where having issues on update and destroy operation. The computed would address the update scenario only indeed.