crossplane-contrib / provider-tf-equinix-metal

DEPRECATED: use provider-jet-equinix :warning:
https://github.com/crossplane-contrib/provider-jet-equinix
Apache License 2.0
5 stars 3 forks source link

Can not delete device resources #8

Closed displague closed 7 months ago

displague commented 2 years ago

What happened?

The resource delete hangs with the following warnings:

  Warning  CannotObserveExternalResource  30m                  managed/device.equinixmetal.jet.crossplane.io/v1alpha1, kind=device  cannot get connection details: cannot get connection details: cannot not get a string for fieldpath "user_data": user_data: not a string
  Warning  CannotObserveExternalResource  82s (x248 over 31m)  managed/device.equinixmetal.jet.crossplane.io/v1alpha1, kind=device  cannot get connection details: cannot get connection details: cannot not get a string for fieldpath "custom_data": custom_data: not a string

How can we reproduce it?

Create the resources from examples/ and then try to delete it. Describe the resource.

What environment did it happen in?

Crossplane version: provider-tf-equinix-metal 0.2.2

displague commented 2 years ago

These optional, sensitive fields (userdata, custom_data) are set on device create. These fields are not forcenew. They are not refreshed on subsequent reads. https://github.com/equinix/terraform-provider-metal/blob/main/metal/resource_metal_device.go#L268-L282

The terrajet warning/error is coming from: https://github.com/crossplane-contrib/terrajet/blob/6abd89a444231a61237788434f38d6e92dbae150/pkg/resource/sensitive.go#L119

muvaf commented 2 years ago

Hmm it looks like we expect it to be present in all cases. @displague would you be interested in contributing a fix? I believe a if error is not found, continue statement before the err check should be sufficient 🙂

displague commented 2 years ago

@muvaf I attempted to create a fix for this, but I think the problem runs deeper. I wonder if paved.GetString should have a paved.GetOptionalString complement.

