Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating ArrayNode ExternalResourceInfo ID #4677

Merged
merged 5 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 39 additions & 15 deletions flytepropeller/pkg/controller/nodes/array/event_recorder.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,24 @@
import (
"context"
"fmt"
"strconv"
"time"

"github.com/golang/protobuf/ptypes"

idlcore "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/event"
"github.com/flyteorg/flyte/flyteplugins/go/tasks/pluginmachinery/encoding"
"github.com/flyteorg/flyte/flytepropeller/pkg/apis/flyteworkflow/v1alpha1"
"github.com/flyteorg/flyte/flytepropeller/pkg/controller/config"
"github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/common"
"github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/interfaces"
"github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/task"
)

type arrayEventRecorder interface {
interfaces.EventRecorder
process(ctx context.Context, nCtx interfaces.NodeExecutionContext, index int, retryAttempt uint32)
process(ctx context.Context, nCtx interfaces.NodeExecutionContext, index int, retryAttempt uint32) error
finalize(ctx context.Context, nCtx interfaces.NodeExecutionContext, taskPhase idlcore.TaskExecution_Phase, taskPhaseVersion uint32, eventConfig *config.EventConfig) error
finalizeRequired(ctx context.Context) bool
}
Expand All @@ -39,8 +42,23 @@
return nil
}

func (e *externalResourcesEventRecorder) process(ctx context.Context, nCtx interfaces.NodeExecutionContext, index int, retryAttempt uint32) {
externalResourceID := fmt.Sprintf("%s-%d", buildSubNodeID(nCtx, index), retryAttempt)
func (e *externalResourcesEventRecorder) process(ctx context.Context, nCtx interfaces.NodeExecutionContext, index int, retryAttempt uint32) error {
// generate externalResourceID
currentNodeUniqueID := nCtx.NodeID()
if nCtx.ExecutionContext().GetEventVersion() != v1alpha1.EventVersion0 {
var err error
currentNodeUniqueID, err = common.GenerateUniqueID(nCtx.ExecutionContext().GetParentInfo(), nCtx.NodeID())
if err != nil {
return err
}

Check warning on line 53 in flytepropeller/pkg/controller/nodes/array/event_recorder.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/controller/nodes/array/event_recorder.go#L52-L53

Added lines #L52 - L53 were not covered by tests
}

uniqueID, err := encoding.FixedLengthUniqueIDForParts(task.IDMaxLength, []string{nCtx.NodeExecutionMetadata().GetOwnerID().Name, currentNodeUniqueID, strconv.Itoa(int(retryAttempt))})
if err != nil {
return err
}

Check warning on line 59 in flytepropeller/pkg/controller/nodes/array/event_recorder.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/controller/nodes/array/event_recorder.go#L58-L59

Added lines #L58 - L59 were not covered by tests

externalResourceID := fmt.Sprintf("%s-n%d-%d", uniqueID, index, retryAttempt)

// process events
cacheStatus := idlcore.CatalogCacheStatus_CACHE_DISABLED
Expand Down Expand Up @@ -83,6 +101,8 @@
// clear nodeEvents and taskEvents
e.nodeEvents = e.nodeEvents[:0]
e.taskEvents = e.taskEvents[:0]

return nil
}

func (e *externalResourcesEventRecorder) finalize(ctx context.Context, nCtx interfaces.NodeExecutionContext,
Expand All @@ -94,6 +114,17 @@
return err
}

var taskID *idlcore.Identifier
subNode := nCtx.Node().GetArrayNode().GetSubNodeSpec()
if subNode != nil && subNode.Kind == v1alpha1.NodeKindTask {
executableTask, err := nCtx.ExecutionContext().GetTask(*subNode.GetTaskID())
if err != nil {
return err
}

Check warning on line 123 in flytepropeller/pkg/controller/nodes/array/event_recorder.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/controller/nodes/array/event_recorder.go#L120-L123

Added lines #L120 - L123 were not covered by tests

taskID = executableTask.CoreTask().GetId()

Check warning on line 125 in flytepropeller/pkg/controller/nodes/array/event_recorder.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/controller/nodes/array/event_recorder.go#L125

Added line #L125 was not covered by tests
}

nodeExecutionID := *nCtx.NodeExecutionMetadata().GetNodeExecutionID()
if nCtx.ExecutionContext().GetEventVersion() != v1alpha1.EventVersion0 {
currentNodeUniqueID, err := common.GenerateUniqueID(nCtx.ExecutionContext().GetParentInfo(), nodeExecutionID.NodeId)
Expand All @@ -103,26 +134,18 @@
nodeExecutionID.NodeId = currentNodeUniqueID
}

workflowExecutionID := nodeExecutionID.ExecutionId

taskExecutionEvent := &event.TaskExecutionEvent{
TaskId: &idlcore.Identifier{
ResourceType: idlcore.ResourceType_TASK,
Project: workflowExecutionID.Project,
Domain: workflowExecutionID.Domain,
Name: nCtx.NodeID(),
Version: "v1", // this value is irrelevant but necessary for the identifier to be valid
},
TaskId: taskID,
ParentNodeExecutionId: &nodeExecutionID,
RetryAttempt: 0, // ArrayNode will never retry
Phase: taskPhase,
PhaseVersion: taskPhaseVersion,
OccurredAt: occurredAt,
Metadata: &event.TaskExecutionMetadata{
ExternalResources: e.externalResources,
PluginIdentifier: "container",
PluginIdentifier: "k8s-array",
},
TaskType: "k8s-array",
TaskType: "container_array",
EventVersion: 1,
}

Expand Down Expand Up @@ -165,7 +188,8 @@
interfaces.EventRecorder
}

