crossplane-contrib / provider-jet-aws

AWS Provider for Crossplane that is built with Terrajet.
https://crossplane.io
Apache License 2.0
37 stars 30 forks source link

ECS Cluster: lifecycle.prevent_destroy set #168

Closed stevendborrelli closed 2 years ago

stevendborrelli commented 2 years ago

What happened?

When creating an ECS cluster using the example manifest, I get the following errors.

I get the following errors:

0s          Warning   CannotObserveExternalResource     cluster/sample-cluster                                                                   cannot run plan: plan failed: Instance cannot be destroyed: Resource aws_ecs_cluster.sample-cluster has lifecycle.prevent_destroy set, but the plan calls for this resource to be destroyed. To avoid this error and continue with the plan, either disable lifecycle.prevent_destroy or reduce the scope of the plan using the -target flag.: File name: main.tf.json

How can we reproduce it?

I created the following resource from a composition:

apiVersion: ecs.aws.jet.crossplane.io/v1alpha2
kind: Cluster
metadata:
  name: sample-cluster
spec:
  forProvider:
    region: us-west-2
0s          Warning   CannotObserveExternalResource     cluster/sample-cluster                                                                   cannot run plan: plan failed: Instance cannot be destroyed: Resource aws_ecs_cluster.sample-cluster has lifecycle.prevent_destroy set, but the plan calls for this resource to be destroyed. To avoid this error and continue with the plan, either disable lifecycle.prevent_destroy or reduce the scope of the plan using the -target flag.: File name: main.tf.json

What environment did it happen in?

muvaf commented 2 years ago

Could you share the YAML you used?

stevendborrelli commented 2 years ago

It's in the ticket. I used the example manifest after getting this error in a composition.

stevendborrelli commented 2 years ago

Outputs from setting the controller to debug:

{\"@level\":\"info\",\"@message\":\"Terraform 1.0.5\",\"@module\":\"terraform.ui\",\"@timestamp\":\"2022-03-10T19:02:09.596087Z\",\"terraform\":\"1.0.5\",\"type\":\"version\",\"ui\":\"0.1.0\"}\n{\"@level\":\"info\",\"@message\":\"aws_ecs_cluster.client1-borrelli-xvbsj: Refreshing state... [id=arn:aws:ecs:us-west-2:1234567:cluster/client1-borrelli-xvbsj]\",\"@module\":\"terraform.ui\",\"@timestamp\":\"2022-03-10T19:02:12.546430Z\",\"hook\":{\"resource\":{\"addr\":\"aws_ecs_cluster.client1-borrelli-xvbsj\",\"module\":\"\",\"resource\":\"aws_ecs_cluster.client1-borrelli-xvbsj\",\"implied_provider\":\"aws\",\"resource_type\":\"aws_ecs_cluster\",\"resource_name\":\"client1-borrelli-xvbsj\",\"resource_key\":null},\"id_key\":\"id\",\"id_value\":\"arn:aws:ecs:us-west-2:1234567:cluster/client1-borrelli-xvbsj\"},\"type\":\"refresh_start\"}\n{\"@level\":\"info\",\"@message\":\"aws_ecs_cluster.client1-borrelli-xvbsj: Refresh complete [id=arn:aws:ecs:us-west-2:1234567:cluster/client1-borrelli-xvbsj]\",\"@module\":\"terraform.ui\",\"@timestamp\":\"2022-03-10T19:02:12.605037Z\",\"hook\":{\"resource\":{\"addr\":\"aws_ecs_cluster.client1-borrelli-xvbsj\",\"module\":\"\",\"resource\":\"aws_ecs_cluster.client1-borrelli-xvbsj\",\"implied_provider\":\"aws\",\"resource_type\":\"aws_ecs_cluster\",\"resource_name\":\"client1-borrelli-xvbsj\",\"resource_key\":null},\"id_key\":\"id\",\"id_value\":\"arn:aws:ecs:us-west-2:1234567:cluster/client1-borrelli-xvbsj\"},\"type\":\"refresh_complete\"}\n{\"@level\":\"info\",\"@message\":\"Plan: 0 to add, 0 to change, 0 to destroy.\",\"@module\":\"terraform.ui\",\"@timestamp\":\"2022-03-10T19:02:12.609863Z\",\"changes\":{\"add\":0,\"change\":0,\"remove\":0,\"operation\":\"plan\"},\"type\":\"change_summary\"}\n{\"@level\":\"info\",\"@message\":\"Apply complete! Resources: 0 added, 0 changed, 0 destroyed.\",\"@module\":\"terraform.ui\",\"@timestamp\":\"2022-03-10T19:02:12.612563Z\",\"changes\":{\"add\":0,\"change\":0,\"remove\":0,\"operation\":\"apply\"},\"type\":\"change_summary\"}\n{\"@level\":\"info\",\"@message\":\"Outputs: 0\",\"@module\":\"terraform.ui\",\"@timestamp\":\"2022-03-10T19:02:12.612601Z\",\"outputs\":{},\"type\":\"outputs\"}\n
--
ezgidemirel commented 2 years ago

@turkenh encountered a similar issue while creating GKE cluster and a node pool

  ----     ------                         ----                 ----                                                            -------
  Normal   CreatedExternalResource        12m                  managed/container.gcp.jet.crossplane.io/v1alpha2, kind=cluster  Successfully requested creation of external resource
  Warning  CannotObserveExternalResource  47s (x8 over 3m25s)  managed/container.gcp.jet.crossplane.io/v1alpha2, kind=cluster  cannot run plan: plan failed: Instance cannot be destroyed: Resource google_container_cluster.ezgi-gke-cluster has lifecycle.prevent_destroy set, but the plan calls for this resource to be destroyed. To avoid this error and continue with the plan, either disable lifecycle.prevent_destroy or reduce the scope of the plan using the -target flag.: File name: main.tf.json
jessesanford commented 2 years ago

Sounds like there might need to be more finesse on when to release the prevent_destroy lifecycle ... it looks like right now it only get's unset when crossplane metadata thinks the resource was explicitly being deleted?

Some operations will require resources to get "replaced" right?

Or is this a case of needing to more nimbly apply ignore changes https://www.terraform.io/language/meta-arguments/lifecycle#ignore_changes to fields on resources that drift between reconcile cycles? Not quite sure why that would happen in practice unless one of the fields was using a data lookup for something like the "latest available ami" or some other data with filter that shifts over time?

https://github.com/crossplane/terrajet/blob/4a4dc50a18541a9a46f0e08f621ee938d119cc08/pkg/terraform/files.go#L151

jbw976 commented 2 years ago

Thanks for addressing the external name issue in #172 @ulucinar! is there more to look into here regarding lifecycle.prevent_destroy?

ulucinar commented 2 years ago

In the context of XRM, we did not allow remote infrastructure to be implicitly destructed and recreated in response to desired state (configuration) changes, that was the reason we resorted to always using the prevent_destroy lifecycle meta-argument except when the managed resource has a non-zero deletionTimestamp.

With the introduction of the validating admission webhooks in Crossplane, we can now treat such fields as immutable once set. We should already be able to identify such arguments from the Terraform schema but I didn't check if this piece of schema info is also available in the JSON schemas. I've opened this issue for further discussion on the topic.

The recent issues #168, #170 and the provider-jet-azure issue surface with no intervention on the desired state.

ulucinar commented 2 years ago

Closing this issue as https://github.com/crossplane-contrib/provider-jet-aws/releases/tag/v0.4.2 containing the proposed fix has been released.