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

core: Allow refresh of local state with no resources #7320

Merged
merged 3 commits into from
Oct 14, 2016
Merged

core: Allow refresh of local state with no resources #7320

merged 3 commits into from
Oct 14, 2016

Conversation

dtolnay
Copy link
Contributor

@dtolnay dtolnay commented Jun 24, 2016

Fixes #4154 and fixes #5410.

HasResources is copied from @apparentlymart's work in #7026.

@dtolnay dtolnay changed the title Allow refresh of local state with no resources core: Allow refresh of local state with no resources Jul 2, 2016
@dtolnay
Copy link
Contributor Author

dtolnay commented Aug 18, 2016

@jen20 Is there anything I can do to move this along?

@@ -64,8 +64,13 @@ func (s *CacheState) RefreshState() error {
// Cache is newer than remote. Not a big deal, user can just
// persist to get correct state.
s.refreshResult = CacheRefreshLocalNewer
case cached == nil && durable != nil:
case !cached.HasResources() && durable != nil:
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably verify that cached is not nil here since the HasResources() method accesses members.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HasResources returns false if the state is nil.

@jen20
Copy link
Contributor

jen20 commented Sep 3, 2016

HI @dtolnay! Looks like we have a problem with the test applied to master:

--- FAIL: TestCacheState_RefreshState (0.01s)
panic: missing module path [recovered]
        panic: missing module path

goroutine 39 [running]:
panic(0x345f40, 0xc42021e050)
        /usr/local/Cellar/go/1.7/libexec/src/runtime/panic.go:500 +0x1a1
testing.tRunner.func1(0xc4201d4840)
        /usr/local/Cellar/go/1.7/libexec/src/testing/testing.go:579 +0x25d
panic(0x345f40, 0xc42021e050)
        /usr/local/Cellar/go/1.7/libexec/src/runtime/panic.go:458 +0x243
github.com/hashicorp/terraform/terraform.(*State).ModuleByPath(0xc420216960, 0x56bd10, 0x1, 0x1, 0xc420090838)
        /Users/James/Code/go/src/github.com/hashicorp/terraform/terraform/state.go:145 +0x1e2
github.com/hashicorp/terraform/terraform.(*State).init(0xc420216960)
        /Users/James/Code/go/src/github.com/hashicorp/terraform/terraform/state.go:580 +0x62
github.com/hashicorp/terraform/terraform.WriteState(0xc420216960, 0x553a40, 0xc420090a78, 0x0, 0x0)
        /Users/James/Code/go/src/github.com/hashicorp/terraform/terraform/state.go:1775 +0x51
github.com/hashicorp/terraform/state.(*LocalState).WriteState(0xc4201f1380, 0xc420216960, 0x0, 0x0)
        /Users/James/Code/go/src/github.com/hashicorp/terraform/state/local.go:69 +0x19c
github.com/hashicorp/terraform/state.(*CacheState).WriteState(0xc420051ec8, 0xc420216960, 0xc4201f1600, 0x3c)
        /Users/James/Code/go/src/github.com/hashicorp/terraform/state/cache.go:28 +0x41
github.com/hashicorp/terraform/state.TestCacheState_RefreshState(0xc4201d4840)
        /Users/James/Code/go/src/github.com/hashicorp/terraform/state/cache_test.go:91 +0x3f2
testing.tRunner(0xc4201d4840, 0x3fb5f8)
        /usr/local/Cellar/go/1.7/libexec/src/testing/testing.go:610 +0x81
created by testing.(*T).Run
        /usr/local/Cellar/go/1.7/libexec/src/testing/testing.go:646 +0x2ec
FAIL    github.com/hashicorp/terraform/state    0.044s

One of us will pick this up next week unless you get a chance to do so beforehand. Thanks!

@dtolnay
Copy link
Contributor Author

dtolnay commented Sep 8, 2016

I'm surprised it still merges cleanly after 2.5 months. I will let you folks take care of the test failure. I enabled the brand new "allow edits from maintainers" to make it easier.

@dtolnay
Copy link
Contributor Author

dtolnay commented Sep 8, 2016

Never mind, it was simple enough I fixed it myself.

@dtolnay
Copy link
Contributor Author

dtolnay commented Sep 8, 2016

git bisect points to #8120 as the point it started failing.

@zachgersh
Copy link
Contributor

I would love to +1 this (can it be merged soonish) - just needed this today and thought terraform would work this way when it clearly does not.

@dtolnay
Copy link
Contributor Author

dtolnay commented Sep 30, 2016

@jen20 is there anything else I can do to help you merge this?

@jen20 jen20 merged commit afe2d7b into hashicorp:master Oct 14, 2016
@dtolnay dtolnay deleted the conflict branch October 14, 2016 16:03
@ghost
Copy link

ghost commented Apr 21, 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 21, 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.

Re-configuring remote state raises conflict on pull Local and remote state conflict on no changes
3 participants