diff --git a/e2e/system/build_from_git_test.go b/e2e/system/build_from_git_test.go index a2fcb4d..add4297 100644 --- a/e2e/system/build_from_git_test.go +++ b/e2e/system/build_from_git_test.go @@ -65,14 +65,12 @@ func (s *Suite) TestBuildFromGitWithModifications() { s.Require().NoError(err) s.T().Log("Setting git repo") - err = s.RetryOperation(func() error { - return target.Build().SetGitRepo(ctx, builder.GitContext{ - Repo: gitRepo, - Branch: gitBranch, - Username: "", - Password: "", - }) - }, maxRetries) + err = target.Build().SetGitRepo(ctx, builder.GitContext{ + Repo: gitRepo, + Branch: gitBranch, + Username: "", + Password: "", + }) s.Require().NoError(err) s.Require().NoError(target.Build().SetStartCommand("sleep", "infinity")) diff --git a/e2e/system/file_test.go b/e2e/system/file_test.go index 7ea5f37..dcb20b5 100644 --- a/e2e/system/file_test.go +++ b/e2e/system/file_test.go @@ -74,13 +74,11 @@ func (s *Suite) TestDownloadFileFromRunningInstance() { namePrefix = "download-file-running" ) - // Setup - target, err := s.Knuu.NewInstance(namePrefix + "-target") s.Require().NoError(err) ctx := context.Background() - s.Require().NoError(target.Build().SetImage(ctx, "alpine:latest")) + s.Require().NoError(target.Build().SetImage(ctx, alpineImage)) s.Require().NoError(target.Build().SetArgs("tail", "-f", "/dev/null")) // Keep the container running s.Require().NoError(target.Build().Commit(ctx)) s.Require().NoError(target.Execution().Start(ctx)) @@ -100,6 +98,38 @@ func (s *Suite) TestDownloadFileFromRunningInstance() { s.Assert().Equal(fileContent, string(gotContent)) } +func (s *Suite) TestDownloadFileFromBuilder() { + const namePrefix = "download-file-builder" + + target, err := s.Knuu.NewInstance(namePrefix + "-target") + s.Require().NoError(err) + + ctx := context.Background() + s.Require().NoError(target.Build().SetImage(ctx, alpineImage)) + + s.T().Cleanup(func() { + if err := target.Execution().Destroy(ctx); err != nil { + s.T().Logf("error destroying instance: %v", err) + } + }) + + // Test logic + const ( + fileContent = "Hello World!" + filePath = "/hello.txt" + ) + + s.Require().NoError(target.Storage().AddFileBytes([]byte(fileContent), filePath, "0:0")) + + // The commit is required to make the changes persistent to the image + s.Require().NoError(target.Build().Commit(ctx)) + + // Now test if the file can be downloaded correctly from the built image + gotContent, err := target.Storage().GetFileBytes(ctx, filePath) + s.Require().NoError(err, "Error getting file bytes") + + s.Assert().Equal(fileContent, string(gotContent)) +} func (s *Suite) TestMinio() { const ( @@ -112,7 +142,7 @@ func (s *Suite) TestMinio() { s.Require().NoError(err) ctx := context.Background() - s.Require().NoError(target.Build().SetImage(ctx, "alpine:latest")) + s.Require().NoError(target.Build().SetImage(ctx, alpineImage)) s.Require().NoError(target.Build().SetArgs("tail", "-f", "/dev/null")) // Keep the container running s.Require().NoError(target.Build().Commit(ctx)) s.Require().NoError(target.Execution().Start(ctx)) diff --git a/e2e/system/suite_setup_test.go b/e2e/system/suite_setup_test.go index 19f423c..d851992 100644 --- a/e2e/system/suite_setup_test.go +++ b/e2e/system/suite_setup_test.go @@ -17,6 +17,7 @@ import ( const ( testTimeout = time.Minute * 15 // the same time that is used in the ci/cd pipeline + alpineImage = "alpine:latest" resourcesHTML = "resources/html" resourcesFileCMToFolder = "resources/file_cm_to_folder" ) diff --git a/pkg/container/docker.go b/pkg/container/docker.go index 43274e5..6457785 100644 --- a/pkg/container/docker.go +++ b/pkg/container/docker.go @@ -2,20 +2,13 @@ package container import ( - "archive/tar" - "bytes" "context" "crypto/sha256" "fmt" - "io" "os" - "os/exec" "path/filepath" "strings" - "time" - "github.com/docker/docker/api/types/container" - "github.com/docker/docker/client" "github.com/sirupsen/logrus" "github.com/celestiaorg/knuu/pkg/builder" @@ -26,24 +19,18 @@ type BuilderFactory struct { imageNameFrom string imageNameTo string imageBuilder builder.Builder - cli *client.Client dockerFileInstructions []string buildContext string } // NewBuilderFactory creates a new instance of BuilderFactory. func NewBuilderFactory(imageName, buildContext string, imageBuilder builder.Builder) (*BuilderFactory, error) { - cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) - if err != nil { - return nil, ErrCreatingDockerClient.Wrap(err) - } - err = os.MkdirAll(buildContext, 0755) - if err != nil { + if err := os.MkdirAll(buildContext, 0755); err != nil { return nil, ErrFailedToCreateContextDir.Wrap(err) } + return &BuilderFactory{ imageNameFrom: imageName, - cli: cli, dockerFileInstructions: []string{"FROM " + imageName}, buildContext: buildContext, imageBuilder: imageBuilder, @@ -55,104 +42,24 @@ func (f *BuilderFactory) ImageNameFrom() string { return f.imageNameFrom } -// ExecuteCmdInBuilder runs the provided command in the context of the given builder. -// It returns the command's output or any error encountered. -func (f *BuilderFactory) ExecuteCmdInBuilder(command []string) (string, error) { +// AddCmdToBuilder adds the provided command to be run in the context of the builder. +func (f *BuilderFactory) AddCmdToBuilder(command []string) { f.dockerFileInstructions = append(f.dockerFileInstructions, "RUN "+strings.Join(command, " ")) - // FIXME: does not return expected output - return "", nil } // AddToBuilder adds a file from the source path to the destination path in the image, with the specified ownership. -func (f *BuilderFactory) AddToBuilder(srcPath, destPath, chown string) error { +func (f *BuilderFactory) AddToBuilder(srcPath, destPath, chown string) { f.dockerFileInstructions = append(f.dockerFileInstructions, "ADD --chown="+chown+" "+srcPath+" "+destPath) - return nil -} - -// ReadFileFromBuilder reads a file from the given builder's mount point. -// It returns the file's content or any error encountered. -func (f *BuilderFactory) ReadFileFromBuilder(filePath string) ([]byte, error) { - if f.imageNameTo == "" { - return nil, ErrNoImageNameProvided - } - containerConfig := &container.Config{ - Image: f.imageNameTo, - Cmd: []string{"tail", "-f", "/dev/null"}, // This keeps the container running - } - resp, err := f.cli.ContainerCreate( - context.Background(), - containerConfig, - nil, - nil, - nil, - "", - ) - if err != nil { - return nil, ErrFailedToCreateContainer.Wrap(err) - } - - defer func() { - // Stop the container - timeout := int(time.Duration(10) * time.Second) - stopOptions := container.StopOptions{ - Timeout: &timeout, - } - - if err := f.cli.ContainerStop(context.Background(), resp.ID, stopOptions); err != nil { - logrus.Warn(ErrFailedToStopContainer.Wrap(err)) - } - - // Remove the container - if err := f.cli.ContainerRemove(context.Background(), resp.ID, container.RemoveOptions{}); err != nil { - logrus.Warn(ErrFailedToRemoveContainer.Wrap(err)) - } - }() - - if err := f.cli.ContainerStart(context.Background(), resp.ID, container.StartOptions{}); err != nil { - return nil, ErrFailedToStartContainer.Wrap(err) - } - - // Now you can copy the file - reader, _, err := f.cli.CopyFromContainer(context.Background(), resp.ID, filePath) - if err != nil { - return nil, ErrFailedToCopyFileFromContainer.Wrap(err) - } - defer reader.Close() - - tarReader := tar.NewReader(reader) - - for { - header, err := tarReader.Next() - - if err == io.EOF { - break // End of archive - } - if err != nil { - return nil, ErrFailedToReadFromTar.Wrap(err) - } - - if header.Typeflag == tar.TypeReg { // if it's a file then extract it - data, err := io.ReadAll(tarReader) - if err != nil { - return nil, ErrFailedToReadFileFromTar.Wrap(err) - } - return data, nil - } - } - - return nil, ErrFileNotFoundInTar } // SetEnvVar sets the value of an environment variable in the builder. -func (f *BuilderFactory) SetEnvVar(name, value string) error { +func (f *BuilderFactory) SetEnvVar(name, value string) { f.dockerFileInstructions = append(f.dockerFileInstructions, "ENV "+name+"="+value) - return nil } // SetUser sets the user in the builder. -func (f *BuilderFactory) SetUser(user string) error { +func (f *BuilderFactory) SetUser(user string) { f.dockerFileInstructions = append(f.dockerFileInstructions, "USER "+user) - return nil } // Changed returns true if the builder has been modified, false otherwise. @@ -178,6 +85,7 @@ func (f *BuilderFactory) PushBuilderImage(ctx context.Context, imageName string) return ErrFailedToCreateContextDir.Wrap(err) } } + dockerFile := strings.Join(f.dockerFileInstructions, "\n") err := os.WriteFile(dockerFilePath, []byte(dockerFile), 0644) if err != nil { @@ -240,17 +148,6 @@ func (f *BuilderFactory) BuildImageFromGitRepo(ctx context.Context, gitCtx build return err } -func runCommand(cmd *exec.Cmd) error { // nolint: unused - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - err := cmd.Run() - if err != nil { - return fmt.Errorf("command failed: %w\nstdout: %s\nstderr: %s", err, stdout.String(), stderr.String()) - } - return nil -} - // GenerateImageHash creates a hash value based on the contents of the Dockerfile instructions and all files in the build context. func (f *BuilderFactory) GenerateImageHash() (string, error) { hasher := sha256.New() diff --git a/pkg/instance/build.go b/pkg/instance/build.go index a15b1ba..0d2f085 100644 --- a/pkg/instance/build.go +++ b/pkg/instance/build.go @@ -119,10 +119,7 @@ func (b *build) ExecuteCommand(command ...string) error { return ErrAddingCommandNotAllowed.WithParams(b.instance.state.String()) } - _, err := b.builderFactory.ExecuteCmdInBuilder(command) - if err != nil { - return ErrExecutingCommandInInstance.WithParams(command, b.instance.name).Wrap(err) - } + b.builderFactory.AddCmdToBuilder(command) return nil } @@ -133,9 +130,7 @@ func (b *build) SetUser(user string) error { return ErrSettingUserNotAllowed.WithParams(b.instance.state.String()) } - if err := b.builderFactory.SetUser(user); err != nil { - return ErrSettingUser.WithParams(user, b.instance.name).Wrap(err) - } + b.builderFactory.SetUser(user) b.instance.Logger.WithFields(logrus.Fields{ "instance": b.instance.name, "user": user, @@ -224,14 +219,10 @@ func (b *build) getBuildDir() string { } // addFileToBuilder adds a file to the builder -func (b *build) addFileToBuilder(src, dest, chown string) error { - _ = src +func (b *build) addFileToBuilder(src, dest, chown string) { // dest is the same as src here, as we copy the file to the build dir with the subfolder structure of dest - err := b.builderFactory.AddToBuilder(dest, dest, chown) - if err != nil { - return ErrAddingFileToInstance.WithParams(dest, b.instance.name).Wrap(err) - } - return nil + _ = src + b.builderFactory.AddToBuilder(dest, dest, chown) } // SetEnvironmentVariable sets the given environment variable in the instance @@ -247,7 +238,8 @@ func (b *build) SetEnvironmentVariable(key, value string) error { }).Debugf("Setting environment variable") if b.instance.state == StatePreparing { - return b.builderFactory.SetEnvVar(key, value) + b.builderFactory.SetEnvVar(key, value) + return nil } b.env[key] = value return nil diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go index 4cee01b..42d3d3f 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -13,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "github.com/celestiaorg/knuu/pkg/k8s" + "github.com/celestiaorg/knuu/pkg/names" ) type storage struct { @@ -47,7 +48,8 @@ func (s *storage) AddFile(src string, dest string, chown string) error { switch s.instance.state { case StatePreparing: - return s.instance.build.addFileToBuilder(src, dest, chown) + s.instance.build.addFileToBuilder(src, dest, chown) + return nil case StateCommitted: return s.addFileToInstance(dstPath, dest, chown) } @@ -185,7 +187,7 @@ func (s *storage) GetFileBytes(ctx context.Context, file string) ([]byte, error) } if s.instance.state != StateStarted { - bytes, err := s.instance.build.builderFactory.ReadFileFromBuilder(file) + bytes, err := s.readFileFromImage(ctx, file) if err != nil { return nil, ErrGettingFile.WithParams(file, s.instance.name).Wrap(err) } @@ -370,6 +372,48 @@ func (s *storage) destroyFiles(ctx context.Context) error { return nil } +func (s *storage) readFileFromImage(ctx context.Context, filePath string) ([]byte, error) { + // Another way to implement this is to download all the layers of the image and then + // extract the file from them, but it seems hacky and will run on the user's machine. + // Therefore, we will use the tmp instance to get the file from the image + + tmpName, err := names.NewRandomK8("tmp-dl") + if err != nil { + return nil, err + } + + ti, err := New(tmpName, s.instance.SystemDependencies) + if err != nil { + return nil, err + } + if err := ti.build.SetImage(ctx, s.instance.build.ImageName()); err != nil { + return nil, err + } + + if err := ti.build.SetStartCommand("sleep", "infinity"); err != nil { + return nil, err + } + + if err := ti.build.Commit(ctx); err != nil { + return nil, err + } + + if err := ti.execution.Start(ctx); err != nil { + return nil, err + } + defer func() { + if err := ti.execution.Destroy(ctx); err != nil { + ti.Logger.Errorf("failed to destroy tmp instance %s: %v", ti.k8sName, err) + } + }() + + output, err := ti.execution.ExecuteCommand(ctx, "cat", filePath) + if err != nil { + return nil, err + } + return []byte(output), nil +} + func (s *storage) clone() *storage { if s == nil { return nil