Skip to content

Commit

Permalink
fix(build, docker, dockerfile): remove ONBUILD support in base images…
Browse files Browse the repository at this point in the history
… to reduce overhead

Drop ONBUILD support from base images due to minimal usage among target users and significant overhead for all users.

Maintaining ONBUILD support comes with high costs, including:
- A registry request is made at each build, causing performance issues.
- There is no guarantee that the retrieved manifest matches the previously built image, as it may have been updated.

Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
  • Loading branch information
alexey-igrychev committed Oct 17, 2024
1 parent 3155a4f commit 6209ac0
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 125 deletions.
133 changes: 11 additions & 122 deletions pkg/build/stage/full_dockerfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"os"
"path/filepath"
"strconv"
"strings"

"github.com/moby/buildkit/frontend/dockerfile/instructions"
"github.com/moby/buildkit/frontend/dockerfile/parser"
Expand Down Expand Up @@ -105,19 +104,16 @@ type DockerStages struct {
dockerMetaArgs []instructions.ArgCommand
dockerStageArgsHash map[int]map[string]string
dockerStageEnvs map[int]map[string]string

imageOnBuildInstructions map[string][]string
}

func NewDockerStages(dockerStages []instructions.Stage, dockerBuildArgsHash map[string]string, dockerMetaArgs []instructions.ArgCommand, dockerTargetStageIndex int) *DockerStages {
ds := &DockerStages{
dockerStages: dockerStages,
dockerTargetStageIndex: dockerTargetStageIndex,
dockerBuildArgsHash: dockerBuildArgsHash,
dockerMetaArgs: dockerMetaArgs,
dockerStageArgsHash: map[int]map[string]string{},
dockerStageEnvs: map[int]map[string]string{},
imageOnBuildInstructions: map[string][]string{},
dockerStages: dockerStages,
dockerTargetStageIndex: dockerTargetStageIndex,
dockerBuildArgsHash: dockerBuildArgsHash,
dockerMetaArgs: dockerMetaArgs,
dockerStageArgsHash: map[int]map[string]string{},
dockerStageEnvs: map[int]map[string]string{},
}

return ds
Expand Down Expand Up @@ -295,109 +291,11 @@ type dockerfileInstructionInterface interface {
Name() string
}

func (s *FullDockerfileStage) FetchDependencies(ctx context.Context, c Conveyor, containerBackend container_backend.ContainerBackend, dockerRegistry docker_registry.GenericApiInterface) error {
resolvedDependenciesArgsHash := ResolveDependenciesArgs(s.targetPlatform, s.dependencies, c)

var resolvedDockerMetaArgsHash map[string]string
{
metaArgs, err := s.resolveDockerMetaArgs(resolvedDependenciesArgsHash)
if err != nil {
return fmt.Errorf("unable to resolve docker meta args: %w", err)
}

platformMetaArgs, err := platformutil.GetPlatformMetaArgsMap(s.targetPlatform)
if err != nil {
return fmt.Errorf("unable to get platform args: %w", err)
}

resolvedDockerMetaArgsHash = util.MergeMaps(platformMetaArgs, metaArgs)
}

outerLoop:
for ind, stage := range s.dockerStages {
resolvedBaseName, err := s.ShlexProcessWordWithMetaArgs(stage.BaseName, resolvedDockerMetaArgsHash)
if err != nil {
return err
}

if resolvedBaseName == "" {
return ErrInvalidBaseImage
}

for relatedStageIndex, relatedStage := range s.dockerStages {
if ind == relatedStageIndex {
continue
}

if resolvedBaseName == relatedStage.Name {
continue outerLoop
}
}

_, ok := s.imageOnBuildInstructions[resolvedBaseName]
if ok || resolvedBaseName == "scratch" {
continue
}

getBaseImageOnBuildLocally := func() ([]string, error) {
info, err := containerBackend.GetImageInfo(ctx, resolvedBaseName, container_backend.GetImageInfoOpts{})
if err != nil {
return nil, err
}

if info == nil {
return nil, errImageNotExistLocally
}

return info.OnBuild, nil
}

getBaseImageOnBuildRemotely := func() ([]string, error) {
configFile, err := dockerRegistry.GetRepoImageConfigFile(ctx, resolvedBaseName)
if err != nil {
return nil, fmt.Errorf("get repo image %q config file failed: %w", resolvedBaseName, err)
}

return configFile.Config.OnBuild, nil
}

var onBuild []string
if onBuild, err = getBaseImageOnBuildLocally(); err != nil && err != errImageNotExistLocally {
return err
} else if err == errImageNotExistLocally {
var getRemotelyErr error
if onBuild, getRemotelyErr = getBaseImageOnBuildRemotely(); getRemotelyErr != nil {
if isUnsupportedMediaTypeError(getRemotelyErr) {
logboek.Context(ctx).Warn().LogF("WARNING: Could not get base image manifest from local docker and from docker registry: %s\n", getRemotelyErr)
logboek.Context(ctx).Warn().LogLn("WARNING: The base image pulling is necessary for calculating digest of image correctly\n")
if err := logboek.Context(ctx).Default().LogProcess("Pulling base image %s", resolvedBaseName).DoError(func() error {
return containerBackend.Pull(ctx, resolvedBaseName, container_backend.PullOpts{})
}); err != nil {
return err
}

if onBuild, err = getBaseImageOnBuildLocally(); err != nil {
return err
}
} else {
return getRemotelyErr
}
}
}

s.imageOnBuildInstructions[resolvedBaseName] = onBuild
}

func (s *FullDockerfileStage) FetchDependencies(_ context.Context, _ Conveyor, _ container_backend.ContainerBackend, _ docker_registry.GenericApiInterface) error {
return nil
}

func isUnsupportedMediaTypeError(err error) bool {
return strings.Contains(err.Error(), "unsupported MediaType")
}

var errImageNotExistLocally = errors.New("IMAGE_NOT_EXIST_LOCALLY")

func (s *FullDockerfileStage) GetDependencies(ctx context.Context, c Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) {
func (s *FullDockerfileStage) GetDependencies(ctx context.Context, c Conveyor, _ container_backend.ContainerBackend, _, _ *StageImage, _ container_backend.BuildContextArchiver) (string, error) {
resolvedDependenciesArgsHash := ResolveDependenciesArgs(s.targetPlatform, s.dependencies, c)

var resolvedDockerMetaArgsHash map[string]string
Expand Down Expand Up @@ -429,20 +327,11 @@ func (s *FullDockerfileStage) GetDependencies(ctx context.Context, c Conveyor, c
return "", err
}

dependencies = append(dependencies, resolvedBaseName)

onBuildInstructions, ok := s.imageOnBuildInstructions[resolvedBaseName]
if ok {
for _, instruction := range onBuildInstructions {
_, iOnBuildDependencies, err := s.dockerfileOnBuildInstructionDependencies(ctx, c.GiterminismManager(), resolvedDockerMetaArgsHash, resolvedDependenciesArgsHash, ind, instruction, true)
if err != nil {
return "", err
}

dependencies = append(dependencies, iOnBuildDependencies...)
}
if resolvedBaseName == "" {
return "", ErrInvalidBaseImage
}

dependencies = append(dependencies, resolvedBaseName)
for _, cmd := range stage.Commands {
cmdDependencies, cmdOnBuildDependencies, err := s.dockerfileInstructionDependencies(ctx, c.GiterminismManager(), resolvedDockerMetaArgsHash, resolvedDependenciesArgsHash, ind, cmd, false, false)
if err != nil {
Expand Down
4 changes: 1 addition & 3 deletions pkg/build/stage/full_dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,7 @@ RUN echo hello

containerBackend := NewContainerBackendStub()

dockerRegistry := NewDockerRegistryApiStub()

err := stage.FetchDependencies(ctx, conveyor, containerBackend, dockerRegistry)
_, err := stage.GetDependencies(ctx, conveyor, containerBackend, nil, nil, nil)
Expect(IsErrInvalidBaseImage(err)).To(BeTrue())
})
})
Expand Down

0 comments on commit 6209ac0

Please sign in to comment.