Migrate Manta Remote state to be a backend #16296

stack72 · 2017-10-09T18:51:35Z

This is a WIP to show the progress of this work

jwreagor · 2017-10-17T20:38:25Z

backend/init/init.go

@@ -47,6 +48,7 @@ func init() {
 			`Warning: "azure" name is deprecated, please use "azurerm"`),
 		"azurerm": func() backend.Backend { return backendAzure.New() },
 		"etcdv3":  func() backend.Backend { return backendetcdv3.New() },
+		"manta":   func() backend.Backend { return backendManta.New() },


For historical reasons this was called Manta but arguably that's an internal name for the project now. From the product side of things its Triton Object Storage (hence the reason github.com/joyent/triton-go/storage). I'm wondering if we should rename this to Triton and deprecate the Manta brand from TF completely.

Thoughts?

Just to make it known, @cheapRoc and I discussed this internally and decided to leave it as manta :)

Just in case it wasn't clear, the deprecateBackend function used just above here in the azure backend can be used to simply deprecate the old name if you want, but I'll leave the final decision up to you.

stack72 · 2017-10-19T21:14:57Z

@jbardin would love your thoughts on this please?

jbardin · 2017-10-20T12:42:10Z

Thanks @stack72, I'll take a look at it ASAP. I'd like to get this and the gcs backend conversion both in before the next beta.

jbardin

Thanks for putting the work in to update this remote state!

I have some comments inline. The backends works like the old providers, where we have to run the ACC tests manually.

jbardin · 2017-10-21T16:17:53Z

backend/init/init.go

@@ -47,6 +48,7 @@ func init() {
 			`Warning: "azure" name is deprecated, please use "azurerm"`),
 		"azurerm": func() backend.Backend { return backendAzure.New() },
 		"etcdv3":  func() backend.Backend { return backendetcdv3.New() },
+		"manta":   func() backend.Backend { return backendManta.New() },


Just in case it wasn't clear, the deprecateBackend function used just above here in the azure backend can be used to simply deprecate the old name if you want, but I'll leave the final decision up to you.

jbardin · 2017-10-21T16:24:18Z

backend/remote-state/manta/backend_state.go

+)
+
+func (b *Backend) States() ([]string, error) {
+	prefix := b.objectName + keyEnvPrefix


This looks like a breaking change for the backend if you're expecting this to upgrade from the legacy remote state, since you're adding keyEnvPrefix to all named states. I think the original remote state would be at objectName by itself, correct?

You should use a workspace-specific suffix like this for the objectName only when it's not the DefaultStateName (there's a number of ways to structure it). Just for context, the keyEnvPrefix as a directory as used in S3 was a mistake copied form the consul directory layout, but we're stuck with it.

For backends that don't need to upgrade from existing remote state, it seems to make more sense to only configure a "path", and have that be a directory containing the state files each of which are named after the respective workspace.

jbardin · 2017-10-21T16:27:33Z

backend/remote-state/manta/backend_state.go

+const (
+	// This will be used as directory name, the odd looking colon is simply to
+	// reduce the chance of name conflicts with existing objects.
+	keyEnvPrefix = "env:"


I think you want to update the comment here, and maybe the variable name, since your usage is a little different the S3.

jbardin · 2017-10-21T16:33:41Z

backend/remote-state/manta/backend_state.go

+}
+
+func (b *Backend) State(name string) (state.State, error) {
+


Here, and in DeleteState, it doesn't look like you're building the objectName with the workspace/state name.

jbardin · 2017-10-21T16:37:20Z

backend/remote-state/manta/client.go

+}
+
+func (c *RemoteClient) Lock(info *state.LockInfo) (string, error) {
+	info.Path = path.Join(c.directoryName, lockFileName)


Locks need to be per named state, so you need the final objectName in here somewhere too. I thought the tests covered conflicting lock paths, but I'll have to update them if they don't

jbardin · 2017-10-21T16:40:15Z

backend/remote-state/manta/client.go

+	contentType := "application/json"
+	contentLength := int64(len(data))
+
+	params := &storage.PutObjectInput{


I'm not familiar with manta at all. I assume manta is strongly consistent, so we can actually use this as a lock. Does the api prevent writing to an existing key though, or would this simply replace an existing lock? (the tests should cover this, but have the acc tests been run on this yet?)

Manta is a strongly consistent backend so this is correct.

stack72 · 2017-10-23T14:29:24Z

@jbardin so I took the liberty to remove anything to do with env: - it shouldn't even have been in the PR in the first place

I am working on the tests right now

stack72 · 2017-10-30T16:16:54Z

@jbardin finally done :)

terraform [manta-backend●] % acctests backend/remote-state/manta 2>~/testing-output.txt
=== RUN   TestBackend_impl
--- PASS: TestBackend_impl (0.00s)
=== RUN   TestBackend
--- PASS: TestBackend (41.38s)
=== RUN   TestBackendLocked
--- PASS: TestBackendLocked (49.91s)
	backend_test.go:63: TestBackend: testing state locking for *manta.Backend
=== RUN   TestRemoteClient_impl
--- PASS: TestRemoteClient_impl (0.00s)
=== RUN   TestRemoteClient
--- PASS: TestRemoteClient (5.10s)
=== RUN   TestRemoteClientLocks
--- PASS: TestRemoteClientLocks (10.71s)
PASS
ok  	github.com/hashicorp/terraform/backend/remote-state/manta	107.115s

This PR changes manta from being a legacy remote state client to a new backend type. This also includes creating a simple lock within manta This PR also unifies the way the triton client is configured (the schema) and also uses the same env vars to set the backend up It is important to note that if the remote state path does not exist, then the backend will create that path. This means the user doesn't need to fall into a chicken and egg situation of creating the directory in advance before interacting with it

jbardin

Thanks @stack72, this looks good!

The tests pass, so I'll work on getting this into the upcoming beta.

stack72 · 2017-11-02T13:45:58Z

Thanks for the merge here @jbardin - glad this will be part of 0.11 GA :)

ghost · 2020-04-06T02:13:13Z

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.

stack72 force-pushed the manta-backend branch from 0c4b12b to 748931c Compare October 17, 2017 20:34

jwreagor reviewed Oct 17, 2017

View reviewed changes

stack72 force-pushed the manta-backend branch 5 times, most recently from b5868ec to 8bc8fd8 Compare October 18, 2017 01:40

stack72 changed the title ~~[WIP] Migrate Manta Remote state to be a backend~~ Migrate Manta Remote state to be a backend Oct 18, 2017

stack72 force-pushed the manta-backend branch from 8bc8fd8 to 2c68eac Compare October 18, 2017 01:44

apparentlymart added the new-backend label Oct 20, 2017

jbardin reviewed Oct 21, 2017

View reviewed changes

stack72 force-pushed the manta-backend branch from 2c68eac to e2001b8 Compare October 23, 2017 14:23

stack72 force-pushed the manta-backend branch from e2001b8 to 9b18e93 Compare October 25, 2017 09:18

stack72 force-pushed the manta-backend branch from 9b18e93 to 1fd0f80 Compare October 30, 2017 16:37

jbardin approved these changes Oct 30, 2017

View reviewed changes

jbardin merged commit 229c70c into hashicorp:master Nov 1, 2017

stack72 deleted the manta-backend branch November 2, 2017 13:45

tombuildsstuff added a commit that referenced this pull request Nov 3, 2017

Updating to include #16296

84d18ad

ghost locked and limited conversation to collaborators Apr 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Manta Remote state to be a backend #16296

Migrate Manta Remote state to be a backend #16296

stack72 commented Oct 9, 2017

jwreagor Oct 17, 2017 •

edited

Loading

stack72 Oct 18, 2017

jbardin Oct 21, 2017

stack72 commented Oct 19, 2017

jbardin commented Oct 20, 2017

jbardin left a comment

jbardin Oct 21, 2017

jbardin Oct 21, 2017

jbardin Oct 21, 2017

jbardin Oct 21, 2017

jbardin Oct 21, 2017

jbardin Oct 21, 2017

sean- Oct 22, 2017

stack72 commented Oct 23, 2017

stack72 commented Oct 30, 2017

jbardin left a comment

stack72 commented Nov 2, 2017

ghost commented Apr 6, 2020

		}

		func (b *Backend) State(name string) (state.State, error) {

Migrate Manta Remote state to be a backend #16296

Migrate Manta Remote state to be a backend #16296

Conversation

stack72 commented Oct 9, 2017

jwreagor Oct 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stack72 commented Oct 19, 2017

jbardin commented Oct 20, 2017

jbardin 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

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

stack72 commented Oct 23, 2017

stack72 commented Oct 30, 2017

jbardin left a comment

Choose a reason for hiding this comment

stack72 commented Nov 2, 2017

ghost commented Apr 6, 2020

jwreagor Oct 17, 2017 •

edited

Loading