Enhahance Terraformer.IsStateEmpty to check for presence of terraformer finalizer

[vpnachev]

How to categorize this PR?

/kind bug
/priority normal

What this PR does / why we need it:
After gardener/terraformer#65 the terraform finalizers are removed at the end of a successful destroy. However, the finalizer deletion can be rejected for some reason by the API and the whole operation needs to executed again. At this point, most of the extensions skips to run a destroy pod because of empty state, ref: https://github.com/gardener/gardener-extension-provider-aws/blob/d4c1eaf8590296add7a4bca19903b6eee3cd293b/pkg/controller/infrastructure/actuator_delete.go#L62-L65, hence there is no controller to remove the finalizer.

Update:
Now the IsStateEmpty check is enhanced to also verify for presence of the terraformer finalizer.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
Thanks @ialidzhikov for spotting it.

Release note:

A bug in the extension library that was preventing the deletion of TF secret and configmaps with empty state is now fixed. 

[ialidzhikov]

/lgtm

[rfranzke]

Wouldn't a better approach be to make the extensions not skip the destruction in case the state is already empty and rather let the Terraformer remove the finalizers that it previously added?

[vpnachev]

Wouldn't a better approach be to make the extensions not skip the destruction in case the state is already empty and rather let the Terraformer remove the finalizers that it previously added?

I believe It was introduced on purpose, for example provider-aws. Now the terraformer finalizer has broken this shortcut, and from my point of view the correct solution was to enhance the CleanupConfiguration.

[rfranzke]

The Terraformer library is not dealing with the finalizer yet and should not start doing so, I believe. It should rather properly instrument the Terraformer - now that we have this new behaviour.
It might work at the moment as you propose, but it has some potential to hit us later again.
Instead of existing early in the extension it should probably only do the LB cleanup if the state is not empty.
WDYT?

[danielfoehrKn]

I do not know all the details here, but why would the extensions remove a finalized from the terraformer? Wouldnt it be better to fix the terraformer to ensure the finalizer is removed from all the secrets (like @rfranzke suggests)? The extension would
In case the terraformer fails to remove the secret and the pod is terminated, the terraformer pod should be re-spawned to try again, no?

Instead of existing early in the extension it should probably only do the LB cleanup if the state is not empty.

Is this to make sure that the terraformer pod is re-created and removes the finalized? How is this related to the LB cleanup?

[timebertt]

Thanks.
/lgtm

[ialidzhikov]

/lgtm