Fail refreshes with init errors. (#1882)

And ensure that refreshes continue on errors.

Fixes #1881.

pkg/engine/lifecycle_test.go

@@ -1119,6 +1119,14 @@ func TestExternalRefresh(t *testing.T) {
 }
 
 func TestRefreshInitFailure(t *testing.T) {
+	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`.
@@ -1134,11 +1142,13 @@ func TestRefreshInitFailure(t *testing.T) {
 				ReadF: func(
 					urn resource.URN, id resource.ID, props resource.PropertyMap,
 				) (resource.PropertyMap, resource.Status, error) {
-					if refreshShouldFail {
+					if refreshShouldFail && urn == resURN {
 						err := &plugin.InitError{
 							Reasons: []string{"Refresh reports continued to fail to initialize"},
 						}
 						return resource.PropertyMap{}, resource.StatusPartialFailure, err
+					} else if urn == res2URN {
+						return res2Outputs, resource.StatusOK, nil
 					}
 					return resource.PropertyMap{}, resource.StatusOK, nil
 				},
@@ -1154,26 +1164,31 @@ func TestRefreshInitFailure(t *testing.T) {
 	})
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
-	p := &TestPlan{
-		Options: UpdateOptions{host: host},
-	}
-
-	provURN := p.NewProviderURN("pkgA", "default", "")
-	resURN := p.NewURN("pkgA:m:typA", "resA", "")
+	p.Options.host = host
 
 	//
 	// 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"},
-		}},
+		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{},
+			},
+		},
 	}
 
 	//
@@ -1188,6 +1203,8 @@ func TestRefreshInitFailure(t *testing.T) {
 			// break
 		case resURN:
 			assert.Empty(t, resource.InitErrors)
+		case res2URN:
+			assert.Equal(t, res2Outputs, resource.Outputs)
 		default:
 			t.Fatalf("unexpected resource %v", urn)
 		}
@@ -1197,7 +1214,7 @@ func TestRefreshInitFailure(t *testing.T) {
 	// Refresh DOES fail, causing the new initialization error to appear.
 	//
 	refreshShouldFail = true
-	p.Steps = []TestStep{{Op: Refresh}}
+	p.Steps = []TestStep{{Op: Refresh, ExpectFailure: true}}
 	snap = p.Run(t, old)
 	for _, resource := range snap.Resources {
 		switch urn := resource.URN; urn {
@@ -1205,6 +1222,8 @@ func TestRefreshInitFailure(t *testing.T) {
 			// 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)
 		}

pkg/resource/deploy/plan_executor.go

@@ -93,7 +93,7 @@ func (pe *planExecutor) Execute(callerCtx context.Context, opts Options, preview
 
 	// Set up a step generator and executor for this plan.
 	pe.stepGen = newStepGenerator(pe.plan, opts)
-	pe.stepExec = newStepExecutor(ctx, cancel, pe.plan, opts, preview)
+	pe.stepExec = newStepExecutor(ctx, cancel, pe.plan, opts, preview, false)
 
 	// We iterate the source in its own goroutine because iteration is blocking and we want the main loop to be able to
 	// respond to cancellation requests promptly.
@@ -224,7 +224,7 @@ func (pe *planExecutor) refresh(callerCtx context.Context, opts Options, preview
 
 	// Fire up a worker pool and issue each refresh in turn.
 	ctx, cancel := context.WithCancel(callerCtx)
-	stepExec := newStepExecutor(ctx, cancel, pe.plan, opts, preview)
+	stepExec := newStepExecutor(ctx, cancel, pe.plan, opts, preview, true)
 	for i := range steps {
 		if ctx.Err() != nil {
 			break

pkg/resource/deploy/step.go

@@ -607,9 +607,7 @@ func (s *RefreshStep) Apply(preview bool) (resource.Status, StepCompleteFunc, er
 			return rst, nil, err
 		}
 		if initErr, isInitErr := err.(*plugin.InitError); isInitErr {
-			// We clear error in this case because we do not want the refresh to fail in the face of initialization
-			// errors.
-			initErrors, err = initErr.Reasons, nil
+			initErrors = initErr.Reasons
 		}
 	}
 

pkg/resource/deploy/step_executor.go

@@ -52,10 +52,11 @@ type Chain = []Step
 // resolved, we (the engine) can assume that any chain given to us by the step generator is already
 // ready to execute.
 type stepExecutor struct {
-	plan        *Plan    // The plan currently being executed.
-	opts        Options  // The options for this current plan.
-	preview     bool     // Whether or not we are doing a preview.
-	pendingNews sync.Map // Resources that have been created but are pending a RegisterResourceOutputs.
+	plan            *Plan    // The plan currently being executed.
+	opts            Options  // The options for this current plan.
+	preview         bool     // Whether or not we are doing a preview.
+	pendingNews     sync.Map // Resources that have been created but are pending a RegisterResourceOutputs.
+	continueOnError bool     // True if we want to continue the plan after a step error.
 
 	workers        sync.WaitGroup // WaitGroup tracking the worker goroutines that are owned by this step executor.
 	incomingChains chan Chain     // Incoming chains that we are to execute
@@ -173,7 +174,9 @@ func (se *stepExecutor) executeChain(workerID int, chain Chain) {
 
 func (se *stepExecutor) cancelDueToError() {
 	se.sawError.Store(true)
-	se.cancel()
+	if !se.continueOnError {
+		se.cancel()
+	}
 }
 
 //
@@ -285,14 +288,15 @@ func (se *stepExecutor) worker(workerID int) {
 }
 
 func newStepExecutor(ctx context.Context, cancel context.CancelFunc, plan *Plan, opts Options,
-	preview bool) *stepExecutor {
+	preview, continueOnError bool) *stepExecutor {
 	exec := &stepExecutor{
-		plan:           plan,
-		opts:           opts,
-		preview:        preview,
-		incomingChains: make(chan Chain),
-		ctx:            ctx,
-		cancel:         cancel,
+		plan:            plan,
+		opts:            opts,
+		preview:         preview,
+		continueOnError: continueOnError,
+		incomingChains:  make(chan Chain),
+		ctx:             ctx,
+		cancel:          cancel,
 	}
 
 	exec.sawError.Store(false)