Feat: Expose nodeID to container #388
Conversation
Signed-off-by: Dennis Keck <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #388 +/- ##
==========================================
+ Coverage 63.01% 64.15% +1.13%
==========================================
Files 154 156 +2
Lines 13080 10712 -2368
==========================================
- Hits 8243 6872 -1371
+ Misses 4220 3208 -1012
- Partials 617 632 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: Dennis Keck <[email protected]>
| @@ -31,6 +31,15 @@ func GetContextEnvVars(ownerCtx context.Context) []v1.EnvVar { | |||
| }, | |||
| ) | |||
| } | |||
|
|
|||
| if nodeID := contextutils.Value(ownerCtx, contextutils.NodeIDKey); nodeID != "" { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Hi @hamersaw
For our case we need the entire hierarchy, similar to how is is displayed here in the UI:
(ideally without it being shortened to a specific length)
id.GetID().NodeExecutionId.NodeId only gives me the child nodeID, e.g. dn0, which might occur multiple times in a workflow. Instead we need the entire hierarchy to uniquely identify a single node in the graph (e.g. n0/dn0/dn0/dn0/dn0)
There was a problem hiding this comment.
If you use something like:
currentNodeUniqueID, err := common.GenerateUniqueID(nCtx.ExecutionContext().GetParentInfo(), nodeExecutionID.NodeId)
then it should prepend the node ID with the parent info which should give you the n0-0-dn0 / n0-0-dn0-0-dn0 being separated by - rather than /. I think this would be cleaner?
|
edit: Re-opening these PRs as the automation failed. |
|
Hi, we are moving all Flyte development to a monorepo. In order to help the transition period, we're moving open PRs to monorepo automatically and your PR was moved to flyteorg/flyte#4135. Notice that if there are any conflicts in the resulting PR they most likely happen due to the change in the import path of the flyte components. |
TL;DR
Exposes the NodeId to the kubernetes Pod as environment variable.
Type
Are all requirements met?
Complete description
Currently the environment variables do not provide enough information to know the exact task being executed.
For example we intended to create a unique ID for all retries of a task. Without the nodeID this is not possible as tasks might be reused.
One can retrieve this information also from the hostname but this fails for long names due to k8s pod name length limits.