From 358bb5faac6a62468a55d435a23f0d786cb8316e Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Mon, 4 Dec 2023 16:35:21 -0600 Subject: [PATCH] removed composition error from branch node Signed-off-by: Daniel Rammer --- .../pkg/apis/flyteworkflow/v1alpha1/branch.go | 42 +++---------------- .../flyteworkflow/v1alpha1/branch_test.go | 23 +--------- .../v1alpha1/zz_generated.deepcopy.go | 12 +----- ...rol_flow.conditions.multiplier_2_2_wf.yaml | 2 +- ...rol_flow.conditions.multiplier_3_2_wf.yaml | 2 +- ...low.conditions.nested_conditions_2_wf.yaml | 2 +- .../pkg/compiler/transformers/k8s/node.go | 2 +- .../controller/nodes/branch/evaluator_test.go | 6 +-- flytepropeller/pkg/webhook/config/config.go | 3 +- flytepropeller/pkg/webhook/entrypoint.go | 3 +- flytepropeller/pkg/webhook/pod.go | 2 +- 11 files changed, 18 insertions(+), 81 deletions(-) diff --git a/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go b/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go index 07a493b8be..37a54dfffe 100644 --- a/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go +++ b/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch.go @@ -8,35 +8,6 @@ import ( "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" ) -type Error struct { - *core.Error -} - -func (in Error) UnmarshalJSON(b []byte) error { - in.Error = &core.Error{} - return jsonpb.Unmarshal(bytes.NewReader(b), in.Error) -} - -func (in Error) MarshalJSON() ([]byte, error) { - if in.Error == nil { - return nilJSON, nil - } - - var buf bytes.Buffer - if err := marshaler.Marshal(&buf, in.Error); err != nil { - return nil, err - } - return buf.Bytes(), nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Error) DeepCopyInto(out *Error) { - *out = *in - // We do not manipulate the object, so its ok - // Once we figure out the autogenerate story we can replace this - -} - type BooleanExpression struct { *core.BooleanExpression } @@ -79,10 +50,10 @@ func (in *IfBlock) GetThenNode() *NodeID { } type BranchNodeSpec struct { - If IfBlock `json:"if"` - ElseIf []*IfBlock `json:"elseIf,omitempty"` - Else *NodeID `json:"else,omitempty"` - ElseFail *Error `json:"elseFail,omitempty"` + If IfBlock `json:"if"` + ElseIf []*IfBlock `json:"elseIf,omitempty"` + Else *NodeID `json:"else,omitempty"` + ElseFail *core.Error `json:"elseFail,omitempty"` } func (in *BranchNodeSpec) GetIf() ExecutableIfBlock { @@ -102,8 +73,5 @@ func (in *BranchNodeSpec) GetElseIf() []ExecutableIfBlock { } func (in *BranchNodeSpec) GetElseFail() *core.Error { - if in.ElseFail != nil { - return in.ElseFail.Error - } - return nil + return in.ElseFail } diff --git a/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch_test.go b/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch_test.go index 5fe19f6758..5fd2a14218 100644 --- a/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch_test.go +++ b/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/branch_test.go @@ -1,12 +1,10 @@ package v1alpha1 import ( - "bytes" "encoding/json" "io/ioutil" "testing" - "github.com/golang/protobuf/jsonpb" "github.com/stretchr/testify/assert" "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" @@ -27,25 +25,6 @@ func TestMarshalUnMarshal_BranchTask(t *testing.T) { } } -// TestBranchNodeSpecMethods tests the methods of the BranchNodeSpec struct. -func TestErrorMarshalAndUnmarshalJSON(t *testing.T) { - coreError := &core.Error{ - FailedNodeId: "TestNode", - Message: "Test error message", - } - - err := Error{Error: coreError} - data, jErr := err.MarshalJSON() - assert.Nil(t, jErr) - - // Unmarshalling the JSON back to a new core.Error struct - var newCoreError core.Error - uErr := jsonpb.Unmarshal(bytes.NewReader(data), &newCoreError) - assert.Nil(t, uErr) - assert.Equal(t, coreError.Message, newCoreError.Message) - assert.Equal(t, coreError.FailedNodeId, newCoreError.FailedNodeId) -} - func TestBranchNodeSpecMethods(t *testing.T) { // Creating a core.BooleanExpression instance for testing boolExpr := &core.BooleanExpression{} @@ -76,7 +55,7 @@ func TestBranchNodeSpecMethods(t *testing.T) { }, }, Else: &elseNode, - ElseFail: &Error{Error: errorMessage}, + ElseFail: errorMessage, } assert.Equal(t, boolExpr, branchNodeSpec.If.GetCondition()) diff --git a/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/zz_generated.deepcopy.go b/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/zz_generated.deepcopy.go index 80d1f04427..febbca733c 100644 --- a/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/zz_generated.deepcopy.go +++ b/flytepropeller/pkg/apis/flyteworkflow/v1alpha1/zz_generated.deepcopy.go @@ -64,7 +64,7 @@ func (in *BranchNodeSpec) DeepCopyInto(out *BranchNodeSpec) { } if in.ElseFail != nil { in, out := &in.ElseFail, &out.ElseFail - *out = (*in).DeepCopy() + *out = *in } return } @@ -142,16 +142,6 @@ func (in *DynamicNodeStatus) DeepCopy() *DynamicNodeStatus { return out } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Error. -func (in *Error) DeepCopy() *Error { - if in == nil { - return nil - } - out := new(Error) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExecutionConfig) DeepCopyInto(out *ExecutionConfig) { *out = *in diff --git a/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/028_core.control_flow.conditions.multiplier_2_2_wf.yaml b/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/028_core.control_flow.conditions.multiplier_2_2_wf.yaml index 7ec08f7c34..54fb0413d5 100755 --- a/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/028_core.control_flow.conditions.multiplier_2_2_wf.yaml +++ b/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/028_core.control_flow.conditions.multiplier_2_2_wf.yaml @@ -76,7 +76,7 @@ spec: n0: branch: elseFail: - failedNodeId: fractions + failed_node_id: fractions message: The input must be between 0 and 10 elseIf: - condition: diff --git a/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/030_core.control_flow.conditions.multiplier_3_2_wf.yaml b/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/030_core.control_flow.conditions.multiplier_3_2_wf.yaml index 10d028fa4a..07c632c8e0 100755 --- a/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/030_core.control_flow.conditions.multiplier_3_2_wf.yaml +++ b/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/030_core.control_flow.conditions.multiplier_3_2_wf.yaml @@ -82,7 +82,7 @@ spec: n0: branch: elseFail: - failedNodeId: fractions + failed_node_id: fractions message: The input must be between 0 and 10 elseIf: - condition: diff --git a/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/039_core.control_flow.conditions.nested_conditions_2_wf.yaml b/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/039_core.control_flow.conditions.nested_conditions_2_wf.yaml index 8579f95947..abfc362b09 100755 --- a/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/039_core.control_flow.conditions.nested_conditions_2_wf.yaml +++ b/flytepropeller/pkg/compiler/test/testdata/snacks-core/k8s/039_core.control_flow.conditions.nested_conditions_2_wf.yaml @@ -135,7 +135,7 @@ spec: n0-n0: branch: elseFail: - failedNodeId: inner_fractions + failed_node_id: inner_fractions message: Only <0.7 allowed elseIf: - condition: diff --git a/flytepropeller/pkg/compiler/transformers/k8s/node.go b/flytepropeller/pkg/compiler/transformers/k8s/node.go index 2ac06ebd89..7120f3ad90 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/node.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/node.go @@ -229,7 +229,7 @@ func buildBranchNodeSpec(branch *core.BranchNode, tasks []*core.CompiledTask, er childNodes = append(childNodes, ns...) res.Else = refStr(branch.IfElse.GetElseNode().Id) case *core.IfElseBlock_Error: - res.ElseFail = &v1alpha1.Error{Error: branch.IfElse.GetError()} + res.ElseFail = branch.IfElse.GetError() } other := make([]*v1alpha1.IfBlock, 0, len(branch.IfElse.Other)) diff --git a/flytepropeller/pkg/controller/nodes/branch/evaluator_test.go b/flytepropeller/pkg/controller/nodes/branch/evaluator_test.go index 45bb850358..dae8a1337b 100644 --- a/flytepropeller/pkg/controller/nodes/branch/evaluator_test.go +++ b/flytepropeller/pkg/controller/nodes/branch/evaluator_test.go @@ -672,10 +672,8 @@ func TestDecideBranch(t *testing.T) { ThenNode: &n2, }, }, - ElseFail: &v1alpha1.Error{ - Error: &core.Error{ - Message: userError, - }, + ElseFail: &core.Error{ + Message: userError, }, } diff --git a/flytepropeller/pkg/webhook/config/config.go b/flytepropeller/pkg/webhook/config/config.go index a1a6fd94ae..71e901ad5b 100644 --- a/flytepropeller/pkg/webhook/config/config.go +++ b/flytepropeller/pkg/webhook/config/config.go @@ -1,9 +1,10 @@ package config import ( + "os" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" - "os" "github.com/flyteorg/flyte/flytestdlib/config" ) diff --git a/flytepropeller/pkg/webhook/entrypoint.go b/flytepropeller/pkg/webhook/entrypoint.go index ad2c21d6a1..1433c52689 100644 --- a/flytepropeller/pkg/webhook/entrypoint.go +++ b/flytepropeller/pkg/webhook/entrypoint.go @@ -5,10 +5,11 @@ import ( "encoding/json" errors2 "errors" "fmt" + "os" + "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" - "os" "sigs.k8s.io/controller-runtime/pkg/manager" "github.com/flyteorg/flyte/flytepropeller/pkg/controller/config" diff --git a/flytepropeller/pkg/webhook/pod.go b/flytepropeller/pkg/webhook/pod.go index 144c9f0a50..9e3a300bf6 100644 --- a/flytepropeller/pkg/webhook/pod.go +++ b/flytepropeller/pkg/webhook/pod.go @@ -31,7 +31,6 @@ import ( "context" "encoding/json" "fmt" - "k8s.io/apimachinery/pkg/runtime" "net/http" "os" "path/filepath" @@ -40,6 +39,7 @@ import ( admissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/webhook/admission"