Skip to content

Commit

Permalink
[engine] Only record a resource's chosen alias. (#9288)
Browse files Browse the repository at this point in the history 
As we discovered when removing aliases from the state entirely, the
snapshotter needs to be alias-aware so that it can fix up references to
resources that were aliased. After a resource operation finishes, the
snapshotter needs to write out a new copy of the snapshot. However, at
the time we write the snapshot, there may be resources that have not yet
been registered that refer to the just-registered resources by a
different URN due to aliasing. Those references need to be fixed up
prior to writing the snapshot in order to preserve the snapshot's
integrity (in particular, the property that all URNs refer to resources
that exist in the snapshot).

For example, consider the following simple dependency graph: A <-- B.
When that graph is serialized, B will contain a reference to A in its
dependency list. Let the next run of the program produces the graph A'
<-- B where A' is aliased to A. After A' is registered, the snapshotter
needs to write a snapshot that contains its state, but B must also be
updated so it references A' instead of A, which will no longer be in the
snapshot.

These changes take advantage of the fact that although a resource can
provide multiple aliases, it can only ever resolve those aliases to a
single resource in the existing state. Therefore, at the time the
statefile is fixed up, each resource in the statefile could only have
been aliased to a single old resource, and it is sufficient to store
only the URN of the chosen resource rather than all possible aliases. In
addition to preserving the ability to fix up references to aliased
resources, retaining the chosen alias allows the history of a logical
resource to be followed across aliases.
  • Loading branch information
@pgavlin
pgavlin committed Mar 28, 2022
1 parent dc6ac2f commit bac1149
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 46 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG_PENDING.md
View file
@@ -1,5 +1,10 @@
### Improvements

- When a resource is aliased to an existing resource with a different URN, only store
the alias of the existing resource in the statefile rather than storing all possible
aliases.
[#9288](https://github.com/pulumi/pulumi/pull/9288)

- Clear pending operations during `pulumi refresh` or `pulumi up -r`.
[#8435](https://github.com/pulumi/pulumi/pull/8435)

Expand Down
11 changes: 8 additions & 3 deletions pkg/engine/journal.go
View file
Expand Up @@ -28,7 +28,7 @@ type JournalEntry struct {

type JournalEntries []JournalEntry

func (entries JournalEntries) Snap(base *deploy.Snapshot) *deploy.Snapshot {
func (entries JournalEntries) Snap(base *deploy.Snapshot) (*deploy.Snapshot, error) {
// Build up a list of current resources by replaying the journal.
resources, dones := []*resource.State{}, make(map[*resource.State]bool)
ops, doneOps := []resource.Operation{}, make(map[*resource.State]bool)
Expand Down Expand Up @@ -130,8 +130,13 @@ func (entries JournalEntries) Snap(base *deploy.Snapshot) *deploy.Snapshot {

manifest := deploy.Manifest{}
manifest.Magic = manifest.NewMagic()
return deploy.NewSnapshot(manifest, secretsManager, resources, operations)

snap := deploy.NewSnapshot(manifest, secretsManager, resources, operations)
err := snap.NormalizeURNReferences()
if err != nil {
return snap, err
}
return snap, snap.VerifyIntegrity()
}

type Journal struct {
Expand Down Expand Up @@ -189,7 +194,7 @@ func (j *Journal) RecordPlugin(plugin workspace.PluginInfo) error {
return nil
}

func (j *Journal) Snap(base *deploy.Snapshot) *deploy.Snapshot {
func (j *Journal) Snap(base *deploy.Snapshot) (*deploy.Snapshot, error) {
return j.entries.Snap(base)
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/engine/lifeycletest/golang_sdk_test.go
View file
Expand Up @@ -250,7 +250,10 @@ func TestSingleResourceDefaultProviderGolangTransformations(t *testing.T) {
foundRes3 := false
foundRes4Child := false
// foundRes5Child1 := false
for _, res := range entries.Snap(target.Snapshot).Resources {

snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
for _, res := range snap.Resources {
// "res1" has a transformation which adds additionalSecretOutputs
if res.URN.Name() == "res1" {
foundRes1 = true
Expand Down
8 changes: 6 additions & 2 deletions pkg/engine/lifeycletest/provider_test.go
View file
Expand Up @@ -133,7 +133,9 @@ func TestSingleResourceDefaultProviderUpgrade(t *testing.T) {
t.Fatalf("unexpected resource %v", urn)
}
}
assert.Len(t, entries.Snap(target.Snapshot).Resources, 2)
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, 2)
return res
}

Expand Down Expand Up @@ -166,7 +168,9 @@ func TestSingleResourceDefaultProviderUpgrade(t *testing.T) {
}
}
assert.Len(t, deleted, 2)
assert.Len(t, entries.Snap(target.Snapshot).Resources, 0)
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, 0)
return res
},
}}
Expand Down
58 changes: 27 additions & 31 deletions pkg/engine/lifeycletest/pulumi_test.go
View file
Expand Up @@ -1302,7 +1302,7 @@ func TestAliases(t *testing.T) {
}

updateProgramWithResource := func(
snap *deploy.Snapshot, resources []Resource, allowedOps []deploy.StepOp) *deploy.Snapshot {
snap *deploy.Snapshot, resources []Resource, allowedOps []deploy.StepOp, expectFailure bool) *deploy.Snapshot {
program := deploytest.NewLanguageRuntime(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
err := registerResources(t, monitor, resources)
return err
Expand All @@ -1312,7 +1312,8 @@ func TestAliases(t *testing.T) {
Options: UpdateOptions{Host: host},
Steps: []TestStep{
{
Op: Update,
Op: Update,
ExpectFailure: expectFailure,
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
events []Event, res result.Result) result.Result {
for _, event := range events {
Expand Down Expand Up @@ -1347,14 +1348,14 @@ func TestAliases(t *testing.T) {
snap := updateProgramWithResource(nil, []Resource{{
t: "pkgA:index:t1",
name: "n1",
}}, []deploy.StepOp{deploy.OpCreate})
}}, []deploy.StepOp{deploy.OpCreate}, false)

// Ensure that rename produces Same
snap = updateProgramWithResource(snap, []Resource{{
t: "pkgA:index:t1",
name: "n2",
aliases: []resource.URN{"urn:pulumi:test::test::pkgA:index:t1::n1"},
}}, []deploy.StepOp{deploy.OpSame})
}}, []deploy.StepOp{deploy.OpSame}, false)

// Ensure that rename produces Same with multiple aliases
snap = updateProgramWithResource(snap, []Resource{{
Expand All @@ -1364,7 +1365,7 @@ func TestAliases(t *testing.T) {
"urn:pulumi:test::test::pkgA:index:t1::n1",
"urn:pulumi:test::test::pkgA:index:t1::n2",
},
}}, []deploy.StepOp{deploy.OpSame})
}}, []deploy.StepOp{deploy.OpSame}, false)

// Ensure that rename produces Same with multiple aliases (reversed)
snap = updateProgramWithResource(snap, []Resource{{
Expand All @@ -1374,7 +1375,7 @@ func TestAliases(t *testing.T) {
"urn:pulumi:test::test::pkgA:index:t1::n2",
"urn:pulumi:test::test::pkgA:index:t1::n1",
},
}}, []deploy.StepOp{deploy.OpSame})
}}, []deploy.StepOp{deploy.OpSame}, false)

