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

Store resolved providers in state #16586

Merged
merged 16 commits into from
Nov 8, 2017
Merged

Store resolved providers in state #16586

merged 16 commits into from
Nov 8, 2017

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Nov 8, 2017

While resolving provider inheritance completely in the config was conceptually much simpler, the situation where there is no config breaks that implementation. However, now that the provider handling has been simplified in core, moving the inheritance back is much simpler.

This further simplifies the provider handling by removing the dummy provider nodes, the disabled nodes, and combining some of the transformers. A key enhancement here is that resources are now bound directly to the provider instance they logically use, rather than requiring all the providers to be instantiated at each path level.

The bulk of this diff is rewriting all the test state string comparisons to include the now mandatory Provider fields. Most of the core functionality is in 49e6ecf and b9b4912.

maving all inheritance into core
Now that resources can be connected to providers with different paths in
the core graph, handling the inheritance in config makes less sense.

Removing this to make room for core to walk the Tree and connect
resources directly to the proper provider instance.
Implement the adding of provider through the module/providers map in the
configuration.

The way this works is that we start walking the module tree from the
top, and for any instance of a provider that can accept a configuration
through the parent's module/provider map, we add a proxy node that
provides the real name and a pointer to the actual parent provider node.
Multiple proxies can be chained back to the original provider.  When
connecting resources to providers, if that provider is a proxy, we can
then connect the resource directly to the proxied node. The proxies are
later removed by the DisabledProviderTransformer.

This should re-instate the 0.11 beta inheritance behavior, but will
allow us to later store the actual concrete provider used by a resource,
so that it can be re-connected if it's orphaned by removing its module
configuration.
Here we complete the passing of providers between modules via the
module/providers configuration, add another test and update broken test
outputs.

The DisbableProviderTransformer is being removed, since it was really
only for provider configuration inheritance. Since configuration is no
longer inherited, there's no need to keep around unused providers. The
actually shouldn't be any unused providers going into the graph any
longer, but put off verifying that condition for later.  Replace it's
usage with the PruneProviderTransformer, and use that to also remove the
unneeded proxy provider nodes.
Use the ResourceState.Provider field to store the full name of the
provider used during apply. This field is only used when a resource is
removed from the config, and will allow that resource to be removed by
the exact same provider with which it was created.

Modify the locations which might accept the alue of the
ResourceState.Provider field to detect that the name is resolved.
Now that the resolved provider is always stored in state, we need to
udpate all the test data to match. There will probably be some more
breakage once the provider field is properly diffed.
And update the test state strings

Destroying with no config is no longer allowed, run an exlpicit destroy
for the destroyOrder test.
and update the test state strings
and update the test state strings
@jbardin jbardin merged commit 00b7715 into master Nov 8, 2017
@jbardin jbardin deleted the jbardin/providers branch November 8, 2017 19:27
@ghost
Copy link

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

2 participants