Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add files to image issue #596

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions e2e/system/build_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ func (s *Suite) TestBuildFromGit() {
// Setup
ctx := context.Background()

s.T().Log("Creating new instance")
target, err := s.Knuu.NewInstance(namePrefix)
s.Require().NoError(err, "Error creating new instance")

Expand Down Expand Up @@ -61,7 +60,6 @@ func (s *Suite) TestBuildFromGitWithModifications() {
// Setup
ctx := context.Background()

s.T().Log("Creating new instance")
target, err := s.Knuu.NewInstance(namePrefix)
s.Require().NoError(err)

Expand All @@ -79,7 +77,7 @@ func (s *Suite) TestBuildFromGitWithModifications() {
expectedData = "Hello, world!"
)

err = target.Storage().AddFileBytes([]byte(expectedData), filePath, "root:root")
err = target.Storage().AddFileBytes([]byte(expectedData), filePath, "0:0")
s.Require().NoError(err)

s.Require().NoError(target.Build().Commit(ctx))
Expand Down
66 changes: 64 additions & 2 deletions pkg/instance/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,16 @@ import (
"github.com/celestiaorg/knuu/pkg/container"
)

const (
baseInitImageName = "nicolaka/netshoot"
)

type build struct {
instance *Instance
imageName string
imagePullPolicy v1.PullPolicy
initImageName string
builderFactory *container.BuilderFactory
imagePullPolicy v1.PullPolicy
command []string
args []string
env map[string]string
Expand Down Expand Up @@ -160,6 +165,56 @@ func (b *build) SetUser(user string) error {
return nil
}

// buildInitImage builds the init image for the instance
// This function can only be called in the state 'Preparing'
func (b *build) buildInitImage(ctx context.Context) error {
if !b.instance.IsState(StatePreparing) {
return ErrBuildingInitImageNotAllowed.WithParams(b.instance.state.String())
}

buildDir, err := b.getBuildDir()
if err != nil {
return ErrGettingBuildDir.Wrap(err)
}

factory, err := container.NewBuilderFactory(container.BuilderFactoryOptions{
ImageName: baseInitImageName,
BuildContext: buildDir,
ImageBuilder: b.instance.ImageBuilder,
Logger: b.instance.Logger,
})
if err != nil {
return ErrCreatingBuilder.Wrap(err)
}

mustBuild := false
for _, vol := range b.instance.storage.volumes {
for _, f := range vol.Files() {
// the files are copied to the build dir with the subfolder structure of dest
factory.AddToBuilder(f.Dest, f.Dest, f.Chown)
mustBuild = true
}
}

if !mustBuild {
return nil
}

imageHash, err := factory.GenerateImageHash()
if err != nil {
return ErrGeneratingImageHash.Wrap(err)
}

// TODO: update this when the custom registry PR is merged (#593)
imageName, err := getImageRegistry(imageHash)
if err != nil {
return ErrGettingImageRegistry.Wrap(err)
}

b.initImageName = imageName
return factory.PushBuilderImage(ctx, imageName)
}
mojtaba-esk marked this conversation as resolved.
Show resolved Hide resolved

func (b *build) SetNodeSelector(nodeSelector map[string]string) error {
if !b.instance.IsInState(StatePreparing, StateCommitted) {
return ErrSettingNodeSelectorNotAllowed.WithParams(b.instance.state.String())
Expand All @@ -176,6 +231,12 @@ func (b *build) Commit(ctx context.Context) error {
return ErrCommittingNotAllowed.WithParams(b.instance.state.String())
}

if len(b.instance.storage.volumes) > 0 {
if err := b.buildInitImage(ctx); err != nil {
return err
}
}

if !b.builderFactory.Changed() {
b.imageName = b.builderFactory.ImageNameFrom()
b.instance.Logger.WithFields(logrus.Fields{
Expand Down Expand Up @@ -254,7 +315,8 @@ func (b *build) getBuildDir() (string, error) {
if err != nil {
return "", err
}
return filepath.Join(tmpDir, b.instance.name), nil
b.buildDir = filepath.Join(tmpDir, b.instance.name)
return b.buildDir, nil
}

// addFileToBuilder adds a file to the builder
Expand Down
2 changes: 2 additions & 0 deletions pkg/instance/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,6 @@ var (
ErrFailedToGetFileSize = errors.New("FailedToGetFileSize", "failed to get file size")
ErrFileTooLargeCommitted = errors.New("FileTooLargeCommitted", "file '%s' is too large (max 1MiB) to add after instance is committed")
ErrTotalFilesSizeTooLarge = errors.New("TotalFilesSizeTooLarge", "total files size is too large (max 1MiB)")
ErrFileAlreadyExistsInTheVolumePath = errors.New("FileAlreadyExistsInTheVolumePath", "file %s already exists in the volume path %s")
ErrBuildingInitImageNotAllowed = errors.New("BuildingInitImageNotAllowed", "building init image is only allowed in state 'Preparing'. Current state is '%s'")
)
1 change: 1 addition & 0 deletions pkg/instance/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ func (e *execution) prepareReplicaSetConfig() k8s.ReplicaSetConfig {
SecurityContext: e.instance.security.prepareSecurityContext(),
TCPPorts: e.instance.network.portsTCP,
UDPPorts: e.instance.network.portsUDP,
InitImageName: e.instance.build.initImageName,
}

sidecarConfigs := make([]k8s.ContainerConfig, 0)
Expand Down
90 changes: 63 additions & 27 deletions pkg/instance/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,32 +49,27 @@ func (s *storage) AddFile(src string, dest string, chown string) error {
return err
}

switch s.instance.state {
case StatePreparing:
s.instance.build.addFileToBuilder(src, dest, chown)
return nil
case StateCommitted, StateStopped:
srcInfo, err := os.Stat(src)
if s.instance.IsState(StatePreparing) {
volume, err := s.findMatchingVolume(dest)
if err != nil {
return ErrFailedToGetFileSize.Wrap(err)
return err
}
if srcInfo.Size() > maxTotalFilesBytes {
return ErrFileTooLargeCommitted.WithParams(src)
if volume != nil {
return volume.AddFile(s.instance.K8sClient.NewFile(src, dest, chown, ""))
}
return s.addFileToInstance(buildDirPath, dest, chown)

s.instance.build.addFileToBuilder(src, dest, chown)
return nil
mojtaba-esk marked this conversation as resolved.
Show resolved Hide resolved
}

buildDir, err := s.instance.build.getBuildDir()
srcInfo, err := os.Stat(src)
if err != nil {
return ErrGettingBuildDir.Wrap(err)
return ErrFailedToGetFileSize.Wrap(err)
}
s.instance.Logger.WithFields(logrus.Fields{
"file": dest,
"instance": s.instance.name,
"state": s.instance.state,
"build_dir": buildDir,
}).Debug("added file")
return nil
if srcInfo.Size() > maxTotalFilesBytes {
return ErrFileTooLargeCommitted.WithParams(src)
}
return s.addFileToInstance(buildDirPath, dest, chown)
}

// AddFolder adds a folder to the instance
Expand Down Expand Up @@ -168,14 +163,6 @@ func (s *storage) AddFileBytes(bytes []byte, dest string, chown string) error {
// The owner of the volume is set to 0, if you want to set a custom owner use AddVolumeWithOwner
// This function can only be called in the states 'Preparing', 'Committed' and 'Stopped'
func (s *storage) AddVolume(path string, size resource.Quantity) error {
// temporary feat, we will remove it once we can add multiple volumes
if len(s.volumes) > 0 {
s.instance.Logger.WithFields(logrus.Fields{
"instance": s.instance.name,
"volumes": len(s.volumes),
}).Debug("maximum volumes exceeded")
return ErrMaximumVolumesExceeded.WithParams(s.instance.name)
}
return s.AddVolumeWithOwner(path, size, 0)
}

Expand All @@ -193,6 +180,17 @@ func (s *storage) AddVolumeWithOwner(path string, size resource.Quantity, owner
}).Debug("maximum volumes exceeded")
return ErrMaximumVolumesExceeded.WithParams(s.instance.name)
}

// When already a file exists in the volume path, then we cannot add a volume
// b/c the volume must be added first to avoid user error
file, err := s.findMatchingFile(path)
if err != nil {
return err
}
if file != nil {
return ErrFileAlreadyExistsInTheVolumePath.WithParams(file.Dest, path)
}

volume := s.instance.K8sClient.NewVolume(path, size, owner)
s.volumes = append(s.volumes, volume)
s.instance.Logger.WithFields(logrus.Fields{
Expand Down Expand Up @@ -497,3 +495,41 @@ func (s *storage) clone() *storage {
files: filesCopy,
}
}

func (s *storage) findMatchingVolume(filePath string) (*k8s.Volume, error) {
for _, v := range s.volumes {
ok, err := isSubpath(v.Path, filePath)
if err != nil {
return nil, err
}
if ok {
return v, nil
}
}
return nil, nil // no matching volume found
}

func (s *storage) findMatchingFile(volumePath string) (*k8s.File, error) {
for _, f := range s.files {
ok, err := isSubpath(volumePath, f.Dest)
if err != nil {
return nil, err
}
if ok {
return f, nil
}
}
return nil, nil // no matching file found
}

func isSubpath(base, target string) (bool, error) {
base = filepath.Clean(base)
target = filepath.Clean(target)

rel, err := filepath.Rel(base, target)
if err != nil {
return false, err
}

return !strings.HasPrefix(rel, ".."), nil
}
1 change: 1 addition & 0 deletions pkg/k8s/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,5 @@ var (
ErrNoPortsFoundForService = errors.New("NoPortsFoundForService", "no ports found for service %s")
ErrNoValidNodeIPFound = errors.New("NoValidNodeIPFound", "no valid node IP found for service %s")
ErrInvalidClusterDomain = errors.New("InvalidClusterDomain", "invalid cluster domain `%s`")
ErrDestNotSubpath = errors.New("DestNotSubpath", "destination %s is not a subpath of volume path %s")
)
37 changes: 5 additions & 32 deletions pkg/k8s/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const (
type ContainerConfig struct {
Name string // Name to assign to the Container
Image string // Name of the container image to use for the container
InitImageName string // InitImageName to use for the Pod
ImagePullPolicy v1.PullPolicy // Image pull policy for the container
Command []string // Command to run in the container
Args []string // Arguments to pass to the command in the container
Expand Down Expand Up @@ -65,19 +66,6 @@ type PodConfig struct {
NodeSelector map[string]string // NodeSelector to apply to the Pod
}

type Volume struct {
Path string
Size resource.Quantity
Owner int64
}

type File struct {
Source string
Dest string
Chown string
Permission string
}

// DeployPod creates a new pod in the namespace that k8s client is initiate with if it doesn't already exist.
func (c *Client) DeployPod(ctx context.Context, podConfig PodConfig, init bool) (*v1.Pod, error) {
if c.terminated {
Expand All @@ -96,23 +84,6 @@ func (c *Client) DeployPod(ctx context.Context, podConfig PodConfig, init bool)
return createdPod, nil
}

func (c *Client) NewVolume(path string, size resource.Quantity, owner int64) *Volume {
return &Volume{
Path: path,
Size: size,
Owner: owner,
}
}

func (c *Client) NewFile(source, dest, chown, permission string) *File {
return &File{
Source: source,
Dest: dest,
Chown: chown,
Permission: permission,
}
}

func (c *Client) ReplacePodWithGracePeriod(ctx context.Context, podConfig PodConfig, gracePeriod *int64) (*v1.Pod, error) {
c.logger.WithField("name", podConfig.Name).Debug("replacing pod")

Expand Down Expand Up @@ -596,14 +567,16 @@ func prepareContainer(config ContainerConfig) v1.Container {

// prepareInitContainers creates a slice of v1.Container as init containers.
func (c *Client) prepareInitContainers(config ContainerConfig, init bool) []v1.Container {
if !init || (len(config.Volumes) == 0 && len(config.Files) == 0) {
if !init ||
(len(config.Volumes) == 0 && len(config.Files) == 0) ||
config.InitImageName == "" {
return nil
}

return []v1.Container{
{
Name: config.Name + initContainerNameSuffix,
Image: initContainerImage,
Image: config.InitImageName,
SecurityContext: &v1.SecurityContext{
RunAsUser: ptr.To[int64](defaultContainerUser),
},
Expand Down
Loading
Loading