Add support for user configured state object names in swift containers #17465

Elethiomel · 2018-02-28T14:55:42Z

Currently in the swift backend, the object state name is hardcoded to "tfstate.tf". This PR makes that configurable via a "state_name" option. The option is optional and defaults to "tfstate.tf". This puts the backend more in line with the S3 backend and will allow users to have one container with multiple states.

I'm not 100% happy with the option name. I'd prefer to use "key" or something similar, but the backend already uses that internally for the Openstack key and I didn't want to have the option and internal struct members named differently. Any suggestions?

I, of course still need to update the documentation and test it a bit more, so this is very much a WIP.

Elethiomel · 2018-04-13T09:46:04Z

I've now updated the documentation. I think this is ready for someone else to take a look at now :)

paultyng · 2018-04-13T13:51:24Z

backend/remote-state/swift/backend.go

+				Type:        schema.TypeString,
+				Optional:    true,
+				Description: descriptions["state_name"],
+			},


Could you set a default value in here with the original value?

Hi Paul, I actually set a default on lines 289 to 293 in keeping with the style of previous code on lines 283 to 286. Is it OK to do it that way?

You should set the value here, the difference is that if someone then explicitly writes tfstate.tf as a value I believe it will create a diff, although this is a backend so doesn't have the same implications as a provider. But we should use it consistently.

Good point, I've made the change and tested it internally. Looks good :)

paultyng · 2018-04-13T13:53:07Z

website/docs/backends/types/swift.html.md

@@ -19,14 +19,13 @@ Stores the state as an artifact in [Swift](http://docs.openstack.org/developer/s
 ```hcl
 terraform {
  backend "swift" {
-    path = "terraform-state"
+    path = "terraform-state",
+    state_name = "terraform.tfstate"
  }
 }
 ```
 This will create a container called `terraform-state` and an object within that container called `tfstate.tf`.


Need to update this line here as well since you changed the example.

Well spotted! I've made a commit to fix the discrepancies. Basically, the old code was hardwired to use "tfstate.tf". Unfortunately, the documentation referred to "terraform.tfstate" in many places. The new commit fixes the docs to reflect the reality that the state has always been "tfstate.tf"

paultyng · 2018-04-13T13:53:45Z

website/docs/backends/types/swift.html.md

@@ -54,6 +53,9 @@ The following configuration options are supported:
 * `path` - (Optional) DEPRECATED: Use `container` instead.
   The name of the container to create in order to store the state file.

+ * `state_name` - (Optional) The path to state file in the container.
+   Defaults to `terraform.tfstate`.


Defaults to tfstate.tf

Elethiomel

Thanks for the review Paul. I think I'm ready for another round please :)

paultyng

Sorry for the delay here, a couple minor fixes and we are good I think.

paultyng · 2018-05-04T13:55:58Z

backend/remote-state/swift/backend.go

+				Type:        schema.TypeString,
+				Optional:    true,
+				Description: descriptions["state_name"],
+			},


You should set the value here, the difference is that if someone then explicitly writes tfstate.tf as a value I believe it will create a diff, although this is a backend so doesn't have the same implications as a provider. But we should use it consistently.

paultyng · 2018-05-04T13:56:23Z

backend/remote-state/swift/client.go

 // RemoteClient implements the Client interface for an Openstack Swift server.
 type RemoteClient struct {
 	client           *gophercloud.ServiceClient
 	container        string
+	state_name       string


please change this to stateName: https://golang.org/doc/effective_go.html#mixed-caps

paultyng · 2018-05-04T13:59:00Z

backend/remote-state/swift/backend.go

@@ -238,6 +246,7 @@ type Backend struct {
 	archiveContainer string
 	expireSecs       int
 	container        string
+	state_name       string


stateName: https://golang.org/doc/effective_go.html#mixed-caps

Elethiomel

I've updated state_name to stateName as suggested to keep with Go's mixedCaps style

Elethiomel · 2018-05-08T12:20:54Z

backend/remote-state/swift/backend.go

+				Type:        schema.TypeString,
+				Optional:    true,
+				Description: descriptions["state_name"],
+			},


Good point, I've made the change and tested it internally. Looks good :)

paultyng

LGTM! Thanks again for the PR. The merging of this will probably have to wait to be rebased on the 0.12 work currently ongoing, but will be merged once the core of that work lands.

dariakts · 2019-04-29T09:42:22Z

Hello,

Any updates for this pull request ?

Elethiomel · 2019-04-29T15:36:04Z

I believe this was to be included in 0.12

…

On Mon, 29 Apr 2019, 03:43 Daria Kobtseva, ***@***.***> wrote: Hello, Any updates for this pull request ? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#17465 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATNRDZMMHOVUHDVGYZIAC3PS27LFANCNFSM4ES3ER5A> .

pselle · 2019-07-09T16:11:19Z

Hi @Elethiomel! Might I ask for an update to this PR to resolve its conflicts? Thanks so much for your patience here.

Elethiomel · 2019-07-10T09:51:39Z

Quite a few changes have been made to the swift backend to support locking and environments, but I'll give it a go over the next week. I'm pretty swamped at work, but I would really like this PR to be accepted so I'll try me best :)

pselle · 2019-07-10T12:42:29Z

Great @Elethiomel! We're keeping an eye on it now, happy to review (and get this in) when you've had the chance to revisit it!

hashicorp-cla · 2019-07-10T17:06:25Z

All committers have signed the CLA.

hashicorp-cla · 2019-07-10T17:06:25Z

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Colin Fowler seems not to be a GitHub user.
You need a GitHub account to be able to sign the CLA. If you already have a GitHub account, please add the email address used for this commit to your account.

_{Have you signed the CLA already but the status is still pending? Recheck it.}

Elethiomel · 2019-07-10T17:13:31Z

@pselle Rebased and CLA signed :)

