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

Correct paths at terraform topology configs #45

Merged
merged 6 commits into from
Apr 1, 2020
Merged

Correct paths at terraform topology configs #45

merged 6 commits into from
Apr 1, 2020

Conversation

MOZGIII
Copy link
Contributor

@MOZGIII MOZGIII commented Mar 26, 2020

This is a WIP, terraform state that we have at S3 has to be migrated so it doesn't diverge. Or it has to be reset, either one would work for us I guess.

I'll take care of it, and then we can merge this.

@binarylogic
Copy link
Contributor

I'd finish this off but I'm not entirely sure why the state would change here 😄

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Apr 1, 2020

The problem is resources in the existing terraform states are named differently after the update that introduced AWS topologies modules.

If we do that kind of change, we would normally want to add a migration script to upgrade the existing states so that they're compatible. Without it, terraform will abort due to conflicts at the planing state (this can be considered a good case) or at application due to conflicting uniqueness constraints at AWS (a worse case, since it's not caught at planning). The second case is only possible if we remove the state instead of upgrading it.

I wanted to spend some time to assess whether we'll hit the second issue if I just drop the states, but it seems like we can just do that and see if we'll have troubles.

We'll be facing this same issue if we don't introduce state migration logic or be careful with the changes we do at terraform. I'm not sure though it's worth the engineering time to implement, or we can just handle issues each time this happens manually.

@binarylogic
Copy link
Contributor

Ah, wasn't aware. Everything in our test harness should be ephemeral so I don't want to spend time migrating state if it's difficult. We should be able to delete everything and start over (minus the test data).

Regarding shared state, I'm wondering if we even need this. I'm curious if it would be easier for each user to have their own state file, even for global state. My only concern is the new long running tests that @lukesteensen is introducing.

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Apr 1, 2020

It would be great if we can make the test harness truly ephemeral, however we need a better way of cleaning up before that will be possible. Currently, only the VM is removed by the alert timeout.
This is was a problem for Github Actions, and the workaround I employed was to just maintain the state at S3.

I agree we shouldn't really have that state shared, however, without shared state and proper cleanup each test leaves a bit of garbage, quickly exhausting the AWS quotas.

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Apr 1, 2020

Regarding the long-running tests, I think they should be managed somehow explicitly separately from the regular short running tests. That is to avoid disrupting their execution by accident. I'll address this concern after we have the #44 merged.

Use this at your own risk! It's counter-intuitive, but local references
to the remote terraform state are currently shared accross user_id and
test_configuration, and you should not migrate them! Instead, clear local
state when switching the configuration or user id.
It'll be addressed in the future.
@MOZGIII MOZGIII merged commit c389c3d into master Apr 1, 2020
@MOZGIII MOZGIII deleted the tf-fix branch April 1, 2020 21:37
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