make k8s the default builder (#541)

* merge main into it * chore: refactored read file from image to use k8s instead of docker * chore: cleanup * filx: linter complain * Update pkg/container/docker.go Co-authored-by: Matthew Sevey <[email protected]> * chore: merge fix conflicts * fix: remove duplicate test parallel * fix: set state after building image * fix: put the set git repo out of retry loop --------- Co-authored-by: Matthew Sevey <[email protected]>
celestiaorg · Sep 5, 2024 · 60ca33c · 60ca33c
1 parent 224383d
commit 60ca33c
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 140 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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