pselle

Great!

pselle · 2019-07-10T18:04:20Z

@Elethiomel In my enthusiasm, I missed a key step 😅 for the purposes of making sure we cover our bases, would you mind running the acceptance tests for this backend and posting the results here? This is my bad for moving too soon/not realizing to say this earlier.

Comment updated -- I've been advised by the team that this is ok in this case, but if you do feel inspired to post the results, it would be appreciated! Again, mea culpa.

ghost · 2019-08-13T01:58:02Z

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.

Elethiomel changed the title ~~[WIP] Add support for user configured state object names in swift containers~~ Add support for user configured state object names in swift containers Apr 12, 2018

paultyng suggested changes Apr 13, 2018

View reviewed changes

paultyng added enhancement backend/swift labels Apr 13, 2018

Elethiomel commented Apr 16, 2018

View reviewed changes

paultyng suggested changes May 4, 2018

View reviewed changes

Elethiomel commented May 8, 2018

View reviewed changes

paultyng approved these changes May 8, 2018

View reviewed changes

paultyng assigned jbardin May 22, 2018

apparentlymart added this to the v0.12.1 milestone Apr 29, 2019

pselle modified the milestones: v0.12.1, TBD Jun 4, 2019

Elethiomel closed this Jul 10, 2019

rebased to terraform master branch

cd7bfba

Elethiomel reopened this Jul 10, 2019

pselle approved these changes Jul 10, 2019

View reviewed changes

pselle merged commit 9f9ac1b into hashicorp:master Jul 10, 2019

ghost locked and limited conversation to collaborators Aug 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for user configured state object names in swift containers #17465

Add support for user configured state object names in swift containers #17465

Elethiomel commented Feb 28, 2018

Elethiomel commented Apr 13, 2018

paultyng Apr 13, 2018

Elethiomel Apr 16, 2018

paultyng May 4, 2018

Elethiomel May 8, 2018

paultyng Apr 13, 2018

Elethiomel Apr 16, 2018

paultyng Apr 13, 2018

Elethiomel Apr 16, 2018

Elethiomel left a comment

paultyng left a comment

paultyng May 4, 2018

paultyng May 4, 2018

paultyng May 4, 2018

Elethiomel left a comment

Elethiomel May 8, 2018

paultyng left a comment

dariakts commented Apr 29, 2019

Elethiomel commented Apr 29, 2019 via email

pselle commented Jul 9, 2019

Elethiomel commented Jul 10, 2019

pselle commented Jul 10, 2019

hashicorp-cla commented Jul 10, 2019 •

edited

Loading

hashicorp-cla commented Jul 10, 2019

Elethiomel commented Jul 10, 2019

pselle left a comment

pselle commented Jul 10, 2019 •

edited

Loading

ghost commented Aug 13, 2019

Add support for user configured state object names in swift containers #17465

Add support for user configured state object names in swift containers #17465

Conversation

Elethiomel commented Feb 28, 2018

Elethiomel commented Apr 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Elethiomel left a comment

Choose a reason for hiding this comment

paultyng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Elethiomel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paultyng left a comment

Choose a reason for hiding this comment

dariakts commented Apr 29, 2019

Elethiomel commented Apr 29, 2019 via email

pselle commented Jul 9, 2019

Elethiomel commented Jul 10, 2019

pselle commented Jul 10, 2019

hashicorp-cla commented Jul 10, 2019 • edited Loading

hashicorp-cla commented Jul 10, 2019

Elethiomel commented Jul 10, 2019

pselle left a comment

Choose a reason for hiding this comment

pselle commented Jul 10, 2019 • edited Loading

ghost commented Aug 13, 2019

hashicorp-cla commented Jul 10, 2019 •

edited

Loading

pselle commented Jul 10, 2019 •

edited

Loading