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

feat: add ability to manually sync layers #321

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

LucasMrqes
Copy link
Contributor

No description provided.

@Alan-pad
Copy link
Member

Alan-pad commented Sep 2, 2024

@LucasMrqes If you create a TerraformRun directly that would be easier I think and should still work, you'd have to make sure there's no other TerraformRun running but apart from that it should be straightforward to implement plan/apply buttons

@LucasMrqes
Copy link
Contributor Author

LucasMrqes commented Sep 6, 2024

@LucasMrqes If you create a TerraformRun directly that would be easier I think and should still work, you'd have to make sure there's no other TerraformRun running but apart from that it should be straightforward to implement plan/apply buttons

I think using the controller to generate the run is a better way. I tried to create the run resource with the server and encountered the following issues:

  • The status of the layer is not updated, thus making the layer status + run history incoherent and messing with how the server finds the last run in its /layers path. (unless we duplicate this logic in the server as well)
  • Not using the controller to generate runs makes it complicated to debug using the manual sync Button, as the run creation logic is somewhat duplicated and the button would bypass the ususal flow of creating run/pods with Burrito
  • Overall, the required logic is way more complicated

wdyt @Alan-pad ?

@Alan-pad
Copy link
Member

Alan-pad commented Sep 9, 2024

I think we can adapt the controller logic so the new runs are picked up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants