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

update code to fix error on termination time when using sidecar #867

Merged
merged 3 commits into from
Apr 11, 2020

Conversation

ImpSy
Copy link
Contributor

@ImpSy ImpSy commented Apr 10, 2020

Hello,

This PR is related to issue #841

Following the discussion on this issue, i've implemented option 2

Specifically, I've introduced a new State Machine specific to the Driver that allow finer grain detection of application end when using sidecar containers
This is also compatible with kubernetes sidecar implementation on version 1.18

I've also updated the tests to cover sidecar cases

PS: for the review the commit are scoped
bfacfe9 is logic only
dbd2f21 is to fix and update tests

Copy link
Collaborator

@liyinan926 liyinan926 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one minor comment. Thanks for contributing!

@liyinan926 liyinan926 merged commit 2bd7a00 into kubeflow:master Apr 11, 2020
jrj-d added a commit to jrj-d/spark-on-k8s-operator that referenced this pull request May 28, 2020
Hey there,
We are [Data Mechanics](https://www.datamechanics.co/), a managed Spark platform deployed on Kubernetes.
We use the spark-operator in the background; congrats for the great work by the way!
We have already made small contributions to the project (see kubeflow#867) and hope to do more!
Cheers, Julien
jbhalodia-slack pushed a commit to jbhalodia-slack/spark-operator that referenced this pull request Oct 4, 2024
…flow#867)

* update code to fix error on termination time when using sidecar

* fix and add new tests

* fixup! update code to fix error on termination time when using sidecar
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants