From 90e8b920994c741efb0f5a5ab5b9070b8cd47590 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Thu, 8 Aug 2024 15:34:39 +0330 Subject: [PATCH 1/9] merge main into it --- e2e/basic/basic_test.go | 2 +- e2e/executor.go | 2 +- e2e/netshaper/netshaper_test.go | 8 +- e2e/system/build_from_git_test.go | 4 +- e2e/system/env_to_json_test.go | 2 +- e2e/system/external_file_test.go | 2 +- e2e/system/file_test.go | 6 +- e2e/system/file_test_image_cached_test.go | 2 +- e2e/system/files_to_volumes_cm_test.go | 10 +- e2e/system/folder_test.go | 2 +- e2e/system/folder_test_image_cached_test.go | 2 +- e2e/system/start_callback_test.go | 2 +- e2e/tshark/tshark_test.go | 2 +- pkg/container/docker.go | 200 +++++++++----------- pkg/instance/build.go | 26 +-- pkg/instance/storage.go | 3 +- pkg/knuu/instance_old.go | 2 +- pkg/knuu/knuu.go | 2 +- pkg/sidecars/netshaper/netshaper.go | 2 +- pkg/sidecars/observability/obsy.go | 2 +- pkg/sidecars/tshark/tshark.go | 2 +- 21 files changed, 126 insertions(+), 159 deletions(-) diff --git a/e2e/basic/basic_test.go b/e2e/basic/basic_test.go index c36ee05b..07dcd057 100644 --- a/e2e/basic/basic_test.go +++ b/e2e/basic/basic_test.go @@ -22,7 +22,7 @@ func (ts *TestSuite) TestBasic() { ts.Require().NoError(target.Build().SetImage(ctx, testImage)) ts.Require().NoError(target.Build().SetStartCommand("sleep", "infinity")) - ts.Require().NoError(target.Build().Commit()) + ts.Require().NoError(target.Build().Commit(ctx)) ts.T().Cleanup(func() { if err := target.Execution().Destroy(ctx); err != nil { diff --git a/e2e/executor.go b/e2e/executor.go index a0796ba1..ef9b505e 100644 --- a/e2e/executor.go +++ b/e2e/executor.go @@ -34,7 +34,7 @@ func (e *Executor) NewInstance(ctx context.Context, name string) (*instance.Inst return nil, err } - if err := i.Build().Commit(); err != nil { + if err := i.Build().Commit(ctx); err != nil { return nil, err } diff --git a/e2e/netshaper/netshaper_test.go b/e2e/netshaper/netshaper_test.go index ded71ada..9bd08132 100644 --- a/e2e/netshaper/netshaper_test.go +++ b/e2e/netshaper/netshaper_test.go @@ -36,7 +36,7 @@ func (s *Suite) TestNetShaperBandwidth() { s.Require().NoError(iperfMother.Build().SetImage(ctx, iperfImage)) s.Require().NoError(iperfMother.Build().SetStartCommand("iperf3", "-s")) s.Require().NoError(iperfMother.Network().AddPortTCP(iperfPort)) - s.Require().NoError(iperfMother.Build().Commit()) + s.Require().NoError(iperfMother.Build().Commit(ctx)) iperfServer, err := iperfMother.CloneWithName("iperf-server") s.Require().NoError(err) @@ -142,7 +142,7 @@ func (s *Suite) TestNetShaperPacketloss() { s.Require().NoError(err) s.Require().NoError(mother.Network().AddPortTCP(gopingPort)) - s.Require().NoError(mother.Build().Commit()) + s.Require().NoError(mother.Build().Commit(ctx)) err = mother.Build().SetEnvironmentVariable("SERVE_ADDR", fmt.Sprintf("0.0.0.0:%d", gopingPort)) s.Require().NoError(err) @@ -245,7 +245,7 @@ func (s *Suite) TestNetShaperLatency() { s.Require().NoError(err) s.Require().NoError(mother.Network().AddPortTCP(gopingPort)) - s.Require().NoError(mother.Build().Commit()) + s.Require().NoError(mother.Build().Commit(ctx)) err = mother.Build().SetEnvironmentVariable("SERVE_ADDR", fmt.Sprintf("0.0.0.0:%d", gopingPort)) s.Require().NoError(err) @@ -355,7 +355,7 @@ func (s *Suite) TestNetShaperJitter() { s.Require().NoError(err) s.Require().NoError(mother.Network().AddPortTCP(gopingPort)) - s.Require().NoError(mother.Build().Commit()) + s.Require().NoError(mother.Build().Commit(ctx)) err = mother.Build().SetEnvironmentVariable("SERVE_ADDR", fmt.Sprintf("0.0.0.0:%d", gopingPort)) s.Require().NoError(err) diff --git a/e2e/system/build_from_git_test.go b/e2e/system/build_from_git_test.go index 4113e72f..3bbc9cd2 100644 --- a/e2e/system/build_from_git_test.go +++ b/e2e/system/build_from_git_test.go @@ -36,7 +36,7 @@ func (s *Suite) TestBuildFromGit() { } }) - s.Require().NoError(target.Build().Commit()) + s.Require().NoError(target.Build().Commit(ctx)) s.T().Logf("Starting instance") s.Require().NoError(target.Execution().Start(ctx)) @@ -74,7 +74,7 @@ func (s *Suite) TestBuildFromGitWithModifications() { err = target.Storage().AddFileBytes([]byte("Hello, world!"), "/home/hello.txt", "root:root") s.Require().NoError(err, "Error adding file") - s.Require().NoError(target.Build().Commit()) + s.Require().NoError(target.Build().Commit(ctx)) s.T().Cleanup(func() { if err := target.Execution().Destroy(ctx); err != nil { diff --git a/e2e/system/env_to_json_test.go b/e2e/system/env_to_json_test.go index 45a59fc8..e1608931 100644 --- a/e2e/system/env_to_json_test.go +++ b/e2e/system/env_to_json_test.go @@ -40,7 +40,7 @@ func (s *Suite) TestEnvToJSON() { name := fmt.Sprintf("%s-web%d", namePrefix, i+1) ins := s.createNginxInstance(ctx, name) - s.Require().NoError(ins.Build().Commit()) + s.Require().NoError(ins.Build().Commit(ctx)) s.Require().NoError(ins.Execution().Start(ctx)) _, err = ins.Execution().ExecuteCommand(ctx, "mkdir", "-p", nginxHTMLPath) diff --git a/e2e/system/external_file_test.go b/e2e/system/external_file_test.go index a7ef8e6d..bd5c48d7 100644 --- a/e2e/system/external_file_test.go +++ b/e2e/system/external_file_test.go @@ -41,7 +41,7 @@ func (s *Suite) TestExternalFile() { err = server.Storage().AddFile(htmlTmpPath, nginxHTMLPath+"/index.html", "0:0") s.Require().NoError(err) - s.Require().NoError(server.Build().Commit()) + s.Require().NoError(server.Build().Commit(ctx)) s.T().Cleanup(func() { err := instance.BatchDestroy(ctx, executor, server) diff --git a/e2e/system/file_test.go b/e2e/system/file_test.go index 9b24ac55..ca78415a 100644 --- a/e2e/system/file_test.go +++ b/e2e/system/file_test.go @@ -26,7 +26,7 @@ func (s *Suite) TestFile() { err = serverfile.Storage().AddFile(resourcesHTML+"/index.html", nginxHTMLPath+"/index.html", "0:0") s.Require().NoError(err) - s.Require().NoError(serverfile.Build().Commit()) + s.Require().NoError(serverfile.Build().Commit(ctx)) s.T().Cleanup(func() { err := instance.BatchDestroy(ctx, serverfile, executor) @@ -59,7 +59,7 @@ func (s *Suite) TestDownloadFileFromRunningInstance() { ctx := context.Background() s.Require().NoError(target.Build().SetImage(ctx, "alpine:latest")) s.Require().NoError(target.Build().SetArgs("tail", "-f", "/dev/null")) // Keep the container running - s.Require().NoError(target.Build().Commit()) + s.Require().NoError(target.Build().Commit(ctx)) s.Require().NoError(target.Execution().Start(ctx)) s.T().Cleanup(func() { @@ -98,7 +98,7 @@ func (s *Suite) TestMinio() { ctx := context.Background() s.Require().NoError(target.Build().SetImage(ctx, "alpine:latest")) s.Require().NoError(target.Build().SetArgs("tail", "-f", "/dev/null")) // Keep the container running - s.Require().NoError(target.Build().Commit()) + s.Require().NoError(target.Build().Commit(ctx)) s.Require().NoError(target.Execution().Start(ctx)) s.T().Cleanup(func() { diff --git a/e2e/system/file_test_image_cached_test.go b/e2e/system/file_test_image_cached_test.go index 51d375ad..4027413a 100644 --- a/e2e/system/file_test_image_cached_test.go +++ b/e2e/system/file_test_image_cached_test.go @@ -49,7 +49,7 @@ func (s *Suite) TestFileCached() { // Test logic for _, i := range instances { - s.Require().NoError(i.Build().Commit()) + s.Require().NoError(i.Build().Commit(ctx)) s.Require().NoError(i.Execution().StartAsync(ctx)) } diff --git a/e2e/system/files_to_volumes_cm_test.go b/e2e/system/files_to_volumes_cm_test.go index 86e574e0..83410f80 100644 --- a/e2e/system/files_to_volumes_cm_test.go +++ b/e2e/system/files_to_volumes_cm_test.go @@ -24,7 +24,7 @@ func (s *Suite) TestNoVolumesNoFiles() { s.Require().NoError(err) target := s.createNginxInstance(ctx, namePrefix+"-target") - s.Require().NoError(target.Build().Commit()) + s.Require().NoError(target.Build().Commit(ctx)) // Cleanup s.T().Cleanup(func() { @@ -65,7 +65,7 @@ func (s *Suite) TestOneVolumeNoFiles() { err = target.Storage().AddVolumeWithOwner("/opt/vol1", resource.MustParse("1Gi"), 0) s.Require().NoError(err) - s.Require().NoError(target.Build().Commit()) + s.Require().NoError(target.Build().Commit(ctx)) s.T().Cleanup(func() { err := instance.BatchDestroy(ctx, executor, target) @@ -134,7 +134,7 @@ func (s *Suite) TestNoVolumesOneFile() { // Test logic for _, i := range instances { - s.Require().NoError(i.Build().Commit()) + s.Require().NoError(i.Build().Commit(ctx)) s.Require().NoError(i.Execution().StartAsync(ctx)) } @@ -196,7 +196,7 @@ func (s *Suite) TestOneVolumeOneFile() { // Test logic for _, i := range instances { - s.Require().NoError(i.Build().Commit()) + s.Require().NoError(i.Build().Commit(ctx)) s.Require().NoError(i.Execution().StartAsync(ctx)) } @@ -260,7 +260,7 @@ func (s *Suite) TestOneVolumeTwoFiles() { // Test logic for _, i := range instances { - s.Require().NoError(i.Build().Commit()) + s.Require().NoError(i.Build().Commit(ctx)) s.Require().NoError(i.Execution().StartAsync(ctx)) } diff --git a/e2e/system/folder_test.go b/e2e/system/folder_test.go index 635af4fa..66d4c3b8 100644 --- a/e2e/system/folder_test.go +++ b/e2e/system/folder_test.go @@ -21,7 +21,7 @@ func (s *Suite) TestFolder() { err = web.Storage().AddFolder(resourcesHTML, nginxHTMLPath, "0:0") require.NoError(s.T(), err) - require.NoError(s.T(), web.Build().Commit()) + require.NoError(s.T(), web.Build().Commit(ctx)) s.T().Cleanup(func() { err := instance.BatchDestroy(ctx, web, executor) diff --git a/e2e/system/folder_test_image_cached_test.go b/e2e/system/folder_test_image_cached_test.go index e72cc39f..842e8d14 100644 --- a/e2e/system/folder_test_image_cached_test.go +++ b/e2e/system/folder_test_image_cached_test.go @@ -49,7 +49,7 @@ func (s *Suite) TestFolderCached() { // Test logic for _, i := range instances { - s.Require().NoError(i.Build().Commit()) + s.Require().NoError(i.Build().Commit(ctx)) s.Require().NoError(i.Execution().StartAsync(ctx)) } diff --git a/e2e/system/start_callback_test.go b/e2e/system/start_callback_test.go index 6b2ac430..6c4073b9 100644 --- a/e2e/system/start_callback_test.go +++ b/e2e/system/start_callback_test.go @@ -54,7 +54,7 @@ func TestStartWithCallback(t *testing.T) { } }) - require.NoError(t, target.Build().Commit()) + require.NoError(t, target.Build().Commit(ctx)) wg := sync.WaitGroup{} require.NoError(t, target.Execution().StartWithCallback(ctx, func() { diff --git a/e2e/tshark/tshark_test.go b/e2e/tshark/tshark_test.go index c177ea19..ccce5d00 100644 --- a/e2e/tshark/tshark_test.go +++ b/e2e/tshark/tshark_test.go @@ -82,7 +82,7 @@ func TestTshark(t *testing.T) { fileKey = filepath.Join(keyPrefix, filename) ) - require.NoError(t, target.Build().Commit()) + require.NoError(t, target.Build().Commit(ctx)) // Test logic diff --git a/pkg/container/docker.go b/pkg/container/docker.go index 06fcb625..8e962b71 100644 --- a/pkg/container/docker.go +++ b/pkg/container/docker.go @@ -2,52 +2,38 @@ 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" ) -const ( - DefaultTimeout = 2 * time.Minute -) - // BuilderFactory is responsible for creating new instances of buildah.Builder 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) } + + // TODO: remove this line + logrus.SetLevel(logrus.DebugLevel) + return &BuilderFactory{ imageNameFrom: imageName, - cli: cli, dockerFileInstructions: []string{"FROM " + imageName}, buildContext: buildContext, imageBuilder: imageBuilder, @@ -59,104 +45,101 @@ 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 runs the provided command in the context of the given 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 + // TODO: implement this using k8s builder + return nil, fmt.Errorf("not implemented") + + // 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. @@ -166,7 +149,7 @@ func (f *BuilderFactory) Changed() bool { // PushBuilderImage pushes the image from the given builder to a registry. // The image is identified by the provided name. -func (f *BuilderFactory) PushBuilderImage(imageName string) error { +func (f *BuilderFactory) PushBuilderImage(ctx context.Context, imageName string) error { if !f.Changed() { logrus.Debugf("No changes made to image %s, skipping push", f.imageNameFrom) return nil @@ -188,8 +171,10 @@ func (f *BuilderFactory) PushBuilderImage(imageName string) error { return ErrFailedToWriteDockerfile.Wrap(err) } - ctx, cancel := context.WithTimeout(context.Background(), DefaultTimeout) - defer cancel() + // TODO: remove these lines + fmt.Printf("f.buildContext: %v\n", f.buildContext) + fmt.Printf("dockerFile: `%v`\n", dockerFile) + logs, err := f.imageBuilder.Build(ctx, &builder.BuilderOptions{ ImageName: f.imageNameTo, Destination: f.imageNameTo, // in docker the image name and destination are the same @@ -246,17 +231,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 6faf0cc2..d0eac168 100644 --- a/pkg/instance/build.go +++ b/pkg/instance/build.go @@ -108,10 +108,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 } @@ -122,16 +119,14 @@ 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.Debugf("Set user '%s' for instance '%s'", user, b.instance.name) return nil } // Commit commits the instance // This function can only be called in the state 'Preparing' -func (b *build) Commit() error { +func (b *build) Commit(ctx context.Context) error { if !b.instance.IsState(StatePreparing) { return ErrCommittingNotAllowed.WithParams(b.instance.state.String()) } @@ -163,7 +158,7 @@ func (b *build) Commit() error { b.instance.Logger.Debugf("Using cached image for instance '%s'", b.instance.name) } else { b.instance.Logger.Debugf("Cannot use any cached image for instance '%s'", b.instance.name) - err = b.builderFactory.PushBuilderImage(imageName) + err = b.builderFactory.PushBuilderImage(ctx, imageName) if err != nil { return ErrPushingImage.WithParams(b.instance.name).Wrap(err) } @@ -196,14 +191,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 = dest + b.builderFactory.AddToBuilder(src, dest, chown) } // SetEnvironmentVariable sets the given environment variable in the instance @@ -214,7 +205,8 @@ func (b *build) SetEnvironmentVariable(key, value string) error { } b.instance.Logger.Debugf("Setting environment variable '%s' in instance '%s'", key, b.instance.name) if b.instance.state == StatePreparing { - return b.builderFactory.SetEnvVar(key, value) + b.builderFactory.SetEnvVar(key, value) + return nil } b.env[key] = value diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go index 89b70062..a0107dc5 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -46,7 +46,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) } diff --git a/pkg/knuu/instance_old.go b/pkg/knuu/instance_old.go index bb7e66b1..055f5b3d 100644 --- a/pkg/knuu/instance_old.go +++ b/pkg/knuu/instance_old.go @@ -138,7 +138,7 @@ func (i *Instance) SetUser(user string) error { // Deprecated: Use the new package knuu instead. func (i *Instance) Commit() error { - return i.Instance.Build().Commit() + return i.Instance.Build().Commit(context.Background()) } // Deprecated: Use the new package knuu instead. diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go index 7c1c1604..06ce179d 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -104,7 +104,7 @@ func (k *Knuu) handleTimeout(ctx context.Context) error { if err := inst.Build().SetImage(ctx, timeoutHandlerImage); err != nil { return ErrCannotSetImage.Wrap(err) } - if err := inst.Build().Commit(); err != nil { + if err := inst.Build().Commit(ctx); err != nil { return ErrCannotCommitInstance.Wrap(err) } diff --git a/pkg/sidecars/netshaper/netshaper.go b/pkg/sidecars/netshaper/netshaper.go index dbb33e71..3cb06231 100644 --- a/pkg/sidecars/netshaper/netshaper.go +++ b/pkg/sidecars/netshaper/netshaper.go @@ -58,7 +58,7 @@ func (bt *NetShaper) Initialize(ctx context.Context, sysDeps system.SystemDepend return ErrAddingBitTwisterPort.Wrap(err) } - if err := bt.instance.Build().Commit(); err != nil { + if err := bt.instance.Build().Commit(ctx); err != nil { return ErrCommittingBitTwisterInstance.Wrap(err) } diff --git a/pkg/sidecars/observability/obsy.go b/pkg/sidecars/observability/obsy.go index 369c85bc..f332d338 100644 --- a/pkg/sidecars/observability/obsy.go +++ b/pkg/sidecars/observability/obsy.go @@ -110,7 +110,7 @@ func (o *Obsy) Initialize(ctx context.Context, sysDeps system.SystemDependencies if err := o.instance.Resources().SetMemory(otelAgentMemory, otelAgentMemoryLimit); err != nil { return ErrSettingOtelAgentMemory.Wrap(err) } - if err := o.instance.Build().Commit(); err != nil { + if err := o.instance.Build().Commit(ctx); err != nil { return ErrCommittingOtelAgentInstance.Wrap(err) } diff --git a/pkg/sidecars/tshark/tshark.go b/pkg/sidecars/tshark/tshark.go index 45c14d45..c2aa8617 100644 --- a/pkg/sidecars/tshark/tshark.go +++ b/pkg/sidecars/tshark/tshark.go @@ -84,7 +84,7 @@ func (t *Tshark) Initialize(ctx context.Context, sysDeps system.SystemDependenci return ErrSettingTsharkCollectorImage.Wrap(err) } - if err := t.instance.Build().Commit(); err != nil { + if err := t.instance.Build().Commit(ctx); err != nil { return ErrCommittingTsharkCollectorInstance.Wrap(err) } From 6b7a027b2cda4c2fde2f924385995e8ab1bded24 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Fri, 9 Aug 2024 16:51:36 +0330 Subject: [PATCH 2/9] chore: refactored read file from image to use k8s instead of docker --- e2e/system/build_from_git_test.go | 1 - e2e/system/file_test.go | 34 +++++++++++++ pkg/container/docker.go | 85 ++----------------------------- pkg/instance/storage.go | 45 +++++++++++++++- 4 files changed, 83 insertions(+), 82 deletions(-) diff --git a/e2e/system/build_from_git_test.go b/e2e/system/build_from_git_test.go index 3bbc9cd2..1eb98d46 100644 --- a/e2e/system/build_from_git_test.go +++ b/e2e/system/build_from_git_test.go @@ -35,7 +35,6 @@ func (s *Suite) TestBuildFromGit() { s.T().Logf("Error cleaning up knuu: %v", err) } }) - s.Require().NoError(target.Build().Commit(ctx)) s.T().Logf("Starting instance") diff --git a/e2e/system/file_test.go b/e2e/system/file_test.go index ca78415a..8f211f2b 100644 --- a/e2e/system/file_test.go +++ b/e2e/system/file_test.go @@ -83,6 +83,40 @@ func (s *Suite) TestDownloadFileFromRunningInstance() { s.Assert().Equal(fileContent, string(gotContent)) } +func (s *Suite) TestDownloadFileFromBuilder() { + const namePrefix = "download-file-builder" + s.T().Parallel() + // 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.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 ( diff --git a/pkg/container/docker.go b/pkg/container/docker.go index fd67fb99..329a06b3 100644 --- a/pkg/container/docker.go +++ b/pkg/container/docker.go @@ -29,9 +29,6 @@ func NewBuilderFactory(imageName, buildContext string, imageBuilder builder.Buil return nil, ErrFailedToCreateContextDir.Wrap(err) } - // TODO: remove this line - logrus.SetLevel(logrus.DebugLevel) - return &BuilderFactory{ imageNameFrom: imageName, dockerFileInstructions: []string{"FROM " + imageName}, @@ -55,83 +52,6 @@ func (f *BuilderFactory) AddToBuilder(srcPath, destPath, chown string) { f.dockerFileInstructions = append(f.dockerFileInstructions, "ADD --chown="+chown+" "+srcPath+" "+destPath) } -// 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) { - // TODO: implement this using k8s builder - return nil, fmt.Errorf("not implemented") - - // 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) { f.dockerFileInstructions = append(f.dockerFileInstructions, "ENV "+name+"="+value) @@ -165,7 +85,12 @@ func (f *BuilderFactory) PushBuilderImage(ctx context.Context, imageName string) return ErrFailedToCreateContextDir.Wrap(err) } } + dockerFile := strings.Join(f.dockerFileInstructions, "\n") + + fmt.Printf("dockerFilePath: %v\n", dockerFilePath) + fmt.Printf("dockerFile: %v\n", dockerFile) + err := os.WriteFile(dockerFilePath, []byte(dockerFile), 0644) if err != nil { return ErrFailedToWriteDockerfile.Wrap(err) diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go index a0107dc5..28857569 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/celestiaorg/knuu/pkg/k8s" + "github.com/celestiaorg/knuu/pkg/names" "k8s.io/apimachinery/pkg/api/resource" ) @@ -164,7 +165,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) } @@ -346,6 +347,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 From 1ec6b061dd815af1b0dcffa84baac752521e9f09 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Fri, 9 Aug 2024 17:02:04 +0330 Subject: [PATCH 3/9] chore: cleanup --- pkg/container/docker.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/container/docker.go b/pkg/container/docker.go index 329a06b3..a32a07f8 100644 --- a/pkg/container/docker.go +++ b/pkg/container/docker.go @@ -87,10 +87,6 @@ func (f *BuilderFactory) PushBuilderImage(ctx context.Context, imageName string) } dockerFile := strings.Join(f.dockerFileInstructions, "\n") - - fmt.Printf("dockerFilePath: %v\n", dockerFilePath) - fmt.Printf("dockerFile: %v\n", dockerFile) - err := os.WriteFile(dockerFilePath, []byte(dockerFile), 0644) if err != nil { return ErrFailedToWriteDockerfile.Wrap(err) From 07b3ef2cfb2bef35f76bef022407132d25968c5d Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Fri, 9 Aug 2024 17:11:28 +0330 Subject: [PATCH 4/9] filx: linter complain --- pkg/instance/build.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/instance/build.go b/pkg/instance/build.go index d0eac168..11349c9f 100644 --- a/pkg/instance/build.go +++ b/pkg/instance/build.go @@ -193,8 +193,8 @@ func (b *build) getBuildDir() string { // addFileToBuilder adds a file to the builder 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 - src = dest - b.builderFactory.AddToBuilder(src, dest, chown) + _ = src + b.builderFactory.AddToBuilder(dest, dest, chown) } // SetEnvironmentVariable sets the given environment variable in the instance From 6a9c9b714cca6648f9f734e911249531a18e3f81 Mon Sep 17 00:00:00 2001 From: Moji Date: Fri, 16 Aug 2024 08:56:32 +0200 Subject: [PATCH 5/9] Update pkg/container/docker.go Co-authored-by: Matthew Sevey <15232757+MSevey@users.noreply.github.com> --- pkg/container/docker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/container/docker.go b/pkg/container/docker.go index a32a07f8..6457785e 100644 --- a/pkg/container/docker.go +++ b/pkg/container/docker.go @@ -42,7 +42,7 @@ func (f *BuilderFactory) ImageNameFrom() string { return f.imageNameFrom } -// AddCmdToBuilder runs the provided command in the context of the given builder. +// 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, " ")) } From 6af8c7b1a90eff4777ef84dde09e154fca5e99a6 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Wed, 4 Sep 2024 12:21:27 +0330 Subject: [PATCH 6/9] chore: merge fix conflicts --- pkg/instance/storage.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go index 2480313c..42d3d3ff 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -9,13 +9,11 @@ import ( "strconv" "strings" - "github.com/celestiaorg/knuu/pkg/k8s" - "github.com/celestiaorg/knuu/pkg/names" "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/api/resource" "github.com/celestiaorg/knuu/pkg/k8s" + "github.com/celestiaorg/knuu/pkg/names" ) type storage struct { From a2877db056e3787eb838a3514724f854bd9f5700 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Wed, 4 Sep 2024 17:49:22 +0330 Subject: [PATCH 7/9] fix: remove duplicate test parallel --- e2e/system/file_test.go | 10 +++------- e2e/system/suite_setup_test.go | 1 + 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/e2e/system/file_test.go b/e2e/system/file_test.go index e0dd263a..dcb20b5e 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)) @@ -102,14 +100,12 @@ func (s *Suite) TestDownloadFileFromRunningInstance() { } func (s *Suite) TestDownloadFileFromBuilder() { const namePrefix = "download-file-builder" - s.T().Parallel() - // 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.T().Cleanup(func() { if err := target.Execution().Destroy(ctx); err != nil { @@ -146,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 19f423cb..d851992f 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" ) From d7fce49eab683cabb83114ca7fd597640b4d900b Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Wed, 4 Sep 2024 18:18:57 +0330 Subject: [PATCH 8/9] fix: set state after building image --- pkg/instance/build.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/instance/build.go b/pkg/instance/build.go index 0d2f0850..b5aa68be 100644 --- a/pkg/instance/build.go +++ b/pkg/instance/build.go @@ -87,9 +87,12 @@ func (b *build) SetGitRepo(ctx context.Context, gitContext builder.GitContext) e } b.builderFactory = factory b.imageName = imageName - b.instance.SetState(StatePreparing) - return b.builderFactory.BuildImageFromGitRepo(ctx, gitContext, imageName) + if err := b.builderFactory.BuildImageFromGitRepo(ctx, gitContext, imageName); err != nil { + return err + } + b.instance.SetState(StatePreparing) + return nil } // SetStartCommand sets the command to run in the instance From 87dd4d617dbabbd6f1bd682f13f7e88af60f6aa3 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Wed, 4 Sep 2024 18:20:43 +0330 Subject: [PATCH 9/9] fix: put the set git repo out of retry loop --- e2e/system/build_from_git_test.go | 14 ++++++-------- pkg/instance/build.go | 7 ++----- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/e2e/system/build_from_git_test.go b/e2e/system/build_from_git_test.go index a2fcb4d3..add4297b 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/pkg/instance/build.go b/pkg/instance/build.go index b5aa68be..0d2f0850 100644 --- a/pkg/instance/build.go +++ b/pkg/instance/build.go @@ -87,12 +87,9 @@ func (b *build) SetGitRepo(ctx context.Context, gitContext builder.GitContext) e } b.builderFactory = factory b.imageName = imageName - - if err := b.builderFactory.BuildImageFromGitRepo(ctx, gitContext, imageName); err != nil { - return err - } b.instance.SetState(StatePreparing) - return nil + + return b.builderFactory.BuildImageFromGitRepo(ctx, gitContext, imageName) } // SetStartCommand sets the command to run in the instance