Skip to content

Avoid pulling same images multiple times ⚡️#9173

Merged
laurazard merged 14 commits intodocker:v2from
KoditkarVedant:8768-avoid-pulling-same-image-multiple-times
Aug 19, 2022
Merged

Avoid pulling same images multiple times ⚡️#9173
laurazard merged 14 commits intodocker:v2from
KoditkarVedant:8768-avoid-pulling-same-image-multiple-times

Conversation

@KoditkarVedant
Copy link
Contributor

What I did
docker compose pull pulls images even if they are present locally and if there are multiple services using the same image it pulls them multiple times. This pull request avoid pulling image if it is already present and only fetch image once if required by many services.

Related issue
#8768

(not mandatory) A picture of a cute animal, if possible in relation with what you did
image

Signed-off-by: Vedant Koditkar <vedant.koditkar@outlook.com>
@KoditkarVedant KoditkarVedant marked this pull request as ready for review February 18, 2022 14:20
@KoditkarVedant
Copy link
Contributor Author

@ndeloof I can not re-run the failed tests and they are also failing on V2 branch when I run them locally. Can you please look into this? Do let me know in case I can help.

@KoditkarVedant
Copy link
Contributor Author

@ndeloof need your help to merge this pull request.

@ndeloof
Copy link
Contributor

ndeloof commented Mar 4, 2022

need TestComposePull to be fixed

@KoditkarVedant
Copy link
Contributor Author

oh, I thought it may be because of some tests which fails sometime. I will fix it.

@KoditkarVedant
Copy link
Contributor Author

@ndeloof I tried running the test case on my local it ran without any error.

image

Can we rerun the integration pipeline if it gets resolved?

@glours
Copy link
Contributor

glours commented Mar 5, 2022

@KoditkarVedant I just restarted the CI, the TestComposePull test is still in error

@KoditkarVedant
Copy link
Contributor Author

@glours I've merged the latest changes from V2 in my branch. Lets see if the test still fails. No idea why I'm not able to reproduce this locally.

@KoditkarVedant
Copy link
Contributor Author

Finally, Test is failing locally. I will fix it.

Signed-off-by: Vedant Koditkar <vedant.koditkar@outlook.com>
@KoditkarVedant KoditkarVedant force-pushed the 8768-avoid-pulling-same-image-multiple-times branch from 23d0bcf to e24d274 Compare March 5, 2022 18:16
@KoditkarVedant
Copy link
Contributor Author

@ndeloof @glours Fixed the TestComposePull.

@n-rodriguez
Copy link

Hi there! Any news?

@KoditkarVedant
Copy link
Contributor Author

@n-rodriguez this has been delayed for long time. I need to get my local setup up and running again. I will spend some time over the weekend.

@KoditkarVedant
Copy link
Contributor Author

KoditkarVedant commented Aug 6, 2022

@ndeloof Need help to get this PR over the line.

@laurazard laurazard requested review from a team and ndeloof August 6, 2022 12:58
@laurazard laurazard self-assigned this Aug 6, 2022
Signed-off-by: Vedant Koditkar <vedant.koditkar@outlook.com>
@KoditkarVedant KoditkarVedant force-pushed the 8768-avoid-pulling-same-image-multiple-times branch from 796c9cd to 0db6dfe Compare August 10, 2022 16:19
@KoditkarVedant
Copy link
Contributor Author

@laurazard done with the changes

@laurazard
Copy link
Member

@KoditkarVedant looks like there are some CI failures. I can’t really look into it (on mobile) can you take a look and fix it?

The changes to the tests are great ☺️

@KoditkarVedant
Copy link
Contributor Author

@laurazard I have changed the image used for that to avoid any race condition while running the test case. Lets see if that resolves the issue. I do not get any error for TestComposePull when I run it locally.

Signed-off-by: Vedant Koditkar <vedant.koditkar@outlook.com>
@KoditkarVedant KoditkarVedant force-pushed the 8768-avoid-pulling-same-image-multiple-times branch from 460f6d2 to de49bea Compare August 12, 2022 17:37
KoditkarVedant and others added 5 commits August 13, 2022 02:06
Signed-off-by: Vedant Koditkar <vedant.koditkar@outlook.com>
Signed-off-by: Vedant Koditkar <vedant.koditkar@outlook.com>
Signed-off-by: Vedant Koditkar <vedant.koditkar@outlook.com>
….com:KoditkarVedant/compose into 8768-avoid-pulling-same-image-multiple-times
@KoditkarVedant
Copy link
Contributor Author

@laurazard I tried multiple ways but this tests doesn't seem to pass on the CI server. It works as expected on local. can you please have a look at this?

Failing test: TestComposePull/Verify_skipped_pull_if_image_is_already_present_locally
My idea is to pull the required image first so when I try to pull it using the compose tool it will already be present locally.

@laurazard
Copy link
Member

I'll take a look at it :)

@laurazard
Copy link
Member

