Clean docker image cache #1636
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| name: restore-docker-images | ||
| description: upload docker images | ||
| runs: | ||
| using: "composite" | ||
| steps: | ||
|
|
||
| - name: restore images if cache matches | ||
| uses: actions/cache/restore@v4 | ||
| id: images-cache | ||
| with: | ||
| path: /tmp/docker/images | ||
| key: docker-images-cache-${{ hashFiles('**/current-images.txt') }} | ||
|
|
||
| - name: pull docker images | ||
|
Contributor
Author
There was a problem hiding this comment. if cache is not present, issue It also replaces the |
||
| shell: bash | ||
| if: steps.images-cache.outputs.cache-hit != 'true' | ||
| run: | | ||
| echo 'start pulling common images' | ||
| mkdir -p /tmp/docker/images | ||
| result=$(find . -name 'current-images.txt') | ||
| while read line; do | ||
| docker pull "$line" | ||
| replace_slash=$(tr '/' '-' <<< "$line") | ||
| docker save "${line}" > "/tmp/docker/images/${replace_slash}.tar" | ||
| done <$result | ||
|
|
||
| ls /tmp/docker/images | ||
|
|
||
| - name: save docker images | ||
| id: save-docker-images | ||
| uses: actions/cache/save@v4 | ||
| if: steps.images-cache.outputs.cache-hit != 'true' | ||
| with: | ||
| path: | | ||
| /tmp/docker/images | ||
| key: docker-images-cache-${{ hashFiles('**/current-images.txt') }} | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ | |
| import java.time.Duration; | ||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import java.util.Optional; | ||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| import com.github.dockerjava.api.command.ListImagesCmd; | ||
|
|
@@ -57,11 +58,14 @@ public final class Commons { | |
| private static final Log LOG = LogFactory.getLog(Commons.class); | ||
|
|
||
| /** | ||
| * this path is generated by the pipeline of github actions. | ||
| */ | ||
| private static final String TMP_IMAGES = "/tmp/docker/images"; | ||
|
|
||
| /** | ||
| * istio version used in our integration tests. | ||
| */ | ||
| public static final String ISTIO_VERSION = "1.20.5"; | ||
|
|
||
| private Commons() { | ||
| throw new AssertionError("No instance provided"); | ||
|
|
@@ -92,7 +96,7 @@ private Commons() { | |
|
|
||
| private static final K3sContainer CONTAINER = new FixedPortsK3sContainer(DockerImageName.parse(Commons.RANCHER)) | ||
| .configureFixedPorts(EXPOSED_PORTS).withFileSystemBind(TEMP_FOLDER, TEMP_FOLDER) | ||
| .withFileSystemBind(TMP_IMAGES, TMP_IMAGES).withCommand(Commons.RANCHER_COMMAND).withReuse(true); | ||
|
|
||
| public static K3sContainer container() { | ||
| return CONTAINER; | ||
|
|
@@ -168,6 +172,51 @@ public static void loadImage(String image, String tag, String tarName, K3sContai | |
|
|
||
| } | ||
|
|
||
| public static void load(K3sContainer container, String name, String image) { | ||
| File dockerImagesRootDir = Paths.get(Commons.TMP_IMAGES).toFile(); | ||
| if (dockerImagesRootDir.exists() && dockerImagesRootDir.isDirectory()) { | ||
| File[] tars = dockerImagesRootDir.listFiles(); | ||
| if (tars != null && tars.length > 0) { | ||
| Optional<String> found = Arrays.stream(tars).map(File::getName) | ||
| .filter(tarName -> tarName.contains(name)).findFirst(); | ||
| if (found.isPresent()) { | ||
| LOG.info("running in github actions, will load from : " + Commons.TMP_IMAGES); | ||
| Commons.loadImageFromPath(found.get(), container); | ||
|
Contributor
Author
There was a problem hiding this comment. this
Contributor
There was a problem hiding this comment. my 2p is to make the GH workflow do this, as it is distracting in the normal code. In my go code, I only pull if the image doesn't already exist (or it is a locally built one). So, if you can have in the GH action the images safely restored from tar, then "pull if not exists" here for the non-test/tagged images will be simpler code. Also, it centralizes the responsibility vs picking a magic tar directory. When I look at other code that uses k3s or testcontainers it is a lot simpler than the code here, and I think we need to keep in mind how difficult this is vs the intent of testcontainers (just use it sort of thing)
Contributor
Author
There was a problem hiding this comment. Let me see if I got your point correct. You are suggesting to do the "docker load" part in the GH action itself, right? If so, good point, I did not think about it this way
Contributor
There was a problem hiding this comment. yeah in fact when I have something complicated for GH action, I put it into a shell script and have the GH action call that. This way it is easier to ad-hoc test. I have it on my TODO to look at https://dagger.io/ instead of this approach
Contributor
Author
There was a problem hiding this comment. I took a closer look, and I don't think that your suggestion is possible. At this point, there is no instance of k3s starter, so no way to import an image via I am really interested how you are doing it to be honest and where I can take a look.
Contributor
Author
There was a problem hiding this comment. And sorry it took a while to respond, I had to recollect why it is like this, and create that PR. Your comments triggered that, so thank you :)
Contributor
There was a problem hiding this comment. thanks for raising it. Most of the time testcontainers-java is ahead of go, and this is a rare place where we need to catch up the main one. 👍
Contributor
Author
There was a problem hiding this comment. neah, thank you actually. I never saw the problem from this angle until you came here, so all the credits go to you. kudos!
Contributor
There was a problem hiding this comment. Just to be clear when we run the tests locally (or internally on Jenkins) we will still pull the images like we did before because this cache will never exist?
Contributor
Author
There was a problem hiding this comment. that's correct. it's like running the test locally, without having the images already pulled |
||
| return; | ||
| } | ||
| else { | ||
| LOG.info(name + " not found, resorting to pulling the image"); | ||
| } | ||
| } | ||
| else { | ||
| LOG.info("no tars found, will resort to pulling the image"); | ||
| } | ||
| } | ||
| else { | ||
| LOG.info("running outside github actions"); | ||
| } | ||
|
|
||
| try { | ||
| LOG.info("no tars found, will resort to pulling the image"); | ||
| Commons.pullImage(image, Commons.ISTIO_VERSION, container); | ||
| Commons.loadImage(image, Commons.ISTIO_VERSION, name, container); | ||
| } | ||
| catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
|
|
||
| private static void loadImageFromPath(String tarName, K3sContainer container) { | ||
| await().atMost(Duration.ofMinutes(2)).pollInterval(Duration.ofSeconds(1)).until(() -> { | ||
| Container.ExecResult result = container.execInContainer("ctr", "i", "import", TMP_IMAGES + "/" + tarName); | ||
| boolean noErrors = result.getStderr() == null || result.getStderr().isEmpty(); | ||
| if (!noErrors) { | ||
| LOG.info("error is : " + result.getStderr()); | ||
| } | ||
| return noErrors; | ||
| }); | ||
| } | ||
|
|
||
| public static void cleanUp(String image, K3sContainer container) throws Exception { | ||
| container.execInContainer("crictl", "rmi", "docker.io/springcloud/" + image + ":" + pomVersion()); | ||
| container.execInContainer("rm", TEMP_FOLDER + "/" + image + ".tar"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| busybox:1.35 | ||
|
Contributor
Author
There was a problem hiding this comment. what I have realized is that we download docker images specific to integration tests all the time. I am proposing to cache them via github actions instead. In order to do that, we are going to track what images we use and their versions in this file. It is used in the pipeline as the key of the cache : If this file changed, we will re-download them; it if stays the same and such a cache is present, it will download from the cache.
Contributor
There was a problem hiding this comment. I would recommend unit testing this somehow. Possibly you can make a test that dumps the images from k3s ctl and fails if it has something that isn't in here or visa versa? Just a thought, as this is very likely to drift.
Contributor
There was a problem hiding this comment. another way could be to make this like DockerImages.go, and give it a main function that prints out a list like this. Then, as long as you checked out the source first and installed go, you could run that main and also have no drift.
Contributor
Author
There was a problem hiding this comment. this is just a helper, really. It is supposed to help with the times we run integration tests (that we plan to reduce in the long run anyway). So even if an image is not downloaded from the cache, it is still going to be pulled in the test. I, personally, would not test this "too much" and a few runs that I had proved that it works exactly as intended.
Contributor
There was a problem hiding this comment. p.s. I meant DockerImages.java ;) ok you can also solve this by making a comment in the java code that defines the istio versions saying "// please update current-images.txt when you update this" because at the end of the day, it is about drift making the infrastructure added here serve no value.
Contributor
Author
There was a problem hiding this comment. if you look closely into
Contributor
Author
There was a problem hiding this comment. neither does P.S. I doubt that I want to wait until that PR in test-containers is reviewed and released, so I am inclined to suggest merge/review this one the way it is. When that PR sees day light, I will go for another round of minor refactor. |
||
| istio/istioctl:1.20.5 | ||
| istio/proxyv2:1.20.5 | ||
| istio/pilot:1.20.5 | ||
| confluentinc/cp-kafka:7.2.1 | ||
| rabbitmq:3-management | ||
| wiremock/wiremock:3.4.2 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a new stage that basically does this:
docker-images-cache-${{ hashFiles('**/current-images.txt') }}use it and restore to/tmp/docker/imagescurrent-images.txtline by line. For each of those lines, do adocker pullthen adocker saveto/tmp/docker/images/tmp/docker/images