// Ensure that aliasing back to original name is okay
snap = updateProgramWithResource(snap, []Resource{{
Expand All @@ -1385,13 +1386,13 @@ func TestAliases(t *testing.T) {
"urn:pulumi:test::test::pkgA:index:t1::n2",
"urn:pulumi:test::test::pkgA:index:t1::n1",
},
}}, []deploy.StepOp{deploy.OpSame})
}}, []deploy.StepOp{deploy.OpSame}, false)

// Ensure that removing aliases is okay (once old names are gone from all snapshots)
snap = updateProgramWithResource(snap, []Resource{{
t: "pkgA:index:t1",
name: "n1",
}}, []deploy.StepOp{deploy.OpSame})
}}, []deploy.StepOp{deploy.OpSame}, false)

// Ensure that changing the type works
snap = updateProgramWithResource(snap, []Resource{{
Expand All @@ -1400,7 +1401,7 @@ func TestAliases(t *testing.T) {
aliases: []resource.URN{
"urn:pulumi:test::test::pkgA:index:t1::n1",
},
}}, []deploy.StepOp{deploy.OpSame})
}}, []deploy.StepOp{deploy.OpSame}, false)

// Ensure that changing the type again works
snap = updateProgramWithResource(snap, []Resource{{
Expand All @@ -1410,7 +1411,7 @@ func TestAliases(t *testing.T) {
"urn:pulumi:test::test::pkgA:index:t1::n1",
"urn:pulumi:test::test::pkgA:index:t2::n1",
},
}}, []deploy.StepOp{deploy.OpSame})
}}, []deploy.StepOp{deploy.OpSame}, false)

// Ensure that order of aliases doesn't matter
snap = updateProgramWithResource(snap, []Resource{{
Expand All @@ -1421,13 +1422,13 @@ func TestAliases(t *testing.T) {
"urn:pulumi:test::test::pkgA:othermod:t3::n1",
"urn:pulumi:test::test::pkgA:index:t2::n1",
},
}}, []deploy.StepOp{deploy.OpSame})
}}, []deploy.StepOp{deploy.OpSame}, false)