@KoditkarVedant I've been looking at this locally and I can consistently replicate the error. Moreover, I was trying to figure out where the error was coming from and searching all over the project, I could not find where the ... Image is already present locally message is supposed to come from. Did you forget to commit something?

@KoditkarVedant
Copy link
Contributor Author

@laurazard Thank you - I see what I was doing wrong for so long. I was using the old docker-compose build that I build previously so rebuilding the CLI starts failing the tests locally. I have added the fix.

@laurazard laurazard requested a review from a team August 19, 2022 13:59
@laurazard
Copy link
Member

awesome @KoditkarVedant! We'll try to get it reviewed and merged soon :)

Copy link
Member

@nicksieger nicksieger left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks :)

@laurazard laurazard merged commit f880b41 into docker:v2 Aug 19, 2022
@geozeke
Copy link

geozeke commented Aug 20, 2022

Thanks for the hard work on this project!

In this version it looks like docker-compose pull will now not pull the latest updates to an image. For example, if my docker-compose.yml file contains this: image: mariadb:latest, then even if there's an updated mariadb image, docker-compose reports:

mariadb Skipped - Image is already present locally

@hugalafutro
Copy link

hugalafutro commented Aug 20, 2022

I can confirm the same, none of my updated :latest images download, claiming they exist locally. Downgrading to compose 2.9.0 updates local images correctly when running docker compose pull.

Edit: I might also be misunderstanding the purpose of this change, but I have been updating my stacks by running docker compose pull since compose v1 and it always only downloaded image if newer was available.

@geozeke
Copy link

geozeke commented Aug 20, 2022

Same for me, @hugalafutro. I didn't see a documented change in behavior, but docker-compose pull has always worked for me the way you describe.

@martadinata666
Copy link

martadinata666 commented Aug 21, 2022

So related to the issue that this PR want to solve, seems this already solved long ago cant really tell which version, i tested a compose file with same image it will only pull once and the other will wait instead redownload again.
using: docker compose v2.9

First case using docker compose pull

docker compose pull    
[+] Running 13/13
 ⠿ db2 Pulled                                                                                                70.3s
 ⠿ db Pulled                                                                                                 70.3s
   ⠿ a749a280e3e9 Pull complete                                                                              24.3s
   ⠿ 3f7d70702fc5 Pull complete                                                                              24.7s
   ⠿ b9ffe290feb5 Pull complete                                                                              25.9s
   ⠿ 5a430076265b Pull complete                                                                              26.6s
   ⠿ 45c880491c15 Pull complete                                                                              26.8s
   ⠿ 41ca8382cebf Pull complete                                                                              28.1s
   ⠿ 88502488cc76 Pull complete                                                                              28.4s
   ⠿ 8ae7307cb02f Pull complete                                                                              28.7s
   ⠿ df60df6710cc Pull complete                                                                              63.8s
   ⠿ bb82d118ee8f Pull complete                                                                              64.0s
   ⠿ 6ce2f3952ce5 Pull complete  

Second case download image first then issue docker compose pull

dedyms@rpi4 apptest % docker pull mariadb:10.6
10.6: Pulling from library/mariadb
a749a280e3e9: Pull complete 
3f7d70702fc5: Pull complete 
b9ffe290feb5: Pull complete 
5a430076265b: Pull complete 
45c880491c15: Pull complete 
41ca8382cebf: Pull complete 
88502488cc76: Pull complete 
8ae7307cb02f: Pull complete 
df60df6710cc: Pull complete 
bb82d118ee8f: Pull complete 
6ce2f3952ce5: Pull complete 
Digest: sha256:e78a7000d93b2fabc0bfb6ff1504199f2f9bfb4a8b7350922c08dabf2b2bdbea
Status: Downloaded newer image for mariadb:10.6
docker.io/library/mariadb:10.6
dedyms@rpi4 apptest % docker compose pull
[+] Running 3/3
 ⠿ db2 Pulled                                                                                                 3.7s
 ⠿ db3 Pulled                                                                                                 3.7s
 ⠿ db Pulled                  

Docker compose will check registry and compare to local, docker default behaviour i think?

Compose file

version: "3.8"
services:
 db:
    image: mariadb:10.6
    environment:
      - MARIADB_AUTO_UPGRADE=1
      - MARIADB_ROOT_PASSWORD=rootpass
      - TZ=Asia/Jakarta
      - MARIADB_DATABASE=test
      - MARIADB_USER=testuser
      - MARIADB_PASSWORD=testpass
    volumes:
      - ./db:/var/lib/mysql

 db2:
    image: mariadb:10.6
    environment:
      - MARIADB_AUTO_UPGRADE=1
      - MARIADB_ROOT_PASSWORD=rootpass
      - TZ=Asia/Jakarta
      - MARIADB_DATABASE=test
      - MARIADB_USER=testuser
      - MARIADB_PASSWORD=testpass
    volumes:
      - ./db2:/var/lib/mysql

@geozeke
Copy link

geozeke commented Aug 21, 2022

Looks like there's a new issue open for this: #9773

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.

9 participants