Skip to content

Commit

Permalink
Avoid configuring providers twice during preview (#4004)
Browse files Browse the repository at this point in the history
In the very common case where provider configuration does not change, during preview we were calling `Configure` on the cloud provider twice - once for the "old" configuration, and once for the "new" configuration.

This is not necessary, and we can just avoid using the new provider when configuration has not changed, since we will have configured the old provider very early so if we can use that we should.

Note that this technically doesn't prevent the second call to `Configure` from being made, but it prevents us from ever waiting on it.  We may want to go further and avoid even calling `Configure` on the provider in this case.

Part of #3671.
  • Loading branch information
Luke Hoban authored Mar 1, 2020
1 parent 60eeff3 commit 2067e27
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ CHANGELOG
=========

## HEAD (unreleased)
- Avoid Configuring providers which are not used during preview.
[#4004](https://github.com/pulumi/pulumi/pull/4004)

- Fix missing module import on Windows platform.
[#3983](https://github.com/pulumi/pulumi/pull/3983)

Expand Down
8 changes: 4 additions & 4 deletions pkg/resource/deploy/providers/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,12 +298,12 @@ func (r *Registry) Diff(urn resource.URN, id resource.ID, olds, news resource.Pr

// If the diff requires replacement, unload the provider: the engine will reload it during its replacememnt Check.
//
// If the diff does not require replacement and we are running a preview, register it under its current ID so that
// references to the provider from other resources will resolve properly.
if len(diff.ReplaceKeys) != 0 {
// If the diff does not require replacement but does have changes, and we are running a preview, register it under
// its current ID so that references to the provider from other resources will resolve properly.
if diff.Replace() {
closeErr := r.host.CloseProvider(provider)
contract.IgnoreError(closeErr)
} else if r.isPreview {
} else if r.isPreview && diff.Changes == plugin.DiffSome {
r.setProvider(mustNewReference(urn, id), provider)
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/resource/deploy/providers/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,11 +615,11 @@ func TestCRUDPreview(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, plugin.DiffResult{}, diff)

// The new provider should be registered.
// The original provider should be used because the config did not change.
p2, ok := r.GetProvider(Reference{urn: urn, id: id})
assert.True(t, ok)
assert.False(t, p2 == old)
assert.True(t, p2 == p)
assert.True(t, p2 == old)
assert.False(t, p2 == p)
}

// Replace the existing provider for the last entry in olds.
Expand Down

0 comments on commit 2067e27

Please sign in to comment.