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

This change will fix the dobule lock issue when we use data.terrafo… #16852

Merged
merged 2 commits into from
Dec 6, 2017

Conversation

kavehmz
Copy link
Contributor

@kavehmz kavehmz commented Dec 5, 2017

Fixes #16741

There is a reproduceable double lock bug as far as I see. It exists in terraform 0.11 that introduced gcs with lock. You can reproduce the issue using the following setup.

Adding terraform_remote_state is causing a double lock that always fails.
Remove the terraform_remote_state section and all goes fine.

provider "google" {
  credentials = "service-account.json"
  project     = "my-project"
  region      = "europe-west1"
}

terraform {
  backend "gcs" {
    credentials = "service-account.json"
    project     = "my-project"
    bucket      = "my-bucket"
    path        = "test.tfstate"
  }
}

data "terraform_remote_state" "k8s" {
  backend = "gcs"

  config {
    credentials = "service-account.json"
    project     = "my-project"
    bucket      = "my-bucket"
    path        = "test.tfstate"
  }

}

This change just moved the lock into a section that will run only if state does not exists. This seems in line with what I seem in implementation of s3 remote state.

…m_remote_state and gcs remote state at the same time.

  Fixes hashicorp#16741
@apparentlymart
Copy link
Contributor

@jbardin , I remember you spent quite some time iterating on and perfecting the locking semantics on other backends... do you think this looks sound, based on that experience?

Copy link
Member

@jbardin jbardin left a comment

Choose a reason for hiding this comment

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

This looks good to me.
I definitely want to add a test that would have caught this for the implementor, but I can take care of that in another PR.

@apparentlymart apparentlymart merged commit 7507e3c into hashicorp:master Dec 6, 2017
@apparentlymart
Copy link
Contributor

Thanks, @kavehmz! We'll have a look at what tests we can add here to catch this sort of error in future. Currently our terraform_remote_state tests mainly just use the local backend, but it sounds like we should have some "acceptance-test-like" tests for other backends to catch situations where the behavior is subtly different between these two use-cases of backends.

@jbardin
Copy link
Member

jbardin commented Dec 6, 2017

After going to create the test, I'm wondering what the actual use case here should be. Granted this does bring the operation in line the the S3 backend, but the utility of loading the state you're already operating on seem rather dubious. Shouldn't anything in that state already be accessible in a safer manner?

@kavehmz
Copy link
Contributor Author

kavehmz commented Dec 6, 2017

@apparentlymart @jbardin thanks for review and merge.

@jbardin I am using state file which already saves some secrets in plain text to spit them out in next run so I don't look for a way to share my secrets. Terraform and its state file is all I might need in this case.

https://medium.com/@kavehmz/terraform-a-simple-setup-for-a-small-team-848c54d87c8b

@kavehmz kavehmz deleted the fixing_gcs_double_lock branch December 6, 2017 18:55
@jbardin jbardin mentioned this pull request Dec 6, 2017
@kavehmz kavehmz restored the fixing_gcs_double_lock branch January 21, 2018 16:41
@ghost
Copy link

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

Error loading state: writing
3 participants