search

containers / buildah

A tool that facilitates building OCI images.
https://buildah.io
Apache License 2.0
7.41k stars 783 forks source link

Image build-time regression between buildah 1.11.5 and 1.11.6 #2067

Closed jmencak closed 4 years ago

jmencak commented 4 years ago

Description

Version 1.11.5 seems to be the last buildah version which doesn't cause any huge (~80s) delays between COPY and RUN steps when building images with larger amounts of vendored-in files (e.g. OpenShift operator images).

Steps to reproduce the issue:

  1. git clone https://github.com/openshift/cluster-node-tuning-operator
  2. cd cluster-node-tuning-operator
  3. make local-image IMAGE_TAG=origin USE_BUILDAH=1

Describe the results you received: 80s delay between STEP 3: COPY . . and STEP 4: RUN make build

Describe the results you expected: A reasonable delay between the steps as in podman version 1.11.5 and below.

Output of rpm -q buildah or apt list buildah:

buildah-1.12.0-2.fc31.x86_64 # buildah 1.12 and the latest 1.14 has the same issue; for the demo using 1.11.6 (see below)

Output of buildah version: Compiled from source on F31.

Version:         1.11.6
Go Version:      go1.13.5
Image Spec:      1.0.1-dev
Runtime Spec:    1.0.1-dev
CNI Spec:        0.4.0
libcni Version:  v0.7.1
image Version:   5.0.0
Git Commit:      9513cb8c
Built:           Tue Jan  7 09:37:18 2020
OS/Arch:         linux/amd64

*Output of `cat /etc/release`:**

Fedora release 31 (Thirty One)
NAME=Fedora
VERSION="31 (Thirty One)"
ID=fedora
VERSION_ID=31
VERSION_CODENAME=""
PLATFORM_ID="platform:f31"
PRETTY_NAME="Fedora 31 (Thirty One)"
ANSI_COLOR="0;34"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:31"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f31/system-administrators-guide/"
SUPPORT_URL="https://fedoraproject.org/wiki/Communicating_and_getting_help"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=31
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=31
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
Fedora release 31 (Thirty One)
Fedora release 31 (Thirty One)

Output of uname -a:

Linux x1.lan 5.4.7-200.fc31.x86_64 #1 SMP Tue Dec 31 22:25:12 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

Output of cat /etc/containers/storage.conf:

# This file is is the configuration file for all tools
# that use the containers/storage library.
# See man 5 containers-storage.conf for more information
# The "container storage" table contains all of the server options.
[storage]

# Default Storage Driver
driver = "overlay"

# Temporary storage location
runroot = "/var/run/containers/storage"

# Primary Read/Write location of container storage
graphroot = "/var/lib/containers/storage"

[storage.options]
# Storage options to be passed to underlying storage drivers

# AdditionalImageStores is used to pass paths to additional Read/Only image stores
# Must be comma separated list.
additionalimagestores = [
]

# Size is used to set a maximum size of the container image.  Only supported by
# certain container storage drivers.
size = ""

# Path to an helper program to use for mounting the file system instead of mounting it
# directly.
#mount_program = "/usr/bin/fuse-overlayfs"

# OverrideKernelCheck tells the driver to ignore kernel checks based on kernel version
override_kernel_check = "true"

# mountopt specifies comma separated list of extra mount options
mountopt = "nodev,metacopy=on"

# Remap-UIDs/GIDs is the mapping from UIDs/GIDs as they should appear inside of
# a container, to UIDs/GIDs as they should appear outside of the container, and
# the length of the range of UIDs/GIDs.  Additional mapped sets can be listed
# and will be heeded by libraries, but there are limits to the number of
# mappings which the kernel will allow when you later attempt to run a
# container.
#
# remap-uids = 0:1668442479:65536
# remap-gids = 0:1668442479:65536

# Remap-User/Group is a name which can be used to look up one or more UID/GID
# ranges in the /etc/subuid or /etc/subgid file.  Mappings are set up starting
# with an in-container ID of 0 and the a host-level ID taken from the lowest
# range that matches the specified name, and using the length of that range.
# Additional ranges are then assigned, using the ranges which specify the
# lowest host-level IDs first, to the lowest not-yet-mapped container-level ID,
# until all of the entries have been used for maps.
#
# remap-user = "storage"
# remap-group = "storage"

[storage.options.thinpool]
# Storage Options for thinpool

# autoextend_percent determines the amount by which pool needs to be
# grown. This is specified in terms of % of pool size. So a value of 20 means
# that when threshold is hit, pool will be grown by 20% of existing
# pool size.
# autoextend_percent = "20"

# autoextend_threshold determines the pool extension threshold in terms
# of percentage of pool size. For example, if threshold is 60, that means when
# pool is 60% full, threshold has been hit.
# autoextend_threshold = "80"

