Skip to content
This repository has been archived by the owner on Nov 14, 2020. It is now read-only.

Correctly handle "not found" errors #45

Merged
merged 1 commit into from
Jan 7, 2020
Merged

Correctly handle "not found" errors #45

merged 1 commit into from
Jan 7, 2020

Conversation

multani
Copy link
Contributor

@multani multani commented Jan 3, 2020

Probably during the rabbithole update from 7235f3, the errors has currently
checked by the provider don't correctly detect that a resource has gone
missing on the cluster.

This creates unexpected "404 Not Found" errors at the end of any
refresh/plan/apply/destroy if any of the resources previously created by
Terraform (and known in the Terraform state) has been removed from the
RabbitMQ cluster, outside of Terraform.

I only tested a few, but this should affect all the resources: queues,
exchanges, vhost, permissions, users and policies (verified only on
vhosts, permissions, users and policies).

In this case, the RabbitMQ provider tries to read the resource it created
in the past, receives a 404 response from the cluster but doesn't treat it
correctly and returns a generic response "404 Object Not Found".

This change tries to read the error as a rabbithole error and handle the
404 response correctly.

I added a basic test on the "vhost" resource to show that it's working correctly now. What I was trying to do was:

  • Create an initial resource with Terraform.
  • Make the resource "disappear" outside of Terraform.
  • Reexecute the Terraform configuration and expect Terraform to recreate the resource correctly.

I'm not too familiar with Terraform providers so feel free to suggest an alternative way of doing so.
Also, I've only added a test on the vhost resource but the bug affects most if not all the resources. Let me know it that test looks OK and if you would like me to extend it to other resources as well.

@ghost ghost added the size/S label Jan 3, 2020
Copy link
Contributor

@cyrilgdn cyrilgdn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @multani ,

Thanks for opening this PR 👍

The build fails because of dependency troubles in master (I forgot to update the vendors in #31 )

Could you rebase please ? It should work after that.

The test you added is great 👍

@multani
Copy link
Contributor Author

multani commented Jan 6, 2020

The build fails because of dependency troubles in master (I forgot to update the vendors in #31 )

Thanks for the quick fix 👍

Could you rebase please ? It should work after that.

Done! Let's see if Travis is more happy 💃

Probably during the rabbithole update from 7235f3, the errors has currently
checked by the provider don't correctly detect that a resource has gone
missing on the cluster.

This creates unexpected "404 Not Found" errors at the end of any
refresh/plan/apply/destroy if any of the resources previously created by
Terraform (and known in the Terraform state) has been removed from the
RabbitMQ cluster, outside of Terraform.

I only tested a few, but this should affect all the resources: queues,
exchanges, vhost, permissions, users and policies (verified only on
vhosts, permissions, users and policies).

In this case, the RabbitMQ provider tries to read the resource it created
in the past, receives a 404 response from the cluster but doesn't treat it
correctly and returns a generic response "404 Object Not Found".

This change tries to read the error as a rabbithole error and handle the
404 response correctly.
Copy link
Contributor

@cyrilgdn cyrilgdn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot 👍

@cyrilgdn cyrilgdn merged commit 47c5029 into hashicorp:master Jan 7, 2020
@multani multani deleted the fix-error-handling branch January 7, 2020 13:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants