Skip to content
This repository has been archived by the owner on Dec 5, 2020. It is now read-only.

ignition: extend cache to keep files on disk to keep data intact between invocations #50

Closed
wants to merge 2 commits into from

Conversation

abhinavdahiya
Copy link

@abhinavdahiya abhinavdahiya commented May 13, 2019

With errors like issue 116 1, terraform restarts the provider server. And with terraform turning
on AutoMTLS for all plugins 2, the client cannot reattach to already running provider server 3.

Provider's memory only caches creates errors like invalid file \"49e2cc7ba023d87070d9b49a0c8f02ff0f63b7f61b390fbce8f64b011bd42ed7\", unknown file id
because the ignition_config resource tried to retrived a file id stored in cache before provider server was restarted.

This stores the objects in diskv store in users' cache directory, retireving it when the memory cache is not populated.
The in-memory cache keeps providing the no overhead access to objects.

$ make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v  -timeout 120m
?       github.com/terraform-providers/terraform-provider-ignition      [no test files]
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestIngnitionFileReplace
--- PASS: TestIngnitionFileReplace (0.03s)
=== RUN   TestIngnitionFileAppend
--- PASS: TestIngnitionFileAppend (0.03s)
=== RUN   TestIngnitionFileReplaceNoVerification
--- PASS: TestIngnitionFileReplaceNoVerification (0.03s)
=== RUN   TestIngnitionFileAppendNoVerification
--- PASS: TestIngnitionFileAppendNoVerification (0.03s)
=== RUN   TestIgnitionConfigDisks
--- PASS: TestIgnitionConfigDisks (0.04s)
=== RUN   TestIgnitionConfigArrays
--- PASS: TestIgnitionConfigArrays (0.03s)
=== RUN   TestIgnitionConfigFilesystems
--- PASS: TestIgnitionConfigFilesystems (0.04s)
=== RUN   TestIgnitionConfigFiles
--- PASS: TestIgnitionConfigFiles (0.03s)
=== RUN   TestIgnitionConfigSystemd
--- PASS: TestIgnitionConfigSystemd (0.04s)
=== RUN   TestIgnitionConfigNetworkd
--- PASS: TestIgnitionConfigNetworkd (0.03s)
=== RUN   TestIgnitionConfigUsers
--- PASS: TestIgnitionConfigUsers (0.03s)
=== RUN   TestIgnitionConfigGroupss
--- PASS: TestIgnitionConfigGroupss (0.03s)
=== RUN   TestIngnitionDirectory
--- PASS: TestIngnitionDirectory (0.03s)
=== RUN   TestIngnitionDirectoryInvalidMode
--- PASS: TestIngnitionDirectoryInvalidMode (0.01s)
=== RUN   TestIngnitionDirectoryInvalidPath
--- PASS: TestIngnitionDirectoryInvalidPath (0.01s)
=== RUN   TestIngnitionDisk
--- PASS: TestIngnitionDisk (0.03s)
=== RUN   TestIngnitionDiskInvalidDevice
--- PASS: TestIngnitionDiskInvalidDevice (0.00s)
=== RUN   TestIngnitionDiskInvalidPartition
--- PASS: TestIngnitionDiskInvalidPartition (0.01s)
=== RUN   TestIngnitionFile
--- PASS: TestIngnitionFile (0.04s)
=== RUN   TestIngnitionFileInvalidMode
--- PASS: TestIngnitionFileInvalidMode (0.01s)
=== RUN   TestIngnitionFileInvalidPath
--- PASS: TestIngnitionFileInvalidPath (0.01s)
=== RUN   TestIngnitionFilesystem
--- PASS: TestIngnitionFilesystem (0.05s)
=== RUN   TestIngnitionFilesystemInvalidPath
--- PASS: TestIngnitionFilesystemInvalidPath (0.01s)
=== RUN   TestIngnitionFilesystemInvalidPathAndMount
--- PASS: TestIngnitionFilesystemInvalidPathAndMount (0.01s)
=== RUN   TestIngnitionGroup
--- PASS: TestIngnitionGroup (0.04s)
=== RUN   TestIngnitionLink
--- PASS: TestIngnitionLink (0.03s)
=== RUN   TestIngnitionLinkInvalidPath
--- PASS: TestIngnitionLinkInvalidPath (0.00s)
=== RUN   TestIngnitionNetworkdUnit
--- PASS: TestIngnitionNetworkdUnit (0.03s)
=== RUN   TestIngnitionRaid
--- PASS: TestIngnitionRaid (0.03s)
=== RUN   TestIngnitionRaidInvalidLevel
--- PASS: TestIngnitionRaidInvalidLevel (0.01s)
=== RUN   TestIngnitionRaidInvalidDevices
--- PASS: TestIngnitionRaidInvalidDevices (0.00s)
=== RUN   TestIngnitionSystemdUnit
--- PASS: TestIngnitionSystemdUnit (0.03s)
=== RUN   TestIngnitionSystemdUnitEmptyContentWithDropIn
--- PASS: TestIngnitionSystemdUnitEmptyContentWithDropIn (0.04s)
=== RUN   TestIgnitionSystemdUnit_emptyContent
--- PASS: TestIgnitionSystemdUnit_emptyContent (0.04s)
=== RUN   TestIngnitionSystemUnitInvalidName
--- PASS: TestIngnitionSystemUnitInvalidName (0.01s)
=== RUN   TestIngnitionSystemUnitInvalidContent
--- PASS: TestIngnitionSystemUnitInvalidContent (0.01s)
=== RUN   TestIngnitionUser
--- PASS: TestIngnitionUser (0.05s)
PASS
ok      github.com/terraform-providers/terraform-provider-ignition/ignition     0.918s

