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

Restart warm container when it does not exist #3390

Closed
wants to merge 3 commits into from

Conversation

leszko
Copy link
Contributor

@leszko leszko commented Feb 13, 2025

In case of the warm container, go-livepeer should monitor if the container is running and recreate it if it's stopped.

The issue happened during our deployments, when we update runner image, we need to stop it and then go-livepeer should start it again.

@leszko leszko requested review from victorges and mjh1 February 13, 2025 10:48
@github-actions github-actions bot added go Pull requests that update Go code AI Issues and PR related to the AI-video branch. labels Feb 13, 2025
Copy link
Contributor

@mjh1 mjh1 left a comment

Choose a reason for hiding this comment

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

Just wondering if there are some edge cases where watchContainer exits but we're in a state where createContainer will fail, I guess we keep an eye out for the error log. Does createContainer handle the case where the container is in a stopped state and all it needs to do is start it rather than creating a new one?

@leszko
Copy link
Contributor Author

leszko commented Feb 13, 2025

Just wondering if there are some edge cases where watchContainer exits but we're in a state where createContainer will fail, I guess we keep an eye out for the error log. Does createContainer handle the case where the container is in a stopped state and all it needs to do is start it rather than creating a new one?

We actually never "stop" container, because it's always started with the -rm flag. So stopping actually means deleting the container.

@leszko leszko marked this pull request as draft February 13, 2025 14:19
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

Attention: Patch coverage is 47.05882% with 9 lines in your changes missing coverage. Please review.

Project coverage is 32.16082%. Comparing base (1c1c280) to head (b0cfb06).

Files with missing lines Patch % Lines
ai/worker/docker.go 47.05882% 9 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #3390         +/-   ##
===================================================
+ Coverage   32.15960%   32.16082%   +0.00122%     
===================================================
  Files            147         147                 
  Lines          40753       40767         +14     
===================================================
+ Hits           13106       13111          +5     
- Misses         26874       26883          +9     
  Partials         773         773                 
Files with missing lines Coverage Δ
ai/worker/docker.go 73.33333% <47.05882%> (-1.25099%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c1c280...b0cfb06. Read the comment docs.

Files with missing lines Coverage Δ
ai/worker/docker.go 73.33333% <47.05882%> (-1.25099%) ⬇️

@leszko
Copy link
Contributor Author

leszko commented Feb 13, 2025

Ahh, just realized it may not be that simple as this implementation. I'll need to work a little bit more on that part.

@leszko
Copy link
Contributor Author

leszko commented Feb 14, 2025

Closing for now

@leszko leszko closed this Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Issues and PR related to the AI-video branch. go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants