From 2ca5da5c69e65449e9877b9e08af2e980900bd7a Mon Sep 17 00:00:00 2001 From: Phil Porada Date: Wed, 6 Sep 2023 00:20:49 -0400 Subject: [PATCH 1/5] Check if bucket exists before creating it --- integration_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/integration_test.go b/integration_test.go index 8d923f3..8c5cc65 100644 --- a/integration_test.go +++ b/integration_test.go @@ -134,11 +134,14 @@ func TestIntegration(t *testing.T) { } s3Service := s3.NewFromConfig(cfg) - _, err = s3Service.CreateBucket(context.Background(), &s3.CreateBucketInput{ - Bucket: aws.String("bucket"), - }) + _, err = s3Service.HeadBucket(context.TODO(), &s3.HeadBucketInput{Bucket: aws.String("bucket")}) if err != nil { - t.Fatal(err) + _, err = s3Service.CreateBucket(context.Background(), &s3.CreateBucketInput{ + Bucket: aws.String("bucket"), + }) + if err != nil { + t.Fatal(err) + } } ctile := tileCachingHandler{ From b652aaca3b367d758f7bfd1b0f64c69b7d1cfe90 Mon Sep 17 00:00:00 2001 From: Phil Porada Date: Wed, 6 Sep 2023 17:19:10 -0400 Subject: [PATCH 2/5] More robust container cleanup --- integration_test.go | 45 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/integration_test.go b/integration_test.go index 8c5cc65..9e1d137 100644 --- a/integration_test.go +++ b/integration_test.go @@ -27,8 +27,38 @@ import ( "golang.org/x/sync/singleflight" ) +const containerName string = "ctile_integration_test_minio" + +// cleanupContainer stops a running named container and removes its assigned +// name. +func cleanupContainer() { + // We throw away the error here because ps will return code 125 if the + // name doesn't exist, but that's the whole point of this check. + isContainerRunning, _ := exec.Command("podman", "ps", "--filter", fmt.Sprintf("name=%s", containerName), "--format={{ .ID }}").Output() + if len(isContainerRunning) > 0 { + _, err := exec.Command("podman", "stop", containerName).Output() + if err != nil { + panic(err) + } + } + + // We throw away the error here because ps will return code 125 if the + // name doesn't exist, but that's the whole point of this check. + danglingName, _ := exec.Command("podman", "ps", "--all", "--filter", fmt.Sprintf("name=%s", containerName), "--format={{ .ID }}").Output() + if len(danglingName) > 0 { + _, err := exec.Command("podman", "rm", "--force", containerName).Output() + if err != nil { + panic(err) + } + } +} + +func init() { + cleanupContainer() +} + func TestMain(m *testing.M) { - cmd := exec.Command("podman", "run", "-p", "19085:9000", "quay.io/minio/minio", "server", "/data") + cmd := exec.Command("podman", "run", "-p", "19085:9000", "--name", containerName, "quay.io/minio/minio", "server", "/data") stderrPipe, err := cmd.StderrPipe() if err != nil { panic(err) @@ -69,6 +99,8 @@ func TestMain(m *testing.M) { const testLogSaysPastTheEnd = "oh no! we fell off the end of the log!" func TestIntegration(t *testing.T) { + defer cleanupContainer() + // A test CT server that responds to get-entries requests with appropriately JSON-formatted // data, where base64-decoding the LeafInput and ExtraData fields yields a binary encoding // of the position of the given element. @@ -134,14 +166,11 @@ func TestIntegration(t *testing.T) { } s3Service := s3.NewFromConfig(cfg) - _, err = s3Service.HeadBucket(context.TODO(), &s3.HeadBucketInput{Bucket: aws.String("bucket")}) + _, err = s3Service.CreateBucket(context.Background(), &s3.CreateBucketInput{ + Bucket: aws.String("bucket"), + }) if err != nil { - _, err = s3Service.CreateBucket(context.Background(), &s3.CreateBucketInput{ - Bucket: aws.String("bucket"), - }) - if err != nil { - t.Fatal(err) - } + t.Fatal(err) } ctile := tileCachingHandler{ From 4efc53923a058986c5f3e2a292102833200e558f Mon Sep 17 00:00:00 2001 From: Phil Porada Date: Thu, 7 Sep 2023 10:22:57 -0400 Subject: [PATCH 3/5] Improve container startup and tear down --- integration_test.go | 47 +++++++++------------------------------------ 1 file changed, 9 insertions(+), 38 deletions(-) diff --git a/integration_test.go b/integration_test.go index 9e1d137..202a2a8 100644 --- a/integration_test.go +++ b/integration_test.go @@ -30,27 +30,16 @@ import ( const containerName string = "ctile_integration_test_minio" // cleanupContainer stops a running named container and removes its assigned -// name. +// name. This is helpful in the event that a container wasn't properly killed +// during a previous test run or if manual testing was being performed and not +// cleaned up. func cleanupContainer() { - // We throw away the error here because ps will return code 125 if the - // name doesn't exist, but that's the whole point of this check. - isContainerRunning, _ := exec.Command("podman", "ps", "--filter", fmt.Sprintf("name=%s", containerName), "--format={{ .ID }}").Output() - if len(isContainerRunning) > 0 { - _, err := exec.Command("podman", "stop", containerName).Output() - if err != nil { - panic(err) - } - } + // Unconditionally stop the container. + _, _ = exec.Command("podman", "stop", containerName).Output() - // We throw away the error here because ps will return code 125 if the - // name doesn't exist, but that's the whole point of this check. - danglingName, _ := exec.Command("podman", "ps", "--all", "--filter", fmt.Sprintf("name=%s", containerName), "--format={{ .ID }}").Output() - if len(danglingName) > 0 { - _, err := exec.Command("podman", "rm", "--force", containerName).Output() - if err != nil { - panic(err) - } - } + // Unconditionally remove the container name if the operator did manual + // container testing, but didn't clean up the name. + _, _ = exec.Command("podman", "rm", containerName).Output() } func init() { @@ -58,16 +47,10 @@ func init() { } func TestMain(m *testing.M) { - cmd := exec.Command("podman", "run", "-p", "19085:9000", "--name", containerName, "quay.io/minio/minio", "server", "/data") - stderrPipe, err := cmd.StderrPipe() - if err != nil { - panic(err) - } - err = cmd.Start() + _, err := exec.Command("podman", "run", "--rm", "--detach", "-p", "19085:9000", "--name", containerName, "quay.io/minio/minio", "server", "/data").Output() if err != nil { panic(err) } - defer cmd.Process.Kill() for i := 0; i < 1000; i++ { _, err := net.Dial("tcp", "localhost:19085") if errors.Is(err, syscall.ECONNREFUSED) { @@ -81,18 +64,6 @@ func TestMain(m *testing.M) { break } code := m.Run() - err = cmd.Process.Signal(os.Interrupt) - if err != nil { - panic(err) - } - io.Copy(os.Stderr, stderrPipe) - processState, err := cmd.Process.Wait() - if err != nil { - panic(err) - } - if processState.ExitCode() != 0 { - panic(fmt.Errorf("minio exited with code %d", processState.ExitCode())) - } os.Exit(code) } From ec777eace51fead766bd95ce4c78e5878a4d51d6 Mon Sep 17 00:00:00 2001 From: Phil Porada Date: Thu, 7 Sep 2023 14:52:08 -0400 Subject: [PATCH 4/5] Remove TestMain and hoist starting minio into init --- integration_test.go | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/integration_test.go b/integration_test.go index 202a2a8..d3c3407 100644 --- a/integration_test.go +++ b/integration_test.go @@ -10,7 +10,6 @@ import ( "net" "net/http" "net/http/httptest" - "os" "os/exec" "reflect" "strconv" @@ -28,25 +27,9 @@ import ( ) const containerName string = "ctile_integration_test_minio" +const testLogSaysPastTheEnd string = "oh no! we fell off the end of the log!" -// cleanupContainer stops a running named container and removes its assigned -// name. This is helpful in the event that a container wasn't properly killed -// during a previous test run or if manual testing was being performed and not -// cleaned up. -func cleanupContainer() { - // Unconditionally stop the container. - _, _ = exec.Command("podman", "stop", containerName).Output() - - // Unconditionally remove the container name if the operator did manual - // container testing, but didn't clean up the name. - _, _ = exec.Command("podman", "rm", containerName).Output() -} - -func init() { - cleanupContainer() -} - -func TestMain(m *testing.M) { +func startContainer() { _, err := exec.Command("podman", "run", "--rm", "--detach", "-p", "19085:9000", "--name", containerName, "quay.io/minio/minio", "server", "/data").Output() if err != nil { panic(err) @@ -63,11 +46,25 @@ func TestMain(m *testing.M) { fmt.Println("minio is up") break } - code := m.Run() - os.Exit(code) } -const testLogSaysPastTheEnd = "oh no! we fell off the end of the log!" +// cleanupContainer stops a running named container and removes its assigned +// name. This is helpful in the event that a container wasn't properly killed +// during a previous test run or if manual testing was being performed and not +// cleaned up. +func cleanupContainer() { + // Unconditionally stop the container. + _, _ = exec.Command("podman", "stop", containerName).Output() + + // Unconditionally remove the container name if the operator did manual + // container testing, but didn't clean up the name. + _, _ = exec.Command("podman", "rm", containerName).Output() +} + +func init() { + cleanupContainer() + startContainer() +} func TestIntegration(t *testing.T) { defer cleanupContainer() From bde905aed5e77f90f23e9a54c87f2617e5ee9178 Mon Sep 17 00:00:00 2001 From: Phil Porada Date: Thu, 7 Sep 2023 15:09:41 -0400 Subject: [PATCH 5/5] Remove init function --- integration_test.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/integration_test.go b/integration_test.go index d3c3407..28ad2f9 100644 --- a/integration_test.go +++ b/integration_test.go @@ -5,7 +5,6 @@ import ( "encoding/binary" "encoding/json" "errors" - "fmt" "io" "net" "net/http" @@ -29,23 +28,25 @@ import ( const containerName string = "ctile_integration_test_minio" const testLogSaysPastTheEnd string = "oh no! we fell off the end of the log!" -func startContainer() { +func startContainer(t *testing.T) { _, err := exec.Command("podman", "run", "--rm", "--detach", "-p", "19085:9000", "--name", containerName, "quay.io/minio/minio", "server", "/data").Output() if err != nil { - panic(err) + t.Fatalf("minio failed to come up: %v", err) } for i := 0; i < 1000; i++ { _, err := net.Dial("tcp", "localhost:19085") if errors.Is(err, syscall.ECONNREFUSED) { + t.Log("sleeping 10ms waiting for minio to come up") time.Sleep(10 * time.Millisecond) continue } if err != nil { - panic(err) + t.Fatalf("failed to connect to minio: %v", err) } - fmt.Println("minio is up") - break + t.Log("minio is up") + return } + t.Fatalf("failed to connect to minio: %v", err) } // cleanupContainer stops a running named container and removes its assigned @@ -61,12 +62,9 @@ func cleanupContainer() { _, _ = exec.Command("podman", "rm", containerName).Output() } -func init() { - cleanupContainer() - startContainer() -} - func TestIntegration(t *testing.T) { + cleanupContainer() // Clean up old containers and names just in case. + startContainer(t) defer cleanupContainer() // A test CT server that responds to get-entries requests with appropriately JSON-formatted