- go get github.com/peterbourgon/diskv/v3
- go mod tidy && go mod vendor
abhinavdahiya added a commit to abhinavdahiya/installer that referenced this pull request May 13, 2019
…een invocations

With errors like issue 116 [1], terraform restarts the provider server. And with terraform turning
on `AutoMTLS` for all plugins [2], the client cannot reattach to already running provider server [3].

Provider's memory only caches creates errors like `invalid file \"49e2cc7ba023d87070d9b49a0c8f02ff0f63b7f61b390fbce8f64b011bd42ed7\", unknown file id`
because the ignition_config resource tried to retrived a file id stored in cache before provider server was restarted.

This stores the objects in `diskv` store in users' cache directory, retireving it when the memory cache is not populated.
The in-memory cache keeps providing the no overhead access to objects.

[1]: hashicorp/go-plugin#116
[2]: hashicorp/terraform#19560
[3]: https://github.com/hashicorp/go-plugin/blob/5692942914bbdbc03558fde936b1f0bc2af365be/client.go#L203-L204
abhinavdahiya added a commit to abhinavdahiya/installer that referenced this pull request May 14, 2019
abhinavdahiya added a commit to abhinavdahiya/installer that referenced this pull request May 14, 2019
…ted 0.12 terraform source

This commit changes the source for 2 providers `openstack` and `ignition` because:

- openstack
PR 753 [1] moves the openstack provider to use 0.12 terraform libraries.

- ignition

PR 50 [2] is required because old implementation of ignition provider stores objects in memory and with terraform 0.12
errors regarding to reading from plugins servers forces new procress to be created due to AutoMTLS. Check the PR description for more info [3].

[1]: https://github.com/terraform-providers/terraform-provider-openstack/pull/753
[2]: hashicorp/terraform-provider-ignition#50
abhinavdahiya added a commit to abhinavdahiya/installer that referenced this pull request May 15, 2019
…ted 0.12 terraform source

This commit changes the source for 2 providers `openstack` and `ignition` because:

- openstack
PR 753 [1] moves the openstack provider to use 0.12 terraform libraries.

- ignition

PR 50 [2] is required because old implementation of ignition provider stores objects in memory and with terraform 0.12
errors regarding to reading from plugins servers forces new procress to be created due to AutoMTLS. Check the PR description for more info [3].

[1]: https://github.com/terraform-providers/terraform-provider-openstack/pull/753
[2]: hashicorp/terraform-provider-ignition#50
@meyskens meyskens mentioned this pull request Jul 1, 2019
}
}

func providerConfigure(d *schema.ResourceData) (interface{}, error) {
basePath := filepath.Join(os.TempDir(), "terraform-provider-ignition")
Copy link

Choose a reason for hiding this comment

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

Were it possible for more than one Terraform run to be underway at the same time, they would collide here, trying to write to the same files under this path. Consider using ioutil.TempDir instead.

func providerConfigure(d *schema.ResourceData) (interface{}, error) {
basePath := filepath.Join(os.TempDir(), "terraform-provider-ignition")

globalCache.d = diskv.New(diskv.Options{
Copy link

Choose a reason for hiding this comment

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

Given that hashicorp/terraform#6258 remains open, is it correct that we never delete this directory and the files within it? (There is a (*Diskv).EraseAll method.)

@seh seh mentioned this pull request Aug 27, 2019
@seh
Copy link

seh commented Aug 27, 2019

I tried this patch, and found that it worked with Terraform version 0.12.7. Per my comment above in file ignition/provider.go, there's no cleanup of the files written by diskv, so it appears that they'll accumulate without bound over time, until rebooting the machine purges the temporary directory. Can you think of a tenable approach to curbing that growth?

I take it we can't purge these files when the program starts, because we may be restarting in the interest of recovering the cache left behind from a previous—and very recent—run of the program.

@seh
Copy link

seh commented Oct 31, 2019

Now that #56 is committed, it doesn't seem likely that we'll accept this patch, given that it's a different approach to solve the same problem. Shall we abandon this patch?

@LorbusChris
Copy link
Contributor

I believe this issue can be closed now.

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.

3 participants