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

Init terraform state migration RFC #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nicklathe
Copy link

Terraform state migration RFC

Signed-off-by: nicklathe <n.lathe@sap.com>
Copy link

@ncronquist ncronquist left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just a couple of smaller questions.

# Future possibilities
[future-possibilities]: #future-possibilities

As mentioned in this RFC, this is an MVP level solution to onboarding. This

Choose a reason for hiding this comment

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

Should we add something here about how this is specific to importing Stack created infrastructure and that future iterations might support terraform more generally?

the Terraform state from the user's existing S3 bucket, and push it to Scipian's
S3 backend.

# Reference-level explanation

Choose a reason for hiding this comment

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

What is the structure of the state stored in Scipian's S3 bucket? Mostly I'm wondering if the namespace is apart of the structure to avoid different teams accidentally overwriting each other's terraform state? Or maybe we will handle possible overwriting a different way?

Choose a reason for hiding this comment

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

Would it be worth adding a mapping here from what the current stack state looks like to what the scipian state would look like?

Copy link
Author

Choose a reason for hiding this comment

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

I didn't get into the specifics of Scipian's backend structure here, since this RFC will not change that. If we feel it would be helpful to specify that here, I'm happy to add it. It's <scipian-bucket>/namespace/workspace/terraform.tfstate

Copy link
Contributor

@dragan dragan left a comment

Choose a reason for hiding this comment

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

As we discussed yesterday, let's add the changes in regards to keeping the state in the target account instead which will feed the RFC around connecting a cloud provider to scipian.

@nicklathe
Copy link
Author

Update: I changed the wording in the RFC to reference the fact that Scipian will store Terraform state in a cusomter's S3 bucket, vs. Scipian's own S3 bucket.

@nicklathe nicklathe force-pushed the feature/terraform-state-migration branch from bac4cd9 to b89992e Compare July 23, 2019 17:25
@nicklathe
Copy link
Author

After @dragan's and my discussion, I've changed the wording of this RFC back to include a managed Scipian S3 backend.

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

Successfully merging this pull request may close these issues.

None yet

3 participants