Fix: Enable kubeflow plugins to use dynamic log links

[fg91]

Why are the changes needed?

So-called dynamic log links are log links which are not activated by default for all tasks but need to be activated for specific tasks. This is done setting a respective "log link type" in the so-called task config, see e.g. here in the wandb plugin.

These dynamic log links currently don't work for the kubeflow plugins (pytorch, tensorflow, mpi tasks). The reason is that the so-called "task template" in flytepropeller - which contains the task-specific log link config - is not made available to the log link templating engine. This PR fixes this.

How was this patch tested?

Added unit test.

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Summary by Bito

This PR addresses a bug in Kubeflow plugins by enabling dynamic log links through task template integration. The changes modify the GetLogs function signature to include taskTemplate parameter and update PyTorch, TensorFlow, and MPI plugins accordingly. The implementation ensures proper template availability for log link generation with comprehensive test coverage.

Unit tests added: True

Estimated effort to review (1-5, lower is better): 3

[flyte-bot]

Code Review Agent Run #7f6d32

Actionable Suggestions - 10

• flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go - 3

  • Consider adding taskTemplate validation check · Line 225-225
  • Consider using correct type for TemplateURIs · Line 235-235
  • Consider adding parameter validation checks · Line 335-352
• flyteplugins/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow.go - 1

  • Consider validating task template after read · Line 160-164
• flyteplugins/go/tasks/plugins/k8s/kfoperators/mpi/mpi.go - 1

  • Consider adding nil check for taskTemplate · Line 173-173
• flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator.go - 3

  • Consider adding taskTemplate parameter validation · Line 92-92
  • Consider adding taskTemplate validation check · Line 191-191
  • Consider adding taskTemplate nil check · Line 191-191
• flyteplugins/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch.go - 1

  • Consider error handling for GetLogs · Line 221-226
• flyteplugins/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go - 1

  • Consider adding taskTemplate nil validation · Line 582-582

Review Details

• Files reviewed - 8 · Commit Range: 77e05b1..77e05b1

  • flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator.go
  • flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go
  • flyteplugins/go/tasks/plugins/k8s/kfoperators/mpi/mpi.go
  • flyteplugins/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go
  • flyteplugins/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch.go
  • flyteplugins/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch_test.go
  • flyteplugins/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow.go
  • flyteplugins/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow_test.go
• Files skipped - 0
• Tools

  • Golangci-lint (Linter) - ✖︎ Failed
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

AI Code Review powered by

[codecov]

Codecov Report

Attention: Patch coverage is 56.52174% with 10 lines in your changes missing coverage. Please review.

Project coverage is 33.82%. Comparing base (5fe7a73) to head (77e05b1).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...lugins/go/tasks/plugins/k8s/kfoperators/mpi/mpi.go	50.00%	2 Missing and 1 partial ⚠️
...o/tasks/plugins/k8s/kfoperators/pytorch/pytorch.go	50.00%	2 Missing and 1 partial ⚠️
...s/plugins/k8s/kfoperators/tensorflow/tensorflow.go	50.00%	2 Missing and 1 partial ⚠️
.../plugins/k8s/kfoperators/common/common_operator.go	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6284      +/-   ##
==========================================
- Coverage   33.82%   33.82%   -0.01%     
==========================================
  Files        1329     1329              
  Lines      147791   147808      +17     
==========================================
+ Hits        49991    49993       +2     
- Misses      92961    92973      +12     
- Partials     4839     4842       +3     

Flag	Coverage Δ
unittests-datacatalog	48.01% <ø> (ø)
unittests-flyteadmin	50.09% <ø> (-0.03%)	⬇️
unittests-flytecopilot	30.99% <ø> (ø)
unittests-flytectl	58.09% <ø> (ø)
unittests-flyteidl	6.78% <ø> (ø)
unittests-flyteplugins	49.01% <56.52%> (-0.01%)	⬇️
unittests-flytepropeller	36.52% <ø> (ø)
unittests-flytestdlib	50.38% <ø> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[flyte-bot]

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted
Bug Fix - Enable Dynamic Log Links for Kubeflow Plugins	- common_operator.go - Added taskTemplate parameter to GetLogs function to support dynamic log links - mpi.go - Updated GetTaskPhase to pass taskTemplate to GetLogs - pytorch.go - Updated GetTaskPhase to pass taskTemplate to GetLogs - tensorflow.go - Updated GetTaskPhase to pass taskTemplate to GetLogs
Testing - Add Tests for Dynamic Log Links	- common_operator_test.go - Added test cases for dynamic log links and helper functions - mpi_test.go - Updated tests to include taskTemplate parameter - pytorch_test.go - Updated tests to include taskTemplate parameter - tensorflow_test.go - Updated tests to include taskTemplate parameter

[eapolinario]

Great, thank you!

[fg91]

Thank you for the super quick reviews @eapolinario 🙏 🚀