search

pulumi / pulumi-dotnet

.NET support for Pulumi
Apache License 2.0
28 stars 25 forks source link

Automation API support for `pulumi-refresh --preview-only` #244

Open PhilBroderickCrezco opened 8 months ago

PhilBroderickCrezco commented 8 months ago

Hello!

Issue details

Now that the CLI supports the preview-only flag for pulumi refresh (https://github.com/pulumi/pulumi/pull/15330), it would be great to have that available for the .NET Automation API. I was going to take a stab at this myself as it should be relatively straightforward, however looking into the code, the --skip-preview flag is hardcoded into the args: https://github.com/pulumi/pulumi-dotnet/blob/d0039a8809f5ff4ac007c02ac694594d6a26d6f7/sdk/Pulumi.Automation/WorkspaceStack.cs#L547

I thought it best to open a discussion on how we expect this to work.

To give a bit of background: we currently use the Automation API to create/manage some of our microstacks, but we are unsure on the best workflow for incorporating pulumi refresh into this. By it's nature, the Automation API isn't always going to prompt for manual approval, hence the --skip-preview flag I presume, but we want to be able to manage state drift between Pulumi and our Cloud Provider (Azure).

Now, there are certain state differences, especially in Azure, that we are happy to apply (timestamp changes to modified dates etc), but others we don't want to auto apply and at least prompt users/tooling that there has been drift. My initial thoughts were that this would be possible with the RefreshAsync method and the ExpectNoChanges flag, but with the presence of the --skip-preview flag, the changes do get applied, and then Pulumi throws a CommandException, which is obviously too late to do anything about as the state as been updated.

I'd be interested to hear what is the recommended workflow for state drift/applying refreshes. Is this something that we should always just be doing, no questions asked, say before or after an update? Should we always run a refresh pre-update and abort on any changes, or at least get a set of eyes on the proposed changes first? Does it make sense to run refreshes out of process, say on a schedule? It's hard to find a recommended approach for this, especially when dealing with the Automation API.

Affected area/feature

Automation API

komalali commented 8 months ago

Hey @PhilBroderickCrezco thanks for the issue! Lots of great questions. I'll tackle them one at a time.

Here's some prior art for adding preview-only support to automation api (in Go):

Essentially, if we're doing a preview-only refresh we'll want to replace the --skip-preview with --preview-only. I think it's fair to create a separate method for this as we have done in Go.

We also added a Refresh option to Stack.Up() et al to roll the refresh/update command into one - something else that might be worth porting over to .NET: https://github.com/pulumi/pulumi/pull/15350

I'd be interested to hear what is the recommended workflow for state drift/applying refreshes.

I think this is really going to depend on your workflow, the kinds of resources you have, the kind of drift you expect, etc. For instance, you might have resources that you expect to update outside of pulumi, in which case you'd probably want to mark those properties as ignoreChanges to avoid spurious diffs.

There are certainly cases where folks would always want to understand the drift, and other cases where folks just want to keep the pulumi program as the source of truth and always want the drift written over.

Let me know if that helps or you have any other questions.

PhilBroderickCrezco commented 8 months ago

Hi @komalali thanks for the response. That Go PR was the one that prompted me to raise this issue in the first place!

There are certainly cases where folks would always want to understand the drift, and other cases where folks just want to keep the pulumi program as the source of truth and always want the drift written over.

All makes sense and as we've come to the conclusion of; it feels like it needs to be dealt with on a case-by-case basis and no one answer fits every scenario.

We also added a Refresh option to Stack.Up() et al to roll the refresh/update command into one

I wasn't aware of this - that would certainly be useful to have.

For instance, you might have resources that you expect to update outside of pulumi, in which case you'd probably want to mark those properties as ignoreChanges to avoid spurious diffs.

Yes there certainly are, and this is exactly where we use ignoreChanges.

Essentially, if we're doing a preview-only refresh we'll want to replace the --skip-preview with --preview-only. I think it's fair to create a separate method for this as we have done in Go.

Agreed. If you'd be alright with it, I wouldn't mind taking a stab at implementing this and opening a PR. Based on the Go implementation, it doesn't seem too complex now that we've got a shared understanding of how it should work. This is something we'd be very keen to use in our Automation API programs very soon.

komalali commented 8 months ago

If you'd be alright with it, I wouldn't mind taking a stab at implementing this and opening a PR.

@PhilBroderickCrezco Yes please, would love to review a PR for this!