func (*passThroughEventRecorder) process(ctx context.Context, nCtx interfaces.NodeExecutionContext, index int, retryAttempt uint32) {
func (*passThroughEventRecorder) process(ctx context.Context, nCtx interfaces.NodeExecutionContext, index int, retryAttempt uint32) error {
return nil

Check warning on line 192 in flytepropeller/pkg/controller/nodes/array/event_recorder.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/controller/nodes/array/event_recorder.go#L191-L192

Added lines #L191 - L192 were not covered by tests
}

func (*passThroughEventRecorder) finalize(ctx context.Context, nCtx interfaces.NodeExecutionContext,
Expand Down
12 changes: 9 additions & 3 deletions flytepropeller/pkg/controller/nodes/array/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@
logger.Warnf(ctx, "failed to record ArrayNode events: %v", err)
}

eventRecorder.process(ctx, nCtx, i, retryAttempt)
if err := eventRecorder.process(ctx, nCtx, i, retryAttempt); err != nil {
logger.Warnf(ctx, "failed to record ArrayNode events: %v", err)
}

Check warning on line 103 in flytepropeller/pkg/controller/nodes/array/handler.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/controller/nodes/array/handler.go#L102-L103

Added lines #L102 - L103 were not covered by tests
}
}
}
Expand Down Expand Up @@ -241,7 +243,9 @@
logger.Warnf(ctx, "failed to record ArrayNode events: %v", err)
}

eventRecorder.process(ctx, nCtx, i, 0)
if err := eventRecorder.process(ctx, nCtx, i, 0); err != nil {
logger.Warnf(ctx, "failed to record ArrayNode events: %v", err)
}

Check warning on line 248 in flytepropeller/pkg/controller/nodes/array/handler.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/controller/nodes/array/handler.go#L247-L248

Added lines #L247 - L248 were not covered by tests
}

// transition ArrayNode to `ArrayNodePhaseExecuting`
Expand Down Expand Up @@ -331,7 +335,9 @@
}
}
}
eventRecorder.process(ctx, nCtx, index, subNodeStatus.GetAttempts())
if err := eventRecorder.process(ctx, nCtx, index, subNodeStatus.GetAttempts()); err != nil {
return handler.UnknownTransition, err
}

Check warning on line 340 in flytepropeller/pkg/controller/nodes/array/handler.go

View check run for this annotation

Codecov / codecov/patch

flytepropeller/pkg/controller/nodes/array/handler.go#L339-L340

Added lines #L339 - L340 were not covered by tests

// update subNode state
arrayNodeState.SubNodePhases.SetItem(index, uint64(subNodeStatus.GetPhase()))
Expand Down
5 changes: 5 additions & 0 deletions flytepropeller/pkg/controller/nodes/array/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"k8s.io/apimachinery/pkg/types"

idlcore "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/event"
Expand Down Expand Up @@ -145,6 +146,10 @@ func createNodeExecutionContext(dataStore *storage.DataStore, eventRecorder inte
Name: "name",
},
})
nodeExecutionMetadata.OnGetOwnerID().Return(types.NamespacedName{
Namespace: "wf-namespace",
Name: "wf-name",
})
nCtx.OnNodeExecutionMetadata().Return(nodeExecutionMetadata)

// NodeID
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading