Skip to content

Commit

Permalink
fix(buildah): buildah Dockerfile builder was not using layers cache
Browse files Browse the repository at this point in the history
Signed-off-by: Timofey Kirillov <timofey.kirillov@flant.com>
  • Loading branch information
distorhead committed May 31, 2022
1 parent 3f32bf0 commit 8d9326d
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 16 deletions.
34 changes: 19 additions & 15 deletions pkg/buildah/native_linux.go
Expand Up @@ -183,19 +183,23 @@ func (b *NativeBuildah) Push(ctx context.Context, ref string, opts PushOpts) err

func (b *NativeBuildah) BuildFromDockerfile(ctx context.Context, dockerfile []byte, opts BuildFromDockerfileOpts) (string, error) {
buildOpts := define.BuildOptions{
Isolation: define.Isolation(b.Isolation),
Runtime: DefaultRuntime,
Args: opts.BuildArgs,
SignaturePolicyPath: b.SignaturePolicyPath,
ReportWriter: opts.LogWriter,
OutputFormat: buildah.Dockerv2ImageManifest,
SystemContext: &b.DefaultSystemContext,
ConfigureNetwork: define.NetworkEnabled,
CommonBuildOpts: &b.DefaultCommonBuildOptions,
Target: opts.Target,
MaxPullPushRetries: MaxPullPushRetries,
PullPushRetryDelay: PullPushRetryDelay,
Platforms: b.platforms,
Isolation: define.Isolation(b.Isolation),
Runtime: DefaultRuntime,
Args: opts.BuildArgs,
SignaturePolicyPath: b.SignaturePolicyPath,
ReportWriter: opts.LogWriter,
OutputFormat: buildah.Dockerv2ImageManifest,
SystemContext: &b.DefaultSystemContext,
ConfigureNetwork: define.NetworkEnabled,
CommonBuildOpts: &b.DefaultCommonBuildOptions,
Target: opts.Target,
MaxPullPushRetries: MaxPullPushRetries,
PullPushRetryDelay: PullPushRetryDelay,
Platforms: b.platforms,
Layers: true,
RemoveIntermediateCtrs: true,
ForceRmIntermediateCtrs: false,
NoCache: false,
}

errLog := &bytes.Buffer{}
Expand Down Expand Up @@ -350,8 +354,8 @@ func (b *NativeBuildah) Rm(ctx context.Context, ref string, opts RmOpts) error {

func (b *NativeBuildah) Rmi(ctx context.Context, ref string, opts RmiOpts) error {
_, rmiErrors := b.Runtime.RemoveImages(ctx, []string{ref}, &libimage.RemoveImagesOptions{
Force: opts.Force,
Filters: []string{"readonly=false", "intermediate=false", "dangling=true"},
Force: opts.Force,
//Filters: []string{"readonly=false", "intermediate=false", "dangling=true"},
})

var multiErr *multierror.Error
Expand Down
4 changes: 4 additions & 0 deletions pkg/container_backend/buildah_backend.go
Expand Up @@ -559,6 +559,10 @@ func (runtime *BuildahBackend) BuildDockerfile(ctx context.Context, dockerfile [
})
}

func (runtime *BuildahBackend) ShouldCleanupDockerfileImage() bool {
return false
}

func (runtime *BuildahBackend) RefreshImageObject(ctx context.Context, img LegacyImageInterface) error {
if info, err := runtime.GetImageInfo(ctx, img.Name(), GetImageInfoOpts{}); err != nil {
return err
Expand Down
6 changes: 6 additions & 0 deletions pkg/container_backend/docker_server_backend.go
Expand Up @@ -78,6 +78,12 @@ func (runtime *DockerServerBackend) BuildDockerfile(ctx context.Context, _ []byt
return tempID, docker.CliBuild_LiveOutputWithCustomIn(ctx, opts.ContextTar, cliArgs...)
}

// ShouldCleanupDockerfileImage for docker-server backend we should cleanup image built from dockerfrom tagged with tempID
// which is implementation detail of the BuildDockerfile.
func (runtime *DockerServerBackend) ShouldCleanupDockerfileImage() bool {
return true
}

func (runtime *DockerServerBackend) GetImageInfo(ctx context.Context, ref string, opts GetImageInfoOpts) (*image.Info, error) {
inspect, err := docker.ImageInspect(ctx, ref)
if client.IsErrNotFound(err) {
Expand Down
1 change: 1 addition & 0 deletions pkg/container_backend/interface.go
Expand Up @@ -58,6 +58,7 @@ type ContainerBackend interface {
String() string

// Legacy
ShouldCleanupDockerfileImage() bool
RefreshImageObject(ctx context.Context, img LegacyImageInterface) error
PullImageFromRegistry(ctx context.Context, img LegacyImageInterface) error
RenameImage(ctx context.Context, img LegacyImageInterface, newImageName string, removeOldName bool) error
Expand Down
4 changes: 4 additions & 0 deletions pkg/container_backend/perf_check_container_backend.go
Expand Up @@ -19,6 +19,10 @@ func (runtime *PerfCheckContainerBackend) HasStapelBuildSupport() bool {
return runtime.ContainerBackend.HasStapelBuildSupport()
}

func (runtime *PerfCheckContainerBackend) ShouldCleanupDockerfileImage() bool {
return runtime.ContainerBackend.ShouldCleanupDockerfileImage()
}

func (runtime *PerfCheckContainerBackend) GetImageInfo(ctx context.Context, ref string, opts GetImageInfoOpts) (resImg *image.Info, resErr error) {
logboek.Context(ctx).Default().LogProcess("ContainerBackend.GetImageInfo %q", ref).
Do(func() {
Expand Down
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"os"

"github.com/werf/logboek"

"github.com/werf/werf/pkg/container_backend"
)

Expand Down Expand Up @@ -60,13 +62,17 @@ func (b *DockerfileStageBuilder) Build(ctx context.Context) error {
if err != nil {
return fmt.Errorf("error building dockerfile with %s: %w", b.ContainerBackend.String(), err)
}

b.Image.SetBuiltID(builtID)

return nil
}

func (b *DockerfileStageBuilder) Cleanup(ctx context.Context) error {
if !b.ContainerBackend.ShouldCleanupDockerfileImage() {
return nil
}

logboek.Context(ctx).Info().LogF("Cleanup built dockerfile image %q\n", b.Image.BuiltID())
if err := b.ContainerBackend.Rmi(ctx, b.Image.BuiltID(), container_backend.RmiOpts{}); err != nil {
return fmt.Errorf("unable to remove built dockerfile image %q: %w", b.Image.BuiltID(), err)
}
Expand Down

0 comments on commit 8d9326d

Please sign in to comment.