diff --git a/pkg/resource/sensitive.go b/pkg/resource/sensitive.go
index 7afa4cb..707db02 100644
--- a/pkg/resource/sensitive.go
+++ b/pkg/resource/sensitive.go
@@ -116,6 +116,10 @@ func GetSensitiveAttributes(from map[string]interface{}, mapping map[string]stri
                for _, fp := range fieldPaths {
                        v, err := paved.GetString(fp)
                        if err != nil {
+                               // Ignore unset optional sensitive attributes
+                               if err.Error() == fmt.Sprintf("%s: not a string", fp) {
+                                       continue
+                               }
                                return nil, errors.Wrapf(err, errFmtCannotGetStringForFieldPath, fp)
                        }
                        // Note(turkenh): k8s secrets uses a strict regex to validate secret
turkenh commented 2 years ago

@displague Opened https://github.com/crossplane-contrib/terrajet/pull/170. With this, I could verify that it fixes this issue.

I observed another issue with device resource though:


  Warning  CannotObserveExternalResource  36s (x13 over 45s)  managed/device.equinixmetal.jet.crossplane.io/v1alpha1, kind=device  cannot run refresh: refresh failed: Conflicting configuration arguments: "metro": conflicts with facilities: File name: main.tf.json
Conflicting configuration arguments: "facilities": conflicts with metro: File name: main.tf.json
  Warning  CannotObserveExternalResource  34s (x10 over 45s)  managed/device.equinixmetal.jet.crossplane.io/v1alpha1, kind=device  cannot run refresh: refresh failed: Conflicting configuration arguments: "facilities": conflicts with metro: File name: main.tf.json

I used the default example and here is the generated terraform state attributes:

"instances": [
        {
          "schema_version": 0,
          "attributes": {
            "access_private_ipv4": "10.70.112.1",
            "access_public_ipv4": "145.40.80.197",
            "access_public_ipv6": "2604:1380:4641:fa00::1",
            "always_pxe": false,
            "billing_cycle": "hourly",
            "created": "2021-12-06T13:28:47Z",
            "custom_data": null,
            "deployed_facility": "da11",
            "deployed_hardware_reservation_id": null,
            "description": null,
            "facilities": [
              "da11"
            ],
            "force_detach_volumes": false,
            "hardware_reservation_id": null,
            "hostname": "terrajet-example",
            "id": "9f6b2126-51f4-4fd1-8664-91d06728fabd",
            "ip_address": [],
            "ipxe_script_url": "",
            "locked": false,
            "metro": "da",
            "network": [
              {
                "address": "145.40.80.197",
                "cidr": 31,
                "family": 4,
                "gateway": "145.40.80.196",
                "public": true
              },
              {
                "address": "2604:1380:4641:fa00::1",
                "cidr": 127,
                "family": 6,
                "gateway": "2604:1380:4641:fa00::",
                "public": true
              },
              {
                "address": "10.70.112.1",
                "cidr": 31,
                "family": 4,
                "gateway": "10.70.112.0",
                "public": false
              }
            ],
            "network_type": "layer3",
            "operating_system": "ubuntu_20_04",
            "plan": "c3.small.x86",
            "ports": [
              {
                "bonded": true,
                "id": "85f77ea7-6d4e-4d70-89d5-9387217a6de3",
                "mac": "",
                "name": "bond0",
                "type": "NetworkBondPort"
              },
              {
                "bonded": true,
                "id": "125633be-9cfb-4d5c-a14a-54fe019b7e69",
                "mac": "0c:42:a1:97:fc:90",
                "name": "eth0",
                "type": "NetworkPort"
              },
              {
                "bonded": true,
                "id": "2a5cf40b-4c5c-4564-88ca-82f18ff90778",
                "mac": "0c:42:a1:97:fc:91",
                "name": "eth1",
                "type": "NetworkPort"
              }
            ],
            "project_id": "7941906b-ea45-44f5-85b4-df1455d3dc34",
            "project_ssh_key_ids": null,
            "reinstall": [],
            "root_password": "aE5d2oBU^]",
            "ssh_key_ids": [
              "b8627087-c0ef-4c0b-ad38-8533b5fecca6",
              "ac4861d9-979b-42e2-8098-faa7d7e3f5bb"
            ],
            "state": "active",
            "storage": null,
            "tags": [
              "crossplane"
            ],
            "termination_time": "",
            "timeouts": null,
            "updated": "2021-12-06T13:31:17Z",
            "user_data": null,
            "wait_for_reservation_deprovision": false
          },
turkenh commented 2 years ago
  Warning  CannotObserveExternalResource  36s (x13 over 45s)  managed/device.equinixmetal.jet.crossplane.io/v1alpha1, kind=device  cannot run refresh: refresh failed: Conflicting configuration arguments: "metro": conflicts with facilities: File name: main.tf.json
Conflicting configuration arguments: "facilities": conflicts with metro: File name: main.tf.json
  Warning  CannotObserveExternalResource  34s (x10 over 45s)  managed/device.equinixmetal.jet.crossplane.io/v1alpha1, kind=device  cannot run refresh: refresh failed: Conflicting configuration arguments: "facilities": conflicts with metro: File name: main.tf.json

@ulucinar I am wondering if the error above needs a late-init config, similar to what you did in https://github.com/crossplane-contrib/provider-jet-azure/pull/107#pullrequestreview-824152324

ulucinar commented 2 years ago
  Warning  CannotObserveExternalResource  36s (x13 over 45s)  managed/device.equinixmetal.jet.crossplane.io/v1alpha1, kind=device  cannot run refresh: refresh failed: Conflicting configuration arguments: "metro": conflicts with facilities: File name: main.tf.json
Conflicting configuration arguments: "facilities": conflicts with metro: File name: main.tf.json
  Warning  CannotObserveExternalResource  34s (x10 over 45s)  managed/device.equinixmetal.jet.crossplane.io/v1alpha1, kind=device  cannot run refresh: refresh failed: Conflicting configuration arguments: "facilities": conflicts with metro: File name: main.tf.json

@ulucinar I am wondering if the error above needs a late-init config, similar to what you did in crossplane-contrib/provider-jet-azure#107 (review)

Hi @turkenh, Yes, it looks like so. Could you please point me to the corresponding Terraform resource documentation?

turkenh commented 2 years ago

Here it is: https://registry.terraform.io/providers/equinix/metal/latest/docs/resources/device

ulucinar commented 2 years ago

Thanks @turkenh. I think it makes sense to exclude metro from late initialization as it conflicts with facilities:

r.LateInitializer = config.LateInitializer{
            IgnoredFields: []string{"metro"},
        }

in the relevant terrajet resource configuration.

displague commented 7 months ago

Closing as won't fix, but this should be resolved in the replacement project. See #11