Consider moving planning to be performed in Check

[t0yv0]

Consider moving planning to be performed in Check instead of Diff, Create, Update.

Here is a simplified sequence diagram for the calls involved in pulumi up deciding to execute an Update plan on a bridged provider resource as of today.

  sequenceDiagram
      participant Engine
      participant BridgedProvider
      participant TFProvider
      Engine->>BridgedProvider: checkedInputs := Check(oldInputs, config)
      Engine->>BridgedProvider: Diff(oldState, checkedInputs)
      BridgedProvider->>TFProvider: plan := PlanResourceChange(oldState, config=checkedInputs)
      BridgedProvider->>Engine: compare(oldState, plan)
      Engine->>BridgedProvider: Update(oldState, checkedInputs)
      BridgedProvider->>TFProvider: plan := PlanResourceChange(state, config)
      BridgedProvider->>TFProvider: ApplyResourceChange(plan)

Loading

The proposed behavior would instead perform planning during the Check call:

  sequenceDiagram
      participant Engine
      participant BridgedProvider
      participant TFProvider
      Engine->>BridgedProvider: plan := Check(oldState, config)
      BridgedProvider->>TFProvider: plan := PlanResourceChange(oldState, config)
      Engine->>BridgedProvider: Diff(oldState, plan)
      BridgedProvider->>Engine: compare(oldState, plan)
      Engine->>BridgedProvider: Update(oldState, plan)
      BridgedProvider->>TFProvider: ApplyResourceChange(plan)

Loading

Rationale

TF providers do a lot of work in PlanResourceChange (see Lifecycle) that is best treated as a black box by the bridge; this work includes applying defaults and executing PlanModifiers. Explaining the planned state to the engine and making it explicit on the protocol instead of something done implicitly in Diff could be a great simplification. It reduces impedance mismatch and can make debugging and maintaining bridged providers easier.

Necessary Changes

• Pulumi provider protocol might need to be modified to send the entire old state (inputs and outputs) and not just old inputs to Check
• Pulumi developer docs around Check need editing explaining that the output of Check represents the planned state and may encompass both inputs proper as well as predicted outputs
• Modify the SDKv2 bridge
• Modify the PF bridge

Testing

We would need to double check the coverage of all the flows involving Check.

• verify creating a resource
• verify updating a resource
• verify refreshing a resource (pulumi refresh)
• verify importing a resource (pulumi import)
• verify importing a resource through an import option (pulumi up)
• verify resource getters R.get (pulumi up) # TBD does this involve Check?
• verify that detailed diff values are resolved against oldState and plan for the purposes of presenting to the user
• verify ignoreChanges through the above scenarios

Issues Fixed

• Unable to get past the errors when using the import resource option #2272
• Likely a lot more, correlating issues here...

[VenelinMartinov]

Found some engine behaviour which supports that plan should be run in Check: #2660 (comment)

When a resource is going to be replaced the engine re-runs Check with nil Olds. This would solve #2660 if we were running Plan in Check