From 4cce6631d896bf34e28294381a7c404c860c08eb Mon Sep 17 00:00:00 2001 From: troychiu Date: Wed, 27 Mar 2024 14:28:17 -0700 Subject: [PATCH 1/5] Update phase field to use closure's phase Signed-off-by: troychiu --- flyteadmin/pkg/repositories/transformers/execution.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flyteadmin/pkg/repositories/transformers/execution.go b/flyteadmin/pkg/repositories/transformers/execution.go index 368cd8e4bd..b018faf7d7 100644 --- a/flyteadmin/pkg/repositories/transformers/execution.go +++ b/flyteadmin/pkg/repositories/transformers/execution.go @@ -128,7 +128,7 @@ func CreateExecutionModel(input CreateExecutionModelInput) (*models.Execution, e Name: input.WorkflowExecutionID.Name, }, Spec: spec, - Phase: input.Phase.String(), + Phase: closure.Phase.String(), Closure: closureBytes, WorkflowID: input.WorkflowID, ExecutionCreatedAt: &input.CreatedAt, From 1227bdee8381dcaebe89d738d66cfc1a01ffcbca Mon Sep 17 00:00:00 2001 From: troychiu Date: Wed, 27 Mar 2024 14:38:49 -0700 Subject: [PATCH 2/5] Update execution phase in testCreateExecutionModel Signed-off-by: troychiu --- flyteadmin/pkg/repositories/transformers/execution_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/flyteadmin/pkg/repositories/transformers/execution_test.go b/flyteadmin/pkg/repositories/transformers/execution_test.go index f3d0b41a5e..0764ae4dc1 100644 --- a/flyteadmin/pkg/repositories/transformers/execution_test.go +++ b/flyteadmin/pkg/repositories/transformers/execution_test.go @@ -163,6 +163,7 @@ func TestCreateExecutionModel(t *testing.T) { assert.Equal(t, nodeID, execution.ParentNodeExecutionID) assert.Equal(t, sourceID, execution.SourceExecutionID) assert.Equal(t, "launch_plan", execution.LaunchEntity) + assert.Equal(t, execution.Phase, core.WorkflowExecution_FAILED.String()) expectedSpec := execRequest.Spec expectedSpec.Metadata.Principal = principal expectedSpec.Metadata.SystemMetadata = &admin.SystemMetadata{ From ada86113deac721c925e0b4864bc55b843029d55 Mon Sep 17 00:00:00 2001 From: troychiu Date: Thu, 28 Mar 2024 10:04:03 -0700 Subject: [PATCH 3/5] Remove unnecessary phase setting in execution model creation Signed-off-by: troychiu --- flyteadmin/pkg/manager/impl/execution_manager.go | 2 -- .../pkg/repositories/transformers/execution.go | 6 +----- .../repositories/transformers/execution_test.go | 14 ++++++-------- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/flyteadmin/pkg/manager/impl/execution_manager.go b/flyteadmin/pkg/manager/impl/execution_manager.go index e3fd87ba71..fb2e0d377c 100644 --- a/flyteadmin/pkg/manager/impl/execution_manager.go +++ b/flyteadmin/pkg/manager/impl/execution_manager.go @@ -617,7 +617,6 @@ func (m *ExecutionManager) launchSingleTaskExecution( TaskID: taskModel.ID, WorkflowID: workflowModel.ID, // The execution is not considered running until the propeller sends a specific event saying so. - Phase: core.WorkflowExecution_UNDEFINED, CreatedAt: m._clock.Now(), Notifications: notificationsSettings, WorkflowIdentifier: workflow.Id, @@ -1006,7 +1005,6 @@ func (m *ExecutionManager) launchExecutionAndPrepareModel( LaunchPlanID: launchPlanModel.ID, WorkflowID: launchPlanModel.WorkflowID, // The execution is not considered running until the propeller sends a specific event saying so. - Phase: core.WorkflowExecution_UNDEFINED, CreatedAt: m._clock.Now(), Notifications: notificationsSettings, WorkflowIdentifier: workflow.Id, diff --git a/flyteadmin/pkg/repositories/transformers/execution.go b/flyteadmin/pkg/repositories/transformers/execution.go index b018faf7d7..66fb438ea7 100644 --- a/flyteadmin/pkg/repositories/transformers/execution.go +++ b/flyteadmin/pkg/repositories/transformers/execution.go @@ -34,7 +34,6 @@ type CreateExecutionModelInput struct { LaunchPlanID uint WorkflowID uint TaskID uint - Phase core.WorkflowExecution_Phase CreatedAt time.Time Notifications []*admin.Notification WorkflowIdentifier *core.Identifier @@ -76,7 +75,7 @@ func CreateExecutionModel(input CreateExecutionModelInput) (*models.Execution, e } createdAt := timestamppb.New(input.CreatedAt) closure := admin.ExecutionClosure{ - Phase: input.Phase, + Phase: core.WorkflowExecution_UNDEFINED, CreatedAt: createdAt, UpdatedAt: createdAt, Notifications: input.Notifications, @@ -87,9 +86,6 @@ func CreateExecutionModel(input CreateExecutionModelInput) (*models.Execution, e OccurredAt: createdAt, }, } - if input.Phase == core.WorkflowExecution_RUNNING { - closure.StartedAt = createdAt - } if input.Error != nil { closure.Phase = core.WorkflowExecution_FAILED execErr := &core.ExecutionError{ diff --git a/flyteadmin/pkg/repositories/transformers/execution_test.go b/flyteadmin/pkg/repositories/transformers/execution_test.go index 0764ae4dc1..b5641c7c8e 100644 --- a/flyteadmin/pkg/repositories/transformers/execution_test.go +++ b/flyteadmin/pkg/repositories/transformers/execution_test.go @@ -71,7 +71,7 @@ func TestCreateExecutionModel(t *testing.T) { }, } namespace := "ns" - t.Run("running", func(t *testing.T) { + t.Run("successful execution", func(t *testing.T) { execution, err := CreateExecutionModel(CreateExecutionModelInput{ WorkflowExecutionID: core.WorkflowExecutionIdentifier{ Project: "project", @@ -81,7 +81,6 @@ func TestCreateExecutionModel(t *testing.T) { RequestSpec: execRequest.Spec, LaunchPlanID: lpID, WorkflowID: wfID, - Phase: core.WorkflowExecution_RUNNING, CreatedAt: createdAt, WorkflowIdentifier: workflowIdentifier, ParentNodeExecutionID: nodeID, @@ -103,6 +102,7 @@ func TestCreateExecutionModel(t *testing.T) { assert.Equal(t, nodeID, execution.ParentNodeExecutionID) assert.Equal(t, sourceID, execution.SourceExecutionID) assert.Equal(t, "launch_plan", execution.LaunchEntity) + assert.Equal(t, execution.Phase, core.WorkflowExecution_UNDEFINED.String()) expectedSpec := execRequest.Spec expectedSpec.Metadata.Principal = principal expectedSpec.Metadata.SystemMetadata = &admin.SystemMetadata{ @@ -116,9 +116,8 @@ func TestCreateExecutionModel(t *testing.T) { expectedCreatedAt, _ := ptypes.TimestampProto(createdAt) expectedClosure, _ := proto.Marshal(&admin.ExecutionClosure{ - Phase: core.WorkflowExecution_RUNNING, + Phase: core.WorkflowExecution_UNDEFINED, CreatedAt: expectedCreatedAt, - StartedAt: expectedCreatedAt, UpdatedAt: expectedCreatedAt, WorkflowId: workflowIdentifier, StateChangeDetails: &admin.ExecutionStateChangeDetails{ @@ -140,7 +139,6 @@ func TestCreateExecutionModel(t *testing.T) { RequestSpec: execRequest.Spec, LaunchPlanID: lpID, WorkflowID: wfID, - Phase: core.WorkflowExecution_RUNNING, CreatedAt: createdAt, WorkflowIdentifier: workflowIdentifier, ParentNodeExecutionID: nodeID, @@ -163,7 +161,7 @@ func TestCreateExecutionModel(t *testing.T) { assert.Equal(t, nodeID, execution.ParentNodeExecutionID) assert.Equal(t, sourceID, execution.SourceExecutionID) assert.Equal(t, "launch_plan", execution.LaunchEntity) - assert.Equal(t, execution.Phase, core.WorkflowExecution_FAILED.String()) + assert.Equal(t, core.WorkflowExecution_FAILED.String(), execution.Phase) expectedSpec := execRequest.Spec expectedSpec.Metadata.Principal = principal expectedSpec.Metadata.SystemMetadata = &admin.SystemMetadata{ @@ -208,7 +206,6 @@ func TestCreateExecutionModel(t *testing.T) { RequestSpec: execRequest.Spec, LaunchPlanID: lpID, WorkflowID: wfID, - Phase: core.WorkflowExecution_RUNNING, CreatedAt: createdAt, WorkflowIdentifier: workflowIdentifier, ParentNodeExecutionID: nodeID, @@ -231,6 +228,7 @@ func TestCreateExecutionModel(t *testing.T) { assert.Equal(t, nodeID, execution.ParentNodeExecutionID) assert.Equal(t, sourceID, execution.SourceExecutionID) assert.Equal(t, "launch_plan", execution.LaunchEntity) + assert.Equal(t, core.WorkflowExecution_FAILED.String(), execution.Phase) expectedSpec := execRequest.Spec expectedSpec.Metadata.Principal = principal expectedSpec.Metadata.SystemMetadata = &admin.SystemMetadata{ @@ -275,7 +273,6 @@ func TestCreateExecutionModel(t *testing.T) { RequestSpec: execRequest.Spec, LaunchPlanID: lpID, WorkflowID: wfID, - Phase: core.WorkflowExecution_RUNNING, CreatedAt: createdAt, WorkflowIdentifier: workflowIdentifier, ParentNodeExecutionID: nodeID, @@ -298,6 +295,7 @@ func TestCreateExecutionModel(t *testing.T) { assert.Equal(t, nodeID, execution.ParentNodeExecutionID) assert.Equal(t, sourceID, execution.SourceExecutionID) assert.Equal(t, "launch_plan", execution.LaunchEntity) + assert.Equal(t, core.WorkflowExecution_FAILED.String(), execution.Phase) expectedSpec := execRequest.Spec expectedSpec.Metadata.Principal = principal expectedSpec.Metadata.SystemMetadata = &admin.SystemMetadata{ From f757699cb8fcef8c7d88a483d5de18d570d4ed9e Mon Sep 17 00:00:00 2001 From: troychiu Date: Thu, 28 Mar 2024 10:09:40 -0700 Subject: [PATCH 4/5] fix test Signed-off-by: troychiu --- flyteadmin/pkg/repositories/transformers/execution_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/flyteadmin/pkg/repositories/transformers/execution_test.go b/flyteadmin/pkg/repositories/transformers/execution_test.go index b5641c7c8e..e9d12807f5 100644 --- a/flyteadmin/pkg/repositories/transformers/execution_test.go +++ b/flyteadmin/pkg/repositories/transformers/execution_test.go @@ -184,7 +184,6 @@ func TestCreateExecutionModel(t *testing.T) { }, }, CreatedAt: expectedCreatedAt, - StartedAt: expectedCreatedAt, UpdatedAt: expectedCreatedAt, WorkflowId: workflowIdentifier, StateChangeDetails: &admin.ExecutionStateChangeDetails{ @@ -251,7 +250,6 @@ func TestCreateExecutionModel(t *testing.T) { }, }, CreatedAt: expectedCreatedAt, - StartedAt: expectedCreatedAt, UpdatedAt: expectedCreatedAt, WorkflowId: workflowIdentifier, StateChangeDetails: &admin.ExecutionStateChangeDetails{ @@ -318,7 +316,6 @@ func TestCreateExecutionModel(t *testing.T) { }, }, CreatedAt: expectedCreatedAt, - StartedAt: expectedCreatedAt, UpdatedAt: expectedCreatedAt, WorkflowId: workflowIdentifier, StateChangeDetails: &admin.ExecutionStateChangeDetails{ From ec6d6133ce7447fd945db40476b9e6cb0edf0e1b Mon Sep 17 00:00:00 2001 From: troychiu Date: Thu, 28 Mar 2024 10:53:38 -0700 Subject: [PATCH 5/5] Remove undefined phase when creating execution model Signed-off-by: troychiu --- flyteadmin/pkg/repositories/transformers/execution.go | 1 - 1 file changed, 1 deletion(-) diff --git a/flyteadmin/pkg/repositories/transformers/execution.go b/flyteadmin/pkg/repositories/transformers/execution.go index 66fb438ea7..3b8c556abd 100644 --- a/flyteadmin/pkg/repositories/transformers/execution.go +++ b/flyteadmin/pkg/repositories/transformers/execution.go @@ -75,7 +75,6 @@ func CreateExecutionModel(input CreateExecutionModelInput) (*models.Execution, e } createdAt := timestamppb.New(input.CreatedAt) closure := admin.ExecutionClosure{ - Phase: core.WorkflowExecution_UNDEFINED, CreatedAt: createdAt, UpdatedAt: createdAt, Notifications: input.Notifications,