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

Revert #7464 and allow an empty state #7521

Merged
merged 3 commits into from
Jul 7, 2016
Merged

Revert #7464 and allow an empty state #7521

merged 3 commits into from
Jul 7, 2016

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Jul 6, 2016

Reverted the change that caused 7509, and fix the original panic in the terraform provider itself.

Add another test to cover remote state initialization.

Fixes #7509

@jbardin
Copy link
Member Author

jbardin commented Jul 7, 2016

I'd also like to try and get a unit test in here to verify remote state initialization, since that was completely missed when #7464 was merged.

@glasser
Copy link
Contributor

glasser commented Jul 7, 2016

Seeing #7509 here. I'll build with this PR and let you know if this fixes our issue.

@jbardin
Copy link
Member Author

jbardin commented Jul 7, 2016

@glasser: I'm going to back this out mostly, as I've found other corner cases here. Since I've elbow deep in the remote state code already, what issue are you seeing?

@glasser
Copy link
Contributor

glasser commented Jul 7, 2016

Can confirm that with this PR, we aren't seeing #7509 any more.

I don't know what issue #7464 was trying to fix but it definitely stopped us from being able to create new setups with S3 state that doesn't exist yet, with a "no remote state found" error.

@glasser
Copy link
Contributor

glasser commented Jul 7, 2016

(ie, if this isn't clear — I like this PR because it makes terraform work again for us, after #7464 broke it)

jbardin added 2 commits July 7, 2016 16:19
Revert back to using a nil state. The external usage of the state shoudl
always check the Empty() method.
- Check for an empty state. If nothing is referenced from that state in
  the config, there's nothing to do here.
@jbardin jbardin force-pushed the jbardin/GH-7509 branch from f22ae1f to 007dfe1 Compare July 7, 2016 20:20
Verify that a remote state file is correctly initialized in the same
manner as used by the `terraform remote config`
@jbardin jbardin force-pushed the jbardin/GH-7509 branch from 007dfe1 to 7481382 Compare July 7, 2016 20:24
@jbardin
Copy link
Member Author

jbardin commented Jul 7, 2016

@glasser Don't worry, this PR is definitely intended to fix that issue. Just pushed a revised patch which should revert to the old behavior entirely, and added another test to keep it stable.

@glasser
Copy link
Contributor

glasser commented Jul 7, 2016

Confirm that revised patch works for me too.

@phinze
Copy link
Contributor

phinze commented Jul 7, 2016

LGTM

@jbardin jbardin merged commit 4c602d1 into master Jul 7, 2016
@jbardin jbardin deleted the jbardin/GH-7509 branch July 7, 2016 21:00
@ghost
Copy link

ghost commented Apr 24, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't initialize a new remote state
4 participants