Refactor plugin cache implementation #141
Conversation
bobh66
force-pushed
the
refactor_plugin_cache
branch
from
a59161c
to
d2bd0a0
Compare
There was a problem hiding this comment.
We also need to remove PluginCache from ProviderConfig in this case
https://github.com/crossplane-contrib/provider-terraform/blob/master/apis/v1alpha1/types.go#L36-L40
I wondered about that - I didn't know if that would break the existing ProviderConfig objects that have been modified to have that attribute set? Will removing the attribute definition make those objects invalid? |
|
Yeah, it will not match CRD spec anymore, not sure how it will exactly behave as it is additive field. Anyway, v1alpha1 still gives us freedom for breaking changing until we bump maturity level |
I'll build it and see what happens in our test environment |
bobh66
force-pushed
the
refactor_plugin_cache
branch
from
d2bd0a0
to
399b381
Compare
|
I pushed the updated changes - I think it will be ok as long as the provider doesn't access the attribute, which it doesn't. |
apis/v1alpha1/types.go
Outdated
| // https://developer.hashicorp.com/terraform/cli/config/config-file#provider-plugin-cache | ||
| // +optional | ||
| // +kubebuilder:default=true | ||
| PluginCache *bool `json:"pluginCache,omitempty"` |
There was a problem hiding this comment.
Can we test upgrading a running version of provider-terraform to this version to make sure this doesn't break? It's v1alpha1 but may be worth bumping to v1alpha2 and announce it as breaking change. I'd expect it to work as expected since its omitempty already but a validation would be good.
|
I think I might have a way to make this work at the providerConfig/Workspace level - we can unset the TF_PLUGIN_CACHE_DIR environment variable in the process that runs the CLI command, This is some rework but if we went to v1alpha2 that would also be more work, so does it make sense to leave the pluginCache option in place to disable the cache and modify the Init() code to unset the env variable? |
* Remove setenv of TF_PLUGIN_CACHE_DIR * Remove TF_PLUGIN_CACHE_DIR from CLI env when cache is disabled * Set TF_PLUGIN_CACHE_DIR env var in Dockerfile * Set TF_IN_AUTOMATION env var in Dockerfile Signed-off-by: Bob Haddleton <bob.haddleton@nokia.com>
bobh66
force-pushed
the
refactor_plugin_cache
branch
from
399b381
to
09c27bb
Compare
|
I refactored the changes to leave the existing pluginCache flag in ProviderConfig. The default is to use the plugin cache (it is set in the Dockerfile) but it can be disabled by setting pluginCache: false in the ProviderConfig. This removes the TF_PLUGIN_CACHE_DIR env variable from the environment before calling terraform init. I tested that the cache works with pluginCache: true and that it does not get used when pluginCache: false. |
Signed-off-by: Bob Haddleton bob.haddleton@nokia.com
Description of your changes
Fixes #139
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
Tested in a development cluster with 100+ workspaces