Only pull if not exist

[rowleya]

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]

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]

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.

[baparham]

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]

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

[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]

Master also now merged in here

[xihajun]

Master also now merged in here

has this been merged ready?

[rowleya]

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