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

Only pull if not exist #800

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

rowleya
Copy link

@rowleya rowleya commented Nov 11, 2020

First check if a docker image exists locally before trying to pull it. This resolves #791. Note that most of the change shown below is whitespace (from indent), so adding the "w=1" option to the URL shows the actual change, which is to do a "docker inspect" before the "docker pull".

@dakale
Copy link
Contributor

dakale commented Dec 11, 2020

Could we check the actual output of docker inspect instead of catching an exception for control flow? There could be other exceptions that happen when running that command that we would want to fail on or handle differently. I understand its sort of what we are already doing since we arent catching exceptions here anyways, but it might be nicer for long term

@rowleya
Copy link
Author

rowleya commented Dec 14, 2020

Happy to update this if it helps make it get in to the main branch and a future release. Do you know:

  1. What exceptions can come out of this call and what might be done differently on each exception?
  2. What outputs can come out of this call and what might be done differently on each output?

If there is nothing different to do at this point, it may be better doing this when there is; I don't think anything here would prevent a future PR with functionality that does different things if needed at a later date.

@rowleya rowleya requested a review from a team as a code owner February 24, 2021 09:41
@baparham
Copy link

Would it be more versatile to instead add an option to jobs.[job-id].container to skip the pull altogether rather than pull if it doesn't exist?

something like

jobs:
  my_job:
    container:
      image: node:14.16
      # could be simply true/false for now, and extended later to include 'never/false', 'if-not-present', or 'always/true'
      pull: false

and of course, if the await _dockerManger.DockerInspect(executionContext, container.ContainerImage, ""); fails, then the job fails, as expected.

I think it's a valid use case to have basically the equivalent of GitLab's pull-policy: never for Action container support, and this may be a quicker way than fully implementing all the possible pull policy options that GitLab runners support.

@rowleya
Copy link
Author

rowleya commented May 11, 2021

Happy for that idea instead; anything to make it work is good!

@rowleya rowleya mentioned this pull request May 11, 2021
@rowleya
Copy link
Author

rowleya commented Mar 10, 2023

Thanks @SamsRM for approving this, but note that #1086 does more than this by adding an option rather than making it required... Still happy to use this version if preferred though (anything to make it work)!

@rowleya
Copy link
Author

rowleya commented Mar 10, 2023

Master also now merged in here

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.

Option to not do docker pull?
3 participants