From 37ae631ee39ecee59b3d75b78b5427dac7bf67a2 Mon Sep 17 00:00:00 2001 From: Babis Kiosidis Date: Thu, 25 Aug 2022 14:05:54 +0300 Subject: [PATCH 1/3] avoid interacting with nil mutatedWfs Signed-off-by: Babis Kiosidis --- pkg/controller/handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/handler.go b/pkg/controller/handler.go index 95fd46cb1..7707fe284 100644 --- a/pkg/controller/handler.go +++ b/pkg/controller/handler.go @@ -245,7 +245,7 @@ func (p *Propeller) Handle(ctx context.Context, namespace, name string) error { t := p.metrics.RoundTime.Start(ctx) mutatedWf, err := p.TryMutateWorkflow(ctx, w) - if wfClosureCrdFields != nil { + if wfClosureCrdFields != nil && mutatedWf != nil { // strip data populated from WorkflowClosureReference w.SubWorkflows, w.Tasks, w.WorkflowSpec = nil, nil, nil mutatedWf.SubWorkflows, mutatedWf.Tasks, mutatedWf.WorkflowSpec = nil, nil, nil From 7bb6b59f667bdcd47ad70ef0e94e04d8c96d88e5 Mon Sep 17 00:00:00 2001 From: Babis Kiosidis Date: Thu, 25 Aug 2022 17:04:45 +0300 Subject: [PATCH 2/3] only strip crd fields from mutated wfs Signed-off-by: Babis Kiosidis --- pkg/controller/handler.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/controller/handler.go b/pkg/controller/handler.go index 7707fe284..bd742ad6e 100644 --- a/pkg/controller/handler.go +++ b/pkg/controller/handler.go @@ -245,10 +245,12 @@ func (p *Propeller) Handle(ctx context.Context, namespace, name string) error { t := p.metrics.RoundTime.Start(ctx) mutatedWf, err := p.TryMutateWorkflow(ctx, w) - if wfClosureCrdFields != nil && mutatedWf != nil { + if wfClosureCrdFields != nil { // strip data populated from WorkflowClosureReference w.SubWorkflows, w.Tasks, w.WorkflowSpec = nil, nil, nil - mutatedWf.SubWorkflows, mutatedWf.Tasks, mutatedWf.WorkflowSpec = nil, nil, nil + if mutatedWf != nil { + mutatedWf.SubWorkflows, mutatedWf.Tasks, mutatedWf.WorkflowSpec = nil, nil, nil + } } if err != nil { From 0ce86ec56f438550902126f971b2ad5b04f607b8 Mon Sep 17 00:00:00 2001 From: Babis Kiosidis Date: Thu, 25 Aug 2022 17:25:52 +0300 Subject: [PATCH 3/3] test failure in executor Signed-off-by: Babis Kiosidis --- pkg/controller/handler.go | 2 +- pkg/controller/handler_test.go | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/pkg/controller/handler.go b/pkg/controller/handler.go index bd742ad6e..e40e4d045 100644 --- a/pkg/controller/handler.go +++ b/pkg/controller/handler.go @@ -245,7 +245,7 @@ func (p *Propeller) Handle(ctx context.Context, namespace, name string) error { t := p.metrics.RoundTime.Start(ctx) mutatedWf, err := p.TryMutateWorkflow(ctx, w) - if wfClosureCrdFields != nil { + if wfClosureCrdFields != nil { // strip data populated from WorkflowClosureReference w.SubWorkflows, w.Tasks, w.WorkflowSpec = nil, nil, nil if mutatedWf != nil { diff --git a/pkg/controller/handler_test.go b/pkg/controller/handler_test.go index b7892a73e..e81c871bc 100644 --- a/pkg/controller/handler_test.go +++ b/pkg/controller/handler_test.go @@ -894,4 +894,25 @@ func TestPropellerHandler_OffloadedWorkflowClosure(t *testing.T) { err := p.Handle(ctx, namespace, name) assert.Error(t, err) }) + + t.Run("TryMutate failure is handled", func(t *testing.T) { + scope := promutils.NewTestScope() + + protoStore := &storagemocks.ComposedProtobufStore{} + protoStore.OnReadProtobufMatch(mock.Anything, mock.Anything, mock.Anything).Return(fmt.Errorf("foo")) + exec.HandleCb = func(ctx context.Context, w *v1alpha1.FlyteWorkflow) error { + return fmt.Errorf("foo") + } + dataStore := storage.NewCompositeDataStore(storage.URLPathConstructor{}, protoStore) + p := NewPropellerHandler(ctx, cfg, dataStore, s, exec, scope) + + err := p.Handle(ctx, namespace, name) + assert.Error(t, err, "foo") + + r, err := s.Get(ctx, namespace, name) + assert.NoError(t, err) + assert.Nil(t, r.WorkflowSpec) + assert.Nil(t, r.SubWorkflows) + assert.Nil(t, r.Tasks) + }) }