Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Setting primaryContainerName by default on Pod plugin #555

Merged
merged 3 commits into from
Apr 17, 2023

Conversation

hamersaw
Copy link
Contributor

TL;DR

Pulls the changes from this PR into flytepropeller to reduce Flyte overhead on k8s Pod executions. Basically, we rarely need to wait for the entirePod to complete, rather once the primary container (or all containers) complete we can prematurely mark the task as successful. This mitigates time for the Pod operator to detect container completion and mark the Pod as completed.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

^^^

Tracking Issue

NA

Follow-up issue

NA

Signed-off-by: Daniel Rammer <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
@hamersaw hamersaw marked this pull request as ready for review April 14, 2023 15:51
@codecov
Copy link

codecov bot commented Apr 14, 2023

Codecov Report

Merging #555 (644eafd) into master (6cb7322) will increase coverage by 0.42%.
The diff coverage is n/a.

❗ Current head 644eafd differs from pull request most recent head 43c86ba. Consider uploading reports for the commit 43c86ba to get more accurate results

@hamersaw hamersaw merged commit e4ca252 into master Apr 17, 2023
@hamersaw hamersaw deleted the feature/default-primary-container-name branch April 17, 2023 16:12
eapolinario pushed a commit to eapolinario/flytepropeller that referenced this pull request Aug 9, 2023
* updated flyteplugins version

Signed-off-by: Daniel Rammer <[email protected]>

* go mod tidy

Signed-off-by: Daniel Rammer <[email protected]>

---------

Signed-off-by: Daniel Rammer <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants