Warn users when there are pending operations but proceed with deployment #9293
Conversation
| @@ -550,11 +550,6 @@ func TestPreviewWithPendingOperations(t *testing.T) { | |||
| // A preview should succeed despite the pending operations. | |||
| _, res := op.Run(project, target, options, true, nil, nil) | |||
| assert.Nil(t, res) | |||
|
|
|||
| // But an update should fail. | |||
| _, res = op.Run(project, target, options, false, nil, nil) | |||
There was a problem hiding this comment.
We should test we get the expected warning here
There was a problem hiding this comment.
Ah I see this is now just for preview and we have tests elsewhere for updates.
| @@ -605,11 +600,6 @@ func TestRefreshWithPendingOperations(t *testing.T) { | |||
| options := UpdateOptions{Host: deploytest.NewPluginHost(nil, nil, program, loaders...)} | |||
| project, target := p.GetProject(), p.GetTarget(t, old) | |||
|
|
|||
| // Without refreshing, an update should fail. | |||
| // Print a warning for users that there are pending operations. | ||
| // Explain that these operations can be cleared using pulumi refresh (except for CREATE operations) | ||
| // since these require user intevention: | ||
| ex.printPendingOperationsWarning() |
There was a problem hiding this comment.
Similarly to what we did with refresh I think we might want to do something to ensure that after doing the update our snapshot still tracks that there were pending operations that were not resovled.
|
Thanks @Frassle, I will revisit the points and try add some tests 😄 |
|
@Frassle Added tests
Can you please have a look? 🙏 |
| @@ -550,11 +550,6 @@ func TestPreviewWithPendingOperations(t *testing.T) { | |||
| // A preview should succeed despite the pending operations. | |||
| _, res := op.Run(project, target, options, true, nil, nil) | |||
| assert.Nil(t, res) | |||
|
|
|||
| // But an update should fail. | |||
| _, res = op.Run(project, target, options, false, nil, nil) | |||
There was a problem hiding this comment.
Ah I see this is now just for preview and we have tests elsewhere for updates.
| } | ||
|
|
||
| _, res := op.Run(project, target, options, false, nil, validate) | ||
| assert.Nil(t, res) |
There was a problem hiding this comment.
Like the refresh test above can we assert that the pending operations are still present (and we should test both PendingUpdates and PendingCreates are retained)
There was a problem hiding this comment.
I am writing the asserts now and it seems that only pending creates are preserved on an update (just like refresh) is this by design? 🤔
// THIS FAILS: expected 2, actual 1
assert.Equal(t, 2, len(new.PendingOperations))
assert.Equal(t, resource.OperationTypeUpdating, new.PendingOperations[0].Type)
assert.Equal(t, urnA, new.PendingOperations[0].Resource.URN)
assert.Equal(t, resource.OperationTypeCreating, new.PendingOperations[1].Type)
assert.Equal(t, urnB, new.PendingOperations[1].Resource.URN)There was a problem hiding this comment.
Hmm that might be ok, I don't think it's ideal but probably good enough for this work
There was a problem hiding this comment.
Alright, added asserts for retaining pending create operations, this way the behaviour is documented until we revisit a better solution
Description
Fixes #4265
This PR changes the deployment execution such that it no longer errors out when there are pending operations. Instead, it only warns the users and explains that they can use
pulumi refreshto clear the operations and they will no longer see the warning (unless there are pendingcreateoperations)Checklist