// Ensure that removing aliases is okay (once old names are gone from all snapshots)
snap = updateProgramWithResource(snap, []Resource{{
t: "pkgA:othermod:t3",
name: "n1",
}}, []deploy.StepOp{deploy.OpSame})
}}, []deploy.StepOp{deploy.OpSame}, false)

// Ensure that changing everything (including props) leads to update not delete and re-create
snap = updateProgramWithResource(snap, []Resource{{
Expand All @@ -1439,7 +1440,7 @@ func TestAliases(t *testing.T) {
aliases: []resource.URN{
"urn:pulumi:test::test::pkgA:othermod:t3::n1",
},
}}, []deploy.StepOp{deploy.OpUpdate})
}}, []deploy.StepOp{deploy.OpUpdate}, false)

// Ensure that changing everything again (including props) leads to update not delete and re-create
snap = updateProgramWithResource(snap, []Resource{{
Expand All @@ -1451,7 +1452,7 @@ func TestAliases(t *testing.T) {
aliases: []resource.URN{
"urn:pulumi:test::test::pkgA:index:t4::n2",
},
}}, []deploy.StepOp{deploy.OpUpdate})
}}, []deploy.StepOp{deploy.OpUpdate}, false)

// Ensure that changing a forceNew property while also changing type and name leads to replacement not delete+create
snap = updateProgramWithResource(snap, []Resource{{
Expand All @@ -1463,7 +1464,7 @@ func TestAliases(t *testing.T) {
aliases: []resource.URN{
"urn:pulumi:test::test::pkgA:index:t5::n3",
},
}}, []deploy.StepOp{deploy.OpReplace, deploy.OpCreateReplacement, deploy.OpDeleteReplaced})
}}, []deploy.StepOp{deploy.OpReplace, deploy.OpCreateReplacement, deploy.OpDeleteReplaced}, false)

// Ensure that changing a forceNew property and deleteBeforeReplace while also changing type and name leads to
// replacement not delete+create
Expand All @@ -1477,7 +1478,7 @@ func TestAliases(t *testing.T) {
aliases: []resource.URN{
"urn:pulumi:test::test::pkgA:index:t6::n4",
},
}}, []deploy.StepOp{deploy.OpReplace, deploy.OpCreateReplacement, deploy.OpDeleteReplaced})
}}, []deploy.StepOp{deploy.OpReplace, deploy.OpCreateReplacement, deploy.OpDeleteReplaced}, false)

// Start again - this time with two resources with depends on relationship
snap = updateProgramWithResource(nil, []Resource{{
Expand All @@ -1491,7 +1492,7 @@ func TestAliases(t *testing.T) {
t: "pkgA:index:t2",
name: "n2",
dependencies: []resource.URN{"urn:pulumi:test::test::pkgA:index:t1::n1"},
}}, []deploy.StepOp{deploy.OpCreate})
}}, []deploy.StepOp{deploy.OpCreate}, false)

_ = updateProgramWithResource(snap, []Resource{{
t: "pkgA:index:t1-new",
Expand All @@ -1510,7 +1511,7 @@ func TestAliases(t *testing.T) {
aliases: []resource.URN{
"urn:pulumi:test::test::pkgA:index:t2::n2",
},
}}, []deploy.StepOp{deploy.OpSame, deploy.OpReplace, deploy.OpCreateReplacement, deploy.OpDeleteReplaced})
}}, []deploy.StepOp{deploy.OpSame, deploy.OpReplace, deploy.OpCreateReplacement, deploy.OpDeleteReplaced}, false)

// Start again - this time with two resources with parent relationship
snap = updateProgramWithResource(nil, []Resource{{
Expand All @@ -1524,7 +1525,7 @@ func TestAliases(t *testing.T) {
t: "pkgA:index:t2",
name: "n2",
parent: resource.URN("urn:pulumi:test::test::pkgA:index:t1::n1"),
}}, []deploy.StepOp{deploy.OpCreate})
}}, []deploy.StepOp{deploy.OpCreate}, false)

_ = updateProgramWithResource(snap, []Resource{{
t: "pkgA:index:t1-new",
Expand All @@ -1543,27 +1544,22 @@ func TestAliases(t *testing.T) {
aliases: []resource.URN{
"urn:pulumi:test::test::pkgA:index:t1$pkgA:index:t2::n2",
},
}}, []deploy.StepOp{deploy.OpSame, deploy.OpReplace, deploy.OpCreateReplacement, deploy.OpDeleteReplaced})
}}, []deploy.StepOp{deploy.OpSame, deploy.OpReplace, deploy.OpCreateReplacement, deploy.OpDeleteReplaced}, false)

