Skip to content

Commit 2c67b24

Browse files
fix(build, docker, dockerfile): remove ONBUILD support in base images 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>
1 parent 75d972b commit 2c67b24

File tree

2 files changed

+12
-115
lines changed

2 files changed

+12
-115
lines changed

pkg/build/stage/full_dockerfile.go

Lines changed: 11 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"os"
99
"path/filepath"
1010
"strconv"
11-
"strings"
1211

1312
"github.com/moby/buildkit/frontend/dockerfile/instructions"
1413
"github.com/moby/buildkit/frontend/dockerfile/parser"
@@ -104,19 +103,16 @@ type DockerStages struct {
104103
dockerMetaArgs []instructions.ArgCommand
105104
dockerStageArgsHash map[int]map[string]string
106105
dockerStageEnvs map[int]map[string]string
107-
108-
imageOnBuildInstructions map[string][]string
109106
}
110107

111108
func NewDockerStages(dockerStages []instructions.Stage, dockerBuildArgsHash map[string]string, dockerMetaArgs []instructions.ArgCommand, dockerTargetStageIndex int) *DockerStages {
112109
ds := &DockerStages{
113-
dockerStages: dockerStages,
114-
dockerTargetStageIndex: dockerTargetStageIndex,
115-
dockerBuildArgsHash: dockerBuildArgsHash,
116-
dockerMetaArgs: dockerMetaArgs,
117-
dockerStageArgsHash: map[int]map[string]string{},
118-
dockerStageEnvs: map[int]map[string]string{},
119-
imageOnBuildInstructions: map[string][]string{},
110+
dockerStages: dockerStages,
111+
dockerTargetStageIndex: dockerTargetStageIndex,
112+
dockerBuildArgsHash: dockerBuildArgsHash,
113+
dockerMetaArgs: dockerMetaArgs,
114+
dockerStageArgsHash: map[int]map[string]string{},
115+
dockerStageEnvs: map[int]map[string]string{},
120116
}
121117

122118
return ds
@@ -294,99 +290,11 @@ type dockerfileInstructionInterface interface {
294290
Name() string
295291
}
296292

297-
func (s *FullDockerfileStage) FetchDependencies(ctx context.Context, c Conveyor, containerBackend container_backend.ContainerBackend, dockerRegistry docker_registry.GenericApiInterface) error {
298-
resolvedDependenciesArgsHash := ResolveDependenciesArgs(s.targetPlatform, s.dependencies, c)
299-
300-
resolvedDockerMetaArgsHash, err := s.resolveDockerMetaArgs(resolvedDependenciesArgsHash)
301-
if err != nil {
302-
return fmt.Errorf("unable to resolve docker meta args: %w", err)
303-
}
304-
305-
outerLoop:
306-
for ind, stage := range s.dockerStages {
307-
resolvedBaseName, err := s.ShlexProcessWordWithMetaArgs(stage.BaseName, resolvedDockerMetaArgsHash)
308-
if err != nil {
309-
return err
310-
}
311-
312-
if resolvedBaseName == "" {
313-
return ErrInvalidBaseImage
314-
}
315-
316-
for relatedStageIndex, relatedStage := range s.dockerStages {
317-
if ind == relatedStageIndex {
318-
continue
319-
}
320-
321-
if resolvedBaseName == relatedStage.Name {
322-
continue outerLoop
323-
}
324-
}
325-
326-
_, ok := s.imageOnBuildInstructions[resolvedBaseName]
327-
if ok || resolvedBaseName == "scratch" {
328-
continue
329-
}
330-
331-
getBaseImageOnBuildLocally := func() ([]string, error) {
332-
info, err := containerBackend.GetImageInfo(ctx, resolvedBaseName, container_backend.GetImageInfoOpts{})
333-
if err != nil {
334-
return nil, err
335-
}
336-
337-
if info == nil {
338-
return nil, errImageNotExistLocally
339-
}
340-
341-
return info.OnBuild, nil
342-
}
343-
344-
getBaseImageOnBuildRemotely := func() ([]string, error) {
345-
configFile, err := dockerRegistry.GetRepoImageConfigFile(ctx, resolvedBaseName)
346-
if err != nil {
347-
return nil, fmt.Errorf("get repo image %q config file failed: %w", resolvedBaseName, err)
348-
}
349-
350-
return configFile.Config.OnBuild, nil
351-
}
352-
353-
var onBuild []string
354-
if onBuild, err = getBaseImageOnBuildLocally(); err != nil && err != errImageNotExistLocally {
355-
return err
356-
} else if err == errImageNotExistLocally {
357-
var getRemotelyErr error
358-
if onBuild, getRemotelyErr = getBaseImageOnBuildRemotely(); getRemotelyErr != nil {
359-
if isUnsupportedMediaTypeError(getRemotelyErr) {
360-
logboek.Context(ctx).Warn().LogF("WARNING: Could not get base image manifest from local docker and from docker registry: %s\n", getRemotelyErr)
361-
logboek.Context(ctx).Warn().LogLn("WARNING: The base image pulling is necessary for calculating digest of image correctly\n")
362-
if err := logboek.Context(ctx).Default().LogProcess("Pulling base image %s", resolvedBaseName).DoError(func() error {
363-
return containerBackend.Pull(ctx, resolvedBaseName, container_backend.PullOpts{})
364-
}); err != nil {
365-
return err
366-
}
367-
368-
if onBuild, err = getBaseImageOnBuildLocally(); err != nil {
369-
return err
370-
}
371-
} else {
372-
return getRemotelyErr
373-
}
374-
}
375-
}
376-
377-
s.imageOnBuildInstructions[resolvedBaseName] = onBuild
378-
}
379-
293+
func (s *FullDockerfileStage) FetchDependencies(_ context.Context, _ Conveyor, _ container_backend.ContainerBackend, _ docker_registry.GenericApiInterface) error {
380294
return nil
381295
}
382296

383-
func isUnsupportedMediaTypeError(err error) bool {
384-
return strings.Contains(err.Error(), "unsupported MediaType")
385-
}
386-
387-
var errImageNotExistLocally = errors.New("IMAGE_NOT_EXIST_LOCALLY")
388-
389-
func (s *FullDockerfileStage) GetDependencies(ctx context.Context, c Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) {
297+
func (s *FullDockerfileStage) GetDependencies(ctx context.Context, c Conveyor, _ container_backend.ContainerBackend, _, _ *StageImage, _ container_backend.BuildContextArchiver) (string, error) {
390298
resolvedDependenciesArgsHash := ResolveDependenciesArgs(s.targetPlatform, s.dependencies, c)
391299

392300
resolvedDockerMetaArgsHash, err := s.resolveDockerMetaArgs(resolvedDependenciesArgsHash)
@@ -408,20 +316,11 @@ func (s *FullDockerfileStage) GetDependencies(ctx context.Context, c Conveyor, c
408316
return "", err
409317
}
410318

411-
dependencies = append(dependencies, resolvedBaseName)
412-
413-
onBuildInstructions, ok := s.imageOnBuildInstructions[resolvedBaseName]
414-
if ok {
415-
for _, instruction := range onBuildInstructions {
416-
_, iOnBuildDependencies, err := s.dockerfileOnBuildInstructionDependencies(ctx, c.GiterminismManager(), resolvedDockerMetaArgsHash, resolvedDependenciesArgsHash, ind, instruction, true)
417-
if err != nil {
418-
return "", err
419-
}
420-
421-
dependencies = append(dependencies, iOnBuildDependencies...)
422-
}
319+
if resolvedBaseName == "" {
320+
return "", ErrInvalidBaseImage
423321
}
424322

323+
dependencies = append(dependencies, resolvedBaseName)
425324
for _, cmd := range stage.Commands {
426325
cmdDependencies, cmdOnBuildDependencies, err := s.dockerfileInstructionDependencies(ctx, c.GiterminismManager(), resolvedDockerMetaArgsHash, resolvedDependenciesArgsHash, ind, cmd, false, false)
427326
if err != nil {

pkg/build/stage/full_dockerfile_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,9 +297,7 @@ RUN echo hello
297297

298298
containerBackend := NewContainerBackendStub()
299299

300-
dockerRegistry := NewDockerRegistryApiStub()
301-
302-
err := stage.FetchDependencies(ctx, conveyor, containerBackend, dockerRegistry)
300+
_, err := stage.GetDependencies(ctx, conveyor, containerBackend, nil, nil, nil)
303301
Expect(IsErrInvalidBaseImage(err)).To(BeTrue())
304302
})
305303
})

0 commit comments

Comments
 (0)