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

Changes to user requested task kills in custom executor #1006

Merged
merged 5 commits into from Jul 29, 2016
Merged

Conversation

@ssalinas
Copy link
Member

@ssalinas ssalinas commented Apr 22, 2016

Two main changes in this:

  • For any user-requested kill (direct task kill, task destroy, also includes bounce at the moment), send a framework message to the executor to trigger the kill (if using a custom executor)
  • In the SingularityExecutor, don't skip to destroy if we receive a kill (framework message or normal) and there is already a kill process running

In the case when sending the framework message, I am still sending the normal kill as well (now that the executor handles it properly). This is so any other executors (default mesos or another custom) will still receive the kills they expect. I couldn't think of a better way around this since we do not have an easy way to determine that the executor is custom AND is a singularity custom executor, suggestions welcome for easier compatibility with other executors.

@wsorenson @tpetr

@wsorenson
Copy link
Contributor

@wsorenson wsorenson commented Apr 22, 2016

I think we should only send the task destroy as a message.

@ssalinas
Copy link
Member Author

@ssalinas ssalinas commented Apr 22, 2016

That makes sense. I'll keep it as regular kills for any other cases then

@ssalinas ssalinas force-pushed the task_kill branch from 952d22a to f11b4ea Apr 22, 2016
@ssalinas
Copy link
Member Author

@ssalinas ssalinas commented Apr 22, 2016

Updated to only use the framework message on a user initiated task destroy (which is now it's own task cleanup type to help differentiate it). Going to work with the task kill endpoint a bit more to make it clearer when a destroy will happen. Realized we can probably get it a bit cleaner than just using the override query param

@ssalinas ssalinas force-pushed the task_kill branch from 9f0c73e to 16c5fc1 Apr 22, 2016
@ssalinas ssalinas added the hs_staging label Apr 27, 2016
@ssalinas ssalinas modified the milestone: 0.6.0 Apr 27, 2016
@ssalinas ssalinas added the hs_qa label Apr 28, 2016
@tpetr

This comment has been minimized.

Copy link
Member

@tpetr tpetr commented on 16c5fc1 May 5, 2016

🚢

@ssalinas ssalinas added the hs_stable label May 6, 2016
@ssalinas ssalinas modified the milestones: 0.6.0, 0.8.0 May 13, 2016
@ssalinas ssalinas modified the milestone: 0.8.0 Jun 16, 2016
@jonathanwgoodwin

This comment has been minimized.

Copy link
Member

@jonathanwgoodwin jonathanwgoodwin commented on f061918 Jul 20, 2016

🚢

Fix user requested destroy when cleaning
@ssalinas ssalinas modified the milestone: 0.10.0 Jul 29, 2016
@ssalinas
Copy link
Member Author

@ssalinas ssalinas commented Jul 29, 2016

This has been doing well since the fix in #1141 , merging

@ssalinas ssalinas merged commit d054703 into master Jul 29, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ssalinas ssalinas deleted the task_kill branch Jul 29, 2016
@ssalinas ssalinas removed the hs_qa label Jul 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.