Skip to content

Commit

Permalink
fix: track localImages in a new map to enable proper cleanup (runfinc…
Browse files Browse the repository at this point in the history
…h#133)

Issue #, if available: See
https://github.com/runfinch/finch/actions/runs/8424406162/job/23068204038?pr=825#step:10:5839
```
  time="2024-03-25T17:50:43Z" level=info msg="trying next host" error="failed to do request: Head \"http://localhost:51261/v2/docker/library/alpine/manifests/latest\": dial tcp [::1]:51261: connect: connection refused" host="localhost:51261"
  time="2024-03-25T17:50:43Z" level=error msg="server \"localhost:51261\" does not seem to support HTTPS" error="failed to resolve reference \"localhost:51261/docker/library/alpine:latest\": failed to do request: Head \"http://localhost:51261/v2/docker/library/alpine/manifests/latest\": dial tcp [::1]:51261: connect: connection refused"
  time="2024-03-25T17:50:43Z" level=info msg="Hint: you may want to try --insecure-registry to allow plain HTTP (if you are in a trusted network)"
  time="2024-03-25T17:50:43Z" level=fatal msg="failed to resolve reference \"localhost:51261/docker/library/alpine:latest\": failed to do request: Head \"http://localhost:51261/v2/docker/library/alpine/manifests/latest\": dial tcp [::1]:51261: connect: connection refused"
  time="2024-03-25T17:50:43Z" level=fatal msg="exit status 1"
```

This error occurs here because the localImages map already has the
"local" entries, even though `CleanupLocalRegistry` was run (and on top
of that, the VM's disk was completely reset, including the local
registry container. This is an edge case that occurs here because the
`tests` package is imported once, and the global state of the
`localImages` map is preserved.

*Description of changes:*
- Instead of using one map and overriding its entries, store the local
entries in a new map so that `CleanupLocalRegistry` can reset it, and a
subsequent `SetupLocalRegistry` can repopulate it without issue

*Testing done:*
- Tested after running Setup -> Clean -> Setup (by running the exact
same failing e2e test locally), and it now works


- [x] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: Justin Alvarez <[email protected]>
  • Loading branch information
pendo324 authored and Shubhranshu153 committed Mar 26, 2024
1 parent 7d08dd1 commit 5652dcf
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions tests/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@ const (
nginxImage localImage = "nginxImage"
)

var localImages = map[localImage]string{
var remoteImages = map[localImage]string{
defaultImage: alpineImage,
olderAlpineImage: "public.ecr.aws/docker/library/alpine:3.13",
amazonLinux2Image: "public.ecr.aws/amazonlinux/amazonlinux:2",
nginxImage: "public.ecr.aws/docker/library/nginx:latest",
}

var localImages = map[localImage]string{}

// CGMode is the cgroups mode of the host system.
// We copy the struct from containerd/cgroups [1] instead of using it as a library
// because it only builds on linux,
Expand All @@ -80,8 +82,8 @@ const (

// SetupLocalRegistry can be invoked before running the tests to save time when pulling images during tests.
//
// It spins up a local registry, tags all localImages, pushes the new tagged image to local registry,
// and changes map entry to be the one pushed to local registry.
// It spins up a local registry, tags all remoteImages, pushes the new tagged images to the local registry,
// and changes adds corresponding entries to localImages for all of the new tags pushed to local registry.
//
// After all the tests are done, invoke CleanupLocalRegistry to clean up the local registry.
func SetupLocalRegistry(o *option.Option) {
Expand All @@ -94,7 +96,7 @@ func SetupLocalRegistry(o *option.Option) {
command.SetLocalRegistryImageID(imageID)
command.SetLocalRegistryImageName(registryImage)

for k, ref := range localImages {
for k, ref := range remoteImages {
// split image tag according to spec
// https://github.com/distribution/distribution/blob/d0deff9cd6c2b8c82c6f3d1c713af51df099d07b/reference/reference.go
_, name, _ := strings.Cut(ref, "/")
Expand All @@ -114,6 +116,7 @@ func CleanupLocalRegistry(o *option.Option) {
command.Run(o, "rm", "-f", containerID)
imageID := command.StdoutStr(o, "images", "-q")
command.Run(o, "rmi", "-f", imageID)
localImages = map[localImage]string{}
}

func pullImage(o *option.Option, imageName string) {
Expand Down

0 comments on commit 5652dcf

Please sign in to comment.