Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Return debuggable CAS errors on consul state put #33108

Merged
merged 4 commits into from Jun 30, 2023
Merged

Return debuggable CAS errors on consul state put #33108

merged 4 commits into from Jun 30, 2023

Conversation

unRob
Copy link
Contributor

@unRob unRob commented Apr 27, 2023

Ran into this error while running terraform inside a container, after it applied my plans, terraform failed saving state to Consul. I suspect my policy needs tweaking, but it's near impossible to tell with an error like this:

╷
│ Error: Failed to save state
│ 
│ Error saving state: consul CAS failed with transaction errors:
│ [0xc0006e93c8]
╵

This PR changes the the rendering of the error instance to actually print the message, instead of the memory address of said error instance.

Target Release

1.15.x

Draft CHANGELOG entry

BUG FIXES

  • When using consul as a state backend and failing to save state, consul CAS failed with transaction errors no longer shows an error instance memory address but an actual error message.

@unRob
Return debuggeble CAS errors on consul state put
0dda1d5 
ran into this error while running terraform on a container and saving state to Consul. I suspect my policy needs tweaking but it's impossible to tell with an error like this:

```
╷
│ Error: Failed to save state
│ 
│ Error saving state: consul CAS failed with transaction errors:
│ [0xc0006e93c8]
╵
```

This PR makes the will include the error messaage in the details so I can continue debugging
@unRob unRob requested a review from a team as a code owner April 27, 2023 16:10
@unRob unRob changed the title Return debuggeble CAS errors on consul state put Return debuggable CAS errors on consul state put Apr 27, 2023
@unRob
fix breaking code
1b7f772 
this is what i get for submitting PRs late at night
@crw
Copy link
Collaborator

crw commented Apr 27, 2023

Thanks for this submission. I'll send it to the Consul team for review.

cthain
cthain reviewed Jun 23, 2023
View reviewed changes
Copy link
Contributor

@cthain cthain left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I left a suggested update for your consideration.

@unRob @cthain
use go-multierror instead of manually joining error reasons
787cc69 
Co-authored-by: Chris Thain <32781396+cthain@users.noreply.github.com>
@unRob @cthain
use multierror.Append instead of append
8e5a66b 
Co-authored-by: Chris Thain <32781396+cthain@users.noreply.github.com>
cthain
cthain approved these changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@cthain cthain left a comment

Choose a reason for hiding this comment

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

Approving from the Consul side.

@crw
Copy link
Collaborator

crw commented Jun 26, 2023

Thanks, I'll bring this to triage to figure out how and when we can do the Terraform Core review.

@alisdair alisdair added the 1.5-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Jun 30, 2023
@alisdair alisdair merged commit ad2298b into hashicorp:main Jun 30, 2023
5 of 6 checks passed
@github-actions
Copy link

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@teamterraform teamterraform mentioned this pull request Jun 30, 2023
Closed
This was referenced Jun 30, 2023
Closed
Merged
@alisdair
Copy link
Member

Thanks for this fix! It will go out in the next 1.5 patch.

@unRob unRob deleted the patch-1 branch July 3, 2023 04:46
@yeikel yeikel mentioned this pull request Jul 13, 2023
Merged
@unRob
Copy link
Contributor Author

unRob commented Jul 27, 2023

Just wanted to say thanks again, now the error looks like this and I was able to fix my issue after all!

│ Error: Failed to save state
│ 
│ Error saving state: consul CAS failed with transaction errors: 1 error
│ occurred:
│ 	* Permission denied: token with AccessorID
│ 'bc883aa2-...' lacks permission 'key:write' on
│ "prefix/state/module-env:workspace"
│

@github-actions GitHub Actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alisdair alisdair alisdair approved these changes

@cthain cthain cthain approved these changes

Assignees
No one assigned
Labels
1.5-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged backend/consul enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@unRob @crw @alisdair @cthain