Skip to content
This repository was archived by the owner on May 6, 2020. It is now read-only.

ref(scheduler): rework the way deis run is handled, timeouts and more#738

Merged
helgi merged 1 commit intodeis:masterfrom
helgi:deis_run
May 17, 2016
Merged

ref(scheduler): rework the way deis run is handled, timeouts and more#738
helgi merged 1 commit intodeis:masterfrom
helgi:deis_run

Conversation

@helgi
Copy link
Copy Markdown
Contributor

@helgi helgi commented May 16, 2016

Summary of Changes

Use the same "is the pod ready" function as normal deployments, also give deis run 20 mins max (arbitrary but is somewhat what the gunicorn worker can do)

On failure the whole Pod manifest is no longer returned. This change affects how we do tests here but minimally

Mocking was improved to help move a Pod from Pending to Running and if it is a deis run Pod (not attached to RC) then it also goes to Succeeded state. This helps with test coverage but is not the full Pod state transition route

Issue(s) that this PR Closes

Fixes #681
Fixes #590

Associated End To End Test PR(s)

deis run already has e2e

Associated Documentation PR(s)

Documentation still needs to be updated to handle the little change in API

Testing Instructions

Please provide a detailed list for how to test the changes in this PR.

  1. Create a Deis Cluster
  2. Register an app
  3. run a deis run command
  4. things should work!

@helgi helgi self-assigned this May 16, 2016
@helgi helgi added this to the v2.0-rc1 milestone May 16, 2016
@helgi
Copy link
Copy Markdown
Contributor Author

helgi commented May 16, 2016

I also need to update the deis cli - Originally I wasn't really hot on updating the API which would require a CLI update but rc means little to nothing compared to exit_code

@codecov-io
Copy link
Copy Markdown

codecov-io commented May 16, 2016

Current coverage is 85.70%

Merging #738 into master will increase coverage by 1.24%

@@             master       #738   diff @@
==========================================
  Files            29         29          
  Lines          2567       2573     +6   
  Methods           0          0          
  Messages          0          0          
  Branches        404        403     -1   
==========================================
+ Hits           2168       2205    +37   
+ Misses          286        253    -33   
- Partials        113        115     +2   

Powered by Codecov. Last updated by c6ad9f6...40668a5

@helgi
Copy link
Copy Markdown
Contributor Author

helgi commented May 16, 2016

Backing out the API change in this PR and moving it to a separate one that will depend on this one

# TODO: Revisit in the future so it can run longer
state = 'up' # pod is still running
waited = 0
timeout = 1200 # 20 minutes
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could this re-use the actual gunicorn timeout?

from deis import gconf

timeout = gconf.timeout

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can't do that since that's the overall timeout for the worker and there are many other pod waits ahead of this 1200 second limit. If we used the gunicorn timeout then the worker would always timeout before the while loop would catch - I felt it was better to have a hardcoded one, not based on the gunicorn worker limit.

Use the same "is the pod ready" function as normal deployments, also give deis run 20 mins max (arbitrary but is somewhat what the gunicorn worker can do)
This also the API to return exit_code instead of the cryptic rc value

On failure the whole Pod manifest is no longer returned. This change affects how we do tests here but minimally

Mocking was improved to help move a Pod from Pending to Running and if it is a deis run Pod (not attached to RC) then it also goes to Succeeded state. This helps with test coverage but is not the full Pod state transition route
@helgi
Copy link
Copy Markdown
Contributor Author

helgi commented May 17, 2016

This also fixes #590

@helgi helgi merged commit d496e3d into deis:master May 17, 2016
@helgi helgi deleted the deis_run branch May 17, 2016 18:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants