Order UpstreamNodeIds to ensure consistent compiler output (flyteorg#323

) * Order UpstreamNodeIds to ensure consistent compiler output Signed-off-by: Haytham Abuelfutuh <[email protected]> * Unit test & lint Signed-off-by: Haytham Abuelfutuh <[email protected]>
eapolinario · Sep 13, 2021 · 7370e4d · 7370e4d
1 parent 248f4cd
commit 7370e4d
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 9 deletions.
diff --git a/flytepropeller/pkg/compiler/validators/node.go b/flytepropeller/pkg/compiler/validators/node.go
@@ -3,6 +3,7 @@ package validators
 
 import (
 	"fmt"
+	"sort"
 
 	flyte "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
 	c "github.com/flyteorg/flytepropeller/pkg/compiler/common"
@@ -120,6 +121,13 @@ func ValidateNode(w c.WorkflowBuilder, n c.NodeBuilder, validateConditionTypes b
 		validateEffectiveOutputParameters(n, errs.NewScope())
 	}
 
+	if n.GetCoreNode().UpstreamNodeIds == nil {
+		n.GetCoreNode().UpstreamNodeIds = make([]string, 0)
+	}
+
+	// Order upstream node ids to ensure consistent output of the compiler even if client ordering changes.
+	sort.Strings(n.GetCoreNode().UpstreamNodeIds)
+
 	// Validate branch node conditions and inner nodes.
 	if n.GetBranchNode() != nil {
 		if nodes, ok := ValidateBranchNode(w, n, validateConditionTypes, errs.NewScope()); ok {

diff --git a/flytepropeller/pkg/compiler/validators/node_test.go b/flytepropeller/pkg/compiler/validators/node_test.go
@@ -37,13 +37,44 @@ func TestValidateBranchNode(t *testing.T) {
 }
 
 func TestValidateNode(t *testing.T) {
-	n := &mocks.NodeBuilder{}
-	n.OnGetId().Return(common.StartNodeID)
-
-	wf := &mocks.WorkflowBuilder{}
-	errs := errors.NewCompileErrors()
-	ValidateNode(wf, n, true, errs)
-	if errs.HasErrors() {
-		assert.NoError(t, errs)
-	}
+	t.Run("Start-node", func(t *testing.T) {
+		n := &mocks.NodeBuilder{}
+		n.OnGetId().Return(common.StartNodeID)
+
+		wf := &mocks.WorkflowBuilder{}
+		errs := errors.NewCompileErrors()
+		ValidateNode(wf, n, true, errs)
+		if !assert.False(t, errs.HasErrors()) {
+			assert.NoError(t, errs)
+		}
+	})
+
+	t.Run("Sort upstream node ids", func(t *testing.T) {
+		n := &mocks.NodeBuilder{}
+		n.OnGetId().Return("my-node")
+		n.OnGetInterface().Return(&core.TypedInterface{
+			Outputs: &core.VariableMap{},
+			Inputs:  &core.VariableMap{},
+		})
+		n.OnGetOutputAliases().Return(nil)
+		n.OnGetBranchNode().Return(nil)
+		n.OnGetWorkflowNode().Return(nil)
+		n.OnGetTaskNode().Return(nil)
+
+		coreN := &core.Node{}
+		coreN.UpstreamNodeIds = []string{"n1", "n0"}
+		n.OnGetCoreNode().Return(coreN)
+		n.On("GetUpstreamNodeIds").Return(func() []string {
+			return coreN.UpstreamNodeIds
+		})
+
+		wf := &mocks.WorkflowBuilder{}
+		errs := errors.NewCompileErrors()
+		ValidateNode(wf, n, true, errs)
+		if !assert.False(t, errs.HasErrors()) {
+			assert.NoError(t, errs)
+		}
+
+		assert.Equal(t, []string{"n0", "n1"}, n.GetUpstreamNodeIds())
+	})
 }