2020-12-15 22:24:46 +00:00
|
|
|
package lifecycletest
|
|
|
|
|
|
|
|
import (
|
|
|
|
"context"
|
|
|
|
"fmt"
|
|
|
|
"reflect"
|
|
|
|
"strconv"
|
|
|
|
"testing"
|
|
|
|
|
|
|
|
"github.com/blang/semver"
|
|
|
|
combinations "github.com/mxschmitt/golang-combinations"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
|
|
|
2023-11-21 15:16:13 +00:00
|
|
|
. "github.com/pulumi/pulumi/pkg/v3/engine" //nolint:revive
|
2021-03-17 13:20:05 +00:00
|
|
|
"github.com/pulumi/pulumi/pkg/v3/resource/deploy"
|
|
|
|
"github.com/pulumi/pulumi/pkg/v3/resource/deploy/deploytest"
|
|
|
|
"github.com/pulumi/pulumi/pkg/v3/resource/deploy/providers"
|
|
|
|
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
|
|
|
|
"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
|
|
|
|
"github.com/pulumi/pulumi/sdk/v3/go/common/workspace"
|
2020-12-15 22:24:46 +00:00
|
|
|
)
|
|
|
|
|
|
|
|
func TestParallelRefresh(t *testing.T) {
|
2022-03-04 08:17:41 +00:00
|
|
|
t.Parallel()
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
|
|
|
// Create a program that registers four resources, each of which depends on the resource that immediately precedes
|
|
|
|
// it.
|
2023-09-28 21:50:18 +00:00
|
|
|
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
|
2024-04-19 11:08:56 +00:00
|
|
|
respA, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.NoError(t, err)
|
|
|
|
|
2024-04-19 11:08:56 +00:00
|
|
|
respB, err := monitor.RegisterResource("pkgA:m:typA", "resB", true, deploytest.ResourceOptions{
|
|
|
|
Dependencies: []resource.URN{respA.URN},
|
2020-12-15 22:24:46 +00:00
|
|
|
})
|
|
|
|
assert.NoError(t, err)
|
|
|
|
|
2024-04-19 11:08:56 +00:00
|
|
|
respC, err := monitor.RegisterResource("pkgA:m:typA", "resC", true, deploytest.ResourceOptions{
|
|
|
|
Dependencies: []resource.URN{respB.URN},
|
2020-12-15 22:24:46 +00:00
|
|
|
})
|
|
|
|
assert.NoError(t, err)
|
|
|
|
|
2024-04-19 11:08:56 +00:00
|
|
|
_, err = monitor.RegisterResource("pkgA:m:typA", "resD", true, deploytest.ResourceOptions{
|
|
|
|
Dependencies: []resource.URN{respC.URN},
|
2020-12-15 22:24:46 +00:00
|
|
|
})
|
|
|
|
assert.NoError(t, err)
|
|
|
|
|
|
|
|
return nil
|
|
|
|
})
|
2023-09-28 21:50:18 +00:00
|
|
|
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
p := &TestPlan{
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
Options: TestUpdateOptions{
|
|
|
|
T: t,
|
|
|
|
HostF: hostF,
|
|
|
|
UpdateOptions: UpdateOptions{Parallel: 4},
|
|
|
|
// Skip display tests because different ordering makes the colouring different.
|
|
|
|
SkipDisplayTests: true,
|
|
|
|
},
|
2020-12-15 22:24:46 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
p.Steps = []TestStep{{Op: Update}}
|
|
|
|
snap := p.Run(t, nil)
|
|
|
|
|
|
|
|
assert.Len(t, snap.Resources, 5)
|
2023-11-20 08:59:00 +00:00
|
|
|
assert.Equal(t, snap.Resources[0].URN.Name(), "default") // provider
|
|
|
|
assert.Equal(t, snap.Resources[1].URN.Name(), "resA")
|
|
|
|
assert.Equal(t, snap.Resources[2].URN.Name(), "resB")
|
|
|
|
assert.Equal(t, snap.Resources[3].URN.Name(), "resC")
|
|
|
|
assert.Equal(t, snap.Resources[4].URN.Name(), "resD")
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
p.Steps = []TestStep{{Op: Refresh}}
|
|
|
|
snap = p.Run(t, snap)
|
|
|
|
|
|
|
|
assert.Len(t, snap.Resources, 5)
|
2023-11-20 08:59:00 +00:00
|
|
|
assert.Equal(t, snap.Resources[0].URN.Name(), "default") // provider
|
|
|
|
assert.Equal(t, snap.Resources[1].URN.Name(), "resA")
|
|
|
|
assert.Equal(t, snap.Resources[2].URN.Name(), "resB")
|
|
|
|
assert.Equal(t, snap.Resources[3].URN.Name(), "resC")
|
|
|
|
assert.Equal(t, snap.Resources[4].URN.Name(), "resD")
|
2020-12-15 22:24:46 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
func TestExternalRefresh(t *testing.T) {
|
2022-03-04 08:17:41 +00:00
|
|
|
t.Parallel()
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
|
|
|
// Our program reads a resource and exits.
|
2023-09-28 21:50:18 +00:00
|
|
|
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
|
2024-07-26 10:03:18 +00:00
|
|
|
_, _, err := monitor.ReadResource("pkgA:m:typA", "resA", "resA-some-id", "", resource.PropertyMap{}, "", "", "", "")
|
2020-12-15 22:24:46 +00:00
|
|
|
if !assert.NoError(t, err) {
|
|
|
|
t.FailNow()
|
|
|
|
}
|
|
|
|
|
|
|
|
return nil
|
|
|
|
})
|
2023-09-28 21:50:18 +00:00
|
|
|
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
|
2020-12-15 22:24:46 +00:00
|
|
|
p := &TestPlan{
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
Options: TestUpdateOptions{T: t, HostF: hostF},
|
2020-12-15 22:24:46 +00:00
|
|
|
Steps: []TestStep{{Op: Update}},
|
|
|
|
}
|
|
|
|
|
|
|
|
// The read should place "resA" in the snapshot with the "External" bit set.
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
snap := p.RunWithName(t, nil, "0")
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.Len(t, snap.Resources, 2)
|
2023-11-20 08:59:00 +00:00
|
|
|
assert.Equal(t, snap.Resources[0].URN.Name(), "default") // provider
|
|
|
|
assert.Equal(t, snap.Resources[1].URN.Name(), "resA")
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.True(t, snap.Resources[1].External)
|
|
|
|
|
|
|
|
p = &TestPlan{
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
Options: TestUpdateOptions{T: t, HostF: hostF},
|
2020-12-15 22:24:46 +00:00
|
|
|
Steps: []TestStep{{Op: Refresh}},
|
|
|
|
}
|
|
|
|
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
snap = p.RunWithName(t, snap, "1")
|
2020-12-15 22:24:46 +00:00
|
|
|
// A refresh should leave "resA" as it is in the snapshot. The External bit should still be set.
|
|
|
|
assert.Len(t, snap.Resources, 2)
|
2023-11-20 08:59:00 +00:00
|
|
|
assert.Equal(t, snap.Resources[0].URN.Name(), "default") // provider
|
|
|
|
assert.Equal(t, snap.Resources[1].URN.Name(), "resA")
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.True(t, snap.Resources[1].External)
|
|
|
|
}
|
|
|
|
|
Don't call `Diff` when refreshing external resources (#16544)
External resources are resources whose state is tracked but not managed.
That is, they feature in the state file, but Pulumi does not issue
create, read or update operations for them. Typically they arise as the
result of generated `.get` functions and methods in Pulumi programs
(these should not be confused with specific `getX` functions/methods,
which are backed by `Invoke`s and designed to reflect data
sources/arbitrary function calls).
In #16146, `refresh` operations were modified to use actual bonafide
`Diff` calls to compute differences in a manner that would reflect "what
will happen on the next `preview`". In general, this aligns better with
what users expect when running `refresh`, and fixes a number of issues
that have long plagued `refresh`. However, it falls down in the case of
external resources. The existing `Diff` contract takes (somewhat
strangely) _three_ key inputs:
* The old inputs ("what the program used to say")
* The old outputs ("what we got back from the last update")
* The new inputs ("what the program says now")
Intuitively, `Diff`'s job is to return the difference between the old
and new inputs. Aside from old outputs being a bit of a third wheel,
this contract makes sense for an `update` operation, where the inputs
are driving the outputs and changes there will yield changes elsewhere.
In a `refresh` operation, however, we have a different situation -- one
in which _both_ old and new inputs _and_ outputs are available. Alas, we
don't currently have a "four-argument `Diff`" (or a two-argument one we
can call twice with no fear of repercussions), and so the usage in
`refresh` is a little bit of a clever hack that ends up biasing inputs
over outputs (see #16146 or the implementation for details).
For external resources, the old behaviour (where we just compared
outputs) seems like a better fit. This commit therefore adds this
special case and a test to ensure that when we see external resources,
we simply compare outputs and don't call `Diff`.
Fixes #16401
Fixes #16502
---------
Co-authored-by: Thomas Gummerer <t.gummerer@gmail.com>
2024-07-02 10:11:10 +00:00
|
|
|
// External resources should only have their outputs diffed during a refresh, in
|
|
|
|
// line with the "legacy" implementation. Consequently Diff should not be called
|
|
|
|
// for them. This test checks that case.
|
|
|
|
func TestExternalRefreshDoesNotCallDiff(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
readCall := 0
|
|
|
|
diffCalled := false
|
|
|
|
|
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{
|
2024-07-26 12:14:45 +00:00
|
|
|
ReadF: func(_ context.Context, req plugin.ReadRequest) (plugin.ReadResponse, error) {
|
Don't call `Diff` when refreshing external resources (#16544)
External resources are resources whose state is tracked but not managed.
That is, they feature in the state file, but Pulumi does not issue
create, read or update operations for them. Typically they arise as the
result of generated `.get` functions and methods in Pulumi programs
(these should not be confused with specific `getX` functions/methods,
which are backed by `Invoke`s and designed to reflect data
sources/arbitrary function calls).
In #16146, `refresh` operations were modified to use actual bonafide
`Diff` calls to compute differences in a manner that would reflect "what
will happen on the next `preview`". In general, this aligns better with
what users expect when running `refresh`, and fixes a number of issues
that have long plagued `refresh`. However, it falls down in the case of
external resources. The existing `Diff` contract takes (somewhat
strangely) _three_ key inputs:
* The old inputs ("what the program used to say")
* The old outputs ("what we got back from the last update")
* The new inputs ("what the program says now")
Intuitively, `Diff`'s job is to return the difference between the old
and new inputs. Aside from old outputs being a bit of a third wheel,
this contract makes sense for an `update` operation, where the inputs
are driving the outputs and changes there will yield changes elsewhere.
In a `refresh` operation, however, we have a different situation -- one
in which _both_ old and new inputs _and_ outputs are available. Alas, we
don't currently have a "four-argument `Diff`" (or a two-argument one we
can call twice with no fear of repercussions), and so the usage in
`refresh` is a little bit of a clever hack that ends up biasing inputs
over outputs (see #16146 or the implementation for details).
For external resources, the old behaviour (where we just compared
outputs) seems like a better fit. This commit therefore adds this
special case and a test to ensure that when we see external resources,
we simply compare outputs and don't call `Diff`.
Fixes #16401
Fixes #16502
---------
Co-authored-by: Thomas Gummerer <t.gummerer@gmail.com>
2024-07-02 10:11:10 +00:00
|
|
|
readCall++
|
2024-07-26 12:14:45 +00:00
|
|
|
return plugin.ReadResponse{
|
|
|
|
ReadResult: plugin.ReadResult{
|
|
|
|
Outputs: resource.PropertyMap{
|
|
|
|
"o1": resource.NewNumberProperty(float64(readCall)),
|
|
|
|
},
|
Don't call `Diff` when refreshing external resources (#16544)
External resources are resources whose state is tracked but not managed.
That is, they feature in the state file, but Pulumi does not issue
create, read or update operations for them. Typically they arise as the
result of generated `.get` functions and methods in Pulumi programs
(these should not be confused with specific `getX` functions/methods,
which are backed by `Invoke`s and designed to reflect data
sources/arbitrary function calls).
In #16146, `refresh` operations were modified to use actual bonafide
`Diff` calls to compute differences in a manner that would reflect "what
will happen on the next `preview`". In general, this aligns better with
what users expect when running `refresh`, and fixes a number of issues
that have long plagued `refresh`. However, it falls down in the case of
external resources. The existing `Diff` contract takes (somewhat
strangely) _three_ key inputs:
* The old inputs ("what the program used to say")
* The old outputs ("what we got back from the last update")
* The new inputs ("what the program says now")
Intuitively, `Diff`'s job is to return the difference between the old
and new inputs. Aside from old outputs being a bit of a third wheel,
this contract makes sense for an `update` operation, where the inputs
are driving the outputs and changes there will yield changes elsewhere.
In a `refresh` operation, however, we have a different situation -- one
in which _both_ old and new inputs _and_ outputs are available. Alas, we
don't currently have a "four-argument `Diff`" (or a two-argument one we
can call twice with no fear of repercussions), and so the usage in
`refresh` is a little bit of a clever hack that ends up biasing inputs
over outputs (see #16146 or the implementation for details).
For external resources, the old behaviour (where we just compared
outputs) seems like a better fit. This commit therefore adds this
special case and a test to ensure that when we see external resources,
we simply compare outputs and don't call `Diff`.
Fixes #16401
Fixes #16502
---------
Co-authored-by: Thomas Gummerer <t.gummerer@gmail.com>
2024-07-02 10:11:10 +00:00
|
|
|
},
|
2024-07-26 12:14:45 +00:00
|
|
|
Status: resource.StatusOK,
|
|
|
|
}, nil
|
Don't call `Diff` when refreshing external resources (#16544)
External resources are resources whose state is tracked but not managed.
That is, they feature in the state file, but Pulumi does not issue
create, read or update operations for them. Typically they arise as the
result of generated `.get` functions and methods in Pulumi programs
(these should not be confused with specific `getX` functions/methods,
which are backed by `Invoke`s and designed to reflect data
sources/arbitrary function calls).
In #16146, `refresh` operations were modified to use actual bonafide
`Diff` calls to compute differences in a manner that would reflect "what
will happen on the next `preview`". In general, this aligns better with
what users expect when running `refresh`, and fixes a number of issues
that have long plagued `refresh`. However, it falls down in the case of
external resources. The existing `Diff` contract takes (somewhat
strangely) _three_ key inputs:
* The old inputs ("what the program used to say")
* The old outputs ("what we got back from the last update")
* The new inputs ("what the program says now")
Intuitively, `Diff`'s job is to return the difference between the old
and new inputs. Aside from old outputs being a bit of a third wheel,
this contract makes sense for an `update` operation, where the inputs
are driving the outputs and changes there will yield changes elsewhere.
In a `refresh` operation, however, we have a different situation -- one
in which _both_ old and new inputs _and_ outputs are available. Alas, we
don't currently have a "four-argument `Diff`" (or a two-argument one we
can call twice with no fear of repercussions), and so the usage in
`refresh` is a little bit of a clever hack that ends up biasing inputs
over outputs (see #16146 or the implementation for details).
For external resources, the old behaviour (where we just compared
outputs) seems like a better fit. This commit therefore adds this
special case and a test to ensure that when we see external resources,
we simply compare outputs and don't call `Diff`.
Fixes #16401
Fixes #16502
---------
Co-authored-by: Thomas Gummerer <t.gummerer@gmail.com>
2024-07-02 10:11:10 +00:00
|
|
|
},
|
|
|
|
|
2024-07-26 12:14:45 +00:00
|
|
|
DiffF: func(context.Context, plugin.DiffRequest) (plugin.DiffResult, error) {
|
Don't call `Diff` when refreshing external resources (#16544)
External resources are resources whose state is tracked but not managed.
That is, they feature in the state file, but Pulumi does not issue
create, read or update operations for them. Typically they arise as the
result of generated `.get` functions and methods in Pulumi programs
(these should not be confused with specific `getX` functions/methods,
which are backed by `Invoke`s and designed to reflect data
sources/arbitrary function calls).
In #16146, `refresh` operations were modified to use actual bonafide
`Diff` calls to compute differences in a manner that would reflect "what
will happen on the next `preview`". In general, this aligns better with
what users expect when running `refresh`, and fixes a number of issues
that have long plagued `refresh`. However, it falls down in the case of
external resources. The existing `Diff` contract takes (somewhat
strangely) _three_ key inputs:
* The old inputs ("what the program used to say")
* The old outputs ("what we got back from the last update")
* The new inputs ("what the program says now")
Intuitively, `Diff`'s job is to return the difference between the old
and new inputs. Aside from old outputs being a bit of a third wheel,
this contract makes sense for an `update` operation, where the inputs
are driving the outputs and changes there will yield changes elsewhere.
In a `refresh` operation, however, we have a different situation -- one
in which _both_ old and new inputs _and_ outputs are available. Alas, we
don't currently have a "four-argument `Diff`" (or a two-argument one we
can call twice with no fear of repercussions), and so the usage in
`refresh` is a little bit of a clever hack that ends up biasing inputs
over outputs (see #16146 or the implementation for details).
For external resources, the old behaviour (where we just compared
outputs) seems like a better fit. This commit therefore adds this
special case and a test to ensure that when we see external resources,
we simply compare outputs and don't call `Diff`.
Fixes #16401
Fixes #16502
---------
Co-authored-by: Thomas Gummerer <t.gummerer@gmail.com>
2024-07-02 10:11:10 +00:00
|
|
|
diffCalled = true
|
|
|
|
return plugin.DiffResult{}, nil
|
|
|
|
},
|
|
|
|
}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
|
|
|
// Our program reads a resource and exits.
|
|
|
|
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
|
2024-07-26 10:03:18 +00:00
|
|
|
_, _, err := monitor.ReadResource("pkgA:m:typA", "resA", "resA-some-id", "", resource.PropertyMap{}, "", "", "", "")
|
Don't call `Diff` when refreshing external resources (#16544)
External resources are resources whose state is tracked but not managed.
That is, they feature in the state file, but Pulumi does not issue
create, read or update operations for them. Typically they arise as the
result of generated `.get` functions and methods in Pulumi programs
(these should not be confused with specific `getX` functions/methods,
which are backed by `Invoke`s and designed to reflect data
sources/arbitrary function calls).
In #16146, `refresh` operations were modified to use actual bonafide
`Diff` calls to compute differences in a manner that would reflect "what
will happen on the next `preview`". In general, this aligns better with
what users expect when running `refresh`, and fixes a number of issues
that have long plagued `refresh`. However, it falls down in the case of
external resources. The existing `Diff` contract takes (somewhat
strangely) _three_ key inputs:
* The old inputs ("what the program used to say")
* The old outputs ("what we got back from the last update")
* The new inputs ("what the program says now")
Intuitively, `Diff`'s job is to return the difference between the old
and new inputs. Aside from old outputs being a bit of a third wheel,
this contract makes sense for an `update` operation, where the inputs
are driving the outputs and changes there will yield changes elsewhere.
In a `refresh` operation, however, we have a different situation -- one
in which _both_ old and new inputs _and_ outputs are available. Alas, we
don't currently have a "four-argument `Diff`" (or a two-argument one we
can call twice with no fear of repercussions), and so the usage in
`refresh` is a little bit of a clever hack that ends up biasing inputs
over outputs (see #16146 or the implementation for details).
For external resources, the old behaviour (where we just compared
outputs) seems like a better fit. This commit therefore adds this
special case and a test to ensure that when we see external resources,
we simply compare outputs and don't call `Diff`.
Fixes #16401
Fixes #16502
---------
Co-authored-by: Thomas Gummerer <t.gummerer@gmail.com>
2024-07-02 10:11:10 +00:00
|
|
|
if !assert.NoError(t, err) {
|
|
|
|
t.FailNow()
|
|
|
|
}
|
|
|
|
|
|
|
|
return nil
|
|
|
|
})
|
|
|
|
|
|
|
|
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
|
|
|
|
p := &TestPlan{
|
|
|
|
Options: TestUpdateOptions{T: t, HostF: hostF},
|
|
|
|
Steps: []TestStep{{Op: Update}},
|
|
|
|
}
|
|
|
|
|
|
|
|
snap := p.RunWithName(t, nil, "0")
|
|
|
|
|
|
|
|
p = &TestPlan{
|
|
|
|
Options: TestUpdateOptions{T: t, HostF: hostF},
|
|
|
|
Steps: []TestStep{{Op: Refresh}},
|
|
|
|
}
|
|
|
|
|
|
|
|
p.RunWithName(t, snap, "1")
|
|
|
|
|
|
|
|
assert.False(t, diffCalled, "Refresh should not diff external resources")
|
|
|
|
}
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
func TestRefreshInitFailure(t *testing.T) {
|
2022-03-04 08:17:41 +00:00
|
|
|
t.Parallel()
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
p := &TestPlan{}
|
|
|
|
|
|
|
|
provURN := p.NewProviderURN("pkgA", "default", "")
|
|
|
|
resURN := p.NewURN("pkgA:m:typA", "resA", "")
|
|
|
|
res2URN := p.NewURN("pkgA:m:typA", "resB", "")
|
|
|
|
|
|
|
|
res2Outputs := resource.PropertyMap{"foo": resource.NewStringProperty("bar")}
|
|
|
|
|
|
|
|
//
|
|
|
|
// Refresh will persist any initialization errors that are returned by `Read`. This provider
|
|
|
|
// will error out or not based on the value of `refreshShouldFail`.
|
|
|
|
//
|
|
|
|
refreshShouldFail := false
|
|
|
|
|
|
|
|
//
|
|
|
|
// Set up test environment to use `readFailProvider` as the underlying resource provider.
|
|
|
|
//
|
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{
|
2024-07-26 12:14:45 +00:00
|
|
|
ReadF: func(_ context.Context, req plugin.ReadRequest) (plugin.ReadResponse, error) {
|
|
|
|
if refreshShouldFail && req.URN == resURN {
|
2020-12-15 22:24:46 +00:00
|
|
|
err := &plugin.InitError{
|
|
|
|
Reasons: []string{"Refresh reports continued to fail to initialize"},
|
|
|
|
}
|
2024-07-26 12:14:45 +00:00
|
|
|
return plugin.ReadResponse{
|
|
|
|
ReadResult: plugin.ReadResult{
|
|
|
|
Outputs: resource.PropertyMap{},
|
|
|
|
},
|
|
|
|
Status: resource.StatusPartialFailure,
|
|
|
|
}, err
|
|
|
|
} else if req.URN == res2URN {
|
|
|
|
return plugin.ReadResponse{
|
|
|
|
ReadResult: plugin.ReadResult{
|
|
|
|
Outputs: res2Outputs,
|
|
|
|
},
|
|
|
|
Status: resource.StatusOK,
|
|
|
|
}, nil
|
2020-12-15 22:24:46 +00:00
|
|
|
}
|
2024-07-26 12:14:45 +00:00
|
|
|
return plugin.ReadResponse{
|
|
|
|
ReadResult: plugin.ReadResult{
|
|
|
|
Outputs: resource.PropertyMap{},
|
|
|
|
},
|
|
|
|
Status: resource.StatusOK,
|
|
|
|
}, nil
|
2020-12-15 22:24:46 +00:00
|
|
|
},
|
|
|
|
}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
2023-09-28 21:50:18 +00:00
|
|
|
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
|
2024-04-19 11:08:56 +00:00
|
|
|
_, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.NoError(t, err)
|
|
|
|
return nil
|
|
|
|
})
|
2023-09-28 21:50:18 +00:00
|
|
|
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
|
2020-12-15 22:24:46 +00:00
|
|
|
|
2023-09-28 21:50:18 +00:00
|
|
|
p.Options.HostF = hostF
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
p.Options.T = t
|
2020-12-15 22:24:46 +00:00
|
|
|
//
|
|
|
|
// Create an old snapshot with a single initialization failure.
|
|
|
|
//
|
|
|
|
old := &deploy.Snapshot{
|
|
|
|
Resources: []*resource.State{
|
|
|
|
{
|
|
|
|
Type: resURN.Type(),
|
|
|
|
URN: resURN,
|
|
|
|
Custom: true,
|
|
|
|
ID: "0",
|
|
|
|
Inputs: resource.PropertyMap{},
|
|
|
|
Outputs: resource.PropertyMap{},
|
|
|
|
InitErrors: []string{"Resource failed to initialize"},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
Type: res2URN.Type(),
|
|
|
|
URN: res2URN,
|
|
|
|
Custom: true,
|
|
|
|
ID: "1",
|
|
|
|
Inputs: resource.PropertyMap{},
|
|
|
|
Outputs: resource.PropertyMap{},
|
|
|
|
},
|
|
|
|
},
|
|
|
|
}
|
|
|
|
|
|
|
|
//
|
|
|
|
// Refresh DOES NOT fail, causing the initialization error to disappear.
|
|
|
|
//
|
|
|
|
p.Steps = []TestStep{{Op: Refresh}}
|
|
|
|
snap := p.Run(t, old)
|
|
|
|
|
|
|
|
for _, resource := range snap.Resources {
|
|
|
|
switch urn := resource.URN; urn {
|
|
|
|
case provURN:
|
|
|
|
// break
|
|
|
|
case resURN:
|
|
|
|
assert.Empty(t, resource.InitErrors)
|
|
|
|
case res2URN:
|
|
|
|
assert.Equal(t, res2Outputs, resource.Outputs)
|
|
|
|
default:
|
|
|
|
t.Fatalf("unexpected resource %v", urn)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
//
|
|
|
|
// Refresh again, see the resource is in a partial state of failure, but the refresh operation
|
|
|
|
// DOES NOT fail. The initialization error is still persisted.
|
|
|
|
//
|
|
|
|
refreshShouldFail = true
|
|
|
|
p.Steps = []TestStep{{Op: Refresh, SkipPreview: true}}
|
|
|
|
snap = p.Run(t, old)
|
|
|
|
for _, resource := range snap.Resources {
|
|
|
|
switch urn := resource.URN; urn {
|
|
|
|
case provURN:
|
|
|
|
// break
|
|
|
|
case resURN:
|
|
|
|
assert.Equal(t, []string{"Refresh reports continued to fail to initialize"}, resource.InitErrors)
|
|
|
|
case res2URN:
|
|
|
|
assert.Equal(t, res2Outputs, resource.Outputs)
|
|
|
|
default:
|
|
|
|
t.Fatalf("unexpected resource %v", urn)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// Test that tests that Refresh can detect that resources have been deleted and removes them
|
|
|
|
// from the snapshot.
|
|
|
|
func TestRefreshWithDelete(t *testing.T) {
|
2022-03-04 08:17:41 +00:00
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
//nolint:paralleltest // false positive because range var isn't used directly in t.Run(name) arg
|
2024-08-28 13:45:17 +00:00
|
|
|
for _, parallelFactor := range []int32{1, 4} {
|
2022-03-04 08:17:41 +00:00
|
|
|
parallelFactor := parallelFactor
|
2020-12-15 22:24:46 +00:00
|
|
|
t.Run(fmt.Sprintf("parallel-%d", parallelFactor), func(t *testing.T) {
|
2022-03-04 08:17:41 +00:00
|
|
|
t.Parallel()
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{
|
2024-07-26 12:14:45 +00:00
|
|
|
ReadF: func(_ context.Context, req plugin.ReadRequest) (plugin.ReadResponse, error) {
|
2020-12-15 22:24:46 +00:00
|
|
|
// This thing doesn't exist. Returning nil from Read should trigger
|
|
|
|
// the engine to delete it from the snapshot.
|
2024-07-26 12:14:45 +00:00
|
|
|
return plugin.ReadResponse{}, nil
|
2020-12-15 22:24:46 +00:00
|
|
|
},
|
|
|
|
}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
2023-09-28 21:50:18 +00:00
|
|
|
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
|
2024-04-19 11:08:56 +00:00
|
|
|
_, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.NoError(t, err)
|
|
|
|
return err
|
|
|
|
})
|
|
|
|
|
2023-09-28 21:50:18 +00:00
|
|
|
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
p := &TestPlan{
|
|
|
|
Options: TestUpdateOptions{
|
|
|
|
T: t,
|
|
|
|
// Skip display tests because different ordering makes the colouring different.
|
|
|
|
SkipDisplayTests: true,
|
|
|
|
HostF: hostF,
|
|
|
|
UpdateOptions: UpdateOptions{Parallel: parallelFactor},
|
|
|
|
},
|
|
|
|
}
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
p.Steps = []TestStep{{Op: Update}}
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
snap := p.RunWithName(t, nil, "0")
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
p.Steps = []TestStep{{Op: Refresh}}
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
snap = p.RunWithName(t, snap, "1")
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
// Refresh succeeds and records that the resource in the snapshot doesn't exist anymore
|
|
|
|
provURN := p.NewProviderURN("pkgA", "default", "")
|
|
|
|
assert.Len(t, snap.Resources, 1)
|
|
|
|
assert.Equal(t, provURN, snap.Resources[0].URN)
|
|
|
|
})
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// Tests that dependencies are correctly rewritten when refresh removes deleted resources.
|
|
|
|
func TestRefreshDeleteDependencies(t *testing.T) {
|
2022-03-04 08:17:41 +00:00
|
|
|
t.Parallel()
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
names := []string{"resA", "resB", "resC"}
|
|
|
|
|
|
|
|
// Try refreshing a stack with every combination of the three above resources as a target to
|
|
|
|
// refresh.
|
|
|
|
subsets := combinations.All(names)
|
|
|
|
|
|
|
|
// combinations.All doesn't return the empty set. So explicitly test that case (i.e. test no
|
|
|
|
// targets specified)
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
validateRefreshDeleteCombination(t, names, []string{}, "pre")
|
2020-12-15 22:24:46 +00:00
|
|
|
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
for i, subset := range subsets {
|
|
|
|
validateRefreshDeleteCombination(t, names, subset, strconv.Itoa(i))
|
2020-12-15 22:24:46 +00:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
Better handle property dependencies and `deletedWith` (#16088)
A resource `A` depends on a resource `B` if:
1. `B` is `A`'s `Provider`.
2. `B` is `A`'s `Parent`.
3. `B` appears in `A`'s `Dependencies`.
4. `B` appears in one or more of `A`'s `PropertyDependencies`.
5. `B` is referenced by `A`'s `DeletedWith` field.
While cases 1, 2, and 3 (providers, parents, and dependencies) are
handled fairly consistently, there have been a number of cases where the
newer features of `PropertyDependencies` (case 4) and `DeletedWith`
(case 5) have been neglected. This commit addresses some of these
omissions. Specifically:
* When refreshing state, it's important that we remove URNs that point
to resources that we've identified as deleted. Presently we check
pointers to parents and dependencies, but not property dependencies or
`deletedWith`. This commit fixes these gaps where dangling URNs could
lurk.
* Linked to the above, this commit extends snapshot integrity checks to
include property dependencies and `deletedWith`. Some tests that
previously used now invalid states have to be repaired or removed as a
result of this.
* Fixing snapshot integrity checking reveals that dependency graph
checks also fail to consider property dependencies and `deletedWith`.
This probably hasn't bitten us since property dependencies are currently
rolled up into dependencies (for temporary backwards compatibility) and
`deletedWith` is typically an optimisation (moreover, one that only
matters during deletes), so operations being parallelised due a
perceived lack of dependency still succeed. However, tests that
previously passed now fail as we can spot these races with our better
integrity checks. This commit thus fixes up dependency graph
construction and bulks out its test suite to cover the new cases.
These bugs were discovered as part of the investigation into #16052,
though they may not be directly responsible for it (though the issues
with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
|
|
|
func TestRefreshDeletePropertyDependencies(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{
|
2024-07-26 12:14:45 +00:00
|
|
|
ReadF: func(_ context.Context, req plugin.ReadRequest) (plugin.ReadResponse, error) {
|
|
|
|
if req.URN.Name() == "resA" {
|
|
|
|
return plugin.ReadResponse{}, nil
|
Better handle property dependencies and `deletedWith` (#16088)
A resource `A` depends on a resource `B` if:
1. `B` is `A`'s `Provider`.
2. `B` is `A`'s `Parent`.
3. `B` appears in `A`'s `Dependencies`.
4. `B` appears in one or more of `A`'s `PropertyDependencies`.
5. `B` is referenced by `A`'s `DeletedWith` field.
While cases 1, 2, and 3 (providers, parents, and dependencies) are
handled fairly consistently, there have been a number of cases where the
newer features of `PropertyDependencies` (case 4) and `DeletedWith`
(case 5) have been neglected. This commit addresses some of these
omissions. Specifically:
* When refreshing state, it's important that we remove URNs that point
to resources that we've identified as deleted. Presently we check
pointers to parents and dependencies, but not property dependencies or
`deletedWith`. This commit fixes these gaps where dangling URNs could
lurk.
* Linked to the above, this commit extends snapshot integrity checks to
include property dependencies and `deletedWith`. Some tests that
previously used now invalid states have to be repaired or removed as a
result of this.
* Fixing snapshot integrity checking reveals that dependency graph
checks also fail to consider property dependencies and `deletedWith`.
This probably hasn't bitten us since property dependencies are currently
rolled up into dependencies (for temporary backwards compatibility) and
`deletedWith` is typically an optimisation (moreover, one that only
matters during deletes), so operations being parallelised due a
perceived lack of dependency still succeed. However, tests that
previously passed now fail as we can spot these races with our better
integrity checks. This commit thus fixes up dependency graph
construction and bulks out its test suite to cover the new cases.
These bugs were discovered as part of the investigation into #16052,
though they may not be directly responsible for it (though the issues
with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
|
|
|
}
|
|
|
|
|
2024-07-26 12:14:45 +00:00
|
|
|
return plugin.ReadResponse{
|
|
|
|
ReadResult: plugin.ReadResult{Outputs: resource.PropertyMap{}},
|
|
|
|
}, nil
|
Better handle property dependencies and `deletedWith` (#16088)
A resource `A` depends on a resource `B` if:
1. `B` is `A`'s `Provider`.
2. `B` is `A`'s `Parent`.
3. `B` appears in `A`'s `Dependencies`.
4. `B` appears in one or more of `A`'s `PropertyDependencies`.
5. `B` is referenced by `A`'s `DeletedWith` field.
While cases 1, 2, and 3 (providers, parents, and dependencies) are
handled fairly consistently, there have been a number of cases where the
newer features of `PropertyDependencies` (case 4) and `DeletedWith`
(case 5) have been neglected. This commit addresses some of these
omissions. Specifically:
* When refreshing state, it's important that we remove URNs that point
to resources that we've identified as deleted. Presently we check
pointers to parents and dependencies, but not property dependencies or
`deletedWith`. This commit fixes these gaps where dangling URNs could
lurk.
* Linked to the above, this commit extends snapshot integrity checks to
include property dependencies and `deletedWith`. Some tests that
previously used now invalid states have to be repaired or removed as a
result of this.
* Fixing snapshot integrity checking reveals that dependency graph
checks also fail to consider property dependencies and `deletedWith`.
This probably hasn't bitten us since property dependencies are currently
rolled up into dependencies (for temporary backwards compatibility) and
`deletedWith` is typically an optimisation (moreover, one that only
matters during deletes), so operations being parallelised due a
perceived lack of dependency still succeed. However, tests that
previously passed now fail as we can spot these races with our better
integrity checks. This commit thus fixes up dependency graph
construction and bulks out its test suite to cover the new cases.
These bugs were discovered as part of the investigation into #16052,
though they may not be directly responsible for it (though the issues
with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
|
|
|
},
|
|
|
|
}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
|
|
|
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
|
|
|
|
resA, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
|
|
|
|
assert.NoError(t, err)
|
|
|
|
|
|
|
|
_, err = monitor.RegisterResource("pkgA:m:typA", "resB", true, deploytest.ResourceOptions{
|
|
|
|
PropertyDeps: map[resource.PropertyKey][]resource.URN{
|
|
|
|
"propB1": {resA.URN},
|
|
|
|
},
|
|
|
|
})
|
|
|
|
|
|
|
|
assert.NoError(t, err)
|
|
|
|
return nil
|
|
|
|
})
|
|
|
|
|
|
|
|
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
|
|
|
|
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
p := &TestPlan{Options: TestUpdateOptions{T: t, HostF: hostF}}
|
Better handle property dependencies and `deletedWith` (#16088)
A resource `A` depends on a resource `B` if:
1. `B` is `A`'s `Provider`.
2. `B` is `A`'s `Parent`.
3. `B` appears in `A`'s `Dependencies`.
4. `B` appears in one or more of `A`'s `PropertyDependencies`.
5. `B` is referenced by `A`'s `DeletedWith` field.
While cases 1, 2, and 3 (providers, parents, and dependencies) are
handled fairly consistently, there have been a number of cases where the
newer features of `PropertyDependencies` (case 4) and `DeletedWith`
(case 5) have been neglected. This commit addresses some of these
omissions. Specifically:
* When refreshing state, it's important that we remove URNs that point
to resources that we've identified as deleted. Presently we check
pointers to parents and dependencies, but not property dependencies or
`deletedWith`. This commit fixes these gaps where dangling URNs could
lurk.
* Linked to the above, this commit extends snapshot integrity checks to
include property dependencies and `deletedWith`. Some tests that
previously used now invalid states have to be repaired or removed as a
result of this.
* Fixing snapshot integrity checking reveals that dependency graph
checks also fail to consider property dependencies and `deletedWith`.
This probably hasn't bitten us since property dependencies are currently
rolled up into dependencies (for temporary backwards compatibility) and
`deletedWith` is typically an optimisation (moreover, one that only
matters during deletes), so operations being parallelised due a
perceived lack of dependency still succeed. However, tests that
previously passed now fail as we can spot these races with our better
integrity checks. This commit thus fixes up dependency graph
construction and bulks out its test suite to cover the new cases.
These bugs were discovered as part of the investigation into #16052,
though they may not be directly responsible for it (though the issues
with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
|
|
|
|
|
|
|
p.Steps = []TestStep{{Op: Update}}
|
|
|
|
snap := p.Run(t, nil)
|
|
|
|
|
|
|
|
assert.Len(t, snap.Resources, 3)
|
|
|
|
assert.Equal(t, snap.Resources[0].URN.Name(), "default") // provider
|
|
|
|
assert.Equal(t, snap.Resources[1].URN.Name(), "resA")
|
|
|
|
assert.Equal(t, snap.Resources[2].URN.Name(), "resB")
|
|
|
|
assert.Equal(t, snap.Resources[2].PropertyDependencies["propB1"][0].Name(), "resA")
|
|
|
|
|
|
|
|
err := snap.VerifyIntegrity()
|
|
|
|
assert.NoError(t, err)
|
|
|
|
|
|
|
|
p.Steps = []TestStep{{Op: Refresh}}
|
|
|
|
snap = p.Run(t, snap)
|
|
|
|
|
|
|
|
assert.Len(t, snap.Resources, 2)
|
|
|
|
assert.Equal(t, snap.Resources[0].URN.Name(), "default") // provider
|
|
|
|
assert.Equal(t, snap.Resources[1].URN.Name(), "resB")
|
|
|
|
assert.Empty(t, snap.Resources[1].PropertyDependencies)
|
|
|
|
|
|
|
|
err = snap.VerifyIntegrity()
|
|
|
|
assert.NoError(t, err)
|
|
|
|
}
|
|
|
|
|
|
|
|
func TestRefreshDeleteDeletedWith(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{
|
2024-07-26 12:14:45 +00:00
|
|
|
ReadF: func(_ context.Context, req plugin.ReadRequest) (plugin.ReadResponse, error) {
|
|
|
|
if req.URN.Name() == "resA" {
|
|
|
|
return plugin.ReadResponse{}, nil
|
Better handle property dependencies and `deletedWith` (#16088)
A resource `A` depends on a resource `B` if:
1. `B` is `A`'s `Provider`.
2. `B` is `A`'s `Parent`.
3. `B` appears in `A`'s `Dependencies`.
4. `B` appears in one or more of `A`'s `PropertyDependencies`.
5. `B` is referenced by `A`'s `DeletedWith` field.
While cases 1, 2, and 3 (providers, parents, and dependencies) are
handled fairly consistently, there have been a number of cases where the
newer features of `PropertyDependencies` (case 4) and `DeletedWith`
(case 5) have been neglected. This commit addresses some of these
omissions. Specifically:
* When refreshing state, it's important that we remove URNs that point
to resources that we've identified as deleted. Presently we check
pointers to parents and dependencies, but not property dependencies or
`deletedWith`. This commit fixes these gaps where dangling URNs could
lurk.
* Linked to the above, this commit extends snapshot integrity checks to
include property dependencies and `deletedWith`. Some tests that
previously used now invalid states have to be repaired or removed as a
result of this.
* Fixing snapshot integrity checking reveals that dependency graph
checks also fail to consider property dependencies and `deletedWith`.
This probably hasn't bitten us since property dependencies are currently
rolled up into dependencies (for temporary backwards compatibility) and
`deletedWith` is typically an optimisation (moreover, one that only
matters during deletes), so operations being parallelised due a
perceived lack of dependency still succeed. However, tests that
previously passed now fail as we can spot these races with our better
integrity checks. This commit thus fixes up dependency graph
construction and bulks out its test suite to cover the new cases.
These bugs were discovered as part of the investigation into #16052,
though they may not be directly responsible for it (though the issues
with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
|
|
|
}
|
|
|
|
|
2024-07-26 12:14:45 +00:00
|
|
|
return plugin.ReadResponse{
|
|
|
|
ReadResult: plugin.ReadResult{Outputs: resource.PropertyMap{}},
|
|
|
|
Status: resource.StatusOK,
|
|
|
|
}, nil
|
Better handle property dependencies and `deletedWith` (#16088)
A resource `A` depends on a resource `B` if:
1. `B` is `A`'s `Provider`.
2. `B` is `A`'s `Parent`.
3. `B` appears in `A`'s `Dependencies`.
4. `B` appears in one or more of `A`'s `PropertyDependencies`.
5. `B` is referenced by `A`'s `DeletedWith` field.
While cases 1, 2, and 3 (providers, parents, and dependencies) are
handled fairly consistently, there have been a number of cases where the
newer features of `PropertyDependencies` (case 4) and `DeletedWith`
(case 5) have been neglected. This commit addresses some of these
omissions. Specifically:
* When refreshing state, it's important that we remove URNs that point
to resources that we've identified as deleted. Presently we check
pointers to parents and dependencies, but not property dependencies or
`deletedWith`. This commit fixes these gaps where dangling URNs could
lurk.
* Linked to the above, this commit extends snapshot integrity checks to
include property dependencies and `deletedWith`. Some tests that
previously used now invalid states have to be repaired or removed as a
result of this.
* Fixing snapshot integrity checking reveals that dependency graph
checks also fail to consider property dependencies and `deletedWith`.
This probably hasn't bitten us since property dependencies are currently
rolled up into dependencies (for temporary backwards compatibility) and
`deletedWith` is typically an optimisation (moreover, one that only
matters during deletes), so operations being parallelised due a
perceived lack of dependency still succeed. However, tests that
previously passed now fail as we can spot these races with our better
integrity checks. This commit thus fixes up dependency graph
construction and bulks out its test suite to cover the new cases.
These bugs were discovered as part of the investigation into #16052,
though they may not be directly responsible for it (though the issues
with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
|
|
|
},
|
|
|
|
}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
|
|
|
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
|
|
|
|
resA, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
|
|
|
|
assert.NoError(t, err)
|
|
|
|
|
|
|
|
_, err = monitor.RegisterResource("pkgA:m:typA", "resB", true, deploytest.ResourceOptions{
|
|
|
|
DeletedWith: resA.URN,
|
|
|
|
})
|
|
|
|
|
|
|
|
assert.NoError(t, err)
|
|
|
|
return nil
|
|
|
|
})
|
|
|
|
|
|
|
|
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
|
|
|
|
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
p := &TestPlan{Options: TestUpdateOptions{T: t, HostF: hostF}}
|
Better handle property dependencies and `deletedWith` (#16088)
A resource `A` depends on a resource `B` if:
1. `B` is `A`'s `Provider`.
2. `B` is `A`'s `Parent`.
3. `B` appears in `A`'s `Dependencies`.
4. `B` appears in one or more of `A`'s `PropertyDependencies`.
5. `B` is referenced by `A`'s `DeletedWith` field.
While cases 1, 2, and 3 (providers, parents, and dependencies) are
handled fairly consistently, there have been a number of cases where the
newer features of `PropertyDependencies` (case 4) and `DeletedWith`
(case 5) have been neglected. This commit addresses some of these
omissions. Specifically:
* When refreshing state, it's important that we remove URNs that point
to resources that we've identified as deleted. Presently we check
pointers to parents and dependencies, but not property dependencies or
`deletedWith`. This commit fixes these gaps where dangling URNs could
lurk.
* Linked to the above, this commit extends snapshot integrity checks to
include property dependencies and `deletedWith`. Some tests that
previously used now invalid states have to be repaired or removed as a
result of this.
* Fixing snapshot integrity checking reveals that dependency graph
checks also fail to consider property dependencies and `deletedWith`.
This probably hasn't bitten us since property dependencies are currently
rolled up into dependencies (for temporary backwards compatibility) and
`deletedWith` is typically an optimisation (moreover, one that only
matters during deletes), so operations being parallelised due a
perceived lack of dependency still succeed. However, tests that
previously passed now fail as we can spot these races with our better
integrity checks. This commit thus fixes up dependency graph
construction and bulks out its test suite to cover the new cases.
These bugs were discovered as part of the investigation into #16052,
though they may not be directly responsible for it (though the issues
with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
|
|
|
|
|
|
|
p.Steps = []TestStep{{Op: Update}}
|
|
|
|
snap := p.Run(t, nil)
|
|
|
|
|
|
|
|
assert.Len(t, snap.Resources, 3)
|
|
|
|
assert.Equal(t, snap.Resources[0].URN.Name(), "default") // provider
|
|
|
|
assert.Equal(t, snap.Resources[1].URN.Name(), "resA")
|
|
|
|
assert.Equal(t, snap.Resources[2].URN.Name(), "resB")
|
|
|
|
assert.Equal(t, snap.Resources[2].DeletedWith.Name(), "resA")
|
|
|
|
|
|
|
|
err := snap.VerifyIntegrity()
|
|
|
|
assert.NoError(t, err)
|
|
|
|
|
|
|
|
p.Steps = []TestStep{{Op: Refresh}}
|
|
|
|
snap = p.Run(t, snap)
|
|
|
|
|
|
|
|
assert.Len(t, snap.Resources, 2)
|
|
|
|
assert.Equal(t, snap.Resources[0].URN.Name(), "default") // provider
|
|
|
|
assert.Equal(t, snap.Resources[1].URN.Name(), "resB")
|
|
|
|
assert.Empty(t, snap.Resources[1].DeletedWith)
|
|
|
|
|
|
|
|
err = snap.VerifyIntegrity()
|
|
|
|
assert.NoError(t, err)
|
|
|
|
}
|
|
|
|
|
2021-12-09 09:09:48 +00:00
|
|
|
// Looks up the provider ID in newResources and sets "Provider" to reference that in every resource in oldResources.
|
|
|
|
func setProviderRef(t *testing.T, oldResources, newResources []*resource.State, provURN resource.URN) {
|
|
|
|
for _, r := range newResources {
|
|
|
|
if r.URN == provURN {
|
|
|
|
provRef, err := providers.NewReference(r.URN, r.ID)
|
|
|
|
assert.NoError(t, err)
|
|
|
|
for i := range oldResources {
|
|
|
|
oldResources[i].Provider = provRef.String()
|
|
|
|
}
|
|
|
|
break
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
func validateRefreshDeleteCombination(t *testing.T, names []string, targets []string, name string) {
|
2020-12-15 22:24:46 +00:00
|
|
|
p := &TestPlan{}
|
|
|
|
|
|
|
|
const resType = "pkgA:m:typA"
|
|
|
|
|
|
|
|
urnA := p.NewURN(resType, names[0], "")
|
|
|
|
urnB := p.NewURN(resType, names[1], "")
|
|
|
|
urnC := p.NewURN(resType, names[2], "")
|
|
|
|
urns := []resource.URN{urnA, urnB, urnC}
|
|
|
|
|
|
|
|
refreshTargets := []resource.URN{}
|
|
|
|
|
|
|
|
t.Logf("Refreshing targets: %v", targets)
|
|
|
|
for _, target := range targets {
|
|
|
|
refreshTargets = append(refreshTargets, pickURN(t, urns, names, target))
|
|
|
|
}
|
|
|
|
|
2023-05-23 20:17:59 +00:00
|
|
|
p.Options.Targets = deploy.NewUrnTargetsFromUrns(refreshTargets)
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
newResource := func(urn resource.URN, id resource.ID, delete bool, dependencies ...resource.URN) *resource.State {
|
|
|
|
return &resource.State{
|
|
|
|
Type: urn.Type(),
|
|
|
|
URN: urn,
|
|
|
|
Custom: true,
|
|
|
|
Delete: delete,
|
|
|
|
ID: id,
|
|
|
|
Inputs: resource.PropertyMap{},
|
|
|
|
Outputs: resource.PropertyMap{},
|
|
|
|
Dependencies: dependencies,
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
oldResources := []*resource.State{
|
|
|
|
newResource(urnA, "0", false),
|
|
|
|
newResource(urnB, "1", false, urnA),
|
|
|
|
newResource(urnC, "2", false, urnA, urnB),
|
|
|
|
newResource(urnA, "3", true),
|
|
|
|
newResource(urnA, "4", true),
|
|
|
|
newResource(urnC, "5", true, urnA, urnB),
|
|
|
|
}
|
|
|
|
|
|
|
|
old := &deploy.Snapshot{
|
|
|
|
Resources: oldResources,
|
|
|
|
}
|
|
|
|
|
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{
|
2024-07-26 12:14:45 +00:00
|
|
|
ReadF: func(_ context.Context, req plugin.ReadRequest) (plugin.ReadResponse, error) {
|
|
|
|
switch req.ID {
|
2020-12-15 22:24:46 +00:00
|
|
|
case "0", "4":
|
|
|
|
// We want to delete resources A::0 and A::4.
|
2024-07-26 12:14:45 +00:00
|
|
|
return plugin.ReadResponse{}, nil
|
2020-12-15 22:24:46 +00:00
|
|
|
default:
|
2024-07-26 12:14:45 +00:00
|
|
|
return plugin.ReadResponse{
|
|
|
|
ReadResult: plugin.ReadResult{
|
|
|
|
Inputs: req.Inputs,
|
|
|
|
Outputs: req.State,
|
|
|
|
},
|
|
|
|
Status: resource.StatusOK,
|
|
|
|
}, nil
|
2020-12-15 22:24:46 +00:00
|
|
|
}
|
|
|
|
},
|
|
|
|
}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
2023-09-28 21:50:18 +00:00
|
|
|
p.Options.HostF = deploytest.NewPluginHostF(nil, nil, nil, loaders...)
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
p.Options.T = t
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
p.Steps = []TestStep{
|
|
|
|
{
|
|
|
|
Op: Refresh,
|
|
|
|
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
|
2024-05-06 17:34:24 +00:00
|
|
|
_ []Event, err error,
|
2023-10-11 14:44:09 +00:00
|
|
|
) error {
|
2020-12-15 22:24:46 +00:00
|
|
|
// Should see only refreshes.
|
|
|
|
for _, entry := range entries {
|
|
|
|
if len(refreshTargets) > 0 {
|
|
|
|
// should only see changes to urns we explicitly asked to change
|
|
|
|
assert.Containsf(t, refreshTargets, entry.Step.URN(),
|
|
|
|
"Refreshed a resource that wasn't a target: %v", entry.Step.URN())
|
|
|
|
}
|
|
|
|
|
|
|
|
assert.Equal(t, deploy.OpRefresh, entry.Step.Op())
|
|
|
|
}
|
|
|
|
|
2023-10-11 14:44:09 +00:00
|
|
|
return err
|
2020-12-15 22:24:46 +00:00
|
|
|
},
|
|
|
|
},
|
|
|
|
}
|
|
|
|
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
snap := p.RunWithName(t, old, name)
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
provURN := p.NewProviderURN("pkgA", "default", "")
|
|
|
|
|
2021-12-09 09:09:48 +00:00
|
|
|
// The new resources will have had their default provider urn filled in. We fill this in on
|
|
|
|
// the old resources here as well so that the equal checks below pass
|
|
|
|
setProviderRef(t, oldResources, snap.Resources, provURN)
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
for _, r := range snap.Resources {
|
|
|
|
switch urn := r.URN; urn {
|
|
|
|
case provURN:
|
|
|
|
continue
|
|
|
|
case urnA, urnB, urnC:
|
|
|
|
// break
|
|
|
|
default:
|
|
|
|
t.Fatalf("unexpected resource %v", urn)
|
|
|
|
}
|
|
|
|
|
|
|
|
if len(refreshTargets) == 0 || containsURN(refreshTargets, urnA) {
|
|
|
|
// 'A' was deleted, so we should see the impact downstream.
|
|
|
|
|
|
|
|
switch r.ID {
|
|
|
|
case "1":
|
|
|
|
// A::0 was deleted, so B's dependency list should be empty.
|
|
|
|
assert.Equal(t, urnB, r.URN)
|
|
|
|
assert.Empty(t, r.Dependencies)
|
|
|
|
case "2":
|
|
|
|
// A::0 was deleted, so C's dependency list should only contain B.
|
|
|
|
assert.Equal(t, urnC, r.URN)
|
|
|
|
assert.Equal(t, []resource.URN{urnB}, r.Dependencies)
|
|
|
|
case "3":
|
|
|
|
// A::3 should not have changed.
|
|
|
|
assert.Equal(t, oldResources[3], r)
|
|
|
|
case "5":
|
|
|
|
// A::4 was deleted but A::3 was still refernceable by C, so C should not have changed.
|
|
|
|
assert.Equal(t, oldResources[5], r)
|
|
|
|
default:
|
|
|
|
t.Fatalf("Unexpected changed resource when refreshing %v: %v::%v", refreshTargets, r.URN, r.ID)
|
|
|
|
}
|
|
|
|
} else {
|
|
|
|
// A was not deleted. So nothing should be impacted.
|
|
|
|
id, err := strconv.Atoi(r.ID.String())
|
|
|
|
assert.NoError(t, err)
|
|
|
|
assert.Equal(t, oldResources[id], r)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
func containsURN(urns []resource.URN, urn resource.URN) bool {
|
|
|
|
for _, val := range urns {
|
|
|
|
if val == urn {
|
|
|
|
return true
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return false
|
|
|
|
}
|
|
|
|
|
|
|
|
// Tests basic refresh functionality.
|
|
|
|
func TestRefreshBasics(t *testing.T) {
|
2022-03-04 08:17:41 +00:00
|
|
|
t.Parallel()
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
names := []string{"resA", "resB", "resC"}
|
|
|
|
|
|
|
|
// Try refreshing a stack with every combination of the three above resources as a target to
|
|
|
|
// refresh.
|
|
|
|
subsets := combinations.All(names)
|
|
|
|
|
|
|
|
// combinations.All doesn't return the empty set. So explicitly test that case (i.e. test no
|
|
|
|
// targets specified)
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
validateRefreshBasicsCombination(t, names, []string{}, "all")
|
2020-12-15 22:24:46 +00:00
|
|
|
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
for i, subset := range subsets {
|
|
|
|
validateRefreshBasicsCombination(t, names, subset, strconv.Itoa(i))
|
2020-12-15 22:24:46 +00:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
func validateRefreshBasicsCombination(t *testing.T, names []string, targets []string, name string) {
|
2020-12-15 22:24:46 +00:00
|
|
|
p := &TestPlan{}
|
|
|
|
|
|
|
|
const resType = "pkgA:m:typA"
|
|
|
|
|
|
|
|
urnA := p.NewURN(resType, names[0], "")
|
|
|
|
urnB := p.NewURN(resType, names[1], "")
|
|
|
|
urnC := p.NewURN(resType, names[2], "")
|
|
|
|
urns := []resource.URN{urnA, urnB, urnC}
|
|
|
|
|
|
|
|
refreshTargets := []resource.URN{}
|
|
|
|
|
|
|
|
for _, target := range targets {
|
2023-05-23 20:17:59 +00:00
|
|
|
refreshTargets = append(p.Options.Targets.Literals(), pickURN(t, urns, names, target))
|
2020-12-15 22:24:46 +00:00
|
|
|
}
|
|
|
|
|
2023-05-23 20:17:59 +00:00
|
|
|
p.Options.Targets = deploy.NewUrnTargetsFromUrns(refreshTargets)
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
newResource := func(urn resource.URN, id resource.ID, delete bool, dependencies ...resource.URN) *resource.State {
|
|
|
|
return &resource.State{
|
|
|
|
Type: urn.Type(),
|
|
|
|
URN: urn,
|
|
|
|
Custom: true,
|
|
|
|
Delete: delete,
|
|
|
|
ID: id,
|
|
|
|
Inputs: resource.PropertyMap{},
|
|
|
|
Outputs: resource.PropertyMap{},
|
|
|
|
Dependencies: dependencies,
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
oldResources := []*resource.State{
|
|
|
|
newResource(urnA, "0", false),
|
|
|
|
newResource(urnB, "1", false, urnA),
|
|
|
|
newResource(urnC, "2", false, urnA, urnB),
|
|
|
|
newResource(urnA, "3", true),
|
|
|
|
newResource(urnA, "4", true),
|
|
|
|
newResource(urnC, "5", true, urnA, urnB),
|
|
|
|
}
|
|
|
|
|
|
|
|
newStates := map[resource.ID]plugin.ReadResult{
|
|
|
|
// A::0 and A::3 will have no changes.
|
|
|
|
"0": {Outputs: resource.PropertyMap{}, Inputs: resource.PropertyMap{}},
|
|
|
|
"3": {Outputs: resource.PropertyMap{}, Inputs: resource.PropertyMap{}},
|
|
|
|
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
// B::1 has output-only changes which will not be reported as a refresh diff.
|
2020-12-15 22:24:46 +00:00
|
|
|
"1": {Outputs: resource.PropertyMap{"foo": resource.NewStringProperty("bar")}, Inputs: resource.PropertyMap{}},
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
|
|
|
|
// A::4 will have input and output changes. The changes that impact the inputs will be reported
|
|
|
|
// as a refresh diff.
|
2020-12-15 22:24:46 +00:00
|
|
|
"4": {
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
Outputs: resource.PropertyMap{
|
|
|
|
"baz": resource.NewStringProperty("qux"),
|
|
|
|
"oof": resource.NewStringProperty("zab"),
|
|
|
|
},
|
|
|
|
Inputs: resource.PropertyMap{"oof": resource.NewStringProperty("zab")},
|
2020-12-15 22:24:46 +00:00
|
|
|
},
|
|
|
|
|
|
|
|
// C::2 and C::5 will be deleted.
|
|
|
|
"2": {},
|
|
|
|
"5": {},
|
|
|
|
}
|
|
|
|
|
|
|
|
old := &deploy.Snapshot{
|
|
|
|
Resources: oldResources,
|
|
|
|
}
|
|
|
|
|
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{
|
2024-07-26 12:14:45 +00:00
|
|
|
ReadF: func(_ context.Context, req plugin.ReadRequest) (plugin.ReadResponse, error) {
|
|
|
|
new, hasNewState := newStates[req.ID]
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.True(t, hasNewState)
|
2024-07-26 12:14:45 +00:00
|
|
|
return plugin.ReadResponse{
|
|
|
|
ReadResult: new,
|
|
|
|
Status: resource.StatusOK,
|
|
|
|
}, nil
|
2020-12-15 22:24:46 +00:00
|
|
|
},
|
|
|
|
}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
2023-09-28 21:50:18 +00:00
|
|
|
p.Options.HostF = deploytest.NewPluginHostF(nil, nil, nil, loaders...)
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
p.Options.T = t
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
p.Steps = []TestStep{{
|
|
|
|
Op: Refresh,
|
|
|
|
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
|
2024-05-06 17:34:24 +00:00
|
|
|
_ []Event, err error,
|
2023-10-11 14:44:09 +00:00
|
|
|
) error {
|
2020-12-15 22:24:46 +00:00
|
|
|
// Should see only refreshes.
|
|
|
|
for _, entry := range entries {
|
|
|
|
if len(refreshTargets) > 0 {
|
|
|
|
// should only see changes to urns we explicitly asked to change
|
|
|
|
assert.Containsf(t, refreshTargets, entry.Step.URN(),
|
|
|
|
"Refreshed a resource that wasn't a target: %v", entry.Step.URN())
|
|
|
|
}
|
|
|
|
|
|
|
|
assert.Equal(t, deploy.OpRefresh, entry.Step.Op())
|
|
|
|
resultOp := entry.Step.(*deploy.RefreshStep).ResultOp()
|
|
|
|
|
|
|
|
old := entry.Step.Old()
|
|
|
|
if !old.Custom || providers.IsProviderType(old.Type) {
|
|
|
|
// Component and provider resources should never change.
|
|
|
|
assert.Equal(t, deploy.OpSame, resultOp)
|
|
|
|
continue
|
|
|
|
}
|
|
|
|
|
|
|
|
expected, new := newStates[old.ID], entry.Step.New()
|
|
|
|
if expected.Outputs == nil {
|
|
|
|
// If the resource was deleted, we want the result op to be an OpDelete.
|
|
|
|
assert.Nil(t, new)
|
|
|
|
assert.Equal(t, deploy.OpDelete, resultOp)
|
|
|
|
} else {
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
// If there were changes to the inputs, we want the result op to be an
|
|
|
|
// OpUpdate. Otherwise we want an OpSame.
|
|
|
|
if reflect.DeepEqual(old.Inputs, expected.Inputs) {
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.Equal(t, deploy.OpSame, resultOp)
|
|
|
|
} else {
|
|
|
|
assert.Equal(t, deploy.OpUpdate, resultOp)
|
|
|
|
}
|
|
|
|
|
2024-05-09 16:15:41 +00:00
|
|
|
old = old.Copy()
|
|
|
|
new = new.Copy()
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
// Only the inputs and outputs should have changed (if anything changed).
|
|
|
|
old.Inputs = expected.Inputs
|
|
|
|
old.Outputs = expected.Outputs
|
This commit adds the `Created` and `Modified` timestamps to pulumi state that are optional.
`Created`: Created tracks when the remote resource was first added to state by pulumi. Checkpoints prior to early 2023 do not include this. (Create, Import)
`Modified`: Modified tracks when the resource state was last altered. Checkpoints prior to early 2023 do not include this. (Create, Import, Read, Refresh, Update)
When serialized they will follow RFC3339 with nanoseconds captured by a test case.
https://pkg.go.dev/time#RFC3339
Note: Older versions of pulumi may strip these fields when modifying the state.
For future expansion, when we inevitably need to track other timestamps, we'll add a new "operationTimestamps" field (or something similarly named that clarified these are timestamps of the actual Pulumi operations).
operationTimestamps: {
created: ...,
updated: ...,
imported: ...,
}
Fixes https://github.com/pulumi/pulumi/issues/12022
2023-02-06 20:39:11 +00:00
|
|
|
|
|
|
|
// Discard timestamps for refresh test.
|
|
|
|
new.Modified = nil
|
|
|
|
old.Modified = nil
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.Equal(t, old, new)
|
|
|
|
}
|
|
|
|
}
|
2023-10-11 14:44:09 +00:00
|
|
|
return err
|
2020-12-15 22:24:46 +00:00
|
|
|
},
|
|
|
|
}}
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
snap := p.RunWithName(t, old, name)
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
provURN := p.NewProviderURN("pkgA", "default", "")
|
|
|
|
|
2021-12-09 09:09:48 +00:00
|
|
|
// The new resources will have had their default provider urn filled in. We fill this in on
|
|
|
|
// the old resources here as well so that the equal checks below pass
|
|
|
|
setProviderRef(t, oldResources, snap.Resources, provURN)
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
for _, r := range snap.Resources {
|
|
|
|
switch urn := r.URN; urn {
|
|
|
|
case provURN:
|
|
|
|
continue
|
|
|
|
case urnA, urnB, urnC:
|
|
|
|
// break
|
|
|
|
default:
|
|
|
|
t.Fatalf("unexpected resource %v", urn)
|
|
|
|
}
|
|
|
|
|
|
|
|
// The only resources left in the checkpoint should be those that were not deleted by the refresh.
|
|
|
|
expected := newStates[r.ID]
|
|
|
|
assert.NotNil(t, expected)
|
|
|
|
|
|
|
|
idx, err := strconv.ParseInt(string(r.ID), 0, 0)
|
|
|
|
assert.NoError(t, err)
|
|
|
|
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
targetedForRefresh := len(refreshTargets) == 0
|
2021-12-09 09:09:48 +00:00
|
|
|
for _, targetUrn := range refreshTargets {
|
|
|
|
if targetUrn == r.URN {
|
|
|
|
targetedForRefresh = true
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// If targeted for refresh the new resources should be equal to the old resources + the new inputs and outputs
|
2024-04-11 22:54:08 +00:00
|
|
|
// and timestamp.
|
2020-12-15 22:24:46 +00:00
|
|
|
old := oldResources[int(idx)]
|
2021-12-09 09:09:48 +00:00
|
|
|
if targetedForRefresh {
|
|
|
|
old.Inputs = expected.Inputs
|
|
|
|
old.Outputs = expected.Outputs
|
2024-04-11 22:54:08 +00:00
|
|
|
old.Modified = r.Modified
|
2021-12-09 09:09:48 +00:00
|
|
|
}
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.Equal(t, old, r)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// Tests that an interrupted refresh leaves behind an expected state.
|
|
|
|
func TestCanceledRefresh(t *testing.T) {
|
2022-03-04 08:17:41 +00:00
|
|
|
t.Parallel()
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
p := &TestPlan{}
|
|
|
|
|
|
|
|
const resType = "pkgA:m:typA"
|
|
|
|
|
|
|
|
urnA := p.NewURN(resType, "resA", "")
|
|
|
|
urnB := p.NewURN(resType, "resB", "")
|
|
|
|
urnC := p.NewURN(resType, "resC", "")
|
|
|
|
|
|
|
|
newResource := func(urn resource.URN, id resource.ID, delete bool, dependencies ...resource.URN) *resource.State {
|
|
|
|
return &resource.State{
|
|
|
|
Type: urn.Type(),
|
|
|
|
URN: urn,
|
|
|
|
Custom: true,
|
|
|
|
Delete: delete,
|
|
|
|
ID: id,
|
|
|
|
Inputs: resource.PropertyMap{},
|
|
|
|
Outputs: resource.PropertyMap{},
|
|
|
|
Dependencies: dependencies,
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
oldResources := []*resource.State{
|
|
|
|
newResource(urnA, "0", false),
|
|
|
|
newResource(urnB, "1", false),
|
|
|
|
newResource(urnC, "2", false),
|
|
|
|
}
|
|
|
|
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
newStates := map[resource.ID]plugin.ReadResult{
|
|
|
|
// A::0 will have input and output changes. The changes that impact the inputs will be reported
|
|
|
|
// as a refresh diff.
|
|
|
|
"0": {
|
|
|
|
Outputs: resource.PropertyMap{"foo": resource.NewStringProperty("bar")},
|
|
|
|
Inputs: resource.PropertyMap{"oof": resource.NewStringProperty("rab")},
|
|
|
|
},
|
|
|
|
// B::1 will have output changes.
|
|
|
|
"1": {
|
|
|
|
Outputs: resource.PropertyMap{"baz": resource.NewStringProperty("qux")},
|
|
|
|
},
|
|
|
|
// C::2 will be deleted.
|
|
|
|
"2": {},
|
2020-12-15 22:24:46 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
old := &deploy.Snapshot{
|
|
|
|
Resources: oldResources,
|
|
|
|
}
|
|
|
|
|
|
|
|
// Set up a cancelable context for the refresh operation.
|
|
|
|
ctx, cancel := context.WithCancel(context.Background())
|
|
|
|
|
|
|
|
// Serialize all refreshes s.t. we can cancel after the first is issued.
|
|
|
|
refreshes, cancelled := make(chan resource.ID), make(chan bool)
|
|
|
|
go func() {
|
|
|
|
<-refreshes
|
|
|
|
cancel()
|
|
|
|
}()
|
|
|
|
|
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{
|
2024-07-26 12:14:45 +00:00
|
|
|
ReadF: func(_ context.Context, req plugin.ReadRequest) (plugin.ReadResponse, error) {
|
|
|
|
refreshes <- req.ID
|
2020-12-15 22:24:46 +00:00
|
|
|
<-cancelled
|
|
|
|
|
2024-07-26 12:14:45 +00:00
|
|
|
new, hasNewState := newStates[req.ID]
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.True(t, hasNewState)
|
2024-07-26 12:14:45 +00:00
|
|
|
return plugin.ReadResponse{
|
|
|
|
ReadResult: new,
|
|
|
|
Status: resource.StatusOK,
|
|
|
|
}, nil
|
2020-12-15 22:24:46 +00:00
|
|
|
},
|
|
|
|
CancelF: func() error {
|
|
|
|
close(cancelled)
|
|
|
|
return nil
|
|
|
|
},
|
|
|
|
}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
|
|
|
refreshed := make(map[resource.ID]bool)
|
|
|
|
op := TestOp(Refresh)
|
2023-09-28 21:50:18 +00:00
|
|
|
options := TestUpdateOptions{
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
T: t,
|
2023-09-28 21:50:18 +00:00
|
|
|
HostF: deploytest.NewPluginHostF(nil, nil, nil, loaders...),
|
|
|
|
UpdateOptions: UpdateOptions{
|
|
|
|
Parallel: 1,
|
|
|
|
},
|
2020-12-15 22:24:46 +00:00
|
|
|
}
|
2021-12-09 09:09:48 +00:00
|
|
|
project, target := p.GetProject(), p.GetTarget(t, old)
|
2020-12-15 22:24:46 +00:00
|
|
|
validate := func(project workspace.Project, target deploy.Target, entries JournalEntries,
|
2024-05-06 17:34:24 +00:00
|
|
|
_ []Event, err error,
|
2023-10-11 14:44:09 +00:00
|
|
|
) error {
|
2020-12-15 22:24:46 +00:00
|
|
|
for _, entry := range entries {
|
|
|
|
assert.Equal(t, deploy.OpRefresh, entry.Step.Op())
|
|
|
|
resultOp := entry.Step.(*deploy.RefreshStep).ResultOp()
|
|
|
|
|
|
|
|
old := entry.Step.Old()
|
|
|
|
if !old.Custom || providers.IsProviderType(old.Type) {
|
|
|
|
// Component and provider resources should never change.
|
|
|
|
assert.Equal(t, deploy.OpSame, resultOp)
|
|
|
|
continue
|
|
|
|
}
|
|
|
|
|
|
|
|
refreshed[old.ID] = true
|
|
|
|
|
|
|
|
expected, new := newStates[old.ID], entry.Step.New()
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
if expected.Outputs == nil {
|
2020-12-15 22:24:46 +00:00
|
|
|
// If the resource was deleted, we want the result op to be an OpDelete.
|
|
|
|
assert.Nil(t, new)
|
|
|
|
assert.Equal(t, deploy.OpDelete, resultOp)
|
|
|
|
} else {
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
// If there were changes to the inputs, we want the result op to be an
|
|
|
|
// OpUpdate. Otherwise we want an OpSame.
|
|
|
|
if reflect.DeepEqual(old.Inputs, expected.Inputs) {
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.Equal(t, deploy.OpSame, resultOp)
|
|
|
|
} else {
|
|
|
|
assert.Equal(t, deploy.OpUpdate, resultOp)
|
|
|
|
}
|
|
|
|
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
// The inputs, outputs and modified timestamps should have changed (if
|
|
|
|
// anything changed at all).
|
2024-05-09 16:15:41 +00:00
|
|
|
old = old.Copy()
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
old.Inputs = expected.Inputs
|
|
|
|
old.Outputs = expected.Outputs
|
This commit adds the `Created` and `Modified` timestamps to pulumi state that are optional.
`Created`: Created tracks when the remote resource was first added to state by pulumi. Checkpoints prior to early 2023 do not include this. (Create, Import)
`Modified`: Modified tracks when the resource state was last altered. Checkpoints prior to early 2023 do not include this. (Create, Import, Read, Refresh, Update)
When serialized they will follow RFC3339 with nanoseconds captured by a test case.
https://pkg.go.dev/time#RFC3339
Note: Older versions of pulumi may strip these fields when modifying the state.
For future expansion, when we inevitably need to track other timestamps, we'll add a new "operationTimestamps" field (or something similarly named that clarified these are timestamps of the actual Pulumi operations).
operationTimestamps: {
created: ...,
updated: ...,
imported: ...,
}
Fixes https://github.com/pulumi/pulumi/issues/12022
2023-02-06 20:39:11 +00:00
|
|
|
old.Modified = new.Modified
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.Equal(t, old, new)
|
|
|
|
}
|
|
|
|
}
|
2023-10-11 14:44:09 +00:00
|
|
|
return err
|
2020-12-15 22:24:46 +00:00
|
|
|
}
|
|
|
|
|
2023-10-11 14:44:09 +00:00
|
|
|
snap, err := op.RunWithContext(ctx, project, target, options, false, nil, validate)
|
2023-11-16 09:58:30 +00:00
|
|
|
assert.ErrorContains(t, err, "BAIL: canceled")
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.Equal(t, 1, len(refreshed))
|
|
|
|
|
|
|
|
provURN := p.NewProviderURN("pkgA", "default", "")
|
|
|
|
|
2021-12-09 09:09:48 +00:00
|
|
|
// The new resources will have had their default provider urn filled in. We fill this in on
|
|
|
|
// the old resources here as well so that the equal checks below pass
|
|
|
|
setProviderRef(t, oldResources, snap.Resources, provURN)
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
for _, r := range snap.Resources {
|
|
|
|
switch urn := r.URN; urn {
|
|
|
|
case provURN:
|
|
|
|
continue
|
|
|
|
case urnA, urnB, urnC:
|
|
|
|
// break
|
|
|
|
default:
|
|
|
|
t.Fatalf("unexpected resource %v", urn)
|
|
|
|
}
|
|
|
|
|
|
|
|
idx, err := strconv.ParseInt(string(r.ID), 0, 0)
|
|
|
|
assert.NoError(t, err)
|
|
|
|
|
|
|
|
if refreshed[r.ID] {
|
|
|
|
// The refreshed resource should have its new state.
|
|
|
|
expected := newStates[r.ID]
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
if expected.Outputs == nil {
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.Fail(t, "refreshed resource was not deleted")
|
|
|
|
} else {
|
|
|
|
old := oldResources[int(idx)]
|
This commit adds the `Created` and `Modified` timestamps to pulumi state that are optional.
`Created`: Created tracks when the remote resource was first added to state by pulumi. Checkpoints prior to early 2023 do not include this. (Create, Import)
`Modified`: Modified tracks when the resource state was last altered. Checkpoints prior to early 2023 do not include this. (Create, Import, Read, Refresh, Update)
When serialized they will follow RFC3339 with nanoseconds captured by a test case.
https://pkg.go.dev/time#RFC3339
Note: Older versions of pulumi may strip these fields when modifying the state.
For future expansion, when we inevitably need to track other timestamps, we'll add a new "operationTimestamps" field (or something similarly named that clarified these are timestamps of the actual Pulumi operations).
operationTimestamps: {
created: ...,
updated: ...,
imported: ...,
}
Fixes https://github.com/pulumi/pulumi/issues/12022
2023-02-06 20:39:11 +00:00
|
|
|
|
Change `pulumi refresh` to report diff relative to desired state instead of relative to only output changes (#16146)
Presently, the behaviour of diffing during refresh steps is incomplete,
returning only an "output diff" that presents the changes in outputs.
This commit changes refresh steps so that:
* they compute a diff similar to the one that would be computed if a
`preview` were run immediately after the refresh, which is more
typically what users expect and want; and
* `IgnoreChanges` resource options are respected when performing the new
desired-state diffs, so that property additions or changes reported by a
refresh can be ignored.
In particular, `IgnoreChanges` can now be used to acknowledge that part
or all of a resource may change in the provider, but the user is OK with
this and doesn't want to be notified about it during a refresh.
Importantly, this means that the diff won't be reported, but also that
the changes won't be applied to state.
The implementation covers the following:
* A diff is computed using the inputs from the program and then
inverting the result, since in the case of a refresh the diff is being
driven by the provider side and not the program. This doesn't change
what is stored back into the state, but it does produce a diff that is
more aligned with the "true changes to the desired state".
* `IgnoreChanges` resource options are now stored in state, so that this
information can be used in refresh operations that do not have access
to/run the program.
* In the context of a refresh operation, `IgnoreChanges` applies to
*both* input and output properties. This differs from the behaviour of a
normal update operation, where `IgnoreChanges` only considers input
properties.
* The special `"*"` value for `IgnoreChanges` can be used to ignore all
properties. It _also_ ignores the case where the resource cannot be
found in the provider, and instead keeps the resource intact in state
with its existing input and output properties.
Because the program is not run for refresh operations, `IgnoreChanges`
options must be applied separately before a refresh takes place. This
can be accomplished using e.g. a `pulumi up` that applies the options
prior to a refresh. We should investigate perhaps providing a `pulumi
state set ...`-like CLI to make these sorts of changes directly to a
state.
For use cases relying on the legacy refresh diff provider, the
`PULUMI_USE_LEGACY_REFRESH_DIFF` environment variable can be set, which
will disable desired-state diff computation. We only need to perform
checks in `RefreshStep.{ResultOp,Apply}`, since downstream code will
work correctly based on the presence or absence of a `DetailedDiff` in
the step.
### Notes
- https://github.com/pulumi/pulumi/issues/16144 affects some of these
cases - though its technically orthogonal
- https://github.com/pulumi/pulumi/issues/11279 is another technically
orthogonal issue that many providers (at least TFBridge ones) - do not
report back changes to input properties on Read when the input property
(or property path) was missing on the inputs. This is again technically
orthogonal - but leads to cases that appear "wrong" in terms of what is
stored back into the state still - though the same as before this
change.
- Azure Native doesn't seem to handle `ignoreChanges` passed to Diff, so
the ability to ignore changes on refresh doesn't currently work for
Azure Native.
### Fixes
* Fixes #16072
* Fixes #16278
* Fixes #16334
* Not quite #12346, but likely replaces the need for that
Co-authored-by: Will Jones <will@sacharissa.co.uk>
2024-06-12 16:17:05 +00:00
|
|
|
// The inputs, outputs and modified timestamps should have changed (if
|
|
|
|
// anything changed at all).
|
|
|
|
old.Inputs = expected.Inputs
|
|
|
|
old.Outputs = expected.Outputs
|
This commit adds the `Created` and `Modified` timestamps to pulumi state that are optional.
`Created`: Created tracks when the remote resource was first added to state by pulumi. Checkpoints prior to early 2023 do not include this. (Create, Import)
`Modified`: Modified tracks when the resource state was last altered. Checkpoints prior to early 2023 do not include this. (Create, Import, Read, Refresh, Update)
When serialized they will follow RFC3339 with nanoseconds captured by a test case.
https://pkg.go.dev/time#RFC3339
Note: Older versions of pulumi may strip these fields when modifying the state.
For future expansion, when we inevitably need to track other timestamps, we'll add a new "operationTimestamps" field (or something similarly named that clarified these are timestamps of the actual Pulumi operations).
operationTimestamps: {
created: ...,
updated: ...,
imported: ...,
}
Fixes https://github.com/pulumi/pulumi/issues/12022
2023-02-06 20:39:11 +00:00
|
|
|
old.Modified = r.Modified
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.Equal(t, old, r)
|
|
|
|
}
|
|
|
|
} else {
|
|
|
|
// Any resources that were not refreshed should retain their original state.
|
|
|
|
old := oldResources[int(idx)]
|
|
|
|
assert.Equal(t, old, r)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
func TestRefreshStepWillPersistUpdatedIDs(t *testing.T) {
|
2022-03-04 08:17:41 +00:00
|
|
|
t.Parallel()
|
|
|
|
|
2020-12-15 22:24:46 +00:00
|
|
|
p := &TestPlan{}
|
|
|
|
|
|
|
|
provURN := p.NewProviderURN("pkgA", "default", "")
|
|
|
|
resURN := p.NewURN("pkgA:m:typA", "resA", "")
|
|
|
|
idBefore := resource.ID("myid")
|
|
|
|
idAfter := resource.ID("mynewid")
|
|
|
|
outputs := resource.PropertyMap{"foo": resource.NewStringProperty("bar")}
|
|
|
|
|
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{
|
2024-07-26 12:14:45 +00:00
|
|
|
ReadF: func(_ context.Context, req plugin.ReadRequest) (plugin.ReadResponse, error) {
|
|
|
|
return plugin.ReadResponse{
|
|
|
|
ReadResult: plugin.ReadResult{
|
|
|
|
ID: idAfter,
|
|
|
|
Inputs: resource.PropertyMap{},
|
|
|
|
Outputs: outputs,
|
|
|
|
},
|
|
|
|
Status: resource.StatusOK,
|
|
|
|
}, nil
|
2020-12-15 22:24:46 +00:00
|
|
|
},
|
|
|
|
}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
2023-09-28 21:50:18 +00:00
|
|
|
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
|
2024-04-19 11:08:56 +00:00
|
|
|
_, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
|
2020-12-15 22:24:46 +00:00
|
|
|
assert.NoError(t, err)
|
|
|
|
return nil
|
|
|
|
})
|
2023-09-28 21:50:18 +00:00
|
|
|
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
|
2020-12-15 22:24:46 +00:00
|
|
|
|
2023-09-28 21:50:18 +00:00
|
|
|
p.Options.HostF = hostF
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
p.Options.T = t
|
2020-12-15 22:24:46 +00:00
|
|
|
|
|
|
|
old := &deploy.Snapshot{
|
|
|
|
Resources: []*resource.State{
|
|
|
|
{
|
|
|
|
Type: resURN.Type(),
|
|
|
|
URN: resURN,
|
|
|
|
Custom: true,
|
|
|
|
ID: idBefore,
|
|
|
|
Inputs: resource.PropertyMap{},
|
|
|
|
Outputs: outputs,
|
|
|
|
InitErrors: []string{"Resource failed to initialize"},
|
|
|
|
},
|
|
|
|
},
|
|
|
|
}
|
|
|
|
|
|
|
|
p.Steps = []TestStep{{Op: Refresh, SkipPreview: true}}
|
|
|
|
snap := p.Run(t, old)
|
|
|
|
|
|
|
|
for _, resource := range snap.Resources {
|
|
|
|
switch urn := resource.URN; urn {
|
|
|
|
case provURN:
|
|
|
|
// break
|
|
|
|
case resURN:
|
|
|
|
assert.Empty(t, resource.InitErrors)
|
|
|
|
assert.Equal(t, idAfter, resource.ID)
|
|
|
|
default:
|
|
|
|
t.Fatalf("unexpected resource %v", urn)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
2020-12-19 07:03:40 +00:00
|
|
|
|
|
|
|
// TestRefreshUpdateWithDeletedResource validates that the engine handles a deleted resource without error on an
|
|
|
|
// update with refresh.
|
|
|
|
func TestRefreshUpdateWithDeletedResource(t *testing.T) {
|
2022-03-04 08:17:41 +00:00
|
|
|
t.Parallel()
|
|
|
|
|
2020-12-19 07:03:40 +00:00
|
|
|
p := &TestPlan{}
|
|
|
|
|
|
|
|
resURN := p.NewURN("pkgA:m:typA", "resA", "")
|
|
|
|
idBefore := resource.ID("myid")
|
|
|
|
|
|
|
|
loaders := []*deploytest.ProviderLoader{
|
|
|
|
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
|
|
|
|
return &deploytest.Provider{
|
2024-07-26 12:14:45 +00:00
|
|
|
ReadF: func(_ context.Context, req plugin.ReadRequest) (plugin.ReadResponse, error) {
|
|
|
|
return plugin.ReadResponse{}, nil
|
2020-12-19 07:03:40 +00:00
|
|
|
},
|
|
|
|
}, nil
|
|
|
|
}),
|
|
|
|
}
|
|
|
|
|
2023-09-28 21:50:18 +00:00
|
|
|
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
|
2020-12-19 07:03:40 +00:00
|
|
|
return nil
|
|
|
|
})
|
2023-09-28 21:50:18 +00:00
|
|
|
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
|
2020-12-19 07:03:40 +00:00
|
|
|
|
2023-09-28 21:50:18 +00:00
|
|
|
p.Options.HostF = hostF
|
2020-12-19 07:03:40 +00:00
|
|
|
p.Options.Refresh = true
|
Add display to the engine tests (#16050)
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.
It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.
There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.
The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.
I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.
One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.
The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?
---------
Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
|
|
|
p.Options.T = t
|
2020-12-19 07:03:40 +00:00
|
|
|
|
|
|
|
old := &deploy.Snapshot{
|
|
|
|
Resources: []*resource.State{
|
|
|
|
{
|
|
|
|
Type: resURN.Type(),
|
|
|
|
URN: resURN,
|
|
|
|
Custom: true,
|
|
|
|
ID: idBefore,
|
|
|
|
Inputs: resource.PropertyMap{},
|
|
|
|
Outputs: resource.PropertyMap{},
|
|
|
|
},
|
|
|
|
},
|
|
|
|
}
|
|
|
|
|
|
|
|
p.Steps = []TestStep{{Op: Update}}
|
|
|
|
snap := p.Run(t, old)
|
|
|
|
assert.Equal(t, 0, len(snap.Resources))
|
|
|
|
}
|