From 757c9ecff1b03c6b7b2410538d36ddeeb1f28157 Mon Sep 17 00:00:00 2001 From: nouseforaname <34882943+nouseforaname@users.noreply.github.com> Date: Wed, 5 Jun 2024 17:33:52 +0200 Subject: [PATCH 1/3] add flag to provide docker test image as tarball some teams would like to use `kiln test` from concourse. That concourse could be shared, like runway // tpe concourse instances. These, even using authentication, are getting rate limited against dockerhub. Effectively to run kiln test, we'd first need to preload the images required to build the test image ( every FROM arg used in the Dockerfile) avoid the pull ibeing executed implicitly when docker build is run by `kiln test` To avoid having to actually build an image ( which would either require to preload the images for the Dockerfile or being able to pull from dockerhub) we could use this flag to run `docker load -i $IMAGE_PATH` instead of building. this way, the test image can be `acquired` via docker proxy registries.. --- internal/commands/test_tile.go | 15 ++++++++------- internal/commands/test_tile_test.go | 21 +++++++++++++++++++++ internal/test/container.go | 1 + 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/internal/commands/test_tile.go b/internal/commands/test_tile.go index 996e13216..876375beb 100644 --- a/internal/commands/test_tile.go +++ b/internal/commands/test_tile.go @@ -16,13 +16,13 @@ type TileTestFunction func(ctx context.Context, w io.Writer, configuration test. type TileTest struct { Options struct { - TilePath string ` long:"tile-path" default:"." description:"Path to the Tile directory (e.g., ~/workspace/tas/ist)."` - Verbose bool `short:"v" long:"verbose" default:"true" description:"Print info lines. This doesn't affect Ginkgo output."` - Silent bool `short:"s" long:"silent" default:"false" description:"Hide info lines. This doesn't affect Ginkgo output."` - Manifest bool ` long:"manifest" default:"false" description:"Focus the Manifest tests."` - Migrations bool ` long:"migrations" default:"false" description:"Focus the Migration tests."` - Stability bool ` long:"stability" default:"false" description:"Focus the Stability tests."` - + TilePath string ` long:"tile-path" default:"." description:"Path to the Tile directory (e.g., ~/workspace/tas/ist)."` + Verbose bool `short:"v" long:"verbose" default:"true" description:"Print info lines. This doesn't affect Ginkgo output."` + Silent bool `short:"s" long:"silent" default:"false" description:"Hide info lines. This doesn't affect Ginkgo output."` + Manifest bool ` long:"manifest" default:"false" description:"Focus the Manifest tests."` + Migrations bool ` long:"migrations" default:"false" description:"Focus the Migration tests."` + Stability bool ` long:"stability" default:"false" description:"Focus the Stability tests."` + ImagePath string " long:\"image-path\" " EnvironmentVars []string `short:"e" long:"environment-variable" description:"Pass environment variable to the test suites. For example --stability -e 'PRODUCT=srt'."` GingkoFlags string ` long:"ginkgo-flags" default:"-r -p -slowSpecThreshold 15" description:"Flags to pass to the Ginkgo Manifest and Stability test suites."` } @@ -70,6 +70,7 @@ func (cmd TileTest) configuration() (test.Configuration, error) { RunManifest: cmd.Options.Manifest, RunMetadata: cmd.Options.Stability, RunMigrations: cmd.Options.Migrations, + ImagePath: cmd.Options.ImagePath, GinkgoFlags: cmd.Options.GingkoFlags, Environment: cmd.Options.EnvironmentVars, diff --git a/internal/commands/test_tile_test.go b/internal/commands/test_tile_test.go index 69aceab29..174de9997 100644 --- a/internal/commands/test_tile_test.go +++ b/internal/commands/test_tile_test.go @@ -208,6 +208,27 @@ var _ = Describe("kiln test", func() { }) }) + When("when the a test image is provided via --image-path", func() { + It("it sets the path to be loaded in the configuration", func() { + args := []string{"--image-path", "/tmp/test.file"} + + fakeTestFunc := fakes.TestTileFunction{} + fakeTestFunc.Returns(nil) + + err := commands.NewTileTestWithCollaborators(&output, fakeTestFunc.Spy).Execute(args) + Expect(err).NotTo(HaveOccurred()) + + Expect(fakeTestFunc.CallCount()).To(Equal(1)) + + ctx, w, configuration := fakeTestFunc.ArgsForCall(0) + Expect(ctx).NotTo(BeNil()) + Expect(w).NotTo(BeNil()) + Expect(configuration.ImagePath).To(Equal("/tmp/test.file")) + Expect(configuration.RunManifest).To(BeFalse()) + Expect(configuration.RunMetadata).To(BeFalse()) + Expect(configuration.RunMigrations).To(BeFalse()) + }) + }) When("when the stability test is enabled", func() { It("it sets the metadata configuration flag", func() { args := []string{"--stability"} diff --git a/internal/test/container.go b/internal/test/container.go index d1f2269cb..574d999cf 100644 --- a/internal/test/container.go +++ b/internal/test/container.go @@ -89,6 +89,7 @@ type Configuration struct { RunMigrations, RunManifest, RunMetadata bool + ImagePath string GinkgoFlags string Environment []string From 0963425a9e5e29466c2569c73c2a81fc33cb0d0f Mon Sep 17 00:00:00 2001 From: nouseforaname <34882943+nouseforaname@users.noreply.github.com> Date: Thu, 6 Jun 2024 10:31:39 +0200 Subject: [PATCH 2/3] add alternative to execute image-build every time why?: because some CI systems cannot get past the dockerhub rate limitation. We want to use kiln test for the csb tiles. But we use the tpe / runway concourse. These instances make it impossible to communicate with dockerhub because of rate limits ( even with authenticated pulls ) We can make the `kiln test` execution work by pre loading the images that are used as FROM stages in `internal/test/Dockerfile` into the docker host we start in the concourse task. Essentially we end up pulling the images from an accessible repository, then we retag them to match the expected FROM args and that makes the implicit image build work. Instead it would be nice to avoid having to run the build stage and make kiln test consume a provided image instead so it can be run offline. --- internal/test/assets/alpine.tgz | 1 + internal/test/container.go | 48 ++++++--- internal/test/container_test.go | 41 +++++++ internal/test/fakes/moby_client.go | 168 +++++++++++++++++++++++++++++ 4 files changed, 246 insertions(+), 12 deletions(-) create mode 100644 internal/test/assets/alpine.tgz diff --git a/internal/test/assets/alpine.tgz b/internal/test/assets/alpine.tgz new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/internal/test/assets/alpine.tgz @@ -0,0 +1 @@ + diff --git a/internal/test/container.go b/internal/test/container.go index 574d999cf..8f8bfa9b5 100644 --- a/internal/test/container.go +++ b/internal/test/container.go @@ -124,6 +124,7 @@ func (configuration Configuration) commands() ([]string, error) { //counterfeiter:generate -o ./fakes/moby_client.go --fake-name MobyClient . mobyClient type mobyClient interface { DialHijack(ctx context.Context, url, proto string, meta map[string][]string) (net.Conn, error) + ImageLoad(ctx context.Context, input io.Reader, quiet bool) (types.ImageLoadResponse, error) ImageBuild(ctx context.Context, buildContext io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) Ping(ctx context.Context) (types.Ping, error) ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *specV1.Platform, containerName string) (container.CreateResponse, error) @@ -141,22 +142,45 @@ func runTestWithSession(ctx context.Context, logger *log.Logger, w io.Writer, do } var dockerfileTarball bytes.Buffer - if err := createDockerfileTarball(tar.NewWriter(&dockerfileTarball), dockerfile); err != nil { + if err = createDockerfileTarball(tar.NewWriter(&dockerfileTarball), dockerfile); err != nil { return err } - logger.Println("creating test image") - imageBuildResult, err := dockerDaemon.ImageBuild(ctx, &dockerfileTarball, types.ImageBuildOptions{ - Tags: []string{"kiln_test_dependencies:vmware"}, - Version: types.BuilderBuildKit, - SessionID: sessionID, - }) - if err != nil { - return fmt.Errorf("failed to build image: %w", err) - } + if configuration.ImagePath == "" { + logger.Println("creating test image") + imageBuildResult, err := dockerDaemon.ImageBuild(ctx, &dockerfileTarball, types.ImageBuildOptions{ + Tags: []string{"kiln_test_dependencies:vmware"}, + Version: types.BuilderBuildKit, + SessionID: sessionID, + }) + if err != nil { + return fmt.Errorf("failed to build image: %w", err) + } + if err = checkSSHPrivateKeyError(imageBuildResult.Body); err != nil { + return err + } + } else { + logger.Println("loading test image") + imageReader, err := os.Open(configuration.ImagePath) + if err != nil { + return fmt.Errorf("failed to read image '%s': %w", configuration.ImagePath, err) + } - if err := checkSSHPrivateKeyError(imageBuildResult.Body); err != nil { - return err + loadResponse, err := dockerDaemon.ImageLoad( + ctx, + imageReader, + true, + ) + if err != nil { + return fmt.Errorf("failed to import image: %w", err) + } + + respBytes, err := io.ReadAll(loadResponse.Body) + defer loadResponse.Body.Close() + if err != nil { + return fmt.Errorf(`failed to parse load image response: %w`, err) + } + logger.Printf("loaded image %s: \n%s\n", configuration.ImagePath, string(respBytes)) } parentDir := path.Dir(configuration.AbsoluteTileDirectory) diff --git a/internal/test/container_test.go b/internal/test/container_test.go index 0ab4091b7..a4db06465 100644 --- a/internal/test/container_test.go +++ b/internal/test/container_test.go @@ -108,6 +108,43 @@ func Test_configureSession(t *testing.T) { }) } +func Test_loadImage(t *testing.T) { + absoluteTileDirectory := filepath.Join(t.TempDir(), "test") + logger := log.New(io.Discard, "", 0) + t.Run("when loading a provided test image with a wrong path", func(t *testing.T) { + ctx := context.Background() + out := bytes.Buffer{} + configuration := Configuration{ + AbsoluteTileDirectory: absoluteTileDirectory, + ImagePath: "non-existing", + } + client := runTestWithSessionHelper(t, "", container.WaitResponse{ + StatusCode: 0, + }) + + err := runTestWithSession(ctx, logger, &out, client, configuration)("some-session-id") + require.ErrorContains(t, err, "failed to read image 'non-existing': open non-existing: no such file or directory") + + }) + t.Run(`when loading a provided test image with an existing path`, func(t *testing.T) { + ctx := context.Background() + out := bytes.Buffer{} + + configuration := Configuration{ + AbsoluteTileDirectory: absoluteTileDirectory, + ImagePath: `assets/alpine.tgz`, + } + + client := runTestWithSessionHelper(t, "", container.WaitResponse{ + StatusCode: 0, + }) + + err := runTestWithSession(ctx, logger, &out, client, configuration)("some-session-id") + require.NoError(t, err) + + }) +} + func Test_runTestWithSession(t *testing.T) { absoluteTileDirectory := filepath.Join(t.TempDir(), "test") logger := log.New(io.Discard, "", 0) @@ -200,6 +237,10 @@ func runTestWithSessionHelper(t *testing.T, logs string, response container.Wait client.ImageBuildReturns(types.ImageBuildResponse{ Body: io.NopCloser(strings.NewReader("")), }, nil) + client.ImageLoadReturns(types.ImageLoadResponse{ + Body: io.NopCloser(strings.NewReader("")), + }, nil) + client.ContainerStartReturns(nil) client.ContainerLogsReturns(io.NopCloser(strings.NewReader(logs)), nil) diff --git a/internal/test/fakes/moby_client.go b/internal/test/fakes/moby_client.go index 11e4ac597..084265d34 100644 --- a/internal/test/fakes/moby_client.go +++ b/internal/test/fakes/moby_client.go @@ -119,6 +119,37 @@ type MobyClient struct { result1 types.ImageBuildResponse result2 error } + ImageImportStub func(context.Context, types.ImageImportSource, string, types.ImageImportOptions) (io.ReadCloser, error) + imageImportMutex sync.RWMutex + imageImportArgsForCall []struct { + arg1 context.Context + arg2 types.ImageImportSource + arg3 string + arg4 types.ImageImportOptions + } + imageImportReturns struct { + result1 io.ReadCloser + result2 error + } + imageImportReturnsOnCall map[int]struct { + result1 io.ReadCloser + result2 error + } + ImageLoadStub func(context.Context, io.Reader, bool) (types.ImageLoadResponse, error) + imageLoadMutex sync.RWMutex + imageLoadArgsForCall []struct { + arg1 context.Context + arg2 io.Reader + arg3 bool + } + imageLoadReturns struct { + result1 types.ImageLoadResponse + result2 error + } + imageLoadReturnsOnCall map[int]struct { + result1 types.ImageLoadResponse + result2 error + } PingStub func(context.Context) (types.Ping, error) pingMutex sync.RWMutex pingArgsForCall []struct { @@ -596,6 +627,139 @@ func (fake *MobyClient) ImageBuildReturnsOnCall(i int, result1 types.ImageBuildR }{result1, result2} } +func (fake *MobyClient) ImageImport(arg1 context.Context, arg2 types.ImageImportSource, arg3 string, arg4 types.ImageImportOptions) (io.ReadCloser, error) { + fake.imageImportMutex.Lock() + ret, specificReturn := fake.imageImportReturnsOnCall[len(fake.imageImportArgsForCall)] + fake.imageImportArgsForCall = append(fake.imageImportArgsForCall, struct { + arg1 context.Context + arg2 types.ImageImportSource + arg3 string + arg4 types.ImageImportOptions + }{arg1, arg2, arg3, arg4}) + stub := fake.ImageImportStub + fakeReturns := fake.imageImportReturns + fake.recordInvocation("ImageImport", []interface{}{arg1, arg2, arg3, arg4}) + fake.imageImportMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3, arg4) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *MobyClient) ImageImportCallCount() int { + fake.imageImportMutex.RLock() + defer fake.imageImportMutex.RUnlock() + return len(fake.imageImportArgsForCall) +} + +func (fake *MobyClient) ImageImportCalls(stub func(context.Context, types.ImageImportSource, string, types.ImageImportOptions) (io.ReadCloser, error)) { + fake.imageImportMutex.Lock() + defer fake.imageImportMutex.Unlock() + fake.ImageImportStub = stub +} + +func (fake *MobyClient) ImageImportArgsForCall(i int) (context.Context, types.ImageImportSource, string, types.ImageImportOptions) { + fake.imageImportMutex.RLock() + defer fake.imageImportMutex.RUnlock() + argsForCall := fake.imageImportArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 +} + +func (fake *MobyClient) ImageImportReturns(result1 io.ReadCloser, result2 error) { + fake.imageImportMutex.Lock() + defer fake.imageImportMutex.Unlock() + fake.ImageImportStub = nil + fake.imageImportReturns = struct { + result1 io.ReadCloser + result2 error + }{result1, result2} +} + +func (fake *MobyClient) ImageImportReturnsOnCall(i int, result1 io.ReadCloser, result2 error) { + fake.imageImportMutex.Lock() + defer fake.imageImportMutex.Unlock() + fake.ImageImportStub = nil + if fake.imageImportReturnsOnCall == nil { + fake.imageImportReturnsOnCall = make(map[int]struct { + result1 io.ReadCloser + result2 error + }) + } + fake.imageImportReturnsOnCall[i] = struct { + result1 io.ReadCloser + result2 error + }{result1, result2} +} + +func (fake *MobyClient) ImageLoad(arg1 context.Context, arg2 io.Reader, arg3 bool) (types.ImageLoadResponse, error) { + fake.imageLoadMutex.Lock() + ret, specificReturn := fake.imageLoadReturnsOnCall[len(fake.imageLoadArgsForCall)] + fake.imageLoadArgsForCall = append(fake.imageLoadArgsForCall, struct { + arg1 context.Context + arg2 io.Reader + arg3 bool + }{arg1, arg2, arg3}) + stub := fake.ImageLoadStub + fakeReturns := fake.imageLoadReturns + fake.recordInvocation("ImageLoad", []interface{}{arg1, arg2, arg3}) + fake.imageLoadMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *MobyClient) ImageLoadCallCount() int { + fake.imageLoadMutex.RLock() + defer fake.imageLoadMutex.RUnlock() + return len(fake.imageLoadArgsForCall) +} + +func (fake *MobyClient) ImageLoadCalls(stub func(context.Context, io.Reader, bool) (types.ImageLoadResponse, error)) { + fake.imageLoadMutex.Lock() + defer fake.imageLoadMutex.Unlock() + fake.ImageLoadStub = stub +} + +func (fake *MobyClient) ImageLoadArgsForCall(i int) (context.Context, io.Reader, bool) { + fake.imageLoadMutex.RLock() + defer fake.imageLoadMutex.RUnlock() + argsForCall := fake.imageLoadArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *MobyClient) ImageLoadReturns(result1 types.ImageLoadResponse, result2 error) { + fake.imageLoadMutex.Lock() + defer fake.imageLoadMutex.Unlock() + fake.ImageLoadStub = nil + fake.imageLoadReturns = struct { + result1 types.ImageLoadResponse + result2 error + }{result1, result2} +} + +func (fake *MobyClient) ImageLoadReturnsOnCall(i int, result1 types.ImageLoadResponse, result2 error) { + fake.imageLoadMutex.Lock() + defer fake.imageLoadMutex.Unlock() + fake.ImageLoadStub = nil + if fake.imageLoadReturnsOnCall == nil { + fake.imageLoadReturnsOnCall = make(map[int]struct { + result1 types.ImageLoadResponse + result2 error + }) + } + fake.imageLoadReturnsOnCall[i] = struct { + result1 types.ImageLoadResponse + result2 error + }{result1, result2} +} + func (fake *MobyClient) Ping(arg1 context.Context) (types.Ping, error) { fake.pingMutex.Lock() ret, specificReturn := fake.pingReturnsOnCall[len(fake.pingArgsForCall)] @@ -677,6 +841,10 @@ func (fake *MobyClient) Invocations() map[string][][]interface{} { defer fake.dialHijackMutex.RUnlock() fake.imageBuildMutex.RLock() defer fake.imageBuildMutex.RUnlock() + fake.imageImportMutex.RLock() + defer fake.imageImportMutex.RUnlock() + fake.imageLoadMutex.RLock() + defer fake.imageLoadMutex.RUnlock() fake.pingMutex.RLock() defer fake.pingMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} From 2c5ae0885ebf3aaf9238a7c87250d42f68e06b8a Mon Sep 17 00:00:00 2001 From: nouseforaname <34882943+nouseforaname@users.noreply.github.com> Date: Fri, 7 Jun 2024 08:41:03 +0200 Subject: [PATCH 3/3] implement changes requested on review - replace `"` with `\`` quoted strings - use closer helper function and move defer up --- internal/commands/test_tile.go | 2 +- internal/test/container.go | 3 +-- internal/test/container_test.go | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/commands/test_tile.go b/internal/commands/test_tile.go index 876375beb..49d8576d6 100644 --- a/internal/commands/test_tile.go +++ b/internal/commands/test_tile.go @@ -22,7 +22,7 @@ type TileTest struct { Manifest bool ` long:"manifest" default:"false" description:"Focus the Manifest tests."` Migrations bool ` long:"migrations" default:"false" description:"Focus the Migration tests."` Stability bool ` long:"stability" default:"false" description:"Focus the Stability tests."` - ImagePath string " long:\"image-path\" " + ImagePath string ` long:"image-path" ` EnvironmentVars []string `short:"e" long:"environment-variable" description:"Pass environment variable to the test suites. For example --stability -e 'PRODUCT=srt'."` GingkoFlags string ` long:"ginkgo-flags" default:"-r -p -slowSpecThreshold 15" description:"Flags to pass to the Ginkgo Manifest and Stability test suites."` } diff --git a/internal/test/container.go b/internal/test/container.go index 8f8bfa9b5..67a4b5b28 100644 --- a/internal/test/container.go +++ b/internal/test/container.go @@ -174,9 +174,8 @@ func runTestWithSession(ctx context.Context, logger *log.Logger, w io.Writer, do if err != nil { return fmt.Errorf("failed to import image: %w", err) } - + defer closeAndIgnoreError(loadResponse.Body) respBytes, err := io.ReadAll(loadResponse.Body) - defer loadResponse.Body.Close() if err != nil { return fmt.Errorf(`failed to parse load image response: %w`, err) } diff --git a/internal/test/container_test.go b/internal/test/container_test.go index a4db06465..35dc1c4a5 100644 --- a/internal/test/container_test.go +++ b/internal/test/container_test.go @@ -132,7 +132,7 @@ func Test_loadImage(t *testing.T) { configuration := Configuration{ AbsoluteTileDirectory: absoluteTileDirectory, - ImagePath: `assets/alpine.tgz`, + ImagePath: "assets/alpine.tgz", } client := runTestWithSessionHelper(t, "", container.WaitResponse{