runner icon indicating copy to clipboard operation
runner copied to clipboard

Only pull if not exist

Open rowleya opened this issue 5 years ago • 8 comments

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".

rowleya avatar Nov 11 '20 08:11 rowleya

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

dakale avatar Dec 11 '20 18:12 dakale

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 avatar Dec 14 '20 13:12 rowleya

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.

baparham avatar May 11 '21 09:05 baparham

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

rowleya avatar May 11 '21 09:05 rowleya

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 avatar Mar 10 '23 07:03 rowleya

Master also now merged in here

rowleya avatar Mar 10 '23 07:03 rowleya

Master also now merged in here

has this been merged ready?

xihajun avatar Aug 05 '24 14:08 xihajun

Nope; I don't think anyone is looking at these PRs!

rowleya avatar Aug 05 '24 14:08 rowleya