// ensure failure when different resources use duplicate aliases
snap = updateProgramWithResource(nil, []Resource{{
_ = updateProgramWithResource(snap, []Resource{{
t: "pkgA:index:t1",
name: "n1",
name: "n2",
aliases: []resource.URN{
"urn:pulumi:test::test::pkgA:index:t1::n1",
},
}, {
t: "pkgA:index:t2",
name: "n2",
name: "n3",
aliases: []resource.URN{
"urn:pulumi:test::test::pkgA:index:t1::n1",
},
}}, []deploy.StepOp{deploy.OpCreate})

err := snap.NormalizeURNReferences()
assert.Equal(t, err.Error(),
"Two resources ('urn:pulumi:test::test::pkgA:index:t1::n1'"+
" and 'urn:pulumi:test::test::pkgA:index:t2::n2') aliased to the same: 'urn:pulumi:test::test::pkgA:index:t1::n1'")
}}, []deploy.StepOp{deploy.OpCreate}, true)

// ensure different resources can use different aliases
snap = updateProgramWithResource(nil, []Resource{{
Expand All @@ -1578,9 +1574,9 @@ func TestAliases(t *testing.T) {
aliases: []resource.URN{
"urn:pulumi:test::test::pkgA:index:t1::n2",
},
}}, []deploy.StepOp{deploy.OpCreate})
}}, []deploy.StepOp{deploy.OpCreate}, false)

err = snap.NormalizeURNReferences()
err := snap.NormalizeURNReferences()
assert.Nil(t, err)
}

Expand Down
31 changes: 23 additions & 8 deletions pkg/engine/lifeycletest/test_plan.go
View file
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/mitchellh/copystructure"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

. "github.com/pulumi/pulumi/pkg/v3/engine"
"github.com/pulumi/pulumi/pkg/v3/resource/deploy"
Expand Down Expand Up @@ -121,8 +122,10 @@ func (op TestOp) runWithContext(
return plan, nil, res
}

snap := journal.Snap(target.Snapshot)
if res == nil && snap != nil {
snap, err := journal.Snap(target.Snapshot)
if res == nil && err != nil {
res = result.FromError(err)
} else if res == nil && snap != nil {
res = result.WrapIfNonNil(snap.VerifyIntegrity())
}
return nil, snap, res
Expand Down Expand Up @@ -290,7 +293,9 @@ func MakeBasicLifecycleSteps(t *testing.T, resCount int) []TestStep {
op := entry.Step.Op()
assert.True(t, op == deploy.OpCreate || op == deploy.OpRead)
}
assert.Len(t, entries.Snap(target.Snapshot).Resources, resCount)
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, resCount)
return res
},
},
Expand All @@ -305,7 +310,9 @@ func MakeBasicLifecycleSteps(t *testing.T, resCount int) []TestStep {
assert.Equal(t, deploy.OpRefresh, entry.Step.Op())
assert.Equal(t, deploy.OpSame, entry.Step.(*deploy.RefreshStep).ResultOp())
}
assert.Len(t, entries.Snap(target.Snapshot).Resources, resCount)
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, resCount)
return res
},
},
Expand All @@ -320,7 +327,9 @@ func MakeBasicLifecycleSteps(t *testing.T, resCount int) []TestStep {
op := entry.Step.Op()
assert.True(t, op == deploy.OpSame || op == deploy.OpRead)
}
assert.Len(t, entries.Snap(target.Snapshot).Resources, resCount)
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, resCount)
return res
},
},
Expand All @@ -335,7 +344,9 @@ func MakeBasicLifecycleSteps(t *testing.T, resCount int) []TestStep {
assert.Equal(t, deploy.OpRefresh, entry.Step.Op())
assert.Equal(t, deploy.OpSame, entry.Step.(*deploy.RefreshStep).ResultOp())
}
assert.Len(t, entries.Snap(target.Snapshot).Resources, resCount)
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, resCount)
return res
},
},
Expand All @@ -354,7 +365,9 @@ func MakeBasicLifecycleSteps(t *testing.T, resCount int) []TestStep {
assert.Fail(t, "expected OpDelete or OpReadDiscard")
}
}
assert.Len(t, entries.Snap(target.Snapshot).Resources, 0)
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, 0)
return res
},
},
Expand All @@ -365,7 +378,9 @@ func MakeBasicLifecycleSteps(t *testing.T, resCount int) []TestStep {
_ []Event, res result.Result) result.Result {

assert.Len(t, entries, 0)
assert.Len(t, entries.Snap(target.Snapshot).Resources, 0)
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, 0)
return res
},
},
Expand Down

0 comments on commit bac1149

Please sign in to comment.