# basesize specifies the size to use when creating the base device, which
# limits the size of images and containers.
# basesize = "10G"

# blocksize specifies a custom blocksize to use for the thin pool.
# blocksize="64k"

# directlvm_device specifies a custom block storage device to use for the
# thin pool. Required if you setup devicemapper.
# directlvm_device = ""

# directlvm_device_force wipes device even if device already has a filesystem.
# directlvm_device_force = "True"

# fs specifies the filesystem type to use for the base device.
# fs="xfs"

# log_level sets the log level of devicemapper.
# 0: LogLevelSuppress 0 (Default)
# 2: LogLevelFatal
# 3: LogLevelErr
# 4: LogLevelWarn
# 5: LogLevelNotice
# 6: LogLevelInfo
# 7: LogLevelDebug
# log_level = "7"

# min_free_space specifies the min free space percent in a thin pool require for
# new device creation to succeed. Valid values are from 0% - 99%.
# Value 0% disables
# min_free_space = "10%"

# mkfsarg specifies extra mkfs arguments to be used when creating the base.
# device.
# mkfsarg = ""

# use_deferred_removal marks devicemapper block device for deferred removal.
# If the thinpool is in use when the driver attempts to remove it, the driver 
# tells the kernel to remove it as soon as possible. Note this does not free
# up the disk space, use deferred deletion to fully remove the thinpool.
# use_deferred_removal = "True"

# use_deferred_deletion marks thinpool device for deferred deletion.
# If the device is busy when the driver attempts to delete it, the driver
# will attempt to delete device every 30 seconds until successful.
# If the program using the driver exits, the driver will continue attempting
# to cleanup the next time the driver is used. Deferred deletion permanently
# deletes the device and all data stored in device will be lost.
# use_deferred_deletion = "True"

# xfs_nospace_max_retries specifies the maximum number of retries XFS should
# attempt to complete IO when ENOSPC (no space) error is returned by
# underlying storage device.
# xfs_nospace_max_retries = "0"

# If specified, use OSTree to deduplicate files with the overlay backend
ostree_repo = ""

# Set to skip a PRIVATE bind mount on the storage home directory.  Only supported by
# certain container storage drivers
skip_mount_home = "false"

Output of cat ~/.config/containers/storage.conf:

[storage]
  driver = "overlay"
  runroot = "/run/user/1000"
  graphroot = "/home/mencak/.local/share/containers/storage"
  [storage.options]
    size = ""
    remap-uids = ""
    remap-gids = ""
    ignore_chown_errors = ""
    remap-user = ""
    remap-group = ""
    mount_program = "/usr/bin/fuse-overlayfs"
    mountopt = ""
    [storage.options.thinpool]
      autoextend_percent = ""
      autoextend_threshold = ""
      basesize = ""
      blocksize = ""
      directlvm_device = ""
      directlvm_device_force = ""
      fs = ""
      log_level = ""
      min_free_space = ""
      mkfsarg = ""
      mountopt = ""
      use_deferred_deletion = ""
      use_deferred_removal = ""
      xfs_nospace_max_retries = ""

Additional information

TomSweeneyRedHat commented 4 years ago

@nalind thoughts?

nalind commented 4 years ago

Not many merges in between, looks like it's a side-effect of #1914?

rhatdan commented 4 years ago

I thought we asked. @jmencak Do you have a .dockerignore file?

jmencak commented 4 years ago

@rhatdan no .dockerignore file. I was also able to reproduce this on a completely fresh F30 install with buildah-1.12.0. Are you saying you cannot reproduce this?

jmencak commented 4 years ago

Narrowed it down to commit 92ff21584fc46fc9d4a1d24577632b53cb97d892. Commit cd88667465a9f4a46daf3c6c2975a9883885c7fe is the last one that works for me fine.

rhatdan commented 4 years ago

This really looks like a .dockerignore is involved or the code is mistakenly thinking there is a .dockerignore.

rhatdan commented 4 years ago

This really looks like a .dockerignore is involved or the code is mistakenly thinking there is a .dockerignore.

@saschagrunert PTAL You removed the !excludes.Exclusions() call, could this be triggering the error?

-               if excludes == nil || !excludes.Exclusions() {
+               // Copy the whole directory because we do not exclude anything
+               if excludes == nil {
saschagrunert commented 4 years ago

The increased performance delay was expected from my side since #1914 fixes the actual behavior for .dockerignore files. So we now correctly process the files but gain a slight improvement by skipping directories entirely.

It looks like that all the calls to copyFileWithTar are slow together. We probably can improve this by copying the whole directly via copyWithTar...

I see the use case of building inside the container image but in general I think that COPY . . opens up possible security issues.

2070 seems the right fix for this :+1:

rhatdan commented 4 years ago

Fixed in master.