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.

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

🐛 Fix CR Status update in Workspace controller #457

Merged
merged 12 commits into from
Aug 6, 2024

Conversation

arybolovlev
Copy link
Contributor

@arybolovlev arybolovlev commented Aug 1, 2024

Description

This PR aims to update the way the workspace controller updates CR Status. It now uses the patch method instead of update. This primarily helps with the edge case when the CR gets updated during initial reconciliation. In this situation, the controller fails to perform an update operation, leading to a scenario where the Workspace is created in HCP Terraform, but the CR status does not have information about the Workspace ID. Consequently, it attempts to create it again once it recovers from the failure. The controller will continue to fail because the name is already taken.

This PR mainly fixes the Workspace controller, however, the same issue can happen with other controllers. We need to revise all controllers' logic along with predicate logic(described in this PR) to make it more flexible, straightforward, and clean.

Tests

  • E2E on HCP Terraform Operator
  • E2E on HCP Terraform Operator [Helm]
  • E2E on Terraform Enterprise
  • E2E on Terraform Enterprise [Helm]

Usage Example

N/A.

References

Community Note

  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

@github-actions github-actions bot removed the test label Aug 1, 2024
@arybolovlev arybolovlev marked this pull request as ready for review August 5, 2024 09:29
@arybolovlev arybolovlev requested a review from a team as a code owner August 5, 2024 09:29
@arybolovlev arybolovlev changed the title 🐛 Fix status update 🐛 Fix CR Status update in Workspace controller Aug 5, 2024
Copy link
Member

@alexsomesan alexsomesan left a comment

Choose a reason for hiding this comment

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

Makes sense!

@arybolovlev arybolovlev merged commit a3ff38b into main Aug 6, 2024
11 checks passed
@arybolovlev arybolovlev deleted the fix-status-update branch August 6, 2024 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Workspace cannot be updated by TFO to set the workspaceID when a finalizer is added by another controller
2 participants