Recover image pulls by trying to run the container anyways. #3813
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
markusthoemmes
added
docker
invoker
review
rabbah
reviewed
Jun 27, 2018
rabbah
reviewed
Jun 27, 2018
| } | ||
| } else Future.successful(()) | ||
| // Always recover the Future so docker run is tried below in any case, in case we have an image available locally | ||
| docker.pull(image).map(_ => true).recover { case _ => false } |
There was a problem hiding this comment.
https://github.com/apache/incubator-openwhisk/pull/2852/files
not sure if this is better but this was an old attempt to skip docker pull if image is present locally using docker inspect.
Codecov Report
@@ Coverage Diff @@
## master #3813 +/- ##
==========================================
- Coverage 75.82% 71.05% -4.77%
==========================================
Files 145 145
Lines 6919 6921 +2
Branches 409 410 +1
==========================================
- Hits 5246 4918 -328
- Misses 1673 2003 +330
Continue to review full report at Codecov.
|
markusthoemmes
force-pushed
the
recover-pull
branch
from
4a60833
to
ac938fd
Compare
|
@rabbah was right and I added some behavioral changes. I'll email the dev-list to discuss the behavior change contained here. |
markusthoemmes
force-pushed
the
recover-pull
branch
from
ac938fd
to
f6828e9
Compare
rabbah
reviewed
Jul 23, 2018
docs/actions-docker.md
Outdated
| @@ -105,6 +105,10 @@ The instructions that follow show you how to use the OpenWhisk Docker skeleton. | |||
| Notice the use of `--docker` when creating an action. Currently all Docker images are assumed | |||
| to be hosted on Docker Hub. | |||
|
|
|||
| *Note:* It is considered best-practice for production images to be versioned via docker image tags. The tag "latest" or no tag will guarantee to always be on the latest state when creating new containers, but that contains the possibility of failing because pulling the image fails. For tagged images however, the system will allow a failing pull and gracefully recover it by using the image that is already locally available, making it much more resilient against Dockerhub outages etc. | |||
There was a problem hiding this comment.
Suggested rewrite for the first sentence:
The absence of a tag will be treated the same as using the tag "latest" which will pull an image from Dockerhub. Please note that "latest" doesn't mean newest tag, but rather "latest" is an alias for any image built without an explicit tag.
rabbah
approved these changes
Jul 23, 2018
| case Left(userProvided) if userProvided.tag.map(_ == "latest").getOrElse(true) => | ||
| // Iff the image tag is "latest" explicitly (or implicitly because no tag is given at all), failing to pull will | ||
| // fail the whole container bringup process, because it is expected to pick up the very latest version every | ||
| // time. |
There was a problem hiding this comment.
the very latest "untagged" version every time.
A `docker pull` can fail due to various reasons. One of them is network throttling by the image registry. Since we try to pull on each blackbox invocation, high volume load can cause lots of errors due to failing pulls unnecessarily. Instead, we try to run the container even if the pull failed in the first place. If the image is available locally, the container will start just fine and recover the error gracefully. If the image is not available locally, the run will fail as well and return the same error as the docker pull would've returned. This behavior will only be enabled for blackbox actions that specify a tag. Blackbox actions not using a tag *or* using "latest" as a tag will exhibit the very same behavior as today. That is: There will always be a pull before each container start and a failing pull will result in an error reported to the user. This is to enable rapid prototyping on images and enable determinism in the workflow. Updating the action will then force a pull and will fail early if that pull fails. With the new behavior, the developer might be surprised by the image not being updated because the pull is swallowed. For production workload it is considered best-practice to version images through labels, thus we can "safely" assume that we can fall back to a local image in case the pull fails.
markusthoemmes
force-pushed
the
recover-pull
branch
from
f6828e9
to
518c87e
Compare
|
PG3 2559 🔵 |
BillZong
pushed a commit
to BillZong/openwhisk
that referenced
this pull request
Nov 18, 2019
) A `docker pull` can fail due to various reasons. One of them is network throttling by the image registry. Since we try to pull on each blackbox invocation, high volume load can cause lots of errors due to failing pulls unnecessarily. Instead, we try to run the container even if the pull failed in the first place. If the image is available locally, the container will start just fine and recover the error gracefully. If the image is not available locally, the run will fail as well and return the same error as the docker pull would've returned. This behavior will only be enabled for blackbox actions that specify a tag. Blackbox actions not using a tag *or* using "latest" as a tag will exhibit the very same behavior as today. That is: There will always be a pull before each container start and a failing pull will result in an error reported to the user. This is to enable rapid prototyping on images and enable determinism in the workflow. Updating the action will then force a pull and will fail early if that pull fails. With the new behavior, the developer might be surprised by the image not being updated because the pull is swallowed. For production workload it is considered best-practice to version images through labels, thus we can "safely" assume that we can fall back to a local image in case the pull fails.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A
docker pullcan fail due to various reasons. One of them is network throttling by the image registry. Since we try to pull on each blackbox invocation, high volume load can cause lots of errors due to failing pulls unnecessarily.Instead, we try to run the container even if the pull failed in the first place. If the image is available locally, the container will start just fine and recover the error gracefully. If the image is not available locally, the run will fail as well and return the same error as the docker pull would've returned.
This behavior will only be enabled for blackbox actions that specify a tag. Blackbox actions not using a tag or using "latest" as a tag will exhibit the very same behavior as today. That is: There will always be a pull before each container start and a failing pull will result in an error reported to the user. This is to enable rapid prototyping on images and enable determinism in the workflow. Updating the action will then force a pull and will fail early if that pull fails. With the new behavior, the developer might be surprised by the image not being updated because the pull is swallowed.
For production workload it is considered best-practice to version images through labels, thus we can "safely" assume that we can fall back to a local image in case the pull fails.
My changes affect the following components
Types of